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/01/11 15:18:07 UTC
[celix] 08/08: Improves svc dep callback handle to prevent
deadlocks and adds a check to filter out svc dependency to cmp own provided
services
This is an automated email from the ASF dual-hosted git repository.
pnoltes pushed a commit to branch feature/refactor_c_dep_man_service_trackers
in repository https://gitbox.apache.org/repos/asf/celix.git
commit 2dcf74bdfb6a95e7b3ac5700e9f958ac94f742af
Author: Pepijn Noltes <pe...@gmail.com>
AuthorDate: Mon Jan 11 16:16:48 2021 +0100
Improves svc dep callback handle to prevent deadlocks and adds a check to filter out svc dependency to cmp own provided services
---
.../gtest/src/tst_activator.c | 6 +-
.../gtest/src/bundle_context_services_test.cpp | 86 +++++++++++++
libs/framework/src/bundle_context.c | 56 +++++++--
libs/framework/src/bundle_context_private.h | 8 +-
libs/framework/src/dm_component_impl.c | 133 ++++++++++++---------
libs/framework/src/dm_service_dependency.c | 47 +++++---
libs/framework/src/dm_service_dependency_impl.h | 17 ++-
libs/framework/src/framework.c | 40 ++++++-
libs/framework/src/framework_private.h | 1 +
libs/framework/src/service_registry.c | 12 +-
10 files changed, 310 insertions(+), 96 deletions(-)
diff --git a/bundles/remote_services/remote_service_admin_dfi/gtest/src/tst_activator.c b/bundles/remote_services/remote_service_admin_dfi/gtest/src/tst_activator.c
index 0f1cbb9..eae5a1b 100644
--- a/bundles/remote_services/remote_service_admin_dfi/gtest/src/tst_activator.c
+++ b/bundles/remote_services/remote_service_admin_dfi/gtest/src/tst_activator.c
@@ -278,9 +278,9 @@ static bool bndTestCreateDestroyComponentWithRemoteService(void *handle) {
calculator_service_t calcSvc;
calcSvc.handle = NULL;
- calcSvc.add = NULL; //note for this test case the actual services can be NULL
- calcSvc.sub = NULL; //note for this test case the actual services can be NULL
- calcSvc.sqrt = NULL; //note for this test case the actual services can be NULL
+ calcSvc.add = NULL; //note for this test case the actual service methods can be NULL
+ calcSvc.sub = NULL; //note for this test case the actual service methods can be NULL
+ calcSvc.sqrt = NULL; //note for this test case the actual service methods can be NULL
celix_dm_component_t *cmp = celix_dmComponent_create(act->ctx, "test");
celix_dmComponent_addInterface(cmp, CALCULATOR_SERVICE, NULL, &calcSvc, properties);
diff --git a/libs/framework/gtest/src/bundle_context_services_test.cpp b/libs/framework/gtest/src/bundle_context_services_test.cpp
index b19fbe9..c12fc7b 100644
--- a/libs/framework/gtest/src/bundle_context_services_test.cpp
+++ b/libs/framework/gtest/src/bundle_context_services_test.cpp
@@ -1193,4 +1193,90 @@ TEST_F(CelixBundleContextServicesTests, onlyCallAsyncCallbackWithAsyncApi) {
trkId = celix_bundleContext_trackBundlesWithOptions(ctx, &opts3);
EXPECT_GT(trkId, 0);
celix_bundleContext_stopTracker(ctx, trkId);
+}
+
+TEST_F(CelixBundleContextServicesTests, unregisterSvcBeforeAsyncRegistration) {
+ celix_framework_fireGenericEvent(
+ fw,
+ -1,
+ celix_bundle_getId(celix_framework_getFrameworkBundle(fw)),
+ "registerAsync",
+ (void*)ctx,
+ [](void *data) {
+ auto context = (celix_bundle_context_t*)data;
+
+ //note register async. So a event on the event queue, but because this is done on the event queue this cannot be completed
+ long svcId = celix_bundleContext_registerServiceAsync(context, (void*)0x42, "test-service", nullptr);
+
+ celix_bundleContext_unregisterService(context, svcId); //trying to unregister still queued svc registration -> should cancel event.
+ },
+ nullptr,
+ nullptr);
+
+ celix_bundleContext_waitForEvents(ctx);
+ long svcId = celix_bundleContext_findService(ctx, "test-service");
+ EXPECT_LT(svcId, 0);
+}
+
+TEST_F(CelixBundleContextServicesTests, stopSvcTrackerBeforeAsyncTrackerIsCreated) {
+ celix_framework_fireGenericEvent(
+ fw,
+ -1,
+ celix_bundle_getId(celix_framework_getFrameworkBundle(fw)),
+ "create tracker async",
+ (void*)ctx,
+ [](void *data) {
+ auto context = (celix_bundle_context_t*)data;
+
+ //note register async. So a event on the event queue, but because this is done on the event queue this cannot be completed
+ long trkId = celix_bundleContext_trackServicesAsync(context, "test-service", nullptr, nullptr,nullptr);
+
+ celix_bundleContext_stopTracker(context, trkId);
+ },
+ nullptr,
+ nullptr);
+
+ celix_bundleContext_waitForEvents(ctx);
+}
+
+TEST_F(CelixBundleContextServicesTests, stopBundleTrackerBeforeAsyncTrackerIsCreated) {
+ celix_framework_fireGenericEvent(
+ fw,
+ -1,
+ celix_bundle_getId(celix_framework_getFrameworkBundle(fw)),
+ "create tracker async",
+ (void*)ctx,
+ [](void *data) {
+ auto context = (celix_bundle_context_t*)data;
+
+ //note register async. So a event on the event queue, but because this is done on the event queue this cannot be completed
+ long trkId = celix_bundleContext_trackBundlesAsync(context, nullptr, nullptr,nullptr);
+
+ celix_bundleContext_stopTracker(context, trkId);
+ },
+ nullptr,
+ nullptr);
+
+ celix_bundleContext_waitForEvents(ctx);
+}
+
+TEST_F(CelixBundleContextServicesTests, stopMetaTrackerBeforeAsyncTrackerIsCreated) {
+ celix_framework_fireGenericEvent(
+ fw,
+ -1,
+ celix_bundle_getId(celix_framework_getFrameworkBundle(fw)),
+ "create tracker async",
+ (void*)ctx,
+ [](void *data) {
+ auto context = (celix_bundle_context_t*)data;
+
+ //note register async. So a event on the event queue, but because this is done on the event queue this cannot be completed
+ long trkId = celix_bundleContext_trackServiceTrackers(context, "test-service", nullptr, nullptr,nullptr);
+
+ celix_bundleContext_stopTracker(context, trkId);
+ },
+ nullptr,
+ nullptr);
+
+ celix_bundleContext_waitForEvents(ctx);
}
\ No newline at end of file
diff --git a/libs/framework/src/bundle_context.c b/libs/framework/src/bundle_context.c
index 2f1473d..ce4d4ce 100644
--- a/libs/framework/src/bundle_context.c
+++ b/libs/framework/src/bundle_context.c
@@ -661,7 +661,17 @@ static celix_status_t bundleContext_bundleChanged(void *listenerSvc, bundle_even
void celix_bundleContext_trackBundlesWithOptionsCallback(void *data) {
celix_bundle_context_bundle_tracker_entry_t* entry = data;
assert(celix_framework_isCurrentThreadTheEventLoop(entry->ctx->framework));
- fw_addBundleListener(entry->ctx->framework, entry->ctx->bundle, &entry->listener);
+ celixThreadMutex_lock(&entry->ctx->mutex);
+ bool cancelled = entry->cancelled;
+ entry->created = true;
+ celixThreadMutex_unlock(&entry->ctx->mutex);
+ if (cancelled) {
+ fw_log(entry->ctx->framework->logger, CELIX_LOG_LEVEL_DEBUG, "Creation of bundle tracker cancelled. trk id = %li", entry->trackerId);
+ free(entry);
+ } else {
+ fw_addBundleListener(entry->ctx->framework, entry->ctx->bundle, &entry->listener);
+ }
+
}
static long celix_bundleContext_trackBundlesWithOptionsInternal(
@@ -910,6 +920,7 @@ static void celix_bundleContext_stopTrackerInternal(bundle_context_t *ctx, long
}
bool found = false;
+ bool cancelled = false;
celix_bundle_context_bundle_tracker_entry_t *bundleTracker = NULL;
celix_bundle_context_service_tracker_entry_t *serviceTracker = NULL;
celix_bundle_context_service_tracker_tracker_entry_t *svcTrackerTracker = NULL;
@@ -919,15 +930,29 @@ static void celix_bundleContext_stopTrackerInternal(bundle_context_t *ctx, long
if (hashMap_containsKey(ctx->bundleTrackers, (void *) trackerId)) {
found = true;
bundleTracker = hashMap_remove(ctx->bundleTrackers, (void *) trackerId);
+ if (!bundleTracker->created && !async) {
+ //note tracker not yet created, so cancel instead of removing
+ bundleTracker->cancelled = true;
+ cancelled = true;
+ }
} else if (hashMap_containsKey(ctx->serviceTrackers, (void *) trackerId)) {
found = true;
serviceTracker = hashMap_remove(ctx->serviceTrackers, (void *) trackerId);
+ if (serviceTracker->tracker == NULL && !async) {
+ //note tracker not yet created, so cancel instead of removing
+ serviceTracker->cancelled = true;
+ cancelled = true;
+ }
} else if (hashMap_containsKey(ctx->metaTrackers, (void *) trackerId)) {
found = true;
svcTrackerTracker = hashMap_remove(ctx->metaTrackers, (void *) trackerId);
+ //note because a meta tracker is a service listener hook under waiter, no additional cancel is needed (svc reg will be cancelled)
}
- if (found && !async && celix_framework_isCurrentThreadTheEventLoop(ctx->framework)) {
+ if (found && cancelled) {
+ //nop
+ celixThreadMutex_unlock(&ctx->mutex);
+ } else if (found && !async && celix_framework_isCurrentThreadTheEventLoop(ctx->framework)) {
//already on the event loop, stop tracker "traditionally" to keep old behavior
celixThreadMutex_unlock(&ctx->mutex); //note calling remove/stops/unregister out side of locks
@@ -1335,21 +1360,36 @@ long celix_bundleContext_trackServicesAsync(
static void celix_bundleContext_createTrackerOnEventLoop(void *data) {
celix_bundle_context_service_tracker_entry_t* entry = data;
assert(celix_framework_isCurrentThreadTheEventLoop(entry->ctx->framework));
- celix_service_tracker_t *tracker = celix_serviceTracker_createWithOptions(entry->ctx, &entry->opts);
- if (tracker != NULL) {
- celixThreadMutex_lock(&entry->ctx->mutex);
- entry->tracker = tracker;
- celixThreadMutex_unlock(&entry->ctx->mutex);
+ celixThreadMutex_lock(&entry->ctx->mutex);
+ bool cancelled = entry->cancelled;
+ celixThreadMutex_unlock(&entry->ctx->mutex);
+ if (cancelled) {
+ fw_log(entry->ctx->framework->logger, CELIX_LOG_LEVEL_DEBUG, "Creating of service tracker was cancelled. trk id = %li, svc name tracked = %s", entry->trackerId, entry->opts.filter.serviceName);
} else {
- fw_log(entry->ctx->framework->logger, CELIX_LOG_LEVEL_ERROR, "Cannot create tracker for bnd %s (%li)", celix_bundle_getSymbolicName(entry->ctx->bundle), celix_bundle_getId(entry->ctx->bundle));
+ celix_service_tracker_t *tracker = celix_serviceTracker_createWithOptions(entry->ctx, &entry->opts);
+ if (tracker != NULL) {
+ celixThreadMutex_lock(&entry->ctx->mutex);
+ entry->tracker = tracker;
+ celixThreadMutex_unlock(&entry->ctx->mutex);
+ } else {
+ fw_log(entry->ctx->framework->logger, CELIX_LOG_LEVEL_ERROR, "Cannot create tracker for bnd %s (%li)", celix_bundle_getSymbolicName(entry->ctx->bundle), celix_bundle_getId(entry->ctx->bundle));
+ celix_serviceTracker_destroy(tracker);
+ }
}
}
static void celix_bundleContext_doneCreatingTrackerOnEventLoop(void *data) {
celix_bundle_context_service_tracker_entry_t* entry = data;
+ celixThreadMutex_lock(&entry->ctx->mutex);
+ bool cancelled = entry->cancelled;
+ celixThreadMutex_unlock(&entry->ctx->mutex);
if (entry->trackerCreatedCallback != NULL) {
entry->trackerCreatedCallback(entry->trackerCreatedCallbackData);
}
+ if (cancelled) {
+ //tracker creation cancelled -> entry already removed from map, but memory needs to be freed.
+ free(entry);
+ }
}
diff --git a/libs/framework/src/bundle_context_private.h b/libs/framework/src/bundle_context_private.h
index 1b77bf5..6e6d9e5 100644
--- a/libs/framework/src/bundle_context_private.h
+++ b/libs/framework/src/bundle_context_private.h
@@ -33,8 +33,11 @@ typedef struct celix_bundle_context_bundle_tracker_entry {
bundle_listener_t listener;
celix_bundle_tracking_options_t opts;
- //used for sync
- long createEventId;
+ bool created;
+ bool cancelled;
+
+ //used for sync
+ long createEventId;
} celix_bundle_context_bundle_tracker_entry_t;
typedef struct celix_bundle_context_service_tracker_entry {
@@ -48,6 +51,7 @@ typedef struct celix_bundle_context_service_tracker_entry {
//used for sync
long createEventId;
+ bool cancelled; //if tracker is stopped before created async
} celix_bundle_context_service_tracker_entry_t;
typedef struct celix_bundle_context_service_tracker_tracker_entry {
diff --git a/libs/framework/src/dm_component_impl.c b/libs/framework/src/dm_component_impl.c
index d930e52..08a4e01 100644
--- a/libs/framework/src/dm_component_impl.c
+++ b/libs/framework/src/dm_component_impl.c
@@ -44,19 +44,18 @@ struct celix_dm_component_struct {
celix_dm_cmp_lifecycle_fpt callbackDeinit;
celix_thread_mutex_t mutex; //protects below
- celix_array_list_t* providedInterfaces;
- celix_array_list_t* dependencies;
+ celix_thread_cond_t cond;
+ celix_array_list_t* providedInterfaces; //type = dm_interface_t*
+ celix_array_list_t* dependencies; //type = celix_dm_service_dependency_t*
+
+ celix_dm_component_state_t state;
/**
- * TODO needed
- * True if the component provides the same service type as it depends on.
- * In that case a addtional check has to be made that the component does not depend on its own provided service,
- * because this can result in a deadlock (cannot unregister if service is in use and unregister can happen in a
- * remote service tracker callback)
+ * Nr of active set, add or remove svc calls in progress.
+ * Should be 0 before destroying cmp or removing service dependencies
*/
- bool providesSameServiceAsOneOfTheDependencies;
+ size_t nrOfSetAddRemCallsInProgress;
- celix_dm_component_state_t state;
bool isStarted;
};
@@ -65,7 +64,6 @@ typedef struct dm_interface_struct {
const void* service;
celix_properties_t *properties;
long svcId;
- bool unregistering;
} dm_interface_t;
static celix_status_t component_registerServices(celix_dm_component_t *component);
@@ -127,6 +125,7 @@ celix_dm_component_t* celix_dmComponent_createWithUUID(bundle_context_t *context
component->providedInterfaces = celix_arrayList_create();
component->dependencies = celix_arrayList_create();
celixThreadMutex_create(&component->mutex, NULL);
+ celixThreadCondition_init(&component->cond, NULL);
component->isStarted = false;
return component;
}
@@ -146,6 +145,24 @@ void component_destroy(celix_dm_component_t *component) {
celix_dmComponent_destroy(component);
}
+static void celix_dmComponent_waitForNoAddRemOrSetDependencyInProgress(celix_dm_component_t* component, bool lockOnMutex) {
+ if (lockOnMutex) {
+ celixThreadMutex_lock(&component->mutex);
+ }
+ struct timespec start = celix_gettime(CLOCK_MONOTONIC);
+ while (component->nrOfSetAddRemCallsInProgress > 0) {
+ celixThreadCondition_timedwaitRelative(&component->cond, &component->mutex, 1, 0);
+ struct timespec now = celix_gettime(CLOCK_MONOTONIC);
+ if (celix_difftime(&start, &now) > 5) {
+ start = celix_gettime(CLOCK_MONOTONIC);
+ fw_log(component->context->framework->logger, CELIX_LOG_LEVEL_WARNING, "Add, remove or set dependency call still in progress for component %s (uuid=%s)", component->name, component->uuid);
+ }
+ }
+ if (lockOnMutex) {
+ celixThreadMutex_unlock(&component->mutex);
+ }
+}
+
void celix_dmComponent_destroy(celix_dm_component_t *component) {
if (component) {
celix_dmComponent_stop(component); //all service deregistered // all svc tracker stopped
@@ -161,12 +178,16 @@ void celix_dmComponent_destroy(celix_dm_component_t *component) {
}
celix_arrayList_destroy(component->providedInterfaces);
+
+ celix_dmComponent_waitForNoAddRemOrSetDependencyInProgress(component, true);
+
for (int i = 0; i < celix_arrayList_size(component->dependencies); ++i) {
celix_dm_service_dependency_t* dep = celix_arrayList_get(component->dependencies, i);
celix_dmServiceDependency_destroy(dep);
}
celix_arrayList_destroy(component->dependencies);
celixThreadMutex_destroy(&component->mutex);
+ celixThreadCondition_destroy(&component->cond);
free(component);
}
@@ -177,6 +198,22 @@ const char* celix_dmComponent_getUUID(celix_dm_component_t* cmp) {
return cmp->uuid;
}
+//call with lock
+static void celix_dmComponent_updateFilterOutOwnSvcDependencies(celix_dm_component_t* component) {
+ for (int i = 0; i < celix_arrayList_size(component->dependencies); ++i) {
+ celix_dm_service_dependency_t* dep = celix_arrayList_get(component->dependencies, i);
+ bool filterOut = false;
+ for (int k = 0; k < celix_arrayList_size(component->providedInterfaces); ++k) {
+ dm_interface_t* intf = celix_arrayList_get(component->providedInterfaces, k);
+ if (celix_utils_stringEquals(intf->serviceName, dep->serviceName)) {
+ filterOut = true;
+ break;
+ }
+ }
+ celix_dmServiceDependency_setFilterOutOwnSvcDependencies(dep, filterOut);
+ }
+}
+
celix_status_t component_addServiceDependency(celix_dm_component_t *component, celix_dm_service_dependency_t *dep) {
return celix_dmComponent_addServiceDependency(component, dep);
}
@@ -191,7 +228,7 @@ celix_status_t celix_dmComponent_addServiceDependency(celix_dm_component_t *comp
if (startDep) {
celix_serviceDependency_start(dep);
}
-
+ celix_dmComponent_updateFilterOutOwnSvcDependencies(component);
celixThreadMutex_unlock(&component->mutex);
component_handleChange(component);
@@ -201,12 +238,14 @@ celix_status_t celix_dmComponent_addServiceDependency(celix_dm_component_t *comp
celix_status_t celix_dmComponent_removeServiceDependency(celix_dm_component_t *component, celix_dm_service_dependency_t *dep) {
celixThreadMutex_lock(&component->mutex);
+ celix_dmComponent_waitForNoAddRemOrSetDependencyInProgress(component, false);
arrayList_removeElement(component->dependencies, dep);
bool stopDependency = component->state != DM_CMP_STATE_INACTIVE;
if (stopDependency) {
celix_serviceDependency_stop(dep);
}
celix_dmServiceDependency_destroy(dep);
+ celix_dmComponent_updateFilterOutOwnSvcDependencies(component);
celixThreadMutex_unlock(&component->mutex);
component_handleChange(component);
@@ -388,6 +427,7 @@ static celix_status_t component_suspend(celix_dm_component_t *component, celix_d
celix_status_t status = CELIX_SUCCESS;
dm_service_dependency_strategy_t strategy = celix_dmServiceDependency_getStrategy(dependency);
if (strategy == DM_SERVICE_DEPENDENCY_STRATEGY_SUSPEND && component->callbackStop != NULL) {
+ component_unregisterServices(component);
status = component->callbackStop(component->implementation);
}
return status;
@@ -397,73 +437,52 @@ static celix_status_t component_resume(celix_dm_component_t *component, celix_dm
celix_status_t status = CELIX_SUCCESS;
dm_service_dependency_strategy_t strategy = celix_dmServiceDependency_getStrategy(dependency);
if (strategy == DM_SERVICE_DEPENDENCY_STRATEGY_SUSPEND && component->callbackStop != NULL) {
+ component_registerServices(component);
status = component->callbackStart(component->implementation);
}
return status;
}
-static celix_status_t component_handleAdd(celix_dm_component_t *component, const celix_dm_event_t* event) {
+static celix_status_t component_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)) {
+ bool needSuspend = false;
celixThreadMutex_lock(&component->mutex);
+ component->nrOfSetAddRemCallsInProgress += 1;
+ celixThreadCondition_broadcast(&component->cond);
switch (component->state) {
case DM_CMP_STATE_TRACKING_OPTIONAL:
if (celix_serviceDependency_hasAddCallback(event->dep)) { //if to prevent unneeded suspends
- component_suspend(component, event->dep);
- celix_serviceDependency_invokeAdd(event->dep, event->svc, event->props);
- component_resume(component, event->dep);
+ needSuspend = true;
}
break;
default: //DM_CMP_STATE_INACTIVE
- celix_serviceDependency_invokeAdd(event->dep, event->svc, event->props);
break;
}
celixThreadMutex_unlock(&component->mutex);
+ if (needSuspend) {
+ component_suspend(component, event->dep);
+ }
+ setAddOrRemFp(event->dep, event->svc, event->props);
+ if (needSuspend) {
+ component_resume(component, event->dep);
+ }
+ celixThreadMutex_lock(&component->mutex);
+ component->nrOfSetAddRemCallsInProgress -= 1;
+ celixThreadCondition_broadcast(&component->cond);
+ celixThreadMutex_unlock(&component->mutex);
component_handleChange(component);
return CELIX_SUCCESS;
}
-static celix_status_t component_handleRemove(celix_dm_component_t *component, const celix_dm_event_t* event) {
- component_handleChange(component);
- celixThreadMutex_lock(&component->mutex);
- switch (component->state) {
- case DM_CMP_STATE_TRACKING_OPTIONAL:
- if (celix_serviceDependency_hasRemoveCallback(event->dep)) { //if to prevent unneeded suspends
- component_suspend(component, event->dep);
- celix_serviceDependency_invokeRemove(event->dep, event->svc, event->props);
- component_resume(component, event->dep);
- }
- break;
- default: //DM_CMP_STATE_INACTIVE
- celix_serviceDependency_invokeRemove(event->dep, event->svc, event->props);
- break;
- }
- celixThreadMutex_unlock(&component->mutex);
- return CELIX_SUCCESS;
+static celix_status_t component_handleAdd(celix_dm_component_t *component, const celix_dm_event_t* event) {
+ return component_handleEvent(component, event, celix_serviceDependency_invokeAdd);
}
+static celix_status_t component_handleRemove(celix_dm_component_t *component, const celix_dm_event_t* event) {
+ return component_handleEvent(component, event, celix_serviceDependency_invokeRemove);
+}
static celix_status_t component_handleSet(celix_dm_component_t *component, const celix_dm_event_t* event) {
- if (event->svc == NULL) {
- //note set with removes a service -> update state first
- component_handleChange(component);
- }
- celixThreadMutex_lock(&component->mutex);
- switch (component->state) {
- case DM_CMP_STATE_TRACKING_OPTIONAL:
- if (celix_serviceDependency_hasSetCallback(event->dep)) { //if to prevent unneeded suspends
- component_suspend(component, event->dep);
- celix_serviceDependency_invokeSet(event->dep, event->svc, event->props);
- component_resume(component, event->dep);
- }
- break;
- default: //DM_CMP_STATE_INACTIVE
- celix_serviceDependency_invokeSet(event->dep, event->svc, event->props);
- break;
- }
- celixThreadMutex_unlock(&component->mutex);
- if (event->svc != NULL) {
- component_handleChange(component);
- }
- return CELIX_SUCCESS;
+ return component_handleEvent(component, event, celix_serviceDependency_invokeSet);
}
/**
@@ -698,14 +717,14 @@ static celix_status_t component_unregisterServices(celix_dm_component_t *compone
celix_array_list_t* ids = celix_arrayList_create();
for (int i = 0; i < celix_arrayList_size(component->providedInterfaces); ++i) {
dm_interface_t *interface = arrayList_get(component->providedInterfaces, i);
- interface->unregistering = true;
+ celix_arrayList_addLong(ids, interface->svcId);
interface->svcId = -1L;
}
celixThreadMutex_unlock(&component->mutex);
for (int i = 0; i < celix_arrayList_size(ids); ++i) {
long svcId = celix_arrayList_getLong(ids, i);
- celix_bundleContext_unregisterService(component->context, svcId); //TODO make async
+ celix_bundleContext_unregisterService(component->context, svcId);
}
celixThreadMutex_lock(&component->mutex);
celix_arrayList_destroy(ids);
diff --git a/libs/framework/src/dm_service_dependency.c b/libs/framework/src/dm_service_dependency.c
index 5cca374..80bf71a 100644
--- a/libs/framework/src/dm_service_dependency.c
+++ b/libs/framework/src/dm_service_dependency.c
@@ -232,13 +232,23 @@ celix_status_t celix_serviceDependency_stop(celix_dm_service_dependency_t *depen
return CELIX_SUCCESS;
}
+/**
+ * checks whether the service dependency needs to be ignore. This can be needed if a component depends on the same service types as it provides
+ */
+static bool serviceDependency_ignoreSvcCallback(celix_dm_service_dependency_t* dependency, const celix_properties_t* props) {
+ bool filterOut = celix_dmServiceDependency_filterOutOwnSvcDependencies(dependency);
+ if (filterOut) {
+ const char *uuid = celix_dmComponent_getUUID(dependency->component);
+ const char *svcCmpUUID = celix_properties_get(props, CELIX_DM_COMPONENT_UUID, NULL);
+ return svcCmpUUID != NULL && celix_utils_stringEquals(uuid, svcCmpUUID);
+ }
+ return false;
+}
+
static void serviceDependency_setServiceTrackerCallback(void *handle, void *svc, const celix_properties_t *props) {
celix_dm_service_dependency_t* dependency = handle;
- const char *uuid = celix_dmComponent_getUUID(dependency->component);
- const char *svcCmpUUID = celix_properties_get(props, CELIX_DM_COMPONENT_UUID, NULL);
- if (svcCmpUUID != NULL && strncmp(uuid, svcCmpUUID, DM_COMPONENT_MAX_ID_LENGTH) == 0) {
- fprintf(stderr, "TODO warning: ignoring svc of own component\n"); //TODO
+ if (serviceDependency_ignoreSvcCallback(dependency, props)) {
return;
}
@@ -263,10 +273,7 @@ celix_status_t celix_serviceDependency_invokeSet(celix_dm_service_dependency_t *
static void serviceDependency_addServiceTrackerCallback(void *handle, void *svc, const celix_properties_t *props) {
celix_dm_service_dependency_t* dependency = handle;
- const char *uuid = celix_dmComponent_getUUID(dependency->component);
- const char *svcCmpUUID = celix_properties_get(props, CELIX_DM_COMPONENT_UUID, NULL);
- if (svcCmpUUID != NULL && strncmp(uuid, svcCmpUUID, DM_COMPONENT_MAX_ID_LENGTH) == 0) {
- fprintf(stderr, "TODO warning: ignoring svc of own component\n"); //TODO
+ if (serviceDependency_ignoreSvcCallback(dependency, props)) {
return;
}
@@ -283,11 +290,12 @@ static void serviceDependency_addServiceTrackerCallback(void *handle, void *svc,
}
celix_status_t celix_serviceDependency_invokeAdd(celix_dm_service_dependency_t *dependency, void* svc, const celix_properties_t* props) {
+ void *handle = serviceDependency_getCallbackHandle(dependency);
if (dependency->add) {
- dependency->add(serviceDependency_getCallbackHandle(dependency), svc);
+ dependency->add(handle, svc);
}
if (dependency->addWithProperties) {
- dependency->addWithProperties(serviceDependency_getCallbackHandle(dependency), svc, props);
+ dependency->addWithProperties(handle, svc, props);
}
return CELIX_SUCCESS;
}
@@ -295,10 +303,7 @@ celix_status_t celix_serviceDependency_invokeAdd(celix_dm_service_dependency_t *
static void serviceDependency_removeServiceTrackerCallback(void *handle, void *svc, const celix_properties_t *props) {
celix_dm_service_dependency_t* dependency = handle;
- const char *uuid = celix_dmComponent_getUUID(dependency->component);
- const char *svcCmpUUID = celix_properties_get(props, CELIX_DM_COMPONENT_UUID, NULL);
- if (svcCmpUUID != NULL && strncmp(uuid, svcCmpUUID, DM_COMPONENT_MAX_ID_LENGTH) == 0) {
- fprintf(stderr, "TODO warning: ignoring svc of own component\n"); //TODO
+ if (serviceDependency_ignoreSvcCallback(dependency, props)) {
return;
}
@@ -355,6 +360,19 @@ bool celix_dmServiceDependency_isTrackerOpen(celix_dm_service_dependency_t* depe
return started;
}
+bool celix_dmServiceDependency_filterOutOwnSvcDependencies(celix_dm_service_dependency_t* dependency) {
+ celixThreadMutex_lock(&dependency->mutex);
+ bool filterOut = dependency->filterOutOwnSvcDependencies;
+ celixThreadMutex_unlock(&dependency->mutex);
+ return filterOut;
+}
+
+void celix_dmServiceDependency_setFilterOutOwnSvcDependencies(celix_dm_service_dependency_t* dependency, bool filterOut) {
+ celixThreadMutex_lock(&dependency->mutex);
+ dependency->filterOutOwnSvcDependencies = filterOut;
+ celixThreadMutex_unlock(&dependency->mutex);
+}
+
celix_status_t serviceDependency_getServiceDependencyInfo(celix_dm_service_dependency_t *dep, dm_service_dependency_info_t **out) {
if (out != NULL) {
*out = celix_dmServiceDependency_createInfo(dep);
@@ -400,7 +418,6 @@ celix_status_t celix_dmServiceDependency_setCallbackHandle(celix_dm_service_depe
return CELIX_SUCCESS;
}
-
static void* serviceDependency_getCallbackHandle(celix_dm_service_dependency_t *dependency) {
return dependency->callbackHandle == NULL ? component_getImplementation(dependency->component) : dependency->callbackHandle;
}
diff --git a/libs/framework/src/dm_service_dependency_impl.h b/libs/framework/src/dm_service_dependency_impl.h
index a946739..df48b22 100644
--- a/libs/framework/src/dm_service_dependency_impl.h
+++ b/libs/framework/src/dm_service_dependency_impl.h
@@ -40,9 +40,6 @@ struct celix_dm_service_dependency_svc_entry {
};
struct celix_dm_service_dependency {
- celix_dm_component_t *component;
-
- void* callbackHandle; //This handle can be set to be used instead of the component implementation
celix_dm_service_update_fp set;
celix_dm_service_update_fp add;
celix_dm_service_update_fp remove;
@@ -57,11 +54,21 @@ struct celix_dm_service_dependency {
bool required;
dm_service_dependency_strategy_t strategy;
bool addCLanguageFilter;
+ celix_dm_component_t *component;
celix_thread_mutex_t mutex; //protects below
long svcTrackerId;
bool isTrackerOpen;
size_t trackedSvcCount;
+ void* callbackHandle; //This handle can be set to be used instead of the component implementation
+
+ /**
+ * If a component provides the same service type as it depends on, this types need to be filtered out to prevent
+ * a deadlock.
+ * The deadlock occurs when service cannot be unregistered, because it is still in use in due to the a service tracker
+ * of a service dependency of the same type (for the same component)
+ */
+ bool filterOutOwnSvcDependencies;
};
celix_status_t celix_serviceDependency_start(celix_dm_service_dependency_t *dependency);
@@ -81,6 +88,10 @@ bool celix_serviceDependency_isAvailable(celix_dm_service_dependency_t *dependen
bool celix_dmServiceDependency_isRequired(const celix_dm_service_dependency_t* dependency);
bool celix_dmServiceDependency_isTrackerOpen(celix_dm_service_dependency_t* dependency);
+
+bool celix_dmServiceDependency_filterOutOwnSvcDependencies(celix_dm_service_dependency_t* dependency);
+void celix_dmServiceDependency_setFilterOutOwnSvcDependencies(celix_dm_service_dependency_t* dependency, bool filterOut);
+
#ifdef __cplusplus
}
#endif
diff --git a/libs/framework/src/framework.c b/libs/framework/src/framework.c
index 59013c2..957003b 100644
--- a/libs/framework/src/framework.c
+++ b/libs/framework/src/framework.c
@@ -1968,8 +1968,11 @@ static void fw_handleEventRequest(celix_framework_t *framework, celix_framework_
celixThreadMutex_unlock(&framework->frameworkListenersLock);
} else if (event->type == CELIX_REGISTER_SERVICE_EVENT) {
service_registration_t* reg = NULL;
- celix_status_t status;
- if (event->factory != NULL) {
+ celix_status_t status = CELIX_SUCCESS;
+ if (event->cancelled) {
+ fw_log(framework->logger, CELIX_LOG_LEVEL_DEBUG, "CELIX_REGISTER_SERVICE_EVENT for svcId %li (service name = %s) was cancelled. Skipping registration", event->registerServiceId, event->serviceName);
+ celix_properties_destroy(event->properties);
+ } else if (event->factory != NULL) {
status = celix_serviceRegistry_registerServiceFactory(framework->registry, event->bndEntry->bnd, event->serviceName, event->factory, event->properties, event->registerServiceId, ®);
} else {
status = celix_serviceRegistry_registerService(framework->registry, event->bndEntry->bnd, event->serviceName, event->svc, event->properties, event->registerServiceId, ®);
@@ -2415,8 +2418,39 @@ void celix_framework_unregisterAsync(celix_framework_t* fw, celix_bundle_t* bnd,
celix_framework_addToEventQueue(fw, &event);
}
+/**
+ * Checks if there is a pending service registration in the event queue and canels this.
+ *
+ * This can be needed when a service is regsitered async and still on the event queue when an sync unregistration
+ * is made.
+ * @returns true if a service registration is cancelled.
+ */
+static bool celix_framework_cancelServiceRegistrationIfPending(celix_framework_t* fw, celix_bundle_t* bnd, long serviceId) {
+ bool cancelled = false;
+ celixThreadMutex_lock(&fw->dispatcher.mutex);
+ for (int i = 0; i < celix_arrayList_size(fw->dispatcher.dynamicEventQueue); ++i) {
+ celix_framework_event_t *event = celix_arrayList_get(fw->dispatcher.dynamicEventQueue, i);
+ if (event->type == CELIX_REGISTER_SERVICE_EVENT && event->registerServiceId == serviceId) {
+ event->cancelled = true;
+ cancelled = true;
+ }
+ }
+ for (size_t i = 0; i < fw->dispatcher.eventQueueSize; ++i) {
+ size_t index = (fw->dispatcher.eventQueueFirstEntry + i) % CELIX_FRAMEWORK_STATIC_EVENT_QUEUE_SIZE;
+ celix_framework_event_t *event = &fw->dispatcher.eventQueue[index];
+ if (event->type == CELIX_REGISTER_SERVICE_EVENT && event->registerServiceId == serviceId) {
+ event->cancelled = true;
+ cancelled = true;
+ }
+ }
+ celixThreadMutex_unlock(&fw->dispatcher.mutex);
+ return cancelled;
+}
+
void celix_framework_unregister(celix_framework_t* fw, celix_bundle_t* bnd, long serviceId) {
- celix_serviceRegistry_unregisterService(fw->registry, bnd, serviceId);
+ if (!celix_framework_cancelServiceRegistrationIfPending(fw, bnd, serviceId)) {
+ celix_serviceRegistry_unregisterService(fw->registry, bnd, serviceId);
+ }
}
void celix_framework_waitForAsyncRegistration(framework_t *fw, long svcId) {
diff --git a/libs/framework/src/framework_private.h b/libs/framework/src/framework_private.h
index 7eaa0c7..6f77b03 100644
--- a/libs/framework/src/framework_private.h
+++ b/libs/framework/src/framework_private.h
@@ -79,6 +79,7 @@ struct celix_framework_event {
//for register event
long registerServiceId;
+ bool cancelled;
char *serviceName;
void *svc;
celix_service_factory_t* factory;
diff --git a/libs/framework/src/service_registry.c b/libs/framework/src/service_registry.c
index 1b2924c..b0c8154 100644
--- a/libs/framework/src/service_registry.c
+++ b/libs/framework/src/service_registry.c
@@ -1314,11 +1314,13 @@ void celix_serviceRegistry_unregisterService(celix_service_registry_t* registry,
service_registration_t *reg = NULL;
celixThreadRwlock_readLock(®istry->lock);
celix_array_list_t* registrations = hashMap_get(registry->serviceRegistrations, (void*)bnd);
- for (int i = 0; i < celix_arrayList_size(registrations); ++i) {
- service_registration_t *entry = celix_arrayList_get(registrations, i);
- if (serviceRegistration_getServiceId(entry) == serviceId) {
- reg = entry;
- break;
+ if (registrations != NULL) {
+ for (int i = 0; i < celix_arrayList_size(registrations); ++i) {
+ service_registration_t *entry = celix_arrayList_get(registrations, i);
+ if (serviceRegistration_getServiceId(entry) == serviceId) {
+ reg = entry;
+ break;
+ }
}
}
celixThreadRwlock_unlock(®istry->lock);