You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@celix.apache.org by pn...@apache.org on 2021/02/24 18:39:45 UTC
[celix] 01/01: Adds error handling for the dependency manager.
This is an automated email from the ASF dual-hosted git repository.
pnoltes pushed a commit to branch feature/dep_man_error_handling
in repository https://gitbox.apache.org/repos/asf/celix.git
commit 052bc9559494458f3704c2d93c3d5eab54ef141e
Author: Pepijn Noltes <pe...@gmail.com>
AuthorDate: Wed Feb 24 19:39:31 2021 +0100
Adds error handling for the dependency manager.
---
.../gtest/src/DependencyManagerTestSuite.cc | 84 ++++++++++++++++++++++
libs/framework/include/celix/dm/Component.h | 6 ++
libs/framework/include/celix/dm/Component_Impl.h | 53 +++++++-------
libs/framework/src/dm_component_impl.c | 83 ++++++++++++++-------
4 files changed, 176 insertions(+), 50 deletions(-)
diff --git a/libs/framework/gtest/src/DependencyManagerTestSuite.cc b/libs/framework/gtest/src/DependencyManagerTestSuite.cc
index d0a5d99..31152f0 100644
--- a/libs/framework/gtest/src/DependencyManagerTestSuite.cc
+++ b/libs/framework/gtest/src/DependencyManagerTestSuite.cc
@@ -555,3 +555,87 @@ TEST_F(DependencyManagerTestSuite, UnneededSuspendIsPrevented) {
EXPECT_EQ(cmp2.getInstance().startCount, 2); //1x suspend for set nullptr, 1x suspend for rem
EXPECT_EQ(cmp2.getInstance().stopCount, 2); //1x suspend for set nullptr, 1x suspend for rem
}
+
+TEST_F(DependencyManagerTestSuite, ExceptionsInLifecycle) {
+ class ExceptionComponent {
+ public:
+ enum class LifecycleMethod {
+ INIT,
+ START,
+ STOP,
+ DEINIT
+ };
+
+ explicit ExceptionComponent(LifecycleMethod _failAt) : failAt{_failAt} {}
+
+ void failFor(LifecycleMethod lm) {
+ if (lm == failAt) {
+ throw std::logic_error("fail test");
+ }
+ }
+
+ void init() {
+ failFor(LifecycleMethod::INIT);
+ }
+
+ void start() {
+ failFor(LifecycleMethod::START);
+ }
+
+ void stop() {
+ failFor(LifecycleMethod::STOP);
+ }
+
+ void deinit() {
+ failFor(LifecycleMethod::DEINIT);
+ }
+
+ private:
+ const LifecycleMethod failAt;
+ };
+
+ celix::dm::DependencyManager dm{ctx};
+
+ {
+ auto& cmp = dm.createComponent(std::make_shared<ExceptionComponent>(ExceptionComponent::LifecycleMethod::INIT), "FailAtInitCmp")
+ .setCallbacks(&ExceptionComponent::init, &ExceptionComponent::start, &ExceptionComponent::stop, &ExceptionComponent::deinit);
+ EXPECT_EQ(cmp.getState(), ComponentState::INACTIVE);
+ cmp.build(); //fails at init and should disable
+ EXPECT_EQ(cmp.getState(), ComponentState::INACTIVE);
+ dm.clear();
+ }
+
+ {
+ auto& cmp = dm.createComponent(std::make_shared<ExceptionComponent>(ExceptionComponent::LifecycleMethod::START), "FailAtStartCmp")
+ .setCallbacks(&ExceptionComponent::init, &ExceptionComponent::start, &ExceptionComponent::stop, &ExceptionComponent::deinit);
+ EXPECT_EQ(cmp.getState(), ComponentState::INACTIVE);
+ cmp.build(); //fails at init and should disable
+ EXPECT_EQ(cmp.getState(), ComponentState::INACTIVE);
+ dm.clear();
+ }
+
+ {
+ auto& cmp = dm.createComponent(std::make_shared<ExceptionComponent>(ExceptionComponent::LifecycleMethod::STOP), "FailAtStopCmp")
+ .setCallbacks(&ExceptionComponent::init, &ExceptionComponent::start, &ExceptionComponent::stop, &ExceptionComponent::deinit);
+ EXPECT_EQ(cmp.getState(), ComponentState::INACTIVE);
+ cmp.build();
+ EXPECT_EQ(cmp.getState(), ComponentState::TRACKING_OPTIONAL);
+
+ //required service -> should stop, but fails at stop and should become inactive (component will disable itself)
+ cmp.createServiceDependency<TestService>().setRequired(true).build();
+ cmp.wait();
+ EXPECT_EQ(cmp.getState(), ComponentState::INACTIVE);
+ dm.clear();
+ }
+
+ {
+ auto& cmp = dm.createComponent(std::make_shared<ExceptionComponent>(ExceptionComponent::LifecycleMethod::DEINIT), "FailAtDeinit")
+ .setCallbacks(&ExceptionComponent::init, &ExceptionComponent::start, &ExceptionComponent::stop, &ExceptionComponent::deinit);
+ EXPECT_EQ(cmp.getState(), ComponentState::INACTIVE);
+ cmp.build();
+ EXPECT_EQ(cmp.getState(), ComponentState::TRACKING_OPTIONAL);
+
+ //required service -> should stop, but fails at stop and should become inactive (component will disable itself)
+ dm.clear(); //dm clear will deinit component and this should fail, but not deadlock
+ }
+}
\ No newline at end of file
diff --git a/libs/framework/include/celix/dm/Component.h b/libs/framework/include/celix/dm/Component.h
index 79b3982..4aa301a 100644
--- a/libs/framework/include/celix/dm/Component.h
+++ b/libs/framework/include/celix/dm/Component.h
@@ -354,6 +354,12 @@ namespace celix { namespace dm {
* Can be called on the Celix event thread.
*/
Component<T>& buildAsync();
+ private:
+ /**
+ * @brief try to invoke a lifecycle method (init, start, stop and deinit) and catch and log a possible exception.
+ * @returns 0 if no exception occurred else -1.
+ */
+ int invokeLifecycleMethod(const std::string& methodName, void (T::*lifecycleMethod)());
};
}}
diff --git a/libs/framework/include/celix/dm/Component_Impl.h b/libs/framework/include/celix/dm/Component_Impl.h
index 5668994..6d7bebb 100644
--- a/libs/framework/include/celix/dm/Component_Impl.h
+++ b/libs/framework/include/celix/dm/Component_Impl.h
@@ -245,6 +245,30 @@ Component<T>& Component<T>::setInstance(T&& inst) {
}
template<class T>
+int Component<T>::invokeLifecycleMethod(const std::string& methodName, void (T::*lifecycleMethod)()) {
+ try {
+ if (lifecycleMethod != nullptr) {
+ T& inst = getInstance();
+ (inst.*lifecycleMethod)();
+ }
+ } catch (const std::exception& e) {
+ celix_bundleContext_log(context, CELIX_LOG_LEVEL_ERROR, "Error invoking %s for component %s (uuid=%s). Exception: %s",
+ methodName.c_str(),
+ cmpName.c_str(),
+ cmpUUID.c_str(),
+ e.what());
+ return -1;
+ } catch (...) {
+ celix_bundleContext_log(context, CELIX_LOG_LEVEL_ERROR, "Error invoking %s for component %s (uuid=%s).",
+ methodName.c_str(),
+ cmpName.c_str(),
+ cmpUUID.c_str());
+ return -1;
+ }
+ return 0;
+}
+
+template<class T>
Component<T>& Component<T>::setCallbacks(
void (T::*init)(),
void (T::*start)(),
@@ -256,41 +280,22 @@ Component<T>& Component<T>::setCallbacks(
this->stopFp = stop;
this->deinitFp = deinit;
+
int (*cInit)(void *) = [](void *handle) {
Component<T>* cmp = (Component<T>*)(handle);
- T* inst = &cmp->getInstance();
- void (T::*fp)() = cmp->initFp;
- if (fp != nullptr) {
- (inst->*fp)();
- }
- return 0;
+ return cmp->invokeLifecycleMethod("init", cmp->initFp);
};
int (*cStart)(void *) = [](void *handle) {
Component<T>* cmp = (Component<T>*)(handle);
- T* inst = &cmp->getInstance();
- void (T::*fp)() = cmp->startFp;
- if (fp != nullptr) {
- (inst->*fp)();
- }
- return 0;
+ return cmp->invokeLifecycleMethod("start", cmp->startFp);
};
int (*cStop)(void *) = [](void *handle) {
Component<T>* cmp = (Component<T>*)(handle);
- T* inst = &cmp->getInstance();
- void (T::*fp)() = cmp->stopFp;
- if (fp != nullptr) {
- (inst->*fp)();
- }
- return 0;
+ return cmp->invokeLifecycleMethod("stop", cmp->stopFp);
};
int (*cDeinit)(void *) = [](void *handle) {
Component<T>* cmp = (Component<T>*)(handle);
- T* inst = &cmp->getInstance();
- void (T::*fp)() = cmp->deinitFp;
- if (fp != nullptr) {
- (inst->*fp)();
- }
- return 0;
+ return cmp->invokeLifecycleMethod("deinit", cmp->deinitFp);
};
celix_dmComponent_setCallbacks(this->cComponent(), cInit, cStart, cStop, cDeinit);
diff --git a/libs/framework/src/dm_component_impl.c b/libs/framework/src/dm_component_impl.c
index 0631643..c456487 100644
--- a/libs/framework/src/dm_component_impl.c
+++ b/libs/framework/src/dm_component_impl.c
@@ -69,7 +69,7 @@ typedef struct dm_interface_struct {
static celix_status_t celix_dmComponent_registerServices(celix_dm_component_t *component, bool needLock);
static celix_status_t celix_dmComponent_unregisterServices(celix_dm_component_t *component, bool needLock);
static bool celix_dmComponent_areAllRequiredServiceDependenciesResolved(celix_dm_component_t *component);
-static celix_status_t celix_dmComponent_performTransition(celix_dm_component_t *component, celix_dm_component_state_t oldState, celix_dm_component_state_t newState, bool *transition);
+static bool celix_dmComponent_performTransition(celix_dm_component_t *component, celix_dm_component_state_t oldState, celix_dm_component_state_t newState);
static celix_status_t celix_dmComponent_calculateNewState(celix_dm_component_t *component, celix_dm_component_state_t currentState, celix_dm_component_state_t *newState);
static celix_status_t celix_dmComponent_handleChange(celix_dm_component_t *component);
static celix_status_t celix_dmComponent_handleAdd(celix_dm_component_t *component, const celix_dm_event_t* event);
@@ -79,6 +79,8 @@ static celix_status_t celix_dmComponent_enableDependencies(celix_dm_component_t
static celix_status_t celix_dmComponent_suspend(celix_dm_component_t *component, celix_dm_service_dependency_t *dependency);
static celix_status_t celix_dmComponent_resume(celix_dm_component_t *component, celix_dm_service_dependency_t *dependency);
static celix_status_t celix_dmComponent_disable(celix_dm_component_t *component);
+static void celix_dmComponent_disableDirectly(celix_dm_component_t *component);
+static celix_status_t celix_dmComponent_disableDependencies(celix_dm_component_t *component);
static bool celix_dmComponent_isDisabled(celix_dm_component_t *component);
static void celix_dmComponent_cleanupRemovedDependencies(celix_dm_component_t* component);
@@ -318,6 +320,9 @@ celix_status_t celix_private_dmComponent_enable(celix_dm_component_t *component)
return CELIX_SUCCESS;
}
+/**
+ * @brief disables component and effectualize this through normal component transitions.
+ */
static celix_status_t celix_dmComponent_disable(celix_dm_component_t *component) {
celixThreadMutex_lock(&component->mutex);
bool changed = false;
@@ -332,6 +337,18 @@ static celix_status_t celix_dmComponent_disable(celix_dm_component_t *component)
return CELIX_SUCCESS;
}
+/**
+ * Disables component directly, this is needed for an error flow.
+ *
+ * Should be called with component mutex locked.
+ */
+static void celix_dmComponent_disableDirectly(celix_dm_component_t *component) {
+ component->isEnabled = false;
+ component->state = DM_CMP_STATE_INACTIVE;
+ celix_dmComponent_unregisterServices(component, false);
+ celix_dmComponent_disableDependencies(component);
+}
+
static void celix_dmComponent_cleanupRemovedDependencies(celix_dm_component_t* component) {
//note should be called with lock
bool removedDep = true;
@@ -562,8 +579,6 @@ static bool celix_dmComponent_needsSuspend(celix_dm_component_t *component, cons
}
static celix_status_t celix_dmComponent_handleEvent(celix_dm_component_t *component, const celix_dm_event_t* event, celix_status_t (*setAddOrRemFp)(celix_dm_service_dependency_t *dependency, void* svc, const celix_properties_t* props), const char *invokeName) {
- assert(celix_framework_isCurrentThreadTheEventLoop(celix_bundleContext_getFramework(component->context)));
-
celix_bundleContext_log(component->context, CELIX_LOG_LEVEL_TRACE,
"Calling %s service for component %s (uuid=%s) on service dependency with type %s",
invokeName,
@@ -652,8 +667,10 @@ static void celix_dmComponent_handleChangeOnEventThread(void *data) {
oldState = component->state;
celix_status_t status = celix_dmComponent_calculateNewState(component, oldState, &newState);
if (status == CELIX_SUCCESS) {
- status = celix_dmComponent_performTransition(component, oldState, newState, &transition);
- component->state = newState;
+ transition = celix_dmComponent_performTransition(component, oldState, newState);
+ if (transition) {
+ component->state = newState;
+ }
}
if (status != CELIX_SUCCESS) {
@@ -731,11 +748,13 @@ static celix_status_t celix_dmComponent_calculateNewState(celix_dm_component_t *
/**
* perform state transition. This function should be called with the component->mutex locked.
+ *
+ * Function can also indicate if the component needs to be disabled. This can be needed if a stop or deinit fails,
+ * because the component cannot be destroyed but apparently also not cleanly stopped
*/
-static celix_status_t celix_dmComponent_performTransition(celix_dm_component_t *component, celix_dm_component_state_t oldState, celix_dm_component_state_t newState, bool *transition) {
- if (oldState == newState) {
- *transition = false;
- return CELIX_SUCCESS;
+static bool celix_dmComponent_performTransition(celix_dm_component_t *component, celix_dm_component_state_t currentState, celix_dm_component_state_t desiredState) {
+ if (currentState == desiredState) {
+ return false;
}
celix_bundleContext_log(component->context,
@@ -743,42 +762,54 @@ static celix_status_t celix_dmComponent_performTransition(celix_dm_component_t *
"performing transition for component '%s' (uuid=%s) from state %s to state %s",
component->name,
component->uuid,
- celix_dmComponent_stateToString(oldState),
- celix_dmComponent_stateToString(newState));
+ celix_dmComponent_stateToString(currentState),
+ celix_dmComponent_stateToString(desiredState));
celix_status_t status = CELIX_SUCCESS;
- if (oldState == DM_CMP_STATE_INACTIVE && newState == DM_CMP_STATE_WAITING_FOR_REQUIRED) {
+ if (currentState == DM_CMP_STATE_INACTIVE && desiredState == DM_CMP_STATE_WAITING_FOR_REQUIRED) {
celix_dmComponent_enableDependencies(component);
- *transition = true;
- } else if (oldState == DM_CMP_STATE_WAITING_FOR_REQUIRED && newState == DM_CMP_STATE_INSTANTIATED_AND_WAITING_FOR_REQUIRED) {
+ } else if (currentState == DM_CMP_STATE_WAITING_FOR_REQUIRED && desiredState == DM_CMP_STATE_INSTANTIATED_AND_WAITING_FOR_REQUIRED) {
if (component->callbackInit) {
status = component->callbackInit(component->implementation);
}
- *transition = true;
- } else if (oldState == DM_CMP_STATE_INSTANTIATED_AND_WAITING_FOR_REQUIRED && newState == DM_CMP_STATE_TRACKING_OPTIONAL) {
+ } else if (currentState == DM_CMP_STATE_INSTANTIATED_AND_WAITING_FOR_REQUIRED && desiredState == DM_CMP_STATE_TRACKING_OPTIONAL) {
if (component->callbackStart) {
status = component->callbackStart(component->implementation);
}
- celix_dmComponent_registerServices(component, false);
- component->nrOfTimesStarted += 1;
- *transition = true;
- } else if (oldState == DM_CMP_STATE_TRACKING_OPTIONAL && newState == DM_CMP_STATE_INSTANTIATED_AND_WAITING_FOR_REQUIRED) {
+ if (status == CELIX_SUCCESS) {
+ celix_dmComponent_registerServices(component, false);
+ component->nrOfTimesStarted += 1;
+ }
+ } else if (currentState == DM_CMP_STATE_TRACKING_OPTIONAL && desiredState == DM_CMP_STATE_INSTANTIATED_AND_WAITING_FOR_REQUIRED) {
celix_dmComponent_unregisterServices(component, false);
if (component->callbackStop) {
status = component->callbackStop(component->implementation);
}
- *transition = true;
- } else if (oldState == DM_CMP_STATE_INSTANTIATED_AND_WAITING_FOR_REQUIRED && newState == DM_CMP_STATE_WAITING_FOR_REQUIRED) {
+ } else if (currentState == DM_CMP_STATE_INSTANTIATED_AND_WAITING_FOR_REQUIRED && desiredState == DM_CMP_STATE_WAITING_FOR_REQUIRED) {
if (component->callbackDeinit) {
status = component->callbackDeinit(component->implementation);
}
- *transition = true;
- } else if (oldState == DM_CMP_STATE_WAITING_FOR_REQUIRED && newState == DM_CMP_STATE_INACTIVE) {
+ } else if (currentState == DM_CMP_STATE_WAITING_FOR_REQUIRED && desiredState == DM_CMP_STATE_INACTIVE) {
celix_dmComponent_disableDependencies(component);
- *transition = true;
+ } else {
+ assert(false); //should not be reached.
}
- return status;
+ bool transition = true;
+ if (status != CELIX_SUCCESS) {
+ celix_bundleContext_log(component->context, CELIX_LOG_LEVEL_ERROR,
+ "Error in component %s (uuid=%s) transition from %s to %s. Disabling component.",
+ component->name,
+ component->uuid,
+ celix_dmComponent_stateToString(currentState),
+ celix_dmComponent_stateToString(desiredState));
+
+ //If a lifecycle method fails, the component will disable itself (but no higher level transition is needed anymore)
+ transition = false;
+ celix_dmComponent_disableDirectly(component);
+ }
+
+ return transition;
}
/**