This is an automated email from the ASF dual-hosted git repository. pnoltes pushed a commit to branch feature/register_with_reserved_id in repository https://gitbox.apache.org/repos/asf/celix.git
commit 819b9b24e0d7688c0fe99c47e5300ce2524d0a16 Author: Pepijn Noltes <pepijnnol...@gmail.com> AuthorDate: Sun May 24 20:53:49 2020 +0200 Adds support for reserving service id, so that registering service can be done safely outside of locks/mutexes. This can be done, because if an unregister service happens before a register service with a reserved service id, the expected future register service will be (silently) cancelled. --- bundles/logging/log_admin/src/celix_log_admin.c | 16 +- .../tms_tst/disc_mock/disc_mock_activator.c | 1 + .../gtest/src/bundle_context_services_test.cpp | 160 +++++++++++++- libs/framework/include/celix_bundle_context.h | 39 +++- libs/framework/include/service_registry.h | 12 +- libs/framework/src/bundle.c | 2 +- libs/framework/src/bundle_context.c | 245 ++++++++++++++++++--- libs/framework/src/bundle_context_private.h | 4 + libs/framework/src/framework.c | 39 +++- libs/framework/src/framework_private.h | 4 +- libs/framework/src/service_registry.c | 74 ++----- libs/framework/src/service_tracker.c | 28 +-- libs/utils/include/celix_log_utils.h | 5 - libs/utils/src/celix_log_utils.c | 14 +- 14 files changed, 510 insertions(+), 133 deletions(-) diff --git a/bundles/logging/log_admin/src/celix_log_admin.c b/bundles/logging/log_admin/src/celix_log_admin.c index 900940d..ad49461 100644 --- a/bundles/logging/log_admin/src/celix_log_admin.c +++ b/bundles/logging/log_admin/src/celix_log_admin.c @@ -176,6 +176,8 @@ static void celix_logAdmin_addLogSvcForName(celix_log_admin_t* admin, const char celixThreadRwlock_writeLock(&admin->lock); celix_log_service_entry_t* found = hashMap_get(admin->loggers, name); + long reservedId = -1L; + celix_log_service_t *newLogSvc = NULL; if (found == NULL) { //new newEntry = calloc(1, sizeof(*newEntry)); @@ -194,8 +196,12 @@ static void celix_logAdmin_addLogSvcForName(celix_log_admin_t* admin, const char newEntry->logSvc.logDetails = celix_logAdmin_logDetails; newEntry->logSvc.vlog = celix_logAdmin_vlog; newEntry->logSvc.vlogDetails = celix_logAdmin_vlogDetails; + newEntry->logSvcId = celix_bundleContext_reserveSvcId(admin->ctx); hashMap_put(admin->loggers, (void*)newEntry->name, newEntry); + reservedId = newEntry->logSvcId; + newLogSvc = &newEntry->logSvc; + if (celix_utils_stringEquals(newEntry->name, CELIX_LOG_ADMIN_FRAMEWORK_LOG_NAME)) { celix_framework_t* fw = celix_bundleContext_getFramework(admin->ctx); celix_framework_setLogCallback(fw, newEntry, celix_logAdmin_frameworkLogFunction); @@ -205,8 +211,9 @@ static void celix_logAdmin_addLogSvcForName(celix_log_admin_t* admin, const char } celixThreadRwlock_unlock(&admin->lock); - if (newEntry != NULL) { - //register new instance + if (newLogSvc != NULL) { + //register new log service outside of the lock using a reserved service id. + celix_properties_t* props = celix_properties_create(); celix_properties_set(props, CELIX_LOG_SERVICE_PROPERTY_NAME, newEntry->name); if (celix_utils_stringEquals(newEntry->name, CELIX_LOG_ADMIN_DEFAULT_LOG_NAME) == 0) { @@ -218,8 +225,9 @@ static void celix_logAdmin_addLogSvcForName(celix_log_admin_t* admin, const char opts.serviceName = CELIX_LOG_SERVICE_NAME; opts.serviceVersion = CELIX_LOG_SERVICE_VERSION; opts.properties = props; - opts.svc = &newEntry->logSvc; - newEntry->logSvcId = celix_bundleContext_registerServiceWithOptions(admin->ctx, &opts); + opts.svc = newLogSvc; + opts.reservedSvcId = reservedId; + celix_bundleContext_registerServiceWithOptions(admin->ctx, &opts); } } diff --git a/bundles/remote_services/topology_manager/tms_tst/disc_mock/disc_mock_activator.c b/bundles/remote_services/topology_manager/tms_tst/disc_mock/disc_mock_activator.c index c34a67d..40e26f3 100644 --- a/bundles/remote_services/topology_manager/tms_tst/disc_mock/disc_mock_activator.c +++ b/bundles/remote_services/topology_manager/tms_tst/disc_mock/disc_mock_activator.c @@ -112,6 +112,7 @@ celix_status_t bundleActivator_stop(void * userData, celix_bundle_context_t *con celix_status_t status; struct disc_mock_activator * act = userData; + serviceRegistration_unregister(act->endpointListenerService); status = serviceRegistration_unregister(act->reg); return status; diff --git a/libs/framework/gtest/src/bundle_context_services_test.cpp b/libs/framework/gtest/src/bundle_context_services_test.cpp index e544d2f..c62760e 100644 --- a/libs/framework/gtest/src/bundle_context_services_test.cpp +++ b/libs/framework/gtest/src/bundle_context_services_test.cpp @@ -412,9 +412,7 @@ TEST_F(CelixBundleContextServicesTests, servicesTrackerTest) { TEST_F(CelixBundleContextServicesTests, servicesTrackerInvalidArgsTest) { long trackerId = celix_bundleContext_trackServices(nullptr, nullptr, nullptr, nullptr, nullptr); - ASSERT_TRUE(trackerId < 0); //required ctx and service name missing - trackerId = celix_bundleContext_trackServices(ctx, nullptr, nullptr, nullptr, nullptr); - ASSERT_TRUE(trackerId < 0); //required service name missing + ASSERT_TRUE(trackerId < 0); //required ctx missing trackerId = celix_bundleContext_trackServices(ctx, "calc", nullptr, nullptr, nullptr); ASSERT_TRUE(trackerId >= 0); //valid celix_bundleContext_stopTracker(ctx, trackerId); @@ -427,7 +425,9 @@ TEST_F(CelixBundleContextServicesTests, servicesTrackerInvalidArgsTest) { ASSERT_TRUE(trackerId < 0); //required opts missing celix_service_tracking_options_t opts{}; trackerId = celix_bundleContext_trackServicesWithOptions(ctx, &opts); - ASSERT_TRUE(trackerId < 0); //required opts->serviceName missing + ASSERT_TRUE(trackerId >= 0); //valid with empty opts + celix_bundleContext_stopTracker(ctx, trackerId); + opts.filter.serviceName = "calc"; trackerId = celix_bundleContext_trackServicesWithOptions(ctx, &opts); ASSERT_TRUE(trackerId >= 0); //valid @@ -711,8 +711,62 @@ TEST_F(CelixBundleContextServicesTests, servicesTrackerSetTest) { ASSERT_EQ(3, count); //check if the set is called the expected times } -//TODO test tracker with options for properties & service owners +TEST_F(CelixBundleContextServicesTests, trackAllServices) { + std::atomic<size_t> count{0}; + + void *svc1 = (void *) 0x100; //no ranking + void *svc2 = (void *) 0x200; //no ranking + void *svc3 = (void *) 0x300; //10 ranking + void *svc4 = (void *) 0x400; //5 ranking + + long svcId1 = celix_bundleContext_registerService(ctx, svc1, "svc_type1", nullptr); + long svcId2 = celix_bundleContext_registerService(ctx, svc2, "svc_type1", nullptr); + long svcId3 = celix_bundleContext_registerService(ctx, svc3, "svc_type2", nullptr); + long svcId4 = celix_bundleContext_registerService(ctx, svc4, "svc_type2", nullptr); + + celix_service_tracking_options_t opts{}; + opts.callbackHandle = (void *) &count; + opts.filter.serviceName = nullptr; + opts.callbackHandle = (void *) &count; + opts.add = [](void *handle, void *) { + auto c = (std::atomic<size_t> *) handle; + c->fetch_add(1); + }; + long trackerId = celix_bundleContext_trackServicesWithOptions(ctx, &opts); + EXPECT_GE(trackerId, 0); + EXPECT_EQ(4, count.load()); + celix_bundleContext_unregisterService(ctx, svcId1); + celix_bundleContext_unregisterService(ctx, svcId2); + celix_bundleContext_unregisterService(ctx, svcId3); + celix_bundleContext_unregisterService(ctx, svcId4); + celix_bundleContext_stopTracker(ctx, trackerId); +} + +TEST_F(CelixBundleContextServicesTests, metaTrackAllServiceTrackers) { + std::atomic<size_t> count{0}; + auto add = [](void *handle, const celix_service_tracker_info_t*) { + auto *c = (std::atomic<size_t>*)handle; + c->fetch_add(1); + }; + long trkId1 = celix_bundleContext_trackServiceTrackers(ctx, nullptr, (void*)&count, add, nullptr); + EXPECT_TRUE(trkId1 >= 0); + + celix_service_tracking_options_t opts{}; + opts.filter.serviceName = "service1"; + long trkId2 = celix_bundleContext_trackServicesWithOptions(ctx, &opts); + EXPECT_TRUE(trkId2 >= 0); + + opts.filter.serviceName = "service2"; + long trkId3 = celix_bundleContext_trackServicesWithOptions(ctx, &opts); + EXPECT_TRUE(trkId3 >= 0); + + EXPECT_EQ(2, count.load()); + + celix_bundleContext_stopTracker(ctx, trkId1); + celix_bundleContext_stopTracker(ctx, trkId2); + celix_bundleContext_stopTracker(ctx, trkId3); +} TEST_F(CelixBundleContextServicesTests, serviceFactoryTest) { struct calc { @@ -825,3 +879,99 @@ TEST_F(CelixBundleContextServicesTests, trackServiceTrackerTest) { celix_bundleContext_stopTracker(ctx, trackerId); celix_bundleContext_stopTracker(ctx, tracker4); } + +TEST_F(CelixBundleContextServicesTests, reserveServiceId) { + void *svc = (void*)0x42; + + celix_service_registration_options_t opts{}; + opts.serviceName = "test_service"; + opts.reservedSvcId = celix_bundleContext_reserveSvcId(ctx); + opts.svc = &svc; + ASSERT_GT(opts.reservedSvcId, 0); + + long svcId = celix_bundleContext_registerServiceWithOptions(ctx, &opts); + EXPECT_TRUE(svcId >= 0); + EXPECT_EQ(opts.reservedSvcId, svcId); + + long foundSvcId = celix_bundleContext_findService(ctx, "test_service"); + EXPECT_GE(foundSvcId, 0); + + celix_bundleContext_unregisterService(ctx, svcId); + foundSvcId = celix_bundleContext_findService(ctx, "test_service"); + EXPECT_EQ(-1, foundSvcId); +} + +TEST_F(CelixBundleContextServicesTests, reserveServiceIdAndRemoveBeforeRegister) { + void *svc = (void*)0x42; + + celix_service_registration_options_t opts{}; + opts.serviceName = "test_service"; + opts.reservedSvcId = celix_bundleContext_reserveSvcId(ctx); + opts.svc = &svc; + ASSERT_GT(opts.reservedSvcId, 0); + + celix_bundleContext_unregisterService(ctx, opts.reservedSvcId); //will effectually cancel the registering for opts.reservedSvcId + + long svcId = celix_bundleContext_registerServiceWithOptions(ctx, &opts); //will not register service, but + EXPECT_EQ(-2, svcId); + + celix_bundleContext_unregisterService(ctx, svcId); + long foundSvcId = celix_bundleContext_findService(ctx, "test_service"); + EXPECT_EQ(-1, foundSvcId); +} + +TEST_F(CelixBundleContextServicesTests, reserveServiceIdInvalidUse) { + void *svc = (void*)0x42; + + celix_properties_t *props = celix_properties_create(); + celix_properties_set(props, "test.prop", "val"); + + celix_service_registration_options_t opts{}; + opts.serviceName = "test_service"; + opts.reservedSvcId = 42; //NOTE not valid -> not reserved through celix_bundleContext_reserveSvcId. + opts.svc = &svc; + opts.properties = props; + ASSERT_GT(opts.reservedSvcId, 0); + + long svcId = celix_bundleContext_registerServiceWithOptions(ctx, &opts); + EXPECT_EQ(-1, svcId); + + props = celix_properties_create(); + celix_properties_set(props, "test.prop", "val"); + opts.reservedSvcId = celix_bundleContext_reserveSvcId(ctx); + opts.properties = props; + svcId = celix_bundleContext_registerServiceWithOptions(ctx, &opts); + EXPECT_GE(svcId, 0); + + props = celix_properties_create(); + celix_properties_set(props, "test.prop", "val"); + opts.properties = props; + long invalidSvcId = celix_bundleContext_registerServiceWithOptions(ctx, &opts); //using a reservedId again is not valid. + EXPECT_EQ(-1, invalidSvcId); + + celix_bundleContext_unregisterService(ctx, svcId); + celix_bundleContext_unregisterService(ctx, svcId); + celix_bundleContext_unregisterService(ctx, svcId); + celix_bundleContext_unregisterService(ctx, invalidSvcId); + celix_bundleContext_unregisterService(ctx, invalidSvcId); +} + +TEST_F(CelixBundleContextServicesTests, missingStopUnregisters) { + void *svc = (void *) 0x42; + long svcId = celix_bundleContext_registerService(ctx, svc, "test_service", nullptr); //dangling service registration + EXPECT_GE(svcId, 0); + + long reservedId = celix_bundleContext_reserveSvcId(ctx); //dangling resvered id + EXPECT_GE(reservedId, 0); + + long trkId1 = celix_bundleContext_trackBundles(ctx, nullptr, nullptr, nullptr); //dangling bundle tracker + EXPECT_GE(trkId1, 0); + + long trkId2 = celix_bundleContext_trackService(ctx, "test_service", nullptr, nullptr); //dangling svc tracker + EXPECT_GE(trkId2, 0); + + long trkId3 = celix_bundleContext_trackServiceTrackers(ctx, "test_service", nullptr, nullptr, nullptr); //dangling svc tracker tracker + EXPECT_GE(trkId3, 0); + + //note "forgetting" unregister/stopTracking +} diff --git a/libs/framework/include/celix_bundle_context.h b/libs/framework/include/celix_bundle_context.h index 2f11e06..bf11df7 100644 --- a/libs/framework/include/celix_bundle_context.h +++ b/libs/framework/include/celix_bundle_context.h @@ -47,7 +47,7 @@ extern "C" { * @param svc the service object. Normally a pointer to a service struct (i.e. a struct with function pointers) * @param serviceName the service name, cannot be NULL * @param properties The meta properties associated with the service. The service registration will take ownership of the properties (i.e. no destroy needed) -* @return The serviceId (>= 0) or < 0 if the registration was unsuccessful. +* @return The serviceId (>= 0), -1 if the registration was unsuccessful and -2 if the registration was cancel (only possible when using opts.reservedServiceId. */ long celix_bundleContext_registerService(celix_bundle_context_t *ctx, void *svc, const char *serviceName, celix_properties_t *properties); @@ -126,9 +126,35 @@ typedef struct celix_service_registration_options { * for this. */ const char *serviceVersion OPTS_INIT; + + /** + * If set to > 0, this svc id will be used to register the service. + * The reservedSvcId should be reserved with a call to celix_bundleContext_reserveSvcId + */ + long reservedSvcId OPTS_INIT; } celix_service_registration_options_t; /** + * Reserves a service id, which is expected to be used to register a service in the future. + * + * If a celix_bundleContext_unregisterService with the reserved service id is called earlier than + * the celix_bundleContext_registerServiceWithOptions with the reserved service. The registering + * of the service will be cancelled instead. + + * This can help in registering/unregistering services outside of locks without creating race + * conditions. + * Note that if this is used service can "unregistered" before they are actually registered and + * as such the service pointer can be invalid at the time of registering. + * This means that the service registering can be done outside of locks, but accessing service + * should always be done using locks. + * + * + * @param ctx the bundle context. + * @return A service id, bounded to this bundle context. Note in this case the svc id will be > 0. + */ +long celix_bundleContext_reserveSvcId(celix_bundle_context_t* ctx); + +/** * C Macro to create a empty celix_service_registration_options_t type. */ #ifndef __cplusplus @@ -137,7 +163,8 @@ typedef struct celix_service_registration_options { .serviceName = NULL, \ .properties = NULL, \ .serviceLanguage = NULL, \ - .serviceVersion = NULL } + .serviceVersion = NULL, \ + .reservedSvcId = 0 } #endif @@ -192,7 +219,8 @@ celix_array_list_t* celix_bundleContext_findServices(celix_bundle_context_t *ctx */ typedef struct celix_service_filter_options { /** - * The required service name. + * The service name. + * If NULL is used any services which matches the filter string will be tracked. */ const char* serviceName OPTS_INIT; @@ -256,7 +284,8 @@ celix_array_list_t* celix_bundleContext_findServicesWithOptions(celix_bundle_con * If a service is removed a the callback with be called with next highest ranking service or NULL as service. * * @param ctx The bundle context. - * @param serviceName The required service name to track + * @param serviceName The required service name to track. + * If NULL is all service are tracked. * @param callbackHandle The data pointer, which will be used in the callbacks * @param set is a required callback, which will be called when a new highest ranking service is set. * @return the tracker id (>=0) or < 0 if unsuccessful. @@ -273,6 +302,7 @@ long celix_bundleContext_trackService( * * @param ctx The bundle context. * @param serviceName The required service name to track + * If NULL is all service are tracked. * @param callbackHandle The data pointer, which will be used in the callbacks * @param add is a required callback, which will be called when a service is added and initially for the existing service. * @param remove is a required callback, which will be called when a service is removed @@ -815,6 +845,7 @@ typedef struct celix_service_tracker_info { * * @param ctx The bundle context * @param serviceName The target service name for the service tracker to track. + * If NULL is provided, add/remove callbacks will be called for all service trackers in the framework. * @param callbackHandle The callback handle which will be provided as handle in the trackerAdd and trackerRemove callback. * @param trackerAdd Called when a service tracker is added, which tracks the provided service name. Will also be called * for all existing service tracker when this tracker is started. diff --git a/libs/framework/include/service_registry.h b/libs/framework/include/service_registry.h index db93cb5..4697b79 100644 --- a/libs/framework/include/service_registry.h +++ b/libs/framework/include/service_registry.h @@ -60,8 +60,6 @@ serviceRegistry_registerServiceFactory(service_registry_pt registry, celix_bundl celix_status_t serviceRegistry_unregisterService(service_registry_pt registry, celix_bundle_t *bundle, service_registration_pt registration); -celix_status_t serviceRegistry_clearServiceRegistrations(service_registry_pt registry, celix_bundle_t *bundle); - celix_status_t serviceRegistry_getServiceReference(service_registry_pt registry, celix_bundle_t *bundle, service_registration_pt registration, service_reference_pt *reference); @@ -95,6 +93,15 @@ celix_status_t celix_serviceRegistry_addServiceListener(celix_service_registry_t celix_status_t celix_serviceRegistry_removeServiceListener(celix_service_registry_t *reg, celix_service_listener_t *listener); +celix_status_t +celix_serviceRegistry_registerService( + celix_service_registry_t *reg, + const celix_bundle_t *bnd, + const char *serviceName, + void* service, + celix_properties_t* props, + long reserveId, + service_registration_t **registration); celix_status_t celix_serviceRegistry_registerServiceFactory( @@ -103,6 +110,7 @@ celix_serviceRegistry_registerServiceFactory( const char *serviceName, celix_service_factory_t *factory, celix_properties_t* props, + long reserveId, service_registration_t **registration); /** diff --git a/libs/framework/src/bundle.c b/libs/framework/src/bundle.c index 9994675..be747b9 100644 --- a/libs/framework/src/bundle.c +++ b/libs/framework/src/bundle.c @@ -173,7 +173,7 @@ celix_status_t bundle_getEntry(const_bundle_pt bundle, const char* name, char** } celix_status_t bundle_getState(const_bundle_pt bundle, bundle_state_e *state) { - if(bundle==NULL){ + if (bundle==NULL) { *state = OSGI_FRAMEWORK_BUNDLE_UNKNOWN; return CELIX_BUNDLE_EXCEPTION; } diff --git a/libs/framework/src/bundle_context.c b/libs/framework/src/bundle_context.c index b7f5326..b052d2e 100644 --- a/libs/framework/src/bundle_context.c +++ b/libs/framework/src/bundle_context.c @@ -21,6 +21,7 @@ #include <stdio.h> #include <string.h> #include <utils.h> +#include <assert.h> #include "celix_utils.h" #include "celix_constants.h" @@ -34,11 +35,14 @@ #include "dm_dependency_manager_impl.h" #include "celix_array_list.h" #include "module.h" +#include "service_tracker_private.h" +#include "celix_array_list.h" static celix_status_t bundleContext_bundleChanged(void *handle, bundle_event_t *event); static void bundleContext_cleanupBundleTrackers(bundle_context_t *ct); static void bundleContext_cleanupServiceTrackers(bundle_context_t *ctx); static void bundleContext_cleanupServiceTrackerTrackers(bundle_context_t *ctx); +static void bundleContext_cleanupServiceRegistration(bundle_context_t* ctx); celix_status_t bundleContext_create(framework_pt framework, celix_framework_logger_t* logger, bundle_pt bundle, bundle_context_pt *bundle_context) { celix_status_t status = CELIX_SUCCESS; @@ -61,6 +65,7 @@ celix_status_t bundleContext_create(framework_pt framework, celix_framework_logg context->bundleTrackers = hashMap_create(NULL,NULL,NULL,NULL); context->serviceTrackers = hashMap_create(NULL,NULL,NULL,NULL); context->metaTrackers = hashMap_create(NULL,NULL,NULL,NULL); + context->reservedIds = hashMap_create(NULL,NULL,NULL,NULL); context->nextTrackerId = 1L; *bundle_context = context; @@ -80,15 +85,21 @@ celix_status_t bundleContext_destroy(bundle_context_pt context) { celixThreadMutex_lock(&context->mutex); - bundleContext_cleanupBundleTrackers(context); - bundleContext_cleanupServiceTrackers(context); - bundleContext_cleanupServiceTrackerTrackers(context); + assert(hashMap_size(context->bundleTrackers) == 0); + hashMap_destroy(context->bundleTrackers, false, false); + assert(hashMap_size(context->serviceTrackers) == 0); + hashMap_destroy(context->serviceTrackers, false, false); + assert(hashMap_size(context->metaTrackers) == 0); + hashMap_destroy(context->metaTrackers, false, false); + assert(celix_arrayList_size(context->svcRegistrations) == 0); + celix_arrayList_destroy(context->svcRegistrations); + assert(hashMap_size(context->reservedIds) == 0); + hashMap_destroy(context->reservedIds, false, false); - //NOTE still present service registrations will be cleared during bundle stop in the + //NOTE still present service registrations will be cleared during bundle stop in the //service registry (serviceRegistry_clearServiceRegistrations). celixThreadMutex_unlock(&context->mutex); - celixThreadMutex_destroy(&context->mutex); - arrayList_destroy(context->svcRegistrations); + celixThreadMutex_destroy(&context->mutex); if (context->mng != NULL) { celix_dependencyManager_removeAllComponents(context->mng); @@ -106,6 +117,15 @@ celix_status_t bundleContext_destroy(bundle_context_pt context) { return status; } +void celix_bundleContext_cleanup(celix_bundle_context_t *ctx) { + //NOTE not perfect, because stopping of registrations/tracker when the activator is destroyed can lead to segfault. + //but atleast we can try to warn the bundle implementer that some cleanup is missing. + bundleContext_cleanupBundleTrackers(ctx); + bundleContext_cleanupServiceTrackers(ctx); + bundleContext_cleanupServiceTrackerTrackers(ctx); + bundleContext_cleanupServiceRegistration(ctx); +} + celix_status_t bundleContext_getBundle(bundle_context_pt context, bundle_pt *out) { celix_status_t status = CELIX_SUCCESS; celix_bundle_t *bnd = celix_bundleContext_getBundle(context); @@ -451,7 +471,10 @@ long celix_bundleContext_registerServiceFactory(celix_bundle_context_t *ctx, cel long celix_bundleContext_registerServiceWithOptions(bundle_context_t *ctx, const celix_service_registration_options_t *opts) { long svcId = -1; + long reservedId = -1; service_registration_t *reg = NULL; + + //set properties celix_properties_t *props = opts->properties; if (props == NULL) { props = celix_properties_create(); @@ -461,16 +484,43 @@ long celix_bundleContext_registerServiceWithOptions(bundle_context_t *ctx, const } const char *lang = opts->serviceLanguage != NULL && strncmp("", opts->serviceLanguage, 1) != 0 ? opts->serviceLanguage : CELIX_FRAMEWORK_SERVICE_C_LANGUAGE; celix_properties_set(props, CELIX_FRAMEWORK_SERVICE_LANGUAGE, lang); - if (opts->serviceName != NULL && strncmp("", opts->serviceName, 1) != 0) { + + bool valid = opts->serviceName != NULL && strncmp("", opts->serviceName, 1) != 0; + if (!valid) { + fw_log(ctx->framework->logger, CELIX_LOG_LEVEL_ERROR, "Required serviceName argument is NULL"); + } + + //check reserved ids + bool cancelled = false; + if (valid && opts->reservedSvcId > 0) { + celixThreadMutex_lock(&ctx->mutex); + if (hashMap_containsKey(ctx->reservedIds, (void*)opts->reservedSvcId)) { + cancelled = (bool)hashMap_remove(ctx->reservedIds, (void*)opts->reservedSvcId); + if (!cancelled) { + reservedId = opts->reservedSvcId; + } else { + svcId = -2; + } + } else { + //not reserved + valid = false; + fw_log(ctx->framework->logger, CELIX_LOG_LEVEL_ERROR, "Invalid reservedSvcId provided (%li). Ensure that service id are reserved using celix_bundleContext_reserveId and only used once", opts->reservedSvcId); + } + celixThreadMutex_unlock(&ctx->mutex); + } + + if (valid && !cancelled) { if (opts->factory != NULL) { - reg = celix_framework_registerServiceFactory(ctx->framework, ctx->bundle, opts->serviceName, opts->factory, props); + reg = celix_framework_registerServiceFactory(ctx->framework, ctx->bundle, opts->serviceName, opts->factory, props, reservedId); } else { - bundleContext_registerService(ctx, opts->serviceName, opts->svc, props, ®); + reg = celix_framework_registerService(ctx->framework, ctx->bundle, opts->serviceName, opts->svc, props, reservedId); } svcId = serviceRegistration_getServiceId(reg); //save to call with NULL } else { framework_logIfError(ctx->framework->logger, CELIX_ILLEGAL_ARGUMENT, NULL, "Required serviceName argument is NULL"); } + + if (svcId < 0) { properties_destroy(props); } else { @@ -483,6 +533,7 @@ long celix_bundleContext_registerServiceWithOptions(bundle_context_t *ctx, const void celix_bundleContext_unregisterService(bundle_context_t *ctx, long serviceId) { service_registration_t *found = NULL; + bool wasReserved = false; if (ctx != NULL && serviceId >= 0) { celixThreadMutex_lock(&ctx->mutex); unsigned int size = arrayList_size(ctx->svcRegistrations); @@ -499,8 +550,22 @@ void celix_bundleContext_unregisterService(bundle_context_t *ctx, long serviceId } celixThreadMutex_unlock(&ctx->mutex); + celixThreadMutex_lock(&ctx->mutex); + if (hashMap_containsKey(ctx->reservedIds, (void*)serviceId)) { + bool cancelled = (bool)hashMap_get(ctx->reservedIds, (void*)serviceId); + if (cancelled) { + //already cancelled + fw_log(ctx->framework->logger, CELIX_LOG_LEVEL_ERROR, "Service registration for service with a svc id %li already cancelled!", serviceId); + } + wasReserved = true; + hashMap_put(ctx->reservedIds, (void*)serviceId, (void*)true); + } + celixThreadMutex_unlock(&ctx->mutex); + if (found != NULL) { serviceRegistration_unregister(found); + } else if (wasReserved) { + //nop } else { framework_logIfError(ctx->framework->logger, CELIX_ILLEGAL_ARGUMENT, NULL, "No service registered with svc id %li for bundle %s (bundle id: %li)!", serviceId, celix_bundle_getSymbolicName(ctx->bundle), celix_bundle_getId(ctx->bundle)); } @@ -604,36 +669,130 @@ bool celix_bundleContext_useBundle( } static void bundleContext_cleanupBundleTrackers(bundle_context_t *ctx) { + module_pt module; + const char *symbolicName; + bundle_getCurrentModule(ctx->bundle, &module); + module_getSymbolicName(module, &symbolicName); + + celix_array_list_t* danglingTrkIds = NULL; + + celixThreadMutex_lock(&ctx->mutex); hash_map_iterator_t iter = hashMapIterator_construct(ctx->bundleTrackers); while (hashMapIterator_hasNext(&iter)) { - celix_bundle_context_bundle_tracker_entry_t *tracker = hashMapIterator_nextValue(&iter); - fw_removeBundleListener(ctx->framework, ctx->bundle, &tracker->listener); - free(tracker); + long trkId = (long)hashMapIterator_nextKey(&iter); + fw_log(ctx->framework->logger, CELIX_LOG_LEVEL_ERROR, "Dangling bundle tracker with id %li for bundle %s. Add missing 'celix_bundleContext_stopTracker' calls.", trkId, symbolicName); + if (danglingTrkIds == NULL) { + danglingTrkIds = celix_arrayList_create(); + } + celix_arrayList_addLong(danglingTrkIds, trkId); + } + celixThreadMutex_unlock(&ctx->mutex); + + if (danglingTrkIds != NULL) { + for (int i = 0; i < celix_arrayList_size(danglingTrkIds); ++i) { + long trkId = celix_arrayList_getLong(danglingTrkIds, i); + celix_bundleContext_stopTracker(ctx, trkId); + } + celix_arrayList_destroy(danglingTrkIds); } - hashMap_destroy(ctx->bundleTrackers, false, false); } static void bundleContext_cleanupServiceTrackers(bundle_context_t *ctx) { - if(hashMap_size(ctx->serviceTrackers) > 0) { - module_pt module; - const char *symbolicName; - bundle_getCurrentModule(ctx->bundle, &module); - module_getSymbolicName(module, &symbolicName); - fw_log(ctx->framework->logger, CELIX_LOG_LEVEL_WARNING, "Dangling service tracker(s) for bundle %s.", symbolicName); + module_pt module; + const char *symbolicName; + bundle_getCurrentModule(ctx->bundle, &module); + module_getSymbolicName(module, &symbolicName); + + celix_array_list_t* danglingTrkIds = NULL; + + celixThreadMutex_lock(&ctx->mutex); + hash_map_iterator_t iter = hashMapIterator_construct(ctx->serviceTrackers); + while (hashMapIterator_hasNext(&iter)) { + long trkId = (long)hashMapIterator_nextKey(&iter); + celix_service_tracker_t *entry = hashMap_get(ctx->serviceTrackers, (void*)trkId); + fw_log(ctx->framework->logger, CELIX_LOG_LEVEL_ERROR, "Dangling service tracker with trkId %li, for bundle %s and with filter %s. Add missing 'celix_bundleContext_stopTracker' calls.", trkId, symbolicName, entry->filter); + if (danglingTrkIds == NULL) { + danglingTrkIds = celix_arrayList_create(); + } + celix_arrayList_addLong(danglingTrkIds, trkId); + } + celixThreadMutex_unlock(&ctx->mutex); + + if (danglingTrkIds != NULL) { + for (int i = 0; i < celix_arrayList_size(danglingTrkIds); ++i) { + long trkId = celix_arrayList_getLong(danglingTrkIds, i); + celix_bundleContext_stopTracker(ctx, trkId); + } + celix_arrayList_destroy(danglingTrkIds); } - hashMap_destroy(ctx->serviceTrackers, false, false); } static void bundleContext_cleanupServiceTrackerTrackers(bundle_context_t *ctx) { + module_pt module; + const char *symbolicName; + bundle_getCurrentModule(ctx->bundle, &module); + module_getSymbolicName(module, &symbolicName); + + celix_array_list_t* danglingTrkIds = NULL; + + celixThreadMutex_lock(&ctx->mutex); hash_map_iterator_t iter = hashMapIterator_construct(ctx->metaTrackers); while (hashMapIterator_hasNext(&iter)) { - celix_bundle_context_service_tracker_tracker_entry_t *entry = hashMapIterator_nextValue(&iter); - serviceRegistration_unregister(entry->hookReg); - free(entry); + long trkId = (long)hashMapIterator_nextKey(&iter); + celix_bundle_context_service_tracker_tracker_entry_t *entry = hashMap_get(ctx->metaTrackers, (void*)trkId); + fw_log(ctx->framework->logger, CELIX_LOG_LEVEL_ERROR, "Dangling meta tracker (service tracker tracker) with trkId %li, for bundle %s and for the services %s. Add missing 'celix_bundleContext_stopTracker' calls.", trkId, symbolicName, entry->serviceName); + if (danglingTrkIds == NULL) { + danglingTrkIds = celix_arrayList_create(); + } + celix_arrayList_addLong(danglingTrkIds, trkId); + } + celixThreadMutex_unlock(&ctx->mutex); + + if (danglingTrkIds != NULL) { + for (int i = 0; i < celix_arrayList_size(danglingTrkIds); ++i) { + long trkId = celix_arrayList_getLong(danglingTrkIds, i); + celix_bundleContext_stopTracker(ctx, trkId); + } + celix_arrayList_destroy(danglingTrkIds); } - hashMap_destroy(ctx->metaTrackers, false, false); } +static void bundleContext_cleanupServiceRegistration(bundle_context_t* ctx) { + module_pt module; + const char *symbolicName; + bundle_getCurrentModule(ctx->bundle, &module); + module_getSymbolicName(module, &symbolicName); + + celix_array_list_t* danglingSvcIds = NULL; + + celixThreadMutex_lock(&ctx->mutex); + for (int i = 0; i < celix_arrayList_size(ctx->svcRegistrations); ++i) { + service_registration_pt reg = celix_arrayList_get(ctx->svcRegistrations, i); + long svcId = serviceRegistration_getServiceId(reg); + const char* svcName = NULL; + serviceRegistration_getServiceName(reg, &svcName); + fw_log(ctx->framework->logger, CELIX_LOG_LEVEL_ERROR, "Dangling service registration with svcId %li, for bundle %s and with service name %s. Add missing 'celix_bundleContext_unregisterService' calls.", svcId, symbolicName, svcName); + if (danglingSvcIds == NULL) { + danglingSvcIds = celix_arrayList_create(); + } + celix_arrayList_addLong(danglingSvcIds, svcId); + } + hash_map_iterator_t iter = hashMapIterator_construct(ctx->reservedIds); + while (hashMapIterator_hasNext(&iter)) { + long svcId = (long)hashMapIterator_nextKey(&iter); + fw_log(ctx->framework->logger, CELIX_LOG_LEVEL_ERROR, "Dangling reserved svc id %li for bundle %s. Add missing 'celix_bundleContext_unregisterService' calls.", svcId, symbolicName); + } + hashMap_clear(ctx->reservedIds, false, false); + celixThreadMutex_unlock(&ctx->mutex); + + if (danglingSvcIds != NULL) { + for (int i = 0; i < celix_arrayList_size(danglingSvcIds); ++i) { + long svcId = celix_arrayList_getLong(danglingSvcIds, i); + celix_bundleContext_unregisterService(ctx, svcId); + } + celix_arrayList_destroy(danglingSvcIds); + } +} void celix_bundleContext_stopTracker(bundle_context_t *ctx, long trackerId) { if (ctx != NULL && trackerId >0) { @@ -841,12 +1000,23 @@ long celix_bundleContext_trackServices( long celix_bundleContext_trackServicesWithOptions(bundle_context_t *ctx, const celix_service_tracking_options_t *opts) { + if (ctx == NULL) { + return -1L; + } else if (opts == NULL) { + fw_log(ctx->framework->logger, CELIX_LOG_LEVEL_ERROR, "Cannot track services with a NULL service tracking options argument"); + return -1L; + } + + if (opts->filter.serviceName == NULL) { + fw_log(ctx->framework->logger, CELIX_LOG_LEVEL_DEBUG, "Starting a tracker for any services"); + } + long trackerId = -1; celix_service_tracker_t *tracker = celix_serviceTracker_createWithOptions(ctx, opts); if (tracker != NULL) { celixThreadMutex_lock(&ctx->mutex); trackerId = ctx->nextTrackerId++; - hashMap_put(ctx->serviceTrackers, (void*)trackerId, tracker); + hashMap_put(ctx->serviceTrackers, (void *) trackerId, tracker); celixThreadMutex_unlock(&ctx->mutex); } return trackerId; @@ -915,9 +1085,11 @@ static celix_status_t bundleContext_callServicedTrackerTrackerCallback(void *han trkInfo.serviceLanguage = celix_filter_findAttribute(trkInfo.filter, CELIX_FRAMEWORK_SERVICE_LANGUAGE); const char *filterSvcName = celix_filter_findAttribute(trkInfo.filter, OSGI_FRAMEWORK_OBJECTCLASS); - if (add && entry->add != NULL && filterSvcName != NULL && strncmp(filterSvcName, entry->serviceName, 1024*1024) == 0) { + bool match = entry->serviceName == NULL || (filterSvcName != NULL && strncmp(filterSvcName, entry->serviceName, 1024*1024) == 0); + + if (add && entry->add != NULL && match) { entry->add(entry->callbackHandle, &trkInfo); - } else if (entry->remove != NULL && filterSvcName != NULL && strncmp(filterSvcName, entry->serviceName, 1024*1024) == 0) { + } else if (!add && entry->remove != NULL && match) { entry->remove(entry->callbackHandle, &trkInfo); } celix_filter_destroy(trkInfo.filter); @@ -944,8 +1116,7 @@ long celix_bundleContext_trackServiceTrackers( long trackerId = -1L; if (serviceName == NULL) { - fw_log(ctx->framework->logger, CELIX_LOG_LEVEL_ERROR, "Required serviceName not provided for function ", __FUNCTION__); - return trackerId; + fw_log(ctx->framework->logger, CELIX_LOG_LEVEL_DEBUG, "Note starting a meta tracker for all services", __FUNCTION__); } celix_bundle_context_service_tracker_tracker_entry_t *entry = calloc(1, sizeof(*entry)); @@ -953,7 +1124,7 @@ long celix_bundleContext_trackServiceTrackers( entry->callbackHandle = callbackHandle; entry->add = trackerAdd; entry->remove = trackerRemove; - entry->serviceName = strndup(serviceName, 1024*1024); + entry->serviceName = celix_utils_strdup(serviceName); entry->hook.handle = entry; entry->hook.added = bundleContext_callServicedTrackerTrackerAdd; @@ -1052,4 +1223,18 @@ char* celix_bundleContext_getBundleSymbolicName(celix_bundle_context_t *ctx, lon char *name = NULL; celix_framework_useBundle(ctx->framework, false, bndId, &name, celix_bundleContext_getBundleSymbolicNameCallback); return name; +} + +long celix_bundleContext_reserveSvcId(celix_bundle_context_t *ctx) { + celixThreadMutex_lock(&ctx->mutex); + long reservedId = celix_serviceRegistry_nextSvcId(ctx->framework->registry); + if (reservedId == 0) { + //note svcId is the first and valid id, but for reserved id, we want a service id > 0, + //so that a 0 initialized CELIX_EMPTY_SERVICE_REGISTRATION_OPTIONS is configured not to + //use a reservedId + reservedId = celix_serviceRegistry_nextSvcId(ctx->framework->registry); + } + hashMap_put(ctx->reservedIds, (void*)reservedId, (void*)false); + celixThreadMutex_unlock(&ctx->mutex); + return reservedId; } \ No newline at end of file diff --git a/libs/framework/src/bundle_context_private.h b/libs/framework/src/bundle_context_private.h index e3b7619..e442bd1 100644 --- a/libs/framework/src/bundle_context_private.h +++ b/libs/framework/src/bundle_context_private.h @@ -56,7 +56,11 @@ struct celix_bundle_context { hash_map_t *bundleTrackers; //key = trackerId, value = celix_bundle_context_bundle_tracker_entry_t* hash_map_t *serviceTrackers; //key = trackerId, value = celix_service_tracker_t* hash_map_t *metaTrackers; //key = trackerId, value = celix_bundle_context_service_tracker_tracker_entry_t* + hash_map_t *reservedIds; //key = reserved svc id, value = cancelled (bool) }; +void celix_bundleContext_cleanup(celix_bundle_context_t *ctx); + + #endif /* BUNDLE_CONTEXT_PRIVATE_H_ */ diff --git a/libs/framework/src/framework.c b/libs/framework/src/framework.c index 73c9e3a..45ac88a 100644 --- a/libs/framework/src/framework.c +++ b/libs/framework/src/framework.c @@ -1104,7 +1104,8 @@ celix_status_t fw_stopBundle(framework_pt framework, long bndId, bool record) { if (bndId > 0) { celix_serviceTracker_syncForContext(entry->bnd->context); - status = CELIX_DO_IF(status, serviceRegistry_clearServiceRegistrations(framework->registry, entry->bnd)); + celix_bundleContext_cleanup(entry->bnd->context); + celix_serviceTracker_syncForContext(framework->bundle->context); if (status == CELIX_SUCCESS) { module_pt module = NULL; const char *symbolicName = NULL; @@ -1112,8 +1113,6 @@ celix_status_t fw_stopBundle(framework_pt framework, long bndId, bool record) { bundle_getCurrentModule(entry->bnd, &module); module_getSymbolicName(module, &symbolicName); bundle_getBundleId(entry->bnd, &id); - - serviceRegistry_clearReferencesFor(framework->registry, entry->bnd); } if (context != NULL) { @@ -1125,6 +1124,8 @@ celix_status_t fw_stopBundle(framework_pt framework, long bndId, bool record) { } else if (bndId == 0) { //framework bundle celix_serviceTracker_syncForContext(framework->bundle->context); + celix_bundleContext_cleanup(entry->bnd->context); + celix_serviceTracker_syncForContext(framework->bundle->context); } } @@ -2384,9 +2385,9 @@ bool celix_framework_useBundle(framework_t *fw, bool onlyActive, long bundleId, return called; } -service_registration_t* celix_framework_registerServiceFactory(framework_t *fw , const celix_bundle_t *bnd, const char* serviceName, celix_service_factory_t *factory, celix_properties_t *properties) { +service_registration_t* celix_framework_registerServiceFactory(framework_t *fw , celix_bundle_t *bnd, const char* serviceName, celix_service_factory_t *factory, celix_properties_t *properties, long reservedId) { const char *error = NULL; - celix_status_t status = CELIX_SUCCESS; + celix_status_t status; service_registration_t *reg = NULL; long bndId = celix_bundle_getId(bnd); @@ -2394,7 +2395,33 @@ service_registration_t* celix_framework_registerServiceFactory(framework_t *fw , if (serviceName != NULL && factory != NULL) { - status = CELIX_DO_IF(status, celix_serviceRegistry_registerServiceFactory(fw->registry, bnd, serviceName, factory, properties, ®)); + status = celix_serviceRegistry_registerServiceFactory(fw->registry, bnd, serviceName, factory, properties, reservedId, ®); + } else { + fw_log(fw->logger, CELIX_LOG_LEVEL_ERROR, "Invalid arguments serviceName and/or factory argument is NULL (%s/%p)", serviceName, factory); + status = CELIX_ILLEGAL_ARGUMENT; + } + + fw_bundleEntry_decreaseUseCount(entry); + + framework_logIfError(fw->logger, status, error, "Cannot register service factory: %s", serviceName); + + return reg; +} + +service_registration_t* celix_framework_registerService(framework_t *fw , celix_bundle_t *bnd, const char* serviceName, void* svc, celix_properties_t *properties, long reservedId) { + const char *error = NULL; + celix_status_t status; + service_registration_t *reg = NULL; + + long bndId = celix_bundle_getId(bnd); + celix_framework_bundle_entry_t *entry = fw_bundleEntry_getBundleEntryAndIncreaseUseCount(fw, bndId); + + + if (serviceName != NULL && svc != NULL) { + status = celix_serviceRegistry_registerService(fw->registry, bnd, serviceName, svc, properties, reservedId, ®); + } else { + fw_log(fw->logger, CELIX_LOG_LEVEL_ERROR, "Invalid arguments serviceName and/or svc argument is NULL (%s/%p)", serviceName, svc); + status = CELIX_ILLEGAL_ARGUMENT; } fw_bundleEntry_decreaseUseCount(entry); diff --git a/libs/framework/src/framework_private.h b/libs/framework/src/framework_private.h index 4c0b517..bbd6370 100644 --- a/libs/framework/src/framework_private.h +++ b/libs/framework/src/framework_private.h @@ -151,6 +151,8 @@ FRAMEWORK_EXPORT bundle_pt framework_getBundleById(framework_pt framework, long ********************************************************************************************************************** **********************************************************************************************************************/ -service_registration_t* celix_framework_registerServiceFactory(framework_t *fw , const celix_bundle_t *bnd, const char* serviceName, celix_service_factory_t *factory, celix_properties_t *properties); +service_registration_t* celix_framework_registerServiceFactory(framework_t *fw, celix_bundle_t *bnd, const char* serviceName, celix_service_factory_t *factory, celix_properties_t *properties, long reservedSvcId); +service_registration_t* celix_framework_registerService(framework_t *fw, celix_bundle_t *bnd, const char* serviceName, void* svc, celix_properties_t *properties, long reservedSvcId); + #endif /* FRAMEWORK_PRIVATE_H_ */ diff --git a/libs/framework/src/service_registry.c b/libs/framework/src/service_registry.c index 6c7b633..0184f2d 100644 --- a/libs/framework/src/service_registry.c +++ b/libs/framework/src/service_registry.c @@ -36,11 +36,10 @@ #define CHECK_DELETED_REFERENCES false #endif -static celix_status_t serviceRegistry_registerServiceInternal(service_registry_pt registry, bundle_pt bundle, const char* serviceName, const void * serviceObject, properties_pt dictionary, enum celix_service_type svcType, service_registration_pt *registration); +static celix_status_t serviceRegistry_registerServiceInternal(service_registry_pt registry, bundle_pt bundle, const char* serviceName, const void * serviceObject, properties_pt dictionary, long reservedId, enum celix_service_type svcType, service_registration_pt *registration); static celix_status_t serviceRegistry_addHooks(service_registry_pt registry, const char* serviceName, const void *serviceObject, service_registration_pt registration); static celix_status_t serviceRegistry_removeHook(service_registry_pt registry, service_registration_pt registration); static void serviceRegistry_logWarningServiceReferenceUsageCount(service_registry_pt registry, bundle_pt bundle, service_reference_pt ref, size_t usageCount, size_t refCount); -static void serviceRegistry_logWarningServiceRegistration(service_registry_pt registry, service_registration_pt reg); static celix_status_t serviceRegistry_checkReference(service_registry_pt registry, service_reference_pt ref, reference_status_t *refStatus); static void serviceRegistry_logIllegalReference(service_registry_pt registry, service_reference_pt reference, @@ -79,8 +78,8 @@ celix_status_t serviceRegistry_create(framework_pt framework, service_registry_p reg->serviceRegistrations = hashMap_create(NULL, NULL, NULL, NULL); reg->framework = framework; - reg->nextServiceId = 1L; - reg->serviceReferences = hashMap_create(NULL, NULL, NULL, NULL); + reg->nextServiceId = 1L; + reg->serviceReferences = hashMap_create(NULL, NULL, NULL, NULL); reg->checkDeletedReferences = CHECK_DELETED_REFERENCES; reg->deletedServiceReferences = hashMap_create(NULL, NULL, NULL, NULL); @@ -198,16 +197,16 @@ celix_status_t serviceRegistry_getRegisteredServices(service_registry_pt registr } celix_status_t serviceRegistry_registerService(service_registry_pt registry, bundle_pt bundle, const char* serviceName, const void* serviceObject, properties_pt dictionary, service_registration_pt *registration) { - return serviceRegistry_registerServiceInternal(registry, bundle, serviceName, serviceObject, dictionary, CELIX_PLAIN_SERVICE, registration); + return serviceRegistry_registerServiceInternal(registry, bundle, serviceName, serviceObject, dictionary, 0 /*TODO*/, CELIX_PLAIN_SERVICE, registration); } celix_status_t serviceRegistry_registerServiceFactory(service_registry_pt registry, bundle_pt bundle, const char* serviceName, service_factory_pt factory, properties_pt dictionary, service_registration_pt *registration) { - return serviceRegistry_registerServiceInternal(registry, bundle, serviceName, (const void *) factory, dictionary, CELIX_DEPRECATED_FACTORY_SERVICE, registration); + return serviceRegistry_registerServiceInternal(registry, bundle, serviceName, (const void *) factory, dictionary, 0 /*TODO*/, CELIX_DEPRECATED_FACTORY_SERVICE, registration); } -static celix_status_t serviceRegistry_registerServiceInternal(service_registry_pt registry, bundle_pt bundle, const char* serviceName, const void * serviceObject, properties_pt dictionary, enum celix_service_type svcType, service_registration_pt *registration) { +static celix_status_t serviceRegistry_registerServiceInternal(service_registry_pt registry, bundle_pt bundle, const char* serviceName, const void * serviceObject, properties_pt dictionary, long reservedId, enum celix_service_type svcType, service_registration_pt *registration) { array_list_pt regs; - long svcId = celix_serviceRegistry_nextSvcId(registry); + long svcId = reservedId > 0 ? reservedId : celix_serviceRegistry_nextSvcId(registry); if (svcType == CELIX_DEPRECATED_FACTORY_SERVICE) { *registration = serviceRegistration_createServiceFactory(registry->callback, bundle, serviceName, @@ -299,50 +298,6 @@ celix_status_t serviceRegistry_unregisterService(service_registry_pt registry, b return CELIX_SUCCESS; } -celix_status_t serviceRegistry_clearServiceRegistrations(service_registry_pt registry, bundle_pt bundle) { - celix_status_t status = CELIX_SUCCESS; - array_list_pt registrations = NULL; - bool registrationsLeft; - - celixThreadRwlock_writeLock(®istry->lock); - registrations = hashMap_get(registry->serviceRegistrations, bundle); - registrationsLeft = (registrations != NULL); - if (registrationsLeft) { - registrationsLeft = (arrayList_size(registrations) > 0); - } - celixThreadRwlock_unlock(®istry->lock); - - while (registrationsLeft) { - service_registration_pt reg = arrayList_get(registrations, 0); - - serviceRegistry_logWarningServiceRegistration(registry, reg); - - if (serviceRegistration_isValid(reg)) { - serviceRegistration_unregister(reg); - } - else { - arrayList_remove(registrations, 0); - } - - // not removed by last unregister call? - celixThreadRwlock_writeLock(®istry->lock); - registrations = hashMap_get(registry->serviceRegistrations, bundle); - registrationsLeft = (registrations != NULL); - if (registrationsLeft) { - registrationsLeft = (arrayList_size(registrations) > 0); - } - celixThreadRwlock_unlock(®istry->lock); - } - - return status; -} - -static void serviceRegistry_logWarningServiceRegistration(service_registry_pt registry __attribute__((unused)), service_registration_pt reg) { - const char *servName = NULL; - serviceRegistration_getServiceName(reg, &servName); - fw_log(registry->framework->logger, CELIX_LOG_LEVEL_WARNING, "Dangling service registration for service %s. Look for missing bundleContext_unregisterService/serviceRegistration_unregister calls.", servName); -} - celix_status_t serviceRegistry_getServiceReference(service_registry_pt registry, bundle_pt owner, service_registration_pt registration, service_reference_pt *out) { celix_status_t status = CELIX_SUCCESS; @@ -959,8 +914,21 @@ celix_serviceRegistry_registerServiceFactory( const char *serviceName, celix_service_factory_t *factory, celix_properties_t* props, + long reserveId, + service_registration_t **registration) { + return serviceRegistry_registerServiceInternal(reg, (celix_bundle_t*)bnd, serviceName, (const void *) factory, props, reserveId, CELIX_FACTORY_SERVICE, registration); +} + +celix_status_t +celix_serviceRegistry_registerService( + celix_service_registry_t *reg, + const celix_bundle_t *bnd, + const char *serviceName, + void* service, + celix_properties_t* props, + long reserveId, service_registration_t **registration) { - return serviceRegistry_registerServiceInternal(reg, (celix_bundle_t*)bnd, serviceName, (const void *) factory, props, CELIX_FACTORY_SERVICE, registration); + return serviceRegistry_registerServiceInternal(reg, (celix_bundle_t*)bnd, serviceName, (const void *) service, props, reserveId, CELIX_PLAIN_SERVICE, registration); } static celix_service_registry_listener_hook_entry_t* celix_createHookEntry(long svcId, celix_listener_hook_service_t *hook) { diff --git a/libs/framework/src/service_tracker.c b/libs/framework/src/service_tracker.c index 53fd31a..8775c23 100644 --- a/libs/framework/src/service_tracker.c +++ b/libs/framework/src/service_tracker.c @@ -687,11 +687,12 @@ celix_service_tracker_t* celix_serviceTracker_createWithOptions( const celix_service_tracking_options_t *opts ) { celix_service_tracker_t *tracker = NULL; - if (ctx != NULL && opts != NULL && opts->filter.serviceName != NULL) { + const char* serviceName = opts->filter.serviceName == NULL ? "*" : opts->filter.serviceName; + if (ctx != NULL && opts != NULL) { tracker = calloc(1, sizeof(*tracker)); if (tracker != NULL) { tracker->context = ctx; - tracker->serviceName = celix_utils_strdup(opts->filter.serviceName); + tracker->serviceName = celix_utils_strdup(serviceName); //setting callbacks tracker->callbackHandle = opts->callbackHandle; @@ -738,23 +739,23 @@ celix_service_tracker_t* celix_serviceTracker_createWithOptions( //setting filter if (opts->filter.ignoreServiceLanguage) { if (opts->filter.filter != NULL && versionRange != NULL) { - asprintf(&tracker->filter, "(&(%s=%s)%s%s)", OSGI_FRAMEWORK_OBJECTCLASS, opts->filter.serviceName, versionRange, opts->filter.filter); + asprintf(&tracker->filter, "(&(%s=%s)%s%s)", OSGI_FRAMEWORK_OBJECTCLASS, serviceName, versionRange, opts->filter.filter); } else if (versionRange != NULL) { - asprintf(&tracker->filter, "(&(%s=%s)%s)", OSGI_FRAMEWORK_OBJECTCLASS, opts->filter.serviceName, versionRange); + asprintf(&tracker->filter, "(&(%s=%s)%s)", OSGI_FRAMEWORK_OBJECTCLASS, serviceName, versionRange); } else if (opts->filter.filter != NULL) { - asprintf(&tracker->filter, "(&(%s=%s)%s)", OSGI_FRAMEWORK_OBJECTCLASS, opts->filter.serviceName, opts->filter.filter); + asprintf(&tracker->filter, "(&(%s=%s)%s)", OSGI_FRAMEWORK_OBJECTCLASS, serviceName, opts->filter.filter); } else { - asprintf(&tracker->filter, "(&(%s=%s))", OSGI_FRAMEWORK_OBJECTCLASS, opts->filter.serviceName); + asprintf(&tracker->filter, "(&(%s=%s))", OSGI_FRAMEWORK_OBJECTCLASS, serviceName); } } else { if (opts->filter.filter != NULL && versionRange != NULL) { - asprintf(&tracker->filter, "(&(%s=%s)(%s=%s)%s%s)", OSGI_FRAMEWORK_OBJECTCLASS, opts->filter.serviceName, CELIX_FRAMEWORK_SERVICE_LANGUAGE, lang, versionRange, opts->filter.filter); + asprintf(&tracker->filter, "(&(%s=%s)(%s=%s)%s%s)", OSGI_FRAMEWORK_OBJECTCLASS, serviceName, CELIX_FRAMEWORK_SERVICE_LANGUAGE, lang, versionRange, opts->filter.filter); } else if (versionRange != NULL) { - asprintf(&tracker->filter, "(&(%s=%s)(%s=%s)%s)", OSGI_FRAMEWORK_OBJECTCLASS, opts->filter.serviceName, CELIX_FRAMEWORK_SERVICE_LANGUAGE, lang, versionRange); + asprintf(&tracker->filter, "(&(%s=%s)(%s=%s)%s)", OSGI_FRAMEWORK_OBJECTCLASS, serviceName, CELIX_FRAMEWORK_SERVICE_LANGUAGE, lang, versionRange); } else if (opts->filter.filter != NULL) { - asprintf(&tracker->filter, "(&(%s=%s)(%s=%s)%s)", OSGI_FRAMEWORK_OBJECTCLASS, opts->filter.serviceName, CELIX_FRAMEWORK_SERVICE_LANGUAGE, lang, opts->filter.filter); + asprintf(&tracker->filter, "(&(%s=%s)(%s=%s)%s)", OSGI_FRAMEWORK_OBJECTCLASS, serviceName, CELIX_FRAMEWORK_SERVICE_LANGUAGE, lang, opts->filter.filter); } else { - asprintf(&tracker->filter, "(&(%s=%s)(%s=%s))", OSGI_FRAMEWORK_OBJECTCLASS, opts->filter.serviceName, CELIX_FRAMEWORK_SERVICE_LANGUAGE, lang); + asprintf(&tracker->filter, "(&(%s=%s)(%s=%s))", OSGI_FRAMEWORK_OBJECTCLASS, serviceName, CELIX_FRAMEWORK_SERVICE_LANGUAGE, lang); } } @@ -765,12 +766,7 @@ celix_service_tracker_t* celix_serviceTracker_createWithOptions( serviceTracker_open(tracker); } } else { - if (ctx != NULL && opts != NULL && opts->filter.serviceName == NULL) { - framework_log(ctx->framework->logger, CELIX_LOG_LEVEL_ERROR, __FUNCTION__, __BASE_FILE__, __LINE__, - "Error incorrect arguments. Missing service name."); - } else if (ctx != NULL) { - framework_log(ctx->framework->logger, CELIX_LOG_LEVEL_ERROR, __FUNCTION__, __BASE_FILE__, __LINE__, "Error incorrect arguments. Required context (%p) or opts (%p) is NULL", ctx, opts); - } + framework_log(ctx->framework->logger, CELIX_LOG_LEVEL_ERROR, __FUNCTION__, __BASE_FILE__, __LINE__, "Error incorrect arguments. Required context (%p) or opts (%p) is NULL", ctx, opts); } return tracker; } diff --git a/libs/utils/include/celix_log_utils.h b/libs/utils/include/celix_log_utils.h index 43ea714..b90cb4d 100644 --- a/libs/utils/include/celix_log_utils.h +++ b/libs/utils/include/celix_log_utils.h @@ -53,7 +53,6 @@ celix_log_level_e celix_logUtils_logLevelFromStringWithCheck(const char *level, * Logs a message to stdout/stderr using the provided logName and log level. * If the provided log level is higher than info, stderr will be used. * - * function is not thread safe (multiple printf's are used). */ void celix_logUtils_logToStdout(const char *logName, celix_log_level_e level, const char *format, ...); @@ -66,7 +65,6 @@ void celix_logUtils_logToStdout(const char *logName, celix_log_level_e level, co * * If the argument file or function is NULL, the arguments file, function and line are not used. * - * function is not thread safe (multiple printf's are used). */ void celix_logUtils_logToStdoutDetails(const char *logName, celix_log_level_e level, const char* file, const char* function, int line, const char *format, ...); @@ -75,7 +73,6 @@ void celix_logUtils_logToStdoutDetails(const char *logName, celix_log_level_e le * Logs a message to stdout/stderr using the provided logName and log level. * If the provided log level is higher than info, stderr will be used. * - * function is not thread safe (multiple printf's are used). */ void celix_logUtils_vLogToStdout(const char *logName, celix_log_level_e level, const char *format, va_list formatArgs); @@ -88,14 +85,12 @@ void celix_logUtils_vLogToStdout(const char *logName, celix_log_level_e level, c * * If the argument file or function is NULL, the arguments file, function and line are not used. * - * function is not thread safe (multiple printf's are used). */ void celix_logUtils_vLogToStdoutDetails(const char *logName, celix_log_level_e level, const char* file, const char* function, int line, const char *format, va_list formatArgs); /** * Prints a backtrace to the provided output stream. * - * function is not thread safe (multiple printf's are used). */ void celix_logUtils_printBacktrace(FILE* stream); diff --git a/libs/utils/src/celix_log_utils.c b/libs/utils/src/celix_log_utils.c index 236d0d9..7826725 100644 --- a/libs/utils/src/celix_log_utils.c +++ b/libs/utils/src/celix_log_utils.c @@ -26,6 +26,7 @@ #include <stdio.h> #include <time.h> #include <string.h> +#include <pthread.h> static const char * const CELIX_STRING_VALUE_DISABLED = "disabled"; static const char * const CELIX_STRING_VALUE_FATAL = "fatal"; @@ -127,6 +128,8 @@ void celix_logUtils_vLogToStdout(const char *logName, celix_log_level_e level, c celix_logUtils_vLogToStdoutDetails(logName, level, NULL, NULL, 0, format, formatArgs); } +static pthread_mutex_t globalMutex = PTHREAD_MUTEX_INITIALIZER; + void celix_logUtils_vLogToStdoutDetails(const char *logName, celix_log_level_e level, const char* file, const char* function, int line, const char *format, va_list formatArgs) { if (level == CELIX_LOG_LEVEL_DISABLED) { //silently ignore @@ -143,22 +146,21 @@ void celix_logUtils_vLogToStdoutDetails(const char *logName, celix_log_level_e l out = stderr; } + pthread_mutex_lock(&globalMutex); fprintf(out, "[%i-%02i-%02iT%02i:%02i:%02i] ", local.tm_year + 1900, local.tm_mon+1, local.tm_mday, local.tm_hour, local.tm_min, local.tm_sec); if (file != NULL && function != NULL) { (void)file; //note not using file - fprintf(out, "[%s] [%s] [%s:%i] ", celix_logUtils_logLevelToString(level), logName, function, line); + fprintf(out, "[%7s] [%s] [%s:%i] ", celix_logUtils_logLevelToString(level), logName, function, line); } else { - fprintf(out, "[%s] [%s] ", celix_logUtils_logLevelToString(level), logName); + fprintf(out, "[%7s] [%s] ", celix_logUtils_logLevelToString(level), logName); } - vfprintf(out, format, formatArgs); - fprintf(out, "\n"); - fflush(out); if (level >= CELIX_LOG_LEVEL_FATAL) { fprintf(out, "Backtrace:\n"); - fflush(out); celix_logUtils_inlinePrintBacktrace(out); } + fflush(out); + pthread_mutex_unlock(&globalMutex); } \ No newline at end of file