You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@celix.apache.org by GitBox <gi...@apache.org> on 2021/01/26 16:20:15 UTC

[GitHub] [celix] Oipo commented on a change in pull request #313: Feature/refactor c dep man service trackers

Oipo commented on a change in pull request #313:
URL: https://github.com/apache/celix/pull/313#discussion_r559449882



##########
File path: libs/utils/include/version_range.h
##########
@@ -53,7 +54,7 @@ typedef struct versionRange *version_range_pt;
  */
 celix_status_t
 versionRange_createVersionRange(version_pt low, bool isLowInclusive, version_pt high, bool isHighInclusive,
-                                version_range_pt *versionRange);
+                                version_range_pt *versionRange) ;

Review comment:
       Can all instances of `) ;` be replaced with `);` in this file?

##########
File path: libs/framework/include/requirement.h
##########
@@ -31,6 +24,7 @@ typedef struct requirement *requirement_pt;
 
 #include "capability.h"
 #include "hash_map.h"
+#include "celix_version_range.h"
 #include "version_range.h"

Review comment:
       Can `version_range.h` be removed?

##########
File path: libs/framework/src/dm_component_impl.c
##########
@@ -282,78 +304,72 @@ celix_status_t component_removeServiceDependency(celix_dm_component_t *component
     return celix_dmComponent_removeServiceDependency(component, dependency);
 }
 
