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 <[email protected]> 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);