-celix_status_t celix_dmComponent_removeServiceDependency(celix_dm_component_t *component, celix_dm_service_dependency_t *dependency) {
-    celix_status_t status = CELIX_SUCCESS;
-
-    executor_executeTask(component->executor, component, component_removeTask, dependency);
-
-    return status;
-}
-
-static celix_status_t component_removeTask(celix_dm_component_t *component, celix_dm_service_dependency_t *dependency) {
-    celix_status_t status = CELIX_SUCCESS;
-
-    pthread_mutex_lock(&component->mutex);
-    arrayList_removeElement(component->dependencies, dependency);
-    pthread_mutex_unlock(&component->mutex);
-
-    if (component->state != DM_CMP_STATE_INACTIVE) {
-        serviceDependency_stop(dependency);
+celix_status_t celix_private_dmComponent_enable(celix_dm_component_t *component) {
+    celixThreadMutex_lock(&component->mutex);
+    if (!component->isEnabled) {
+        component->isEnabled = true;
     }
+    celixThreadMutex_unlock(&component->mutex);
+    celix_dmComponent_handleChange(component);

Review comment:
       Shouldn't `celix_dmComponent_handleChange` only be called if `isEnabled` actually changed?

##########
File path: libs/framework/src/dm_component_impl.c
##########
@@ -282,78 +304,72 @@ celix_status_t component_removeServiceDependency(celix_dm_component_t *component
     return celix_dmComponent_removeServiceDependency(component, dependency);
 }
 
-celix_status_t celix_dmComponent_removeServiceDependency(celix_dm_component_t *component, celix_dm_service_dependency_t *dependency) {
-    celix_status_t status = CELIX_SUCCESS;
-
-    executor_executeTask(component->executor, component, component_removeTask, dependency);
-
-    return status;
-}
-
-static celix_status_t component_removeTask(celix_dm_component_t *component, celix_dm_service_dependency_t *dependency) {
-    celix_status_t status = CELIX_SUCCESS;
-
-    pthread_mutex_lock(&component->mutex);
-    arrayList_removeElement(component->dependencies, dependency);
-    pthread_mutex_unlock(&component->mutex);
-
-    if (component->state != DM_CMP_STATE_INACTIVE) {
-        serviceDependency_stop(dependency);
+celix_status_t celix_private_dmComponent_enable(celix_dm_component_t *component) {
+    celixThreadMutex_lock(&component->mutex);
+    if (!component->isEnabled) {
+        component->isEnabled = true;
     }
+    celixThreadMutex_unlock(&component->mutex);
+    celix_dmComponent_handleChange(component);
+    return CELIX_SUCCESS;
+}
 
-    pthread_mutex_lock(&component->mutex);
-    array_list_pt events = hashMap_remove(component->dependencyEvents, dependency);
-    pthread_mutex_unlock(&component->mutex);
-
-	serviceDependency_destroy(&dependency);
-
-    while (!arrayList_isEmpty(events)) {
-    	dm_event_pt event = arrayList_remove(events, 0);
-    	event_destroy(&event);
+static celix_status_t celix_dmComponent_disable(celix_dm_component_t *component) {
+    celixThreadMutex_lock(&component->mutex);
+    if (component->isEnabled) {
+        component->isEnabled = false;
     }
-    arrayList_destroy(events);
-
-    component_handleChange(component);
-
-    return status;
+    celixThreadMutex_unlock(&component->mutex);
+    celix_dmComponent_handleChange(component);

Review comment:
       Shouldn't `celix_dmComponent_handleChange` only be called if `isEnabled` actually changed?

##########
File path: libs/utils/src/version_range.c
##########
@@ -16,116 +16,42 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-/**
- * version_range.c
- *
- *  \date       Jul 12, 2010
- *  \author     <a href="mailto:dev@celix.apache.org">Apache Celix Project Team</a>
- *  \copyright  Apache License, Version 2.0
- */
+
 
 #include <stdlib.h>
 #include <string.h>
 #include <stdio.h>
 #include <assert.h>
 
 #include "version_private.h"
+#include "version_range.h"
+#include "celix_version_range.h"
 #include "version_range_private.h"
+#include "celix_version_range.h"
 
 celix_status_t versionRange_createVersionRange(version_pt low, bool isLowInclusive,
             version_pt high, bool isHighInclusive, version_range_pt *range) {
-    assert(low != high);
-    celix_status_t status = CELIX_SUCCESS;
-    *range = (version_range_pt) malloc(sizeof(**range));
-    if (!*range) {
-        status = CELIX_ENOMEM;
-    } else {
-        (*range)->low = low;
-        (*range)->isLowInclusive = isLowInclusive;
-        (*range)->high = high;
-        (*range)->isHighInclusive = isHighInclusive;
-    }
-
-    return status;
+    *range = celix_versionRange_createVersionRange(low, isLowInclusive, high, isHighInclusive);
+    return CELIX_SUCCESS;
 }
 
 celix_status_t versionRange_destroy(version_range_pt range) {
-    if (range->high != NULL) {
-        version_destroy(range->high);
-    }
-    if (range->low != NULL) {
-        version_destroy(range->low);
-    }
-
-    range->high = NULL;
-    range->isHighInclusive = false;
-    range->low = NULL;
-    range->isLowInclusive = false;
-
-    free(range);
-
+    celix_versionRange_destroy(range);
     return CELIX_SUCCESS;
 }
 
 celix_status_t versionRange_createInfiniteVersionRange(version_range_pt *range) {
-    celix_status_t status;
-
-    version_pt version = NULL;
-    status = version_createEmptyVersion(&version);
-    if (status == CELIX_SUCCESS) {
-        status = versionRange_createVersionRange(version, true, NULL, true, range);
-    }
-
-    return status;
+    *range = celix_versionRange_createInfiniteVersionRange();
+    return CELIX_SUCCESS;
 }
 
 celix_status_t versionRange_isInRange(version_range_pt versionRange, version_pt version, bool *inRange) {
-    celix_status_t status;
-    if (versionRange->high == NULL) {
-        int cmp;
-        status = version_compareTo(version, versionRange->low, &cmp);
-        if (status == CELIX_SUCCESS) {
-            *inRange = (cmp >= 0);
-        }
-    } else if (versionRange->isLowInclusive && versionRange->isHighInclusive) {
-        int low, high;
-        status = version_compareTo(version, versionRange->low, &low);
-        if (status == CELIX_SUCCESS) {
-            status = version_compareTo(version, versionRange->high, &high);
-            if (status == CELIX_SUCCESS) {
-                *inRange = (low >= 0) && (high <= 0);
-            }
-        }
-    } else if (versionRange->isHighInclusive) {
-        int low, high;
-        status = version_compareTo(version, versionRange->low, &low);
-        if (status == CELIX_SUCCESS) {
-            status = version_compareTo(version, versionRange->high, &high);
-            if (status == CELIX_SUCCESS) {
-                *inRange = (low > 0) && (high <= 0);
-            }
-        }
-    } else if (versionRange->isLowInclusive) {
-        int low, high;
-        status = version_compareTo(version, versionRange->low, &low);
-        if (status == CELIX_SUCCESS) {
-            status = version_compareTo(version, versionRange->high, &high);
-            if (status == CELIX_SUCCESS) {
-                *inRange = (low >= 0) && (high < 0);
-            }
-        }
+    if (versionRange != NULL && version != NULL) {
+        *inRange = celix_versionRange_isInRange(versionRange, version);
+        return CELIX_SUCCESS;
     } else {
-        int low, high;
-        status = version_compareTo(version, versionRange->low, &low);
-        if (status == CELIX_SUCCESS) {
-            status = version_compareTo(version, versionRange->high, &high);
-            if (status == CELIX_SUCCESS) {
-                *inRange = (low > 0) && (high < 0);
-            }
-        }
+        return CELIX_ILLEGAL_ARGUMENT;
     }
-
-    return status;
 }
 
 celix_status_t versionRange_getLowVersion(version_range_pt versionRange, version_pt *lowVersion) {

Review comment:
       Shouldn't this also refer to `celix_versionRange_getLowVersion`?

##########
File path: libs/framework/include/celix_dependency_manager.h
##########
@@ -30,22 +30,78 @@
 extern "C" {
 #endif
 
+/**
+ * The `celix_dependencyManager_add`, `celix_dependencyManager_remove` and `celix_dependencyManager_removeAllComponents`
+ * funcions for celix_dependency_manager_t should be called outside the Celix event thread.

Review comment:
       `funcions` -> `functions`

##########
File path: libs/framework/include/celix/dm/ServiceDependency.h
##########
@@ -78,14 +70,29 @@ namespace celix { namespace dm {
 
         /**
          * Whether the service dependency is valid.
+         *
+         * Depcrated -> will always return true.

Review comment:
       typo in `Depcrated` -> `Deprecated`

##########
File path: libs/framework/gtest/src/DependencyManagerTestSuite.cc
##########
@@ -63,6 +65,87 @@ TEST_F(DependencyManagerTestSuite, DmCreateComponent) {
     ASSERT_TRUE(celix_dependencyManager_allComponentsActive(mng));
 }
 
+TEST_F(DependencyManagerTestSuite, DmComponentAddRemove) {
+    auto *mng = celix_bundleContext_getDependencyManager(ctx);
+    auto *cmp = celix_dmComponent_create(ctx, "test1");
+    celix_dependencyManager_add(mng, cmp);
+    ASSERT_EQ(1, celix_dependencyManager_nrOfComponents(mng));
+
+    celix_dependencyManager_remove(mng, cmp);
+    ASSERT_EQ(0, celix_dependencyManager_nrOfComponents(mng));
+
+    auto *cmp2 = celix_dmComponent_create(ctx, "test2");
+    auto *cmp3 = celix_dmComponent_create(ctx, "test3");
+    celix_dependencyManager_add(mng, cmp2);
+    celix_dependencyManager_add(mng, cmp3);
+    ASSERT_EQ(2, celix_dependencyManager_nrOfComponents(mng));
+
+    celix_dependencyManager_removeAllComponents(mng);
+    ASSERT_EQ(0, celix_dependencyManager_nrOfComponents(mng));
+}
+
+
+TEST_F(DependencyManagerTestSuite, DmComponentAddRemoveAsync) {
+    auto *mng = celix_bundleContext_getDependencyManager(ctx);
+    auto *cmp1 = celix_dmComponent_create(ctx, "test1");
+    celix_dependencyManager_addAsync(mng, cmp1);
+    celix_dependencyManager_wait(mng);
+    EXPECT_EQ(1, celix_dependencyManager_nrOfComponents(mng));
+
+    std::atomic<std::size_t> count{0};
+    auto cb = [](void *data) {
+        auto* c = static_cast<std::atomic<std::size_t>*>(data);
+        c->fetch_add(1);
+    };
+
+    celix_dependencyManager_removeAsync(mng, cmp1, &count, cb);
+    celix_dependencyManager_wait(mng);
+    EXPECT_EQ(0, celix_dependencyManager_nrOfComponents(mng));
+    EXPECT_EQ(1, count.load());
+}
+
+TEST_F(DependencyManagerTestSuite, DmComponentRemoveAllAsync) {
+    auto *mng = celix_bundleContext_getDependencyManager(ctx);
+    auto *cmp1 = celix_dmComponent_create(ctx, "test1");
+    auto *cmp2 = celix_dmComponent_create(ctx, "test2");
+    celix_dependencyManager_add(mng, cmp1);
+    celix_dependencyManager_add(mng, cmp2);
+    EXPECT_EQ(2, celix_dependencyManager_nrOfComponents(mng));
+
+    std::atomic<std::size_t> count{0};
+    celix_dependencyManager_removeAllComponentsAsync(mng, &count, [](void *data) {
+        auto* c = static_cast<std::atomic<std::size_t>*>(data);
+        c->fetch_add(1);
+    });
+    celix_dependencyManager_wait(mng);
+    EXPECT_EQ(0, celix_dependencyManager_nrOfComponents(mng));
+    EXPECT_EQ(1, count.load());
+}
+
+TEST_F(DependencyManagerTestSuite, CDmGetInfo) {
+    auto* mng = celix_bundleContext_getDependencyManager(ctx);
+    auto* cmp = celix_dmComponent_create(ctx, "test1");
+
+    auto* p = celix_properties_create();
+    celix_properties_set(p, "key", "value");
+    celix_dmComponent_addInterface(cmp, "test-interface", nullptr, (void*)0x42, p);
+
+    auto* dep = celix_dmServiceDependency_create();
+    celix_dmServiceDependency_setService(dep, "test-interface", nullptr, nullptr);
+    celix_dmComponent_addServiceDependency(cmp, dep);
+
+    celix_dependencyManager_add(mng, cmp);
+
+    auto* infos = celix_dependencyManager_createInfos(mng);
+    ASSERT_EQ(1, celix_arrayList_size(infos));
+    auto* dmInfo = (celix_dependency_manager_info_t*)celix_arrayList_get(infos, 0);
+    ASSERT_EQ(1, celix_arrayList_size(dmInfo->components));
+    auto *info = (celix_dm_component_info_t*)celix_arrayList_get(dmInfo->components, 0);
+    EXPECT_EQ(1, celix_arrayList_size(info->interfaces));

Review comment:
       Add check if `celix_properties_size(info->interfaces[0]->properties) == 1`, but then with `celix_arrayList_get` instead of C++ arrays.

##########
File path: libs/framework/include/celix_bundle_context.h
##########
@@ -1143,8 +1178,29 @@ void celix_bundleContext_waitForEvents(celix_bundle_context_t* ctx);
  */
 celix_bundle_t* celix_bundleContext_getBundle(const celix_bundle_context_t *ctx);
 
+
+/**
+ * Returns the bundle if for the bundle of this bundle context.
+ */
+long celix_bundleContext_getBundleId(const celix_bundle_context_t *ctx);
+
 celix_framework_t* celix_bundleContext_getFramework(const celix_bundle_context_t* ctx);
 
+/**
+ * Logs a message to Celix framework logger with the provided log level.
+ * @param ctx       The bundle context
+ * @param level     The log level to use
+ * @param format    printf style format string
+ * @param ...       printf style format arguments
+ */
+void celix_bundleContext_log(const celix_bundle_context_t* ctx, celix_log_level_e level, const char* format, ...);

Review comment:
       :+1: 
   
   Can be used in some older pubsub files where a celix loghelper is not available, without refactoring everything.

##########
File path: libs/framework/include/celix_bundle_context.h
##########
@@ -177,6 +177,10 @@ typedef struct celix_service_registration_options {
     /**
     * Async callback. Will be called after the a service is registered in the service registry using a async call.
     * Will be called on the Celix event loop.
+     *
+     * If a asyns service registration is combined with a _sync_ service unregistration, it can happen that

Review comment:
       shift+r entire file `asyns` -> `async`.

##########
File path: libs/utils/src/utils.c
##########
@@ -61,6 +61,10 @@ bool celix_utils_stringEquals(const char* a, const char* b) {
     }
 }
 
+bool celix_utils_isStringNullOrEmpty(const char* s) {
+    return s == NULL || strlen(s) == 0;

Review comment:
       Probably better to replace this line with `return s == NULL || s[0] == '\0'`

##########
File path: libs/framework/include/celix/dm/DependencyManager_Impl.h
##########
@@ -88,3 +117,81 @@ inline std::size_t DependencyManager::getNrOfComponents() const {
     return celix_dependencyManager_nrOfComponents(cDepMan.get());
 }
 
+template<typename T>
+inline std::shared_ptr<Component<T>> DependencyManager::findComponent(const std::string& uuid) const {
+    std::lock_guard<std::mutex> lck{mutex};
+    std::shared_ptr<BaseComponent> found{nullptr};
+    for (const auto& cmp : components) {
+        if (cmp->getUUID() == uuid) {
+            found = cmp;

Review comment:
       add `break;`

##########
File path: libs/framework/src/dm_component_impl.c
##########
@@ -889,402 +689,158 @@ static celix_status_t component_calculateNewState(celix_dm_component_t *componen
     return status;
 }
 
-static celix_status_t component_performTransition(celix_dm_component_t *component, celix_dm_component_state_t oldState, celix_dm_component_state_t newState, bool *transition) {
-    celix_status_t status = CELIX_SUCCESS;
-    //printf("performing transition for %s in thread %i from %i to %i\n", component->name, (int) pthread_self(), oldState, newState);
-
+/**
+ * perform state transition. This function should be called with the component->mutex locked.
+ */
+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;
-    } else if (oldState == DM_CMP_STATE_INACTIVE && newState == DM_CMP_STATE_WAITING_FOR_REQUIRED) {
-        component_startDependencies(component, component->dependencies);
+        return CELIX_SUCCESS;
+    }
+
+    celix_bundleContext_log(component->context,
+           CELIX_LOG_LEVEL_TRACE,
+           "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_status_t status = CELIX_SUCCESS;
+    if (oldState == DM_CMP_STATE_INACTIVE && newState == 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) {
-        component_invokeAddRequiredDependencies(component);
-        component_invokeAutoConfigDependencies(component);
         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) {
-        component_invokeAddRequiredInstanceBoundDependencies(component);
-        component_invokeAutoConfigInstanceBoundDependencies(component);
-		component_invokeAddOptionalDependencies(component);
         if (component->callbackStart) {
         	status = component->callbackStart(component->implementation);
         }
-        component_registerServices(component);
+        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) {
-        component_unregisterServices(component);
+        celix_dmComponent_unregisterServices(component, false);
         if (component->callbackStop) {
         	status = component->callbackStop(component->implementation);
         }
-		component_invokeRemoveOptionalDependencies(component);
-        component_invokeRemoveInstanceBoundDependencies(component);
         *transition = true;
     } else if (oldState == DM_CMP_STATE_INSTANTIATED_AND_WAITING_FOR_REQUIRED && newState == DM_CMP_STATE_WAITING_FOR_REQUIRED) {
-    	if (component->callbackDeinit) {
-    		status = component->callbackDeinit(component->implementation);
-    	}
-        component_invokeRemoveRequiredDependencies(component);
+        if (component->callbackDeinit) {
+            status = component->callbackDeinit(component->implementation);
+        }
         *transition = true;
     } else if (oldState == DM_CMP_STATE_WAITING_FOR_REQUIRED && newState == DM_CMP_STATE_INACTIVE) {
-        component_stopDependencies(component);
+        celix_dmComponent_disableDependencies(component);
         *transition = true;
     }
 
     return status;
 }
 
-static celix_status_t component_allRequiredAvailable(celix_dm_component_t *component, bool *available) {
-    celix_status_t status = CELIX_SUCCESS;
-
-    pthread_mutex_lock(&component->mutex);
-    *available = true;
-    for (unsigned int i = 0; i < arrayList_size(component->dependencies); i++) {
-        celix_dm_service_dependency_t *dependency = arrayList_get(component->dependencies, i);
-        bool required = false;
-        bool instanceBound = false;
-
-        serviceDependency_isRequired(dependency, &required);
-        serviceDependency_isInstanceBound(dependency, &instanceBound);
-
-        if (required && !instanceBound) {
-            bool isAvailable = false;
-            serviceDependency_isAvailable(dependency, &isAvailable);
-            if (!isAvailable) {
-                *available = false;
-                break;
-            }
-        }
-    }
-    pthread_mutex_unlock(&component->mutex);
-
-    return status;
-}
-
-static celix_status_t component_allInstanceBoundAvailable(celix_dm_component_t *component, bool *available) {
-    celix_status_t status = CELIX_SUCCESS;
-
-    pthread_mutex_lock(&component->mutex);
-    *available = true;
-    for (unsigned int i = 0; i < arrayList_size(component->dependencies); i++) {
-        celix_dm_service_dependency_t *dependency = arrayList_get(component->dependencies, i);
-        bool required = false;
-        bool instanceBound = false;
-
-        serviceDependency_isRequired(dependency, &required);
-        serviceDependency_isInstanceBound(dependency, &instanceBound);
-
-        if (required && instanceBound) {
-            bool isAvailable = false;
-            serviceDependency_isAvailable(dependency, &isAvailable);
-            if (!isAvailable) {
-                *available = false;
-                break;
-            }
-        }
-    }
-    pthread_mutex_unlock(&component->mutex);
-
-    return status;
-}
-
-static celix_status_t component_invokeAddRequiredDependencies(celix_dm_component_t *component) {
-    celix_status_t status = CELIX_SUCCESS;
-
-    pthread_mutex_lock(&component->mutex);
-    for (unsigned int i = 0; i < arrayList_size(component->dependencies); i++) {
-        celix_dm_service_dependency_t *dependency = arrayList_get(component->dependencies, i);
-
-        bool required = false;
-        bool instanceBound = false;
-
-        serviceDependency_isRequired(dependency, &required);
-        serviceDependency_isInstanceBound(dependency, &instanceBound);
-
-        if (required && !instanceBound) {
-            array_list_pt events = hashMap_get(component->dependencyEvents, dependency);
-            if (events) {
-				for (unsigned int j = 0; j < arrayList_size(events); j++) {
-					dm_event_pt event = arrayList_get(events, j);
-					serviceDependency_invokeAdd(dependency, event);
-				}
-            }
-        }
-    }
-    pthread_mutex_unlock(&component->mutex);
-
-    return status;
-}
-
-static celix_status_t component_invokeAutoConfigDependencies(celix_dm_component_t *component) {
-    celix_status_t status = CELIX_SUCCESS;
-
-    pthread_mutex_lock(&component->mutex);
-    for (unsigned int i = 0; i < arrayList_size(component->dependencies); i++) {
-        celix_dm_service_dependency_t *dependency = arrayList_get(component->dependencies, i);
-
-        bool autoConfig = false;
-        bool instanceBound = false;
-
-        serviceDependency_isAutoConfig(dependency, &autoConfig);
-        serviceDependency_isInstanceBound(dependency, &instanceBound);
-
-        if (autoConfig && !instanceBound) {
-            component_configureImplementation(component, dependency);
+/**
+ * Check if all required dependencies are resolved. This function should be called with the component->mutex locked.
+ */
+static bool celix_dmComponent_areAllRequiredServiceDependenciesResolved(celix_dm_component_t *component) {
+    bool allResolved = true;
+    for (int i = 0; i < celix_arrayList_size(component->dependencies); i++) {
+        celix_dm_service_dependency_t *dependency = celix_arrayList_get(component->dependencies, i);
+        bool started = celix_dmServiceDependency_isTrackerOpen(dependency);
+        bool required = celix_dmServiceDependency_isRequired(dependency);
+        bool available = celix_dmServiceDependency_isAvailable(dependency);
+        if (!started) {
+            allResolved = false;
+            break;
         }
-    }
-    pthread_mutex_unlock(&component->mutex);
-
-    return status;
-}
-
-static celix_status_t component_invokeAutoConfigInstanceBoundDependencies(celix_dm_component_t *component) {
-    celix_status_t status = CELIX_SUCCESS;
-
-    pthread_mutex_lock(&component->mutex);
-    for (unsigned int i = 0; i < arrayList_size(component->dependencies); i++) {
-        celix_dm_service_dependency_t *dependency = arrayList_get(component->dependencies, i);
-
-        bool autoConfig = false;
-        bool instanceBound = false;
-
-        serviceDependency_isAutoConfig(dependency, &autoConfig);
-        serviceDependency_isInstanceBound(dependency, &instanceBound);
-
-        if (autoConfig && instanceBound) {
-            component_configureImplementation(component, dependency);
+        if (required && !available) {
+            allResolved = false;
+            break;
         }
     }
-    pthread_mutex_unlock(&component->mutex);
-
-    return status;
+    return allResolved;
 }
 
-static celix_status_t component_invokeAddRequiredInstanceBoundDependencies(celix_dm_component_t *component) {
-    celix_status_t status = CELIX_SUCCESS;
-
-    pthread_mutex_lock(&component->mutex);
-    for (unsigned int i = 0; i < arrayList_size(component->dependencies); i++) {
-        celix_dm_service_dependency_t *dependency = arrayList_get(component->dependencies, i);
-
-        bool required = false;
-        bool instanceBound = false;
-
-        serviceDependency_isRequired(dependency, &required);
-        serviceDependency_isInstanceBound(dependency, &instanceBound);
-
-        if (instanceBound && required) {
-            array_list_pt events = hashMap_get(component->dependencyEvents, dependency);
-            if (events) {
-				for (unsigned int j = 0; j < arrayList_size(events); j++) {
-					dm_event_pt event = arrayList_get(events, j);
-					serviceDependency_invokeAdd(dependency, event);
-				}
-            }
-        }
+/**
+ * Register component services (if not already registered).
+ * If needLock is false, this function should be called with the component->mutex locked.
+ */
+static celix_status_t celix_dmComponent_registerServices(celix_dm_component_t *component, bool needLock) {
+    if (needLock) {
+        celixThreadMutex_lock(&component->mutex);
     }
-    pthread_mutex_unlock(&component->mutex);
-
-    return status;
-}
-
-static celix_status_t component_invokeAddOptionalDependencies(celix_dm_component_t *component) {
-    celix_status_t status = CELIX_SUCCESS;
-
-    pthread_mutex_lock(&component->mutex);
-    for (unsigned int i = 0; i < arrayList_size(component->dependencies); i++) {
-        celix_dm_service_dependency_t *dependency = arrayList_get(component->dependencies, i);
-
-        bool required = false;
-
-        serviceDependency_isRequired(dependency, &required);
-
-        if (!required) {
-            array_list_pt events = hashMap_get(component->dependencyEvents, dependency);
-            if (events) {
-				for (unsigned int j = 0; j < arrayList_size(events); j++) {
-					dm_event_pt event = arrayList_get(events, j);
-					serviceDependency_invokeAdd(dependency, event);
-				}
-            }
-        }
+    for (int i = 0; i < celix_arrayList_size(component->providedInterfaces); i++) {
+        dm_interface_t *interface = arrayList_get(component->providedInterfaces, i);
+        if (interface->svcId == -1L) {
+            celix_properties_t *regProps = celix_properties_copy(interface->properties);
+            celix_service_registration_options_t opts = CELIX_EMPTY_SERVICE_REGISTRATION_OPTIONS;
+            opts.properties = regProps;
+            opts.svc = (void*)interface->service;
+            opts.serviceName = interface->serviceName;
+            opts.serviceLanguage = celix_properties_get(regProps, CELIX_FRAMEWORK_SERVICE_LANGUAGE, NULL);
+            celix_bundleContext_log(component->context, CELIX_LOG_LEVEL_TRACE,
+                   "Async registering service %s for component %s (uuid=%s)",
+                   interface->serviceName,
+                   component->name,
+                   component->uuid);
+            long svcId = celix_bundleContext_registerServiceWithOptionsAsync(component->context, &opts);
+            interface->svcId = svcId;
+        }
+    }
+    if (needLock) {
+        celixThreadMutex_unlock(&component->mutex);
     }
-    pthread_mutex_unlock(&component->mutex);
 
-    return status;
+    return CELIX_SUCCESS;
 }
 
-static celix_status_t component_invokeRemoveOptionalDependencies(celix_dm_component_t *component) {
+/**
+ * Unregister component services. This function should be called with the component->mutex locked.
+ * If needLock is false, this function should be called with the component->mutex locked.

Review comment:
       Add comment that this function unlocks mutex in case of found ids, even if `needLock` is false.

##########
File path: libs/framework/src/dm_component_impl.c
##########
@@ -416,466 +432,250 @@ celix_status_t celix_dmComponent_removeInterface(celix_dm_component_t *component
     celix_status_t status = CELIX_ILLEGAL_ARGUMENT;
 
     celixThreadMutex_lock(&component->mutex);
-    int nof_interfaces = arrayList_size(component->dm_interfaces);
-    for (unsigned int i = 0; i < nof_interfaces; ++i) {
-        dm_interface_t *interface = (dm_interface_t *) arrayList_get(component->dm_interfaces, i);
+    int nof_interfaces = celix_arrayList_size(component->providedInterfaces);
+    dm_interface_t* removedInterface = NULL;
+    for (int i = 0; i < nof_interfaces; ++i) {
+        dm_interface_t *interface = celix_arrayList_get(component->providedInterfaces, i);
         if (interface->service == service) {
-            arrayList_remove(component->dm_interfaces, i);
-            if (component->state == DM_CMP_STATE_TRACKING_OPTIONAL) {
-                celixThreadMutex_unlock(&component->mutex);
-                component_unregisterServices(component);
-                component_registerServices(component);
-                celixThreadMutex_lock(&component->mutex);
-            }
-            status = CELIX_SUCCESS;
+            celix_arrayList_removeAt(component->providedInterfaces, i);
+            removedInterface = interface;
             break;
         }
     }
     celixThreadMutex_unlock(&component->mutex);
 
+    if (removedInterface != NULL) {
+        celix_bundleContext_unregisterService(component->context, removedInterface->svcId);
+        free(removedInterface->serviceName);
+        free(removedInterface);
+    }
+
     return status;
 }
 
-celix_status_t component_getInterfaces(celix_dm_component_t *component, array_list_pt *out) {
+celix_status_t component_getInterfaces(celix_dm_component_t *component, celix_array_list_t **out) {
     return celix_dmComponent_getInterfaces(component, out);
 }
 
-celix_status_t celix_dmComponent_getInterfaces(celix_dm_component_t *component, array_list_pt *out) {
-    celix_status_t status = CELIX_SUCCESS;
-    array_list_pt names = NULL;
-    arrayList_create(&names);
+celix_status_t celix_dmComponent_getInterfaces(celix_dm_component_t *component, celix_array_list_t **out) {
+    celix_array_list_t* names = celix_arrayList_create();
+
     celixThreadMutex_lock(&component->mutex);
-    int size = arrayList_size(component->dm_interfaces);
-    int i;
-    for (i = 0; i < size; i += 1) {
-        dm_interface_info_pt info = calloc(1, sizeof(*info));
-        dm_interface_t *interface = arrayList_get(component->dm_interfaces, i);
-        info->name = strdup(interface->serviceName);
-        properties_copy(interface->properties, &info->properties);
-        arrayList_add(names, info);
+    int size = celix_arrayList_size(component->providedInterfaces);
+    for (int i = 0; i < size; i += 1) {
+        dm_interface_info_t* info = calloc(1, sizeof(*info));
+        dm_interface_t *interface = celix_arrayList_get(component->providedInterfaces, i);
+        info->name = celix_utils_strdup(interface->serviceName);
+        info->properties = celix_properties_copy(interface->properties);
+        celix_arrayList_add(names, info);
     }
     celixThreadMutex_unlock(&component->mutex);
+    *out = names;
 
-    if (status == CELIX_SUCCESS) {
-        *out = names;
-    }
-
-    return status;
-}
-
-celix_status_t celix_private_dmComponent_handleEvent(celix_dm_component_t *component, celix_dm_service_dependency_t *dependency, dm_event_pt event) {
-	celix_status_t status = CELIX_SUCCESS;
-
-	dm_handle_event_type_pt data = calloc(1, sizeof(*data));
-	data->dependency = dependency;
-	data->event = event;
-	data->newEvent = NULL;
-
-	status = executor_executeTask(component->executor, component, component_handleEventTask, data);
-//	component_handleEventTask(component, data);
-
-	return status;
+    return CELIX_SUCCESS;
 }
 
-static celix_status_t component_handleEventTask(celix_dm_component_t *component, dm_handle_event_type_pt data) {
-	celix_status_t status = CELIX_SUCCESS;
-
-	switch (data->event->event_type) {
-		case DM_EVENT_ADDED:
-			component_handleAdded(component,data->dependency, data->event);
-			break;
-		case DM_EVENT_CHANGED:
-			component_handleChanged(component,data->dependency, data->event);
-			break;
-		case DM_EVENT_REMOVED:
-			component_handleRemoved(component,data->dependency, data->event);
-			break;
-		case DM_EVENT_SWAPPED:
-			component_handleSwapped(component,data->dependency, data->event, data->newEvent);
-			break;
-		default:
-			break;
-	}
-
-	free(data);
-
-	return status;
+celix_status_t celix_private_dmComponent_handleEvent(celix_dm_component_t *component, const celix_dm_event_t* event) {
+    celix_status_t status = CELIX_SUCCESS;
+    switch (event->eventType) {
+        case CELIX_DM_EVENT_SVC_ADD:
+            celix_dmComponent_handleAdd(component, event);
+            break;
+        case CELIX_DM_EVENT_SVC_REM:
+            celix_dmComponent_handleRemove(component, event);
+            break;
+        case CELIX_DM_EVENT_SVC_SET:
+            celix_dmComponent_handleSet(component, event);
+            break;
+        default:
+            break;
+    }
+    return status;
 }
 
-static celix_status_t component_suspend(celix_dm_component_t *component, celix_dm_service_dependency_t *dependency) {
+static celix_status_t celix_dmComponent_suspend(celix_dm_component_t *component, celix_dm_service_dependency_t *dependency) {
 	celix_status_t status = CELIX_SUCCESS;
-
-	dm_service_dependency_strategy_t strategy;
-	serviceDependency_getStrategy(dependency, &strategy);
+	dm_service_dependency_strategy_t strategy = celix_dmServiceDependency_getStrategy(dependency);
 	if (strategy == DM_SERVICE_DEPENDENCY_STRATEGY_SUSPEND &&  component->callbackStop != NULL) {
+        celix_bundleContext_log(component->context, CELIX_LOG_LEVEL_TRACE,
+               "Suspending component %s (uuid=%s)",
+               component->name,
+               component->uuid);
+        celix_dmComponent_unregisterServices(component, true);
 		status = component->callbackStop(component->implementation);
 	}
-
 	return status;
 }
 
-static celix_status_t component_resume(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) {
 	celix_status_t status = CELIX_SUCCESS;
-
-	dm_service_dependency_strategy_t strategy;
-	serviceDependency_getStrategy(dependency, &strategy);
-	if (strategy == DM_SERVICE_DEPENDENCY_STRATEGY_SUSPEND &&  component->callbackStop != NULL) {
-		status = component->callbackStart(component->implementation);
+    dm_service_dependency_strategy_t strategy = celix_dmServiceDependency_getStrategy(dependency);
+	if (strategy == DM_SERVICE_DEPENDENCY_STRATEGY_SUSPEND &&  component->callbackStart != NULL) {
+	    //ensure that the current state is still TRACKING_OPTION. Else during a add/rem/set call a required svc dep has been added.
+        celixThreadMutex_lock(&component->mutex);
+        celix_dm_component_state_t currentState = component->state;
+        celixThreadMutex_unlock(&component->mutex);
+        if (currentState == DM_CMP_STATE_TRACKING_OPTIONAL) {
+            celix_bundleContext_log(component->context, CELIX_LOG_LEVEL_TRACE,
+                                    "Resuming component %s (uuid=%s)",
+                                    component->name,
+                                    component->uuid);
+            celix_dmComponent_registerServices(component, true);
+            status = component->callbackStart(component->implementation);
+            component->nrOfTimesResumed += 1;
+        } else {
+            celix_bundleContext_log(component->context, CELIX_LOG_LEVEL_TRACE,
+                                    "Skipping resume because cmp state is now %s",
+                                    celix_dmComponent_stateToString(currentState));
+        }
 	}
-
 	return status;
 }
 
-static celix_status_t component_handleAdded(celix_dm_component_t *component, celix_dm_service_dependency_t *dependency, dm_event_pt event) {
-    celix_status_t status = CELIX_SUCCESS;
-
-    pthread_mutex_lock(&component->mutex);
-    array_list_pt events = hashMap_get(component->dependencyEvents, dependency);
-    arrayList_add(events, event);
-    pthread_mutex_unlock(&component->mutex);
-
-    serviceDependency_setAvailable(dependency, true);
-
+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) {
+    bool needSuspend = false;
+    celixThreadMutex_lock(&component->mutex);
     switch (component->state) {
-        case DM_CMP_STATE_WAITING_FOR_REQUIRED: {
-            serviceDependency_invokeSet(dependency, event);
-            bool required = false;
-            serviceDependency_isRequired(dependency, &required);
-            if (required) {
-                component_handleChange(component);
-            }
-            break;
-        }
-        case DM_CMP_STATE_INSTANTIATED_AND_WAITING_FOR_REQUIRED: {
-            bool instanceBound = false;
-            serviceDependency_isInstanceBound(dependency, &instanceBound);
-            bool required = false;
-            serviceDependency_isRequired(dependency, &required);
-            if (!instanceBound) {
-                if (required) {
-                    serviceDependency_invokeSet(dependency, event);
-                    serviceDependency_invokeAdd(dependency, event);
-                }
-                dm_event_pt event = NULL;
-                component_getDependencyEvent(component, dependency, &event);
-                component_updateInstance(component, dependency, event, false, true);
-            }
-
-            if (required) {
-                component_handleChange(component);
-            }
-            break;
-        }
         case DM_CMP_STATE_TRACKING_OPTIONAL:
-		    component_suspend(component,dependency);
-            serviceDependency_invokeSet(dependency, event);
-            serviceDependency_invokeAdd(dependency, event);
-		    component_resume(component,dependency);
-            dm_event_pt event = NULL;
-            component_getDependencyEvent(component, dependency, &event);
-            component_updateInstance(component, dependency, event, false, true);
+            needSuspend = true;
             break;
-        default:
+        default: //DM_CMP_STATE_INACTIVE
             break;
     }
-
-    return status;
-}
-
-static celix_status_t component_handleChanged(celix_dm_component_t *component, celix_dm_service_dependency_t *dependency, dm_event_pt event) {
-    celix_status_t status = CELIX_SUCCESS;
-
-    pthread_mutex_lock(&component->mutex);
-    array_list_pt events = hashMap_get(component->dependencyEvents, dependency);
-    int index = arrayList_indexOf(events, event);
-    if (index < 0) {
-	pthread_mutex_unlock(&component->mutex);
-        status = CELIX_BUNDLE_EXCEPTION;
-    } else {
-        dm_event_pt old = arrayList_remove(events, (unsigned int) index);
-        arrayList_add(events, event);
-        pthread_mutex_unlock(&component->mutex);
-
-        serviceDependency_invokeSet(dependency, event);
-        switch (component->state) {
-            case DM_CMP_STATE_TRACKING_OPTIONAL:
-			    component_suspend(component,dependency);
-                serviceDependency_invokeChange(dependency, event);
-			    component_resume(component,dependency);
-                dm_event_pt hevent = NULL;
-                component_getDependencyEvent(component, dependency, &hevent);
-                component_updateInstance(component, dependency, hevent, true, false);
-                break;
-            case DM_CMP_STATE_INSTANTIATED_AND_WAITING_FOR_REQUIRED: {
-                bool instanceBound = false;
-                serviceDependency_isInstanceBound(dependency, &instanceBound);
-                if (!instanceBound) {
-                    serviceDependency_invokeChange(dependency, event);
-                    dm_event_pt hevent = NULL;
-                    component_getDependencyEvent(component, dependency, &hevent);
-                    component_updateInstance(component, dependency, hevent, true, false);
-                }
-                break;
-            }
-            default:
-                break;
-        }
-
-        event_destroy(&old);
-    }
-
-    return status;
+    celixThreadMutex_unlock(&component->mutex);
+    if (needSuspend) {
+        celix_dmComponent_suspend(component, event->dep);
+    }
+    celix_bundleContext_log(component->context, CELIX_LOG_LEVEL_TRACE,
+        "Calling %s service for component %s (uuid=%s) on service dependency with type %s",
+        invokeName,
+        component->name,
+        component->uuid,
+        event->dep->serviceName);
+    setAddOrRemFp(event->dep, event->svc, event->props);
+    //note that after after a set/add/rem a new svc dependency can be added, removed, etc
+    if (needSuspend) {
+        celix_dmComponent_resume(component, event->dep);
+    }
+    celix_dmComponent_handleChange(component);
+    return CELIX_SUCCESS;
 }
 
-static celix_status_t component_handleRemoved(celix_dm_component_t *component, celix_dm_service_dependency_t *dependency, dm_event_pt event) {
-    celix_status_t status = CELIX_SUCCESS;
-
-    pthread_mutex_lock(&component->mutex);
-    array_list_pt events = hashMap_get(component->dependencyEvents, dependency);
-    int size = arrayList_size(events);
-    if (arrayList_contains(events, event)) {
-        size--;
-    }
-    pthread_mutex_unlock(&component->mutex);
-    serviceDependency_setAvailable(dependency, size > 0);
-    component_handleChange(component);
-
-    pthread_mutex_lock(&component->mutex);
-    int index = arrayList_indexOf(events, event);
-    if (index < 0) {
-	pthread_mutex_unlock(&component->mutex);
-        status = CELIX_BUNDLE_EXCEPTION;
-    } else {
-        dm_event_pt old = arrayList_remove(events, (unsigned int) index);
-        pthread_mutex_unlock(&component->mutex);
-
-
-        switch (component->state) {
-            case DM_CMP_STATE_INSTANTIATED_AND_WAITING_FOR_REQUIRED: {
-                serviceDependency_invokeSet(dependency, event);
-                bool instanceBound = false;
-                serviceDependency_isInstanceBound(dependency, &instanceBound);
-                if (!instanceBound) {
-                    bool required = false;
-                    serviceDependency_isRequired(dependency, &required);
-                    if (required) {
-                        serviceDependency_invokeRemove(dependency, event);
-                    }
-                    dm_event_pt hevent = NULL;
-                    component_getDependencyEvent(component, dependency, &hevent);
-                    component_updateInstance(component, dependency, hevent, false, false);
-                }
-                break;
-            }
-            case DM_CMP_STATE_TRACKING_OPTIONAL:
-			    component_suspend(component,dependency);
-                serviceDependency_invokeSet(dependency, event);
-                serviceDependency_invokeRemove(dependency, event);
-			    component_resume(component,dependency);
-                dm_event_pt hevent = NULL;
-                component_getDependencyEvent(component, dependency, &hevent);
-                component_updateInstance(component, dependency, hevent, false, false);
-                break;
-            default:
-                break;
-        }
-
-        event_destroy(&event);
-        if (old) {
-            event_destroy(&old);
-        }
-    }
-
-    return status;
+static celix_status_t celix_dmComponent_handleAdd(celix_dm_component_t *component, const celix_dm_event_t* event) {
+    return celix_dmComponent_handleEvent(component, event, celix_dmServiceDependency_invokeAdd, "add");
 }
 
-static celix_status_t component_handleSwapped(celix_dm_component_t *component, celix_dm_service_dependency_t *dependency, dm_event_pt event, dm_event_pt newEvent) {
-    celix_status_t status = CELIX_SUCCESS;
-
-    pthread_mutex_lock(&component->mutex);
-    array_list_pt events = hashMap_get(component->dependencyEvents, dependency);
-    int index = arrayList_indexOf(events, event);
-    if (index < 0) {
-	pthread_mutex_unlock(&component->mutex);
-        status = CELIX_BUNDLE_EXCEPTION;
-    } else {
-        dm_event_pt old = arrayList_remove(events, (unsigned int) index);
-        arrayList_add(events, newEvent);
-        pthread_mutex_unlock(&component->mutex);
-
-        serviceDependency_invokeSet(dependency, event);
-
-        switch (component->state) {
-            case DM_CMP_STATE_WAITING_FOR_REQUIRED:
-                break;
-            case DM_CMP_STATE_INSTANTIATED_AND_WAITING_FOR_REQUIRED: {
-                bool instanceBound = false;
-                serviceDependency_isInstanceBound(dependency, &instanceBound);
-                if (!instanceBound) {
-                    bool required = false;
-                    serviceDependency_isRequired(dependency, &required);
-                    if (required) {
-                        serviceDependency_invokeSwap(dependency, event, newEvent);
-                    }
-                }
-                break;
-            }
-            case DM_CMP_STATE_TRACKING_OPTIONAL:
-			    component_suspend(component,dependency);
-                serviceDependency_invokeSwap(dependency, event, newEvent);
-			    component_resume(component,dependency);
-                break;
-            default:
-                break;
-        }
-
-        event_destroy(&event);
-        if (old) {
-            event_destroy(&old);
-        }
-    }
-
-    return status;
+static celix_status_t celix_dmComponent_handleRemove(celix_dm_component_t *component, const celix_dm_event_t* event) {
+    return celix_dmComponent_handleEvent(component, event, celix_dmServiceDependency_invokeRemove, "remove");
 }
 
-static celix_status_t component_updateInstance(celix_dm_component_t *component, celix_dm_service_dependency_t *dependency, dm_event_pt event, bool update, bool add) {
-    celix_status_t status = CELIX_SUCCESS;
-
-    bool autoConfig = false;
-
-    serviceDependency_isAutoConfig(dependency, &autoConfig);
-
-    if (autoConfig) {
-        const void *service = NULL;
-        const void **field = NULL;
-
-        if (event != NULL) {
-            event_getService(event, &service);
-        }
-        serviceDependency_getAutoConfig(dependency, &field);
-        serviceDependency_lock(dependency);
-        *field = service;
-        serviceDependency_unlock(dependency);
-    }
-
-    return status;
+static celix_status_t celix_dmComponent_handleSet(celix_dm_component_t *component, const celix_dm_event_t* event) {
+    return celix_dmComponent_handleEvent(component, event, celix_dmServiceDependency_invokeSet, "set");
 }
 
-static celix_status_t component_startDependencies(celix_dm_component_t *component __attribute__((unused)), array_list_pt dependencies) {
-    celix_status_t status = CELIX_SUCCESS;
-    array_list_pt required_dependencies = NULL;
-    arrayList_create(&required_dependencies);
-
-    for (unsigned int i = 0; i < arrayList_size(dependencies); i++) {
-        celix_dm_service_dependency_t *dependency = arrayList_get(dependencies, i);
-        bool required = false;
-        serviceDependency_isRequired(dependency, &required);
-        if (required) {
-            arrayList_add(required_dependencies, dependency);
-            continue;
-        }
-
-        serviceDependency_start(dependency);
-    }
-
-    for (unsigned int i = 0; i < arrayList_size(required_dependencies); i++) {
-        celix_dm_service_dependency_t *dependency = arrayList_get(required_dependencies, i);
-        serviceDependency_start(dependency);
+/**
+ * perform state transition. This function should be called with the component->mutex locked.
+ */
+static celix_status_t celix_dmComponent_enableDependencies(celix_dm_component_t *component) {
+    for (int i = 0; i < celix_arrayList_size(component->dependencies); i++) {
+        celix_dm_service_dependency_t *dependency = arrayList_get(component->dependencies, i);
+        celix_dmServiceDependency_enable(dependency);
     }
-
-    arrayList_destroy(required_dependencies);
-
-    return status;
+    return CELIX_SUCCESS;
 }
 
-static celix_status_t component_stopDependencies(celix_dm_component_t *component) {
-    celix_status_t status = CELIX_SUCCESS;
-
-    pthread_mutex_lock(&component->mutex);
-    for (unsigned int i = 0; i < arrayList_size(component->dependencies); i++) {
+/**
+ * perform state transition. This function should be called with the component->mutex locked.
+ */
+static celix_status_t celix_dmComponent_disableDependencies(celix_dm_component_t *component) {
+    for (int i = 0; i < celix_arrayList_size(component->dependencies); i++) {
         celix_dm_service_dependency_t *dependency = arrayList_get(component->dependencies, i);
-        pthread_mutex_unlock(&component->mutex);
-        serviceDependency_stop(dependency);
-        pthread_mutex_lock(&component->mutex);
+        celix_dmServiceDependency_disable(dependency);
     }
-    pthread_mutex_unlock(&component->mutex);
-
-    return status;
+    return CELIX_SUCCESS;
 }
 
-static celix_status_t component_handleChange(celix_dm_component_t *component) {
-    celix_status_t status = CELIX_SUCCESS;
 
+/**
+ * Calculate and handle state change. This function should be called with the component->mutex locked.

Review comment:
       Comment says that mutex should already be locked. But function also locks mutex. One of the two is incorrect.

##########
File path: libs/framework/src/dm_component_impl.c
##########
@@ -889,402 +689,158 @@ static celix_status_t component_calculateNewState(celix_dm_component_t *componen
     return status;
 }
 
-static celix_status_t component_performTransition(celix_dm_component_t *component, celix_dm_component_state_t oldState, celix_dm_component_state_t newState, bool *transition) {
-    celix_status_t status = CELIX_SUCCESS;
-    //printf("performing transition for %s in thread %i from %i to %i\n", component->name, (int) pthread_self(), oldState, newState);
-
+/**
+ * perform state transition. This function should be called with the component->mutex locked.
+ */
+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;
-    } else if (oldState == DM_CMP_STATE_INACTIVE && newState == DM_CMP_STATE_WAITING_FOR_REQUIRED) {
-        component_startDependencies(component, component->dependencies);
+        return CELIX_SUCCESS;
+    }
+
+    celix_bundleContext_log(component->context,
+           CELIX_LOG_LEVEL_TRACE,
+           "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_status_t status = CELIX_SUCCESS;
+    if (oldState == DM_CMP_STATE_INACTIVE && newState == 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) {
-        component_invokeAddRequiredDependencies(component);
-        component_invokeAutoConfigDependencies(component);
         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) {
-        component_invokeAddRequiredInstanceBoundDependencies(component);
-        component_invokeAutoConfigInstanceBoundDependencies(component);
-		component_invokeAddOptionalDependencies(component);
         if (component->callbackStart) {
         	status = component->callbackStart(component->implementation);
         }
-        component_registerServices(component);
+        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) {
-        component_unregisterServices(component);
+        celix_dmComponent_unregisterServices(component, false);
         if (component->callbackStop) {
         	status = component->callbackStop(component->implementation);
         }
-		component_invokeRemoveOptionalDependencies(component);
-        component_invokeRemoveInstanceBoundDependencies(component);
         *transition = true;
     } else if (oldState == DM_CMP_STATE_INSTANTIATED_AND_WAITING_FOR_REQUIRED && newState == DM_CMP_STATE_WAITING_FOR_REQUIRED) {
-    	if (component->callbackDeinit) {
-    		status = component->callbackDeinit(component->implementation);
-    	}
-        component_invokeRemoveRequiredDependencies(component);
+        if (component->callbackDeinit) {
+            status = component->callbackDeinit(component->implementation);
+        }
         *transition = true;
     } else if (oldState == DM_CMP_STATE_WAITING_FOR_REQUIRED && newState == DM_CMP_STATE_INACTIVE) {
-        component_stopDependencies(component);
+        celix_dmComponent_disableDependencies(component);
         *transition = true;
     }
 
     return status;
 }
 
-static celix_status_t component_allRequiredAvailable(celix_dm_component_t *component, bool *available) {
-    celix_status_t status = CELIX_SUCCESS;
-
-    pthread_mutex_lock(&component->mutex);
-    *available = true;
-    for (unsigned int i = 0; i < arrayList_size(component->dependencies); i++) {
-        celix_dm_service_dependency_t *dependency = arrayList_get(component->dependencies, i);
-        bool required = false;
-        bool instanceBound = false;
-
-        serviceDependency_isRequired(dependency, &required);
-        serviceDependency_isInstanceBound(dependency, &instanceBound);
-
-        if (required && !instanceBound) {
-            bool isAvailable = false;
-            serviceDependency_isAvailable(dependency, &isAvailable);
-            if (!isAvailable) {
-                *available = false;
-                break;
-            }
-        }
-    }
-    pthread_mutex_unlock(&component->mutex);
-
-    return status;
-}
-
-static celix_status_t component_allInstanceBoundAvailable(celix_dm_component_t *component, bool *available) {
-    celix_status_t status = CELIX_SUCCESS;
-
-    pthread_mutex_lock(&component->mutex);
-    *available = true;
-    for (unsigned int i = 0; i < arrayList_size(component->dependencies); i++) {
-        celix_dm_service_dependency_t *dependency = arrayList_get(component->dependencies, i);
-        bool required = false;
-        bool instanceBound = false;
-
-        serviceDependency_isRequired(dependency, &required);
-        serviceDependency_isInstanceBound(dependency, &instanceBound);
-
-        if (required && instanceBound) {
-            bool isAvailable = false;
-            serviceDependency_isAvailable(dependency, &isAvailable);
-            if (!isAvailable) {
-                *available = false;
-                break;
-            }
-        }
-    }
-    pthread_mutex_unlock(&component->mutex);
-
-    return status;
-}
-
-static celix_status_t component_invokeAddRequiredDependencies(celix_dm_component_t *component) {
-    celix_status_t status = CELIX_SUCCESS;
-
-    pthread_mutex_lock(&component->mutex);
-    for (unsigned int i = 0; i < arrayList_size(component->dependencies); i++) {
-        celix_dm_service_dependency_t *dependency = arrayList_get(component->dependencies, i);
-
-        bool required = false;
-        bool instanceBound = false;
-
-        serviceDependency_isRequired(dependency, &required);
-        serviceDependency_isInstanceBound(dependency, &instanceBound);
-
-        if (required && !instanceBound) {
-            array_list_pt events = hashMap_get(component->dependencyEvents, dependency);
-            if (events) {
-				for (unsigned int j = 0; j < arrayList_size(events); j++) {
-					dm_event_pt event = arrayList_get(events, j);
-					serviceDependency_invokeAdd(dependency, event);
-				}
-            }
-        }
-    }
-    pthread_mutex_unlock(&component->mutex);
-
-    return status;
-}
-
-static celix_status_t component_invokeAutoConfigDependencies(celix_dm_component_t *component) {
-    celix_status_t status = CELIX_SUCCESS;
-
-    pthread_mutex_lock(&component->mutex);
-    for (unsigned int i = 0; i < arrayList_size(component->dependencies); i++) {
-        celix_dm_service_dependency_t *dependency = arrayList_get(component->dependencies, i);
-
-        bool autoConfig = false;
-        bool instanceBound = false;
-
-        serviceDependency_isAutoConfig(dependency, &autoConfig);
-        serviceDependency_isInstanceBound(dependency, &instanceBound);
-
-        if (autoConfig && !instanceBound) {
-            component_configureImplementation(component, dependency);
+/**
+ * Check if all required dependencies are resolved. This function should be called with the component->mutex locked.
+ */
+static bool celix_dmComponent_areAllRequiredServiceDependenciesResolved(celix_dm_component_t *component) {
+    bool allResolved = true;
+    for (int i = 0; i < celix_arrayList_size(component->dependencies); i++) {
+        celix_dm_service_dependency_t *dependency = celix_arrayList_get(component->dependencies, i);
+        bool started = celix_dmServiceDependency_isTrackerOpen(dependency);
+        bool required = celix_dmServiceDependency_isRequired(dependency);
+        bool available = celix_dmServiceDependency_isAvailable(dependency);
+        if (!started) {
+            allResolved = false;
+            break;
         }
-    }
-    pthread_mutex_unlock(&component->mutex);
-
-    return status;
-}
-
-static celix_status_t component_invokeAutoConfigInstanceBoundDependencies(celix_dm_component_t *component) {
-    celix_status_t status = CELIX_SUCCESS;
-
-    pthread_mutex_lock(&component->mutex);
-    for (unsigned int i = 0; i < arrayList_size(component->dependencies); i++) {
-        celix_dm_service_dependency_t *dependency = arrayList_get(component->dependencies, i);
-
-        bool autoConfig = false;
-        bool instanceBound = false;
-
-        serviceDependency_isAutoConfig(dependency, &autoConfig);
-        serviceDependency_isInstanceBound(dependency, &instanceBound);
-
-        if (autoConfig && instanceBound) {
-            component_configureImplementation(component, dependency);
+        if (required && !available) {
+            allResolved = false;
+            break;
         }
     }
-    pthread_mutex_unlock(&component->mutex);
-
-    return status;
+    return allResolved;
 }
 
-static celix_status_t component_invokeAddRequiredInstanceBoundDependencies(celix_dm_component_t *component) {
-    celix_status_t status = CELIX_SUCCESS;
-
-    pthread_mutex_lock(&component->mutex);
-    for (unsigned int i = 0; i < arrayList_size(component->dependencies); i++) {
-        celix_dm_service_dependency_t *dependency = arrayList_get(component->dependencies, i);
-
-        bool required = false;
-        bool instanceBound = false;
-
-        serviceDependency_isRequired(dependency, &required);
-        serviceDependency_isInstanceBound(dependency, &instanceBound);
-
-        if (instanceBound && required) {
-            array_list_pt events = hashMap_get(component->dependencyEvents, dependency);
-            if (events) {
-				for (unsigned int j = 0; j < arrayList_size(events); j++) {
-					dm_event_pt event = arrayList_get(events, j);
-					serviceDependency_invokeAdd(dependency, event);
-				}
-            }
-        }
+/**
+ * Register component services (if not already registered).
+ * If needLock is false, this function should be called with the component->mutex locked.
+ */
+static celix_status_t celix_dmComponent_registerServices(celix_dm_component_t *component, bool needLock) {
+    if (needLock) {
+        celixThreadMutex_lock(&component->mutex);
     }
-    pthread_mutex_unlock(&component->mutex);
-
-    return status;
-}
-
-static celix_status_t component_invokeAddOptionalDependencies(celix_dm_component_t *component) {
-    celix_status_t status = CELIX_SUCCESS;
-
-    pthread_mutex_lock(&component->mutex);
-    for (unsigned int i = 0; i < arrayList_size(component->dependencies); i++) {
-        celix_dm_service_dependency_t *dependency = arrayList_get(component->dependencies, i);
-
-        bool required = false;
-
-        serviceDependency_isRequired(dependency, &required);
-
-        if (!required) {
-            array_list_pt events = hashMap_get(component->dependencyEvents, dependency);
-            if (events) {
-				for (unsigned int j = 0; j < arrayList_size(events); j++) {
-					dm_event_pt event = arrayList_get(events, j);
-					serviceDependency_invokeAdd(dependency, event);
-				}
-            }
-        }
+    for (int i = 0; i < celix_arrayList_size(component->providedInterfaces); i++) {
+        dm_interface_t *interface = arrayList_get(component->providedInterfaces, i);
+        if (interface->svcId == -1L) {
+            celix_properties_t *regProps = celix_properties_copy(interface->properties);
+            celix_service_registration_options_t opts = CELIX_EMPTY_SERVICE_REGISTRATION_OPTIONS;
+            opts.properties = regProps;
+            opts.svc = (void*)interface->service;
+            opts.serviceName = interface->serviceName;
+            opts.serviceLanguage = celix_properties_get(regProps, CELIX_FRAMEWORK_SERVICE_LANGUAGE, NULL);
+            celix_bundleContext_log(component->context, CELIX_LOG_LEVEL_TRACE,
+                   "Async registering service %s for component %s (uuid=%s)",
+                   interface->serviceName,
+                   component->name,
+                   component->uuid);
+            long svcId = celix_bundleContext_registerServiceWithOptionsAsync(component->context, &opts);
+            interface->svcId = svcId;
+        }
+    }
+    if (needLock) {
+        celixThreadMutex_unlock(&component->mutex);
     }
-    pthread_mutex_unlock(&component->mutex);
 
-    return status;
+    return CELIX_SUCCESS;
 }
 
-static celix_status_t component_invokeRemoveOptionalDependencies(celix_dm_component_t *component) {
+/**
+ * Unregister component services. This function should be called with the component->mutex locked.

Review comment:
       Redundant sentence about locking mutex, `needLock` comment is clear enough.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org