This is an automated email from the ASF dual-hosted git repository. pnoltes pushed a commit to branch feature/framework_racecondition in repository https://gitbox.apache.org/repos/asf/celix.git
commit f4671e0fa3a00da28bc1279a9375677b0ade2615 Author: Pepijn Noltes <[email protected]> AuthorDate: Sun Mar 8 21:07:45 2020 +0100 Refactors the handling of service listeners. This is moved to the service registry and the service listener is now retroactively creating events. This is done because there was a race condition in the service trackers when adding service listener and getting a list of already registered services. This was done with two calls and as result introduced a race condition. Because the service listeners are changed in behaviour (now retroactively), the direct use if also deprecated; use the service tracker instead. --- .../example/refining_driver/CMakeLists.txt | 2 + .../example/refining_driver/src/refining_driver.c | 4 +- .../topology_manager/CMakeLists.txt | 1 - .../topology_manager/src/activator.c | 72 ++-- .../topology_manager/src/topology_manager.c | 168 +++----- .../topology_manager/src/topology_manager.h | 4 +- libs/framework/CMakeLists.txt | 2 + .../gtest/src/bundle_context_services_test.cpp | 80 ++-- libs/framework/include/bundle_context.h | 2 +- libs/framework/include/celix_bundle_context.h | 6 +- libs/framework/include/service_reference.h | 3 + libs/framework/include/service_registry.h | 15 +- libs/framework/include/service_tracker.h | 5 +- libs/framework/private/mock/bundle_context_mock.c | 3 +- .../private/mock/service_reference_mock.c | 6 + .../framework/private/mock/service_registry_mock.c | 22 +- .../private/test/service_registration_test.cpp | 24 -- .../private/test/service_registry_test.cpp | 161 +++----- libs/framework/src/bundle.c | 8 +- libs/framework/src/bundle_context.c | 13 +- libs/framework/src/framework.c | 282 +------------ libs/framework/src/framework_private.h | 3 - libs/framework/src/registry_callback_private.h | 1 - libs/framework/src/service_reference.c | 12 + libs/framework/src/service_registration.c | 9 - libs/framework/src/service_registration_private.h | 8 +- libs/framework/src/service_registry.c | 439 +++++++++++++++++---- libs/framework/src/service_registry_private.h | 38 +- libs/framework/src/service_tracker.c | 168 +++----- 29 files changed, 737 insertions(+), 824 deletions(-) diff --git a/bundles/device_access/example/refining_driver/CMakeLists.txt b/bundles/device_access/example/refining_driver/CMakeLists.txt index ba0469c..5f55828 100644 --- a/bundles/device_access/example/refining_driver/CMakeLists.txt +++ b/bundles/device_access/example/refining_driver/CMakeLists.txt @@ -27,3 +27,5 @@ add_celix_bundle(char_refiningdriver target_include_directories(char_refiningdriver PRIVATE src) target_include_directories(char_refiningdriver PUBLIC include) target_link_libraries(char_refiningdriver PRIVATE Celix::device_access_api base_driver) +target_compile_options(char_refiningdriver PRIVATE -Wno-deprecated-declarations) #char_refiningdriver still uses service listeners, refactor this + diff --git a/bundles/device_access/example/refining_driver/src/refining_driver.c b/bundles/device_access/example/refining_driver/src/refining_driver.c index cafcd63..0af416d 100644 --- a/bundles/device_access/example/refining_driver/src/refining_driver.c +++ b/bundles/device_access/example/refining_driver/src/refining_driver.c @@ -110,9 +110,9 @@ static celix_status_t refiningDriver_stopDevice(refining_driver_device_pt device } -static celix_status_t refiningDriver_serviceChanged(celix_service_listener_t *listener, celix_service_event_t *event) { +static celix_status_t refiningDriver_serviceChanged(void *handle, celix_service_event_t *event) { celix_status_t status = CELIX_SUCCESS; - refining_driver_device_pt device = listener->handle; + refining_driver_device_pt device = handle; if (event->type == OSGI_FRAMEWORK_SERVICE_EVENT_UNREGISTERING) { bool equal = false; status = serviceReference_equals(device->baseServiceReference, event->reference, &equal); diff --git a/bundles/remote_services/topology_manager/CMakeLists.txt b/bundles/remote_services/topology_manager/CMakeLists.txt index 189d2a1..ad4c574 100644 --- a/bundles/remote_services/topology_manager/CMakeLists.txt +++ b/bundles/remote_services/topology_manager/CMakeLists.txt @@ -31,7 +31,6 @@ if (RSA_TOPOLOGY_MANAGER) target_include_directories(rsa_topology_manager PRIVATE include) target_link_libraries(rsa_topology_manager PRIVATE Celix::log_helper Celix::rsa_spi) - if (ENABLE_TESTING) find_package(CppUTest REQUIRED) find_package(Jansson REQUIRED) diff --git a/bundles/remote_services/topology_manager/src/activator.c b/bundles/remote_services/topology_manager/src/activator.c index 4a4a6df..4aa28f1 100644 --- a/bundles/remote_services/topology_manager/src/activator.c +++ b/bundles/remote_services/topology_manager/src/activator.c @@ -50,7 +50,7 @@ struct activator { service_tracker_t *endpointListenerTracker; service_tracker_t *remoteServiceAdminTracker; - celix_service_listener_t *serviceListener; + service_tracker_t *exportedServicesTracker; endpoint_listener_t *endpointListener; service_registration_t *endpointListenerService; @@ -67,7 +67,7 @@ struct activator { static celix_status_t bundleActivator_createEPLTracker(struct activator *activator, service_tracker_t **tracker); static celix_status_t bundleActivator_createRSATracker(struct activator *activator, service_tracker_t **tracker); -static celix_status_t bundleActivator_createServiceListener(struct activator *activator, celix_service_listener_t **listener); +static celix_status_t bundleActivator_createExportedServicesTracker(struct activator *activator, service_tracker_t **tracker); celix_status_t bundleActivator_create(celix_bundle_context_t *context, void **userData) { celix_status_t status = CELIX_SUCCESS; @@ -86,7 +86,7 @@ celix_status_t bundleActivator_create(celix_bundle_context_t *context, void **us activator->hook = NULL; activator->manager = NULL; activator->remoteServiceAdminTracker = NULL; - activator->serviceListener = NULL; + activator->exportedServicesTracker = NULL; activator->scopeService = calloc(1, sizeof(*(activator->scopeService))); if (activator->scopeService == NULL) { @@ -111,7 +111,7 @@ celix_status_t bundleActivator_create(celix_bundle_context_t *context, void **us if (status == CELIX_SUCCESS) { status = bundleActivator_createRSATracker(activator, &activator->remoteServiceAdminTracker); if (status == CELIX_SUCCESS) { - status = bundleActivator_createServiceListener(activator, &activator->serviceListener); + status = bundleActivator_createExportedServicesTracker(activator, &activator->exportedServicesTracker); if (status == CELIX_SUCCESS) { *userData = activator; } @@ -128,45 +128,33 @@ celix_status_t bundleActivator_create(celix_bundle_context_t *context, void **us static celix_status_t bundleActivator_createEPLTracker(struct activator *activator, service_tracker_t **tracker) { celix_status_t status; - service_tracker_customizer_t *customizer = NULL; - status = serviceTrackerCustomizer_create(activator->manager, topologyManager_endpointListenerAdding, topologyManager_endpointListenerAdded, topologyManager_endpointListenerModified, topologyManager_endpointListenerRemoved, &customizer); - if (status == CELIX_SUCCESS) { status = serviceTracker_create(activator->context, (char *) OSGI_ENDPOINT_LISTENER_SERVICE, customizer, tracker); } - return status; } static celix_status_t bundleActivator_createRSATracker(struct activator *activator, service_tracker_t **tracker) { celix_status_t status; - service_tracker_customizer_t *customizer = NULL; - status = serviceTrackerCustomizer_create(activator->manager, topologyManager_rsaAdding, topologyManager_rsaAdded, topologyManager_rsaModified, topologyManager_rsaRemoved, &customizer); - if (status == CELIX_SUCCESS) { status = serviceTracker_create(activator->context, OSGI_RSA_REMOTE_SERVICE_ADMIN, customizer, tracker); } - return status; } -static celix_status_t bundleActivator_createServiceListener(struct activator *activator, celix_service_listener_t **listener) { - celix_status_t status = CELIX_SUCCESS; - - *listener = malloc(sizeof(**listener)); - if (!*listener) { - return CELIX_ENOMEM; - } - - (*listener)->handle = activator->manager; - (*listener)->serviceChanged = topologyManager_serviceChanged; - - return status; +static celix_status_t bundleActivator_createExportedServicesTracker(struct activator *activator, service_tracker_t **tracker) { + celix_status_t status; + service_tracker_customizer_t *customizer = NULL; + status = serviceTrackerCustomizer_create(activator->manager, NULL, topologyManager_addExportedService, NULL, topologyManager_removeExportedService, &customizer); + if (status == CELIX_SUCCESS) { + status = serviceTracker_createWithFilter(activator->context, "(service.exported.interfaces=*)", customizer, tracker); + } + return status; } celix_status_t bundleActivator_start(void * userData, celix_bundle_context_t *context) { @@ -210,30 +198,21 @@ celix_status_t bundleActivator_start(void * userData, celix_bundle_context_t *co hookService->removed = topologyManager_listenerRemoved; activator->hookService = hookService; - bundleContext_registerService(context, (char *) OSGI_FRAMEWORK_LISTENER_HOOK_SERVICE_NAME, hookService, NULL, &activator->hook); - bundleContext_addServiceListener(context, activator->serviceListener, "(service.exported.interfaces=*)"); - - if (status == CELIX_SUCCESS) { - serviceTracker_open(activator->remoteServiceAdminTracker); - } + bundleContext_registerService(context, (char *) TOPOLOGYMANAGER_SCOPE_SERVICE, activator->scopeService, NULL, &activator->scopeReg); - if (status == CELIX_SUCCESS) { - status = serviceTracker_open(activator->endpointListenerTracker); - } + bundleContext_registerService(context, (char *) OSGI_FRAMEWORK_LISTENER_HOOK_SERVICE_NAME, hookService, NULL, &activator->hook); - bundleContext_registerService(context, (char *) TOPOLOGYMANAGER_SCOPE_SERVICE, activator->scopeService, NULL, &activator->scopeReg); + if (status == CELIX_SUCCESS) { + status = serviceTracker_open(activator->exportedServicesTracker); + } - array_list_pt references = NULL; - bundleContext_getServiceReferences(context, NULL, "(service.exported.interfaces=*)", &references); - int i; - for (i = 0; i < arrayList_size(references); i++) { - service_reference_pt reference = arrayList_get(references, i); - const char* serviceId = NULL; - status = CELIX_DO_IF(status, serviceReference_getProperty(reference, OSGI_FRAMEWORK_SERVICE_ID, &serviceId)); + if (status == CELIX_SUCCESS) { + serviceTracker_open(activator->remoteServiceAdminTracker); + } - CELIX_DO_IF(status, topologyManager_addExportedService(activator->manager, reference, (char*)serviceId)); - } - arrayList_destroy(references); + if (status == CELIX_SUCCESS) { + status = serviceTracker_open(activator->endpointListenerTracker); + } return status; } @@ -250,8 +229,9 @@ celix_status_t bundleActivator_stop(void * userData, celix_bundle_context_t *con serviceTracker_destroy(activator->endpointListenerTracker); } - bundleContext_removeServiceListener(context, activator->serviceListener); - free(activator->serviceListener); + if (serviceTracker_close(activator->exportedServicesTracker) == CELIX_SUCCESS) { + serviceTracker_destroy(activator->exportedServicesTracker); + } serviceRegistration_unregister(activator->hook); free(activator->hookService); diff --git a/bundles/remote_services/topology_manager/src/topology_manager.c b/bundles/remote_services/topology_manager/src/topology_manager.c index 82c12d1..193b882 100644 --- a/bundles/remote_services/topology_manager/src/topology_manager.c +++ b/bundles/remote_services/topology_manager/src/topology_manager.c @@ -27,6 +27,7 @@ #include <stdio.h> #include <stdlib.h> #include <string.h> +#include <assert.h> #include "celixbool.h" #include "topology_manager.h" @@ -201,91 +202,75 @@ celix_status_t topologyManager_rsaAdding(void * handle, service_reference_pt ref return status; } -celix_status_t topologyManager_rsaAdded(void * handle, service_reference_pt reference, void * service) { - celix_status_t status; +celix_status_t topologyManager_rsaAdded(void * handle, service_reference_pt unusedRef __attribute__((unused)), void * service) { topology_manager_pt manager = (topology_manager_pt) handle; celix_properties_t *serviceProperties = NULL; remote_service_admin_service_t *rsa = (remote_service_admin_service_t *) service; logHelper_log(manager->loghelper, OSGI_LOGSERVICE_INFO, "TOPOLOGY_MANAGER: Added RSA"); - status = celixThreadMutex_lock(&manager->rsaListLock); - - if (status == CELIX_SUCCESS) { - arrayList_add(manager->rsaList, rsa); - status = celixThreadMutex_unlock(&manager->rsaListLock); - } + celixThreadMutex_lock(&manager->rsaListLock); + arrayList_add(manager->rsaList, rsa); + celixThreadMutex_unlock(&manager->rsaListLock); // add already imported services to new rsa - if (status == CELIX_SUCCESS) { - status = celixThreadMutex_lock(&manager->importedServicesLock); + celixThreadMutex_lock(&manager->importedServicesLock); + hash_map_iterator_pt importedServicesIterator = hashMapIterator_create(manager->importedServices); - if (status == CELIX_SUCCESS) { - hash_map_iterator_pt importedServicesIterator = hashMapIterator_create(manager->importedServices); + while (hashMapIterator_hasNext(importedServicesIterator)) { + hash_map_entry_pt entry = hashMapIterator_nextEntry(importedServicesIterator); + endpoint_description_t *endpoint = hashMapEntry_getKey(entry); + if (scope_allowImport(manager->scope, endpoint)) { + import_registration_t *import = NULL; + celix_status_t status = rsa->importService(rsa->admin, endpoint, &import); - while (hashMapIterator_hasNext(importedServicesIterator)) { - hash_map_entry_pt entry = hashMapIterator_nextEntry(importedServicesIterator); - endpoint_description_t *endpoint = hashMapEntry_getKey(entry); - if (scope_allowImport(manager->scope, endpoint)) { - import_registration_t *import = NULL; - status = rsa->importService(rsa->admin, endpoint, &import); - - if (status == CELIX_SUCCESS) { - hash_map_pt imports = hashMapEntry_getValue(entry); - - if (imports == NULL) { - imports = hashMap_create(NULL, NULL, NULL, NULL); - hashMap_put(manager->importedServices,endpoint,imports); - } + if (status == CELIX_SUCCESS) { + hash_map_pt imports = hashMapEntry_getValue(entry); - hashMap_put(imports, service, import); - } - } - } + if (imports == NULL) { + imports = hashMap_create(NULL, NULL, NULL, NULL); + hashMap_put(manager->importedServices, endpoint, imports); + } - hashMapIterator_destroy(importedServicesIterator); + hashMap_put(imports, service, import); + } + } + } - celixThreadMutex_unlock(&manager->importedServicesLock); - } - } + hashMapIterator_destroy(importedServicesIterator); + celixThreadMutex_unlock(&manager->importedServicesLock); // add already exported services to new rsa - if (status == CELIX_SUCCESS) { - status = celixThreadMutex_lock(&manager->exportedServicesLock); + celixThreadMutex_lock(&manager->exportedServicesLock); + hash_map_iterator_pt exportedServicesIterator = hashMapIterator_create(manager->exportedServices); - if (status == CELIX_SUCCESS) { - hash_map_iterator_pt exportedServicesIterator = hashMapIterator_create(manager->exportedServices); + while (hashMapIterator_hasNext(exportedServicesIterator)) { + hash_map_entry_pt entry = hashMapIterator_nextEntry(exportedServicesIterator); + service_reference_pt reference = hashMapEntry_getKey(entry); + const char* serviceId = NULL; - while (hashMapIterator_hasNext(exportedServicesIterator)) { - hash_map_entry_pt entry = hashMapIterator_nextEntry(exportedServicesIterator); - service_reference_pt reference = hashMapEntry_getKey(entry); - const char* serviceId = NULL; + serviceReference_getProperty(reference, OSGI_FRAMEWORK_SERVICE_ID, &serviceId); - serviceReference_getProperty(reference, OSGI_FRAMEWORK_SERVICE_ID, &serviceId); + scope_getExportProperties(manager->scope, reference, &serviceProperties); - scope_getExportProperties(manager->scope, reference, &serviceProperties); + array_list_pt endpoints = NULL; + celix_status_t status = rsa->exportService(rsa->admin, (char*)serviceId, serviceProperties, &endpoints); - array_list_pt endpoints = NULL; - status = rsa->exportService(rsa->admin, (char*)serviceId, serviceProperties, &endpoints); + if (status == CELIX_SUCCESS) { + hash_map_pt exports = hashMapEntry_getValue(entry); - if (status == CELIX_SUCCESS) { - hash_map_pt exports = hashMapEntry_getValue(entry); + if (exports == NULL) { + exports = hashMap_create(NULL, NULL, NULL, NULL); + hashMap_put(manager->exportedServices,reference,exports); + } - if (exports == NULL) { - exports = hashMap_create(NULL, NULL, NULL, NULL); - hashMap_put(manager->exportedServices,reference,exports); - } + hashMap_put(exports, rsa, endpoints); + topologyManager_notifyListenersEndpointAdded(manager, rsa, endpoints); + } + } - hashMap_put(exports, rsa, endpoints); - status = topologyManager_notifyListenersEndpointAdded(manager, rsa, endpoints); - } - } - - hashMapIterator_destroy(exportedServicesIterator); - - celixThreadMutex_unlock(&manager->exportedServicesLock); - } - } - return status; + hashMapIterator_destroy(exportedServicesIterator); + celixThreadMutex_unlock(&manager->exportedServicesLock); + return CELIX_SUCCESS; } celix_status_t topologyManager_rsaModified(void * handle, service_reference_pt reference, void * service) { @@ -376,43 +361,6 @@ celix_status_t topologyManager_rsaRemoved(void * handle, service_reference_pt re return status; } - -celix_status_t topologyManager_serviceChanged(void *listener, celix_service_event_t *event) { - celix_status_t status = CELIX_SUCCESS; - celix_service_listener_t *listen = listener; - topology_manager_pt manager = listen->handle; - - const char* export = NULL; - const char* serviceId = NULL; - serviceReference_getProperty(event->reference, OSGI_RSA_SERVICE_EXPORTED_INTERFACES, &export); - serviceReference_getProperty(event->reference, OSGI_FRAMEWORK_SERVICE_ID, &serviceId); - - if (!export) { - // Nothing needs to be done: we're not interested... - return status; - } - - switch (event->type) { - case OSGI_FRAMEWORK_SERVICE_EVENT_REGISTERED: - status = topologyManager_addExportedService(manager, event->reference, (char*)serviceId); - break; - case OSGI_FRAMEWORK_SERVICE_EVENT_MODIFIED: - status = topologyManager_removeExportedService(manager, event->reference, (char*)serviceId); - - if (status == CELIX_SUCCESS) { - status = topologyManager_addExportedService(manager, event->reference, (char*)serviceId); - } - break; - case OSGI_FRAMEWORK_SERVICE_EVENT_UNREGISTERING: - status = topologyManager_removeExportedService(manager, event->reference, (char*)serviceId); - break; - case OSGI_FRAMEWORK_SERVICE_EVENT_MODIFIED_ENDMATCH: - break; - } - - return status; -} - celix_status_t topologyManager_exportScopeChanged(void *handle, char *filterStr) { celix_status_t status = CELIX_SUCCESS; topology_manager_pt manager = (topology_manager_pt) handle; @@ -602,11 +550,19 @@ celix_status_t topologyManager_removeImportedService(void *handle, endpoint_desc return status; } -celix_status_t topologyManager_addExportedService(topology_manager_pt manager, service_reference_pt reference, char *serviceId) { +celix_status_t topologyManager_addExportedService(void * handle, service_reference_pt reference, void * service __attribute__((unused))) { + topology_manager_pt manager = handle; celix_status_t status = CELIX_SUCCESS; + long serviceId = serviceReference_getServiceId(reference); + char serviceIdStr[64]; + snprintf(serviceIdStr, 64, "%li", serviceId); celix_properties_t *serviceProperties = NULL; - logHelper_log(manager->loghelper, OSGI_LOGSERVICE_INFO, "TOPOLOGY_MANAGER: Add exported service (%s).", serviceId); + const char *export = NULL; + serviceReference_getProperty(reference, OSGI_RSA_SERVICE_EXPORTED_INTERFACES, &export); + assert(export != NULL); + + logHelper_log(manager->loghelper, OSGI_LOGSERVICE_INFO, "TOPOLOGY_MANAGER: Add exported service (%li).", serviceId); if (celixThreadMutex_lock(&manager->exportedServicesLock) == CELIX_SUCCESS) { scope_getExportProperties(manager->scope, reference, &serviceProperties); @@ -624,7 +580,7 @@ celix_status_t topologyManager_addExportedService(topology_manager_pt manager, s remote_service_admin_service_t *rsa = arrayList_get(manager->rsaList, iter); array_list_pt endpoints = NULL; - celix_status_t substatus = rsa->exportService(rsa->admin, serviceId, serviceProperties, &endpoints); + celix_status_t substatus = rsa->exportService(rsa->admin, serviceIdStr, serviceProperties, &endpoints); if (substatus == CELIX_SUCCESS) { hashMap_put(exports, rsa, endpoints); @@ -641,10 +597,12 @@ celix_status_t topologyManager_addExportedService(topology_manager_pt manager, s return status; } -celix_status_t topologyManager_removeExportedService(topology_manager_pt manager, service_reference_pt reference, char *serviceId) { +celix_status_t topologyManager_removeExportedService(void * handle, service_reference_pt reference, void * service __attribute__((unused))) { + topology_manager_pt manager = handle; celix_status_t status = CELIX_SUCCESS; + long serviceId = serviceReference_getServiceId(reference); - logHelper_log(manager->loghelper, OSGI_LOGSERVICE_INFO, "TOPOLOGY_MANAGER: Remove exported service (%s).", serviceId); + logHelper_log(manager->loghelper, OSGI_LOGSERVICE_INFO, "TOPOLOGY_MANAGER: Remove exported service (%li).", serviceId); if (celixThreadMutex_lock(&manager->exportedServicesLock) == CELIX_SUCCESS) { hash_map_pt exports = hashMap_get(manager->exportedServices, reference); diff --git a/bundles/remote_services/topology_manager/src/topology_manager.h b/bundles/remote_services/topology_manager/src/topology_manager.h index 5f66ccd..8845214 100644 --- a/bundles/remote_services/topology_manager/src/topology_manager.h +++ b/bundles/remote_services/topology_manager/src/topology_manager.h @@ -57,8 +57,8 @@ celix_status_t topologyManager_serviceChanged(void *listener, celix_service_even celix_status_t topologyManager_addImportedService(void *handle, endpoint_description_t *endpoint, char *matchedFilter); celix_status_t topologyManager_removeImportedService(void *handle, endpoint_description_t *endpoint, char *matchedFilter); -celix_status_t topologyManager_addExportedService(topology_manager_pt manager, service_reference_pt reference, char *serviceId); -celix_status_t topologyManager_removeExportedService(topology_manager_pt manager, service_reference_pt reference, char *serviceId); +celix_status_t topologyManager_addExportedService(void * handle, service_reference_pt reference, void * service); +celix_status_t topologyManager_removeExportedService(void * handle, service_reference_pt reference, void * service); celix_status_t topologyManager_listenerAdded(void *handle, array_list_pt listeners); celix_status_t topologyManager_listenerRemoved(void *handle, array_list_pt listeners); diff --git a/libs/framework/CMakeLists.txt b/libs/framework/CMakeLists.txt index a8d551a..70dc025 100644 --- a/libs/framework/CMakeLists.txt +++ b/libs/framework/CMakeLists.txt @@ -39,6 +39,7 @@ target_include_directories(framework PUBLIC $<INSTALL_INTERFACE:include/celix> ) target_compile_options(framework PRIVATE -DUSE_FILE32API) +target_compile_options(framework PRIVATE -Wno-deprecated-declarations) #note part of the api is deprecated, ignore this warning on own api set_target_properties(framework PROPERTIES "SOVERSION" 2) target_link_libraries(framework PUBLIC Celix::utils Celix::dfi ${CELIX_OPTIONAL_EXTRA_LIBS}) @@ -112,6 +113,7 @@ if (ENABLE_TESTING AND FRAMEWORK_TESTS) private/mock/bundle_mock.c private/mock/celix_log_mock.c) target_link_libraries(bundle_context_test ${CPPUTEST_LIBRARY} ${CPPUTEST_EXT_LIBRARY} Celix::utils pthread) + target_compile_options(bundle_context_test PRIVATE -Wno-deprecated-declarations) add_executable(bundle_revision_test private/test/bundle_revision_test.cpp diff --git a/libs/framework/gtest/src/bundle_context_services_test.cpp b/libs/framework/gtest/src/bundle_context_services_test.cpp index fa650a4..17e0d89 100644 --- a/libs/framework/gtest/src/bundle_context_services_test.cpp +++ b/libs/framework/gtest/src/bundle_context_services_test.cpp @@ -111,13 +111,15 @@ TEST_F(CelixBundleContextServicesTests, registerMultipleAndUseServices) { }; int total = 0; - celix_bundleContext_useServices(ctx, "calc", &total, use); + auto count = celix_bundleContext_useServices(ctx, "calc", &total, use); + ASSERT_EQ(3, count); ASSERT_EQ(42 * 3, total); celix_bundleContext_unregisterService(ctx, svcId3); total = 0; - celix_bundleContext_useServices(ctx, "calc", &total, use); + count = celix_bundleContext_useServices(ctx, "calc", &total, use); + ASSERT_EQ(2, count); ASSERT_EQ(42 * 2, total); total = 0; @@ -127,6 +129,10 @@ TEST_F(CelixBundleContextServicesTests, registerMultipleAndUseServices) { celix_bundleContext_unregisterService(ctx, svcId1); celix_bundleContext_unregisterService(ctx, svcId2); + + //NOTE superfluous (note prints errors) + celix_bundleContext_unregisterService(ctx, svcId1); + celix_bundleContext_unregisterService(ctx, svcId2); }; TEST_F(CelixBundleContextServicesTests, registerAndUseService) { @@ -169,50 +175,52 @@ TEST_F(CelixBundleContextServicesTests, registerAndUseService) { }; TEST_F(CelixBundleContextServicesTests, registerAndUseServiceWithTimeout) { - struct calc { - int (*calc)(int); - }; + const int NR_ITERATIONS = 5; //NOTE this test is sensitive for triggering race condition in the celix framework, therefore is used a few times. + for (int i = 0; i < NR_ITERATIONS; ++i) { + printf("Iter %i\n", i); + struct calc { + int (*calc)(int); + }; - const char *calcName = "calc"; - struct calc svc; - svc.calc = [](int n) -> int { - return n * 42; - }; - - celix_service_use_options_t opts{}; - opts.filter.serviceName = "calc"; + const char *calcName = "calc"; + struct calc svc; + svc.calc = [](int n) -> int { + return n * 42; + }; - bool called = celix_bundleContext_useServiceWithOptions(ctx, &opts); - ASSERT_TRUE(!called); //service not avail. + celix_service_use_options_t opts{}; + opts.filter.serviceName = "calc"; - std::future<bool> result{std::async([&]{ - opts.waitTimeoutInSeconds = 5.0; - printf("Trying to call calc with timeout of %f\n", opts.waitTimeoutInSeconds); - bool calledAsync = celix_bundleContext_useServiceWithOptions(ctx, &opts); - printf("returned from use service with timeout. calc called %s.\n", calledAsync ? "true" : "false"); - return calledAsync; - })}; - long svcId = celix_bundleContext_registerService(ctx, &svc, calcName, nullptr); - ASSERT_TRUE(svcId >= 0); + bool called = celix_bundleContext_useServiceWithOptions(ctx, &opts); + EXPECT_FALSE(called); //service not avail. + std::future<bool> result{std::async([&] { + opts.waitTimeoutInSeconds = 2.0; + //printf("Trying to call calc with timeout of %f\n", opts.waitTimeoutInSeconds); + bool calledAsync = celix_bundleContext_useServiceWithOptions(ctx, &opts); + //printf("returned from use service with timeout. calc called %s.\n", calledAsync ? "true" : "false"); + return calledAsync; + })}; + long svcId = celix_bundleContext_registerService(ctx, &svc, calcName, nullptr); + EXPECT_TRUE(svcId >= 0); - ASSERT_TRUE(result.get()); //should return true after waiting for the registered service. + EXPECT_TRUE(result.get()); //should return true after waiting for the registered service. - celix_bundleContext_unregisterService(ctx, svcId); + celix_bundleContext_unregisterService(ctx, svcId); - //check if timeout is triggered - std::future<bool> result2{std::async([&]{ - opts.waitTimeoutInSeconds = 0.5; - printf("Trying to call calc with timeout of %f\n", opts.waitTimeoutInSeconds); - bool calledAsync = celix_bundleContext_useServiceWithOptions(ctx, &opts); - printf("returned from use service with timeout. calc called %s.\n", calledAsync ? "true" : "false"); - return calledAsync; - })}; - ASSERT_TRUE(!result2.get()); - celix_bundleContext_unregisterService(ctx, svcId); + //check if timeout is triggered + std::future<bool> result2{std::async([&] { + opts.waitTimeoutInSeconds = 0.1; + //printf("Trying to call calc with timeout of %f\n", opts.waitTimeoutInSeconds); + bool calledAsync = celix_bundleContext_useServiceWithOptions(ctx, &opts); + //printf("returned from use service with timeout. calc called %s.\n", calledAsync ? "true" : "false"); + return calledAsync; + })}; + EXPECT_FALSE(result2.get()); //note service is away, so even with a wait the service is not found. + } } TEST_F(CelixBundleContextServicesTests, registerAndUseServiceWithCorrectVersion) { diff --git a/libs/framework/include/bundle_context.h b/libs/framework/include/bundle_context.h index 77fbd1c..8bdda05 100644 --- a/libs/framework/include/bundle_context.h +++ b/libs/framework/include/bundle_context.h @@ -125,7 +125,7 @@ FRAMEWORK_EXPORT celix_status_t bundleContext_getBundles(celix_bundle_context_t FRAMEWORK_EXPORT celix_status_t bundleContext_getBundleById(celix_bundle_context_t *context, long id, celix_bundle_t **bundle); FRAMEWORK_EXPORT celix_status_t -bundleContext_addServiceListener(celix_bundle_context_t *context, celix_service_listener_t *listener, const char *filter); +bundleContext_addServiceListener(celix_bundle_context_t *context, celix_service_listener_t *listener, const char *filter) __attribute__((deprecated("using service listeners directly is deprecated, use a service tracker instead!"))); FRAMEWORK_EXPORT celix_status_t bundleContext_removeServiceListener(celix_bundle_context_t *context, celix_service_listener_t *listener); diff --git a/libs/framework/include/celix_bundle_context.h b/libs/framework/include/celix_bundle_context.h index db513c7..559b2a8 100644 --- a/libs/framework/include/celix_bundle_context.h +++ b/libs/framework/include/celix_bundle_context.h @@ -471,8 +471,9 @@ bool celix_bundleContext_useService( * @param serviceName the required service name. * @param callbackHandle The data pointer, which will be used in the callbacks * @param use The callback, which will be called for every service found. + * @return The number of services found and called */ -void celix_bundleContext_useServices( +size_t celix_bundleContext_useServices( celix_bundle_context_t *ctx, const char* serviceName, void *callbackHandle, @@ -568,8 +569,9 @@ bool celix_bundleContext_useServiceWithOptions( * * @param ctx The bundle context. * @param opts The required options. Note that the serviceName is required. + * @return The number of services found and called */ -void celix_bundleContext_useServicesWithOptions( +size_t celix_bundleContext_useServicesWithOptions( celix_bundle_context_t *ctx, const celix_service_use_options_t *opts); diff --git a/libs/framework/include/service_reference.h b/libs/framework/include/service_reference.h index a734909..0ec54a0 100644 --- a/libs/framework/include/service_reference.h +++ b/libs/framework/include/service_reference.h @@ -48,6 +48,9 @@ serviceReference_getPropertyKeys(service_reference_pt reference, char **keys[], FRAMEWORK_EXPORT celix_status_t serviceReference_getServiceRegistration(service_reference_pt reference, service_registration_pt *registration); +FRAMEWORK_EXPORT +long serviceReference_getServiceId(service_reference_pt reference); + FRAMEWORK_EXPORT celix_status_t serviceReference_equals(service_reference_pt reference, service_reference_pt compareTo, bool *equal); diff --git a/libs/framework/include/service_registry.h b/libs/framework/include/service_registry.h index 9f06f8a..28d8afd 100644 --- a/libs/framework/include/service_registry.h +++ b/libs/framework/include/service_registry.h @@ -38,8 +38,7 @@ extern "C" { typedef void (*serviceChanged_function_pt)(celix_framework_t*, celix_service_event_type_t, service_registration_pt, celix_properties_t*); -celix_status_t serviceRegistry_create(celix_framework_t *framework, serviceChanged_function_pt serviceChanged, - service_registry_pt *registry); +celix_status_t serviceRegistry_create(celix_framework_t *framework, service_registry_pt *registry); celix_status_t serviceRegistry_destroy(service_registry_pt registry); @@ -87,13 +86,15 @@ serviceRegistry_ungetService(service_registry_pt registry, celix_bundle_t *bundl celix_status_t serviceRegistry_clearReferencesFor(service_registry_pt registry, celix_bundle_t *bundle); -void serviceRegistry_callHooksForListenerFilter(service_registry_pt registry, celix_bundle_t *owner, const char *filter, bool removed); - size_t serviceRegistry_nrOfHooks(service_registry_pt registry); -celix_status_t -serviceRegistry_servicePropertiesModified(service_registry_pt registry, service_registration_pt registration, - celix_properties_t *oldprops); +/** + * Register a service listener. Will also retroactively call the listener with register events for already registered services. + */ +celix_status_t celix_serviceRegistry_addServiceListener(celix_service_registry_t *reg, celix_bundle_t *bundle, const char *filter, celix_service_listener_t *listener); + +celix_status_t celix_serviceRegistry_removeServiceListener(celix_service_registry_t *reg, celix_service_listener_t *listener); + celix_status_t celix_serviceRegistry_registerServiceFactory( diff --git a/libs/framework/include/service_tracker.h b/libs/framework/include/service_tracker.h index 4b58e10..4e69621 100644 --- a/libs/framework/include/service_tracker.h +++ b/libs/framework/include/service_tracker.h @@ -61,8 +61,6 @@ FRAMEWORK_EXPORT celix_array_list_t *serviceTracker_getServices(service_tracker_ FRAMEWORK_EXPORT void *serviceTracker_getServiceByReference(service_tracker_t *tracker, service_reference_pt reference); -FRAMEWORK_EXPORT void serviceTracker_serviceChanged(celix_service_listener_t *listener, celix_service_event_t *event); - FRAMEWORK_EXPORT size_t serviceTracker_nrOfTrackedServices(service_tracker_t *tracker); @@ -125,8 +123,9 @@ bool celix_serviceTracker_useHighestRankingService( /** * Calls the use callback for every services found by this tracker. + * Returns the number of called services */ -void celix_serviceTracker_useServices( +size_t celix_serviceTracker_useServices( service_tracker_t *tracker, const char* serviceName /*sanity*/, void *callbackHandle, diff --git a/libs/framework/private/mock/bundle_context_mock.c b/libs/framework/private/mock/bundle_context_mock.c index 61642c2..873981d 100644 --- a/libs/framework/private/mock/bundle_context_mock.c +++ b/libs/framework/private/mock/bundle_context_mock.c @@ -317,12 +317,13 @@ bool celix_bundleContext_useServiceWithOptions( } -void celix_bundleContext_useServicesWithOptions( +size_t celix_bundleContext_useServicesWithOptions( celix_bundle_context_t *ctx, const celix_service_use_options_t *opts) { mock_c()->actualCall("celix_bundleContext_useServicesWithOptions") ->withPointerParameters("ctx", ctx) ->withConstPointerParameters("opts", opts); + return mock_c()->returnValue().value.unsignedLongIntValue; } long celix_bundleContext_registerServiceFactory(celix_bundle_context_t *ctx, celix_service_factory_t *factory, const char *serviceName, celix_properties_t *props) { diff --git a/libs/framework/private/mock/service_reference_mock.c b/libs/framework/private/mock/service_reference_mock.c index e9248c0..e2753fe 100644 --- a/libs/framework/private/mock/service_reference_mock.c +++ b/libs/framework/private/mock/service_reference_mock.c @@ -184,3 +184,9 @@ unsigned int serviceReference_hashCode(const void *referenceP) { mock_c()->actualCall("serviceReference_hashCode"); return mock_c()->returnValue().value.intValue; } + +long serviceReference_getServiceId(service_reference_pt reference) { + mock_c()->actualCall("serviceReference_getServiceId") + ->withPointerParameters("reference", reference); + return mock_c()->returnValue().value.longIntValue; +} \ No newline at end of file diff --git a/libs/framework/private/mock/service_registry_mock.c b/libs/framework/private/mock/service_registry_mock.c index 767e507..8df72e9 100644 --- a/libs/framework/private/mock/service_registry_mock.c +++ b/libs/framework/private/mock/service_registry_mock.c @@ -29,10 +29,9 @@ #include "service_registry.h" -celix_status_t serviceRegistry_create(framework_pt framework, serviceChanged_function_pt serviceChanged, service_registry_pt *registry) { +celix_status_t serviceRegistry_create(framework_pt framework, service_registry_pt *registry) { mock_c()->actualCall("serviceRegistry_create") ->withPointerParameters("framework", framework) - ->withPointerParameters("serviceChanged", serviceChanged) ->withOutputParameter("registry", registry); return mock_c()->returnValue().value.intValue; } @@ -219,4 +218,21 @@ celix_array_list_t* celix_serviceRegistry_listServiceIdsForOwner(celix_service_r ->withPointerParameters("registry", registry) ->withLongIntParameters("bndId", bndId); return mock_c()->returnValue().value.pointerValue; -} \ No newline at end of file +} + +celix_status_t celix_serviceRegistry_addServiceListener(celix_service_registry_t *registry, celix_bundle_t *bundle, const char *stringFilter, celix_service_listener_t *listener) { + mock_c()->actualCall("celix_serviceRegistry_addServiceListener") + ->withPointerParameters("registry", registry) + ->withPointerParameters("bundle", bundle) + ->withConstPointerParameters("stringFilter", stringFilter) + ->withPointerParameters("listener", listener); + return mock_c()->returnValue().value.intValue; +} + +celix_status_t celix_serviceRegistry_removeServiceListener(celix_service_registry_t *registry, celix_service_listener_t *listener) { + mock_c()->actualCall("celix_serviceRegistry_removeServiceListener") + ->withPointerParameters("registry", registry) + ->withPointerParameters("listener", listener); + return mock_c()->returnValue().value.intValue; +} + diff --git a/libs/framework/private/test/service_registration_test.cpp b/libs/framework/private/test/service_registration_test.cpp index b1c560e..d31f692 100644 --- a/libs/framework/private/test/service_registration_test.cpp +++ b/libs/framework/private/test/service_registration_test.cpp @@ -344,30 +344,6 @@ TEST(service_registration, getProperties) { free(name); } -TEST(service_registration, setProperties){ - registry_callback_t callback; - callback.modified = (callback_modified_signature) serviceRegistry_servicePropertiesModified; - service_registry_pt registry = (service_registry_pt) 0x10; - callback.handle = registry; - char * name = my_strdup("service_name"); - service_registration_pt registration = serviceRegistration_create(callback, NULL, name, 0, NULL, NULL); - - properties_pt properties = properties_create(); - properties_pt old_properties = registration->properties; - - mock().expectOneCall("serviceRegistry_servicePropertiesModified") - .withParameter("registry", registry) - .withParameter("registration", registration) - .withParameter("oldprops", old_properties); - - serviceRegistration_setProperties(registration, properties); - - POINTERS_EQUAL(properties, registration->properties); - - properties_destroy(old_properties); - serviceRegistration_release(registration); - free(name); -} TEST(service_registration, getServiceName) { registry_callback_t callback; diff --git a/libs/framework/private/test/service_registry_test.cpp b/libs/framework/private/test/service_registry_test.cpp index ee2ae88..67c25f0 100644 --- a/libs/framework/private/test/service_registry_test.cpp +++ b/libs/framework/private/test/service_registry_test.cpp @@ -44,14 +44,6 @@ extern "C" { framework_logger_pt logger = (framework_logger_pt) 0x42; -void serviceRegistryTest_serviceChanged(framework_pt framework, celix_service_event_type_t eventType, service_registration_pt registration, properties_pt oldprops) { - mock_c()->actualCall("serviceRegistryTest_serviceChanged") - ->withPointerParameters("framework", framework) - ->withIntParameters("eventType", eventType) - ->withPointerParameters("registration", registration) - ->withPointerParameters("oldprops", oldprops); -} - static char* my_strdup(const char* s) { if (s == NULL) { return NULL; @@ -75,7 +67,6 @@ static int registry_callback_t_isEqual(const void* object1, const void* object2) registry_callback_t callback2 = *(registry_callback_t*) object2; return callback1.getUsingBundles == callback2.getUsingBundles && callback1.handle == callback2.handle && - callback1.modified == callback2.modified && callback1.unregister == callback2.unregister; } @@ -83,7 +74,7 @@ static const char * registry_callback_t_toString(const void* object) { char buff[512]; registry_callback_t callback = *(registry_callback_t*) object; - snprintf(buff, 512, "getUsingBundles: %p, handle: %p, modified: %p, unregister: %p", callback.getUsingBundles, callback.handle, callback.modified, callback.unregister); + snprintf(buff, 512, "getUsingBundles: %p, handle: %p, unregister: %p", callback.getUsingBundles, callback.handle, callback.unregister); return my_strdup(buff); } @@ -110,10 +101,9 @@ TEST(service_registry, create) { framework_pt framework = (framework_pt) 0x10; service_registry_pt registry = NULL; - serviceRegistry_create(framework, serviceRegistryTest_serviceChanged, ®istry); + serviceRegistry_create(framework, ®istry); POINTERS_EQUAL(framework, registry->framework); - POINTERS_EQUAL(serviceRegistryTest_serviceChanged, registry->serviceChanged); UNSIGNED_LONGS_EQUAL(1UL, registry->currentServiceId); CHECK(registry->listenerHooks != NULL); CHECK(registry->serviceReferences != NULL); @@ -125,7 +115,7 @@ TEST(service_registry, create) { TEST(service_registry, getRegisteredServices) { service_registry_pt registry = NULL; framework_pt framework = (framework_pt) 0x01; - serviceRegistry_create(framework,serviceRegistryTest_serviceChanged, ®istry); + serviceRegistry_create(framework, ®istry); array_list_pt registrations = NULL; arrayList_create(®istrations); service_registration_pt reg = (service_registration_pt) calloc(1,sizeof(struct serviceRegistration)); @@ -139,6 +129,8 @@ TEST(service_registry, getRegisteredServices) { hashMap_put(usages, (void*)reg->serviceId, ref); hashMap_put(registry->serviceReferences, bundle, usages); + mock().expectNCalls(1,"framework_log"); + mock() .expectOneCall("serviceRegistration_isValid") .withParameter("registration", reg) @@ -163,7 +155,7 @@ TEST(service_registry, getRegisteredServices) { TEST(service_registry, getServicesInUse) { service_registry_pt registry = NULL; framework_pt framework = (framework_pt) 0x01; - serviceRegistry_create(framework,serviceRegistryTest_serviceChanged, ®istry); + serviceRegistry_create(framework, ®istry); hash_map_pt usages = hashMap_create(NULL, NULL, NULL, NULL); bundle_pt bundle = (bundle_pt) 0x10; @@ -172,6 +164,8 @@ TEST(service_registry, getServicesInUse) { hashMap_put(usages, reg, ref); hashMap_put(registry->serviceReferences, bundle, usages); + mock().expectNCalls(1,"framework_log"); + array_list_pt inUse = NULL; serviceRegistry_getServicesInUse(registry, bundle, &inUse); LONGS_EQUAL(1, arrayList_size(inUse)); @@ -185,7 +179,7 @@ TEST(service_registry, getServicesInUse) { TEST(service_registry, registerServiceNoProps) { service_registry_pt registry = NULL; framework_pt framework = (framework_pt) 0x01; - serviceRegistry_create(framework,serviceRegistryTest_serviceChanged, ®istry); + serviceRegistry_create(framework, ®istry); bundle_pt bundle = (bundle_pt) 0x10; char * serviceName = my_strdup("service"); @@ -202,11 +196,9 @@ TEST(service_registry, registerServiceNoProps) { .withParameter("dictionary", (void *) NULL) .andReturnValue(reg); - mock().expectOneCall("serviceRegistryTest_serviceChanged") - .withParameter("framework", framework) - .withParameter("eventType", OSGI_FRAMEWORK_SERVICE_EVENT_REGISTERED) - .withParameter("registration", reg) - .withParameter("oldprops", (void*)NULL); + mock().expectNCalls(1,"serviceRegistration_getServiceId") + .withParameter("registration", reg) + .andReturnValue(42); service_registration_pt registration = NULL; serviceRegistry_registerService(registry, bundle, serviceName, service, NULL, ®istration); @@ -221,7 +213,7 @@ TEST(service_registry, registerServiceNoProps) { TEST(service_registry, registerServiceFactoryNoProps) { service_registry_pt registry = NULL; framework_pt framework = (framework_pt) 0x01; - serviceRegistry_create(framework,serviceRegistryTest_serviceChanged, ®istry); + serviceRegistry_create(framework, ®istry); bundle_pt bundle = (bundle_pt) 0x10; char * serviceName = my_strdup("service"); @@ -239,11 +231,9 @@ TEST(service_registry, registerServiceFactoryNoProps) { .withParameter("dictionary", (void *) NULL) .andReturnValue(reg); - mock().expectOneCall("serviceRegistryTest_serviceChanged") - .withParameter("framework", framework) - .withParameter("eventType", OSGI_FRAMEWORK_SERVICE_EVENT_REGISTERED) - .withParameter("registration", reg) - .withParameter("oldprops", (void*)NULL); + mock().expectNCalls(1,"serviceRegistration_getServiceId") + .withParameter("registration", reg) + .andReturnValue(42); service_registration_pt registration = NULL; serviceRegistry_registerServiceFactory(registry, bundle, serviceName, factory, NULL, ®istration); @@ -259,7 +249,7 @@ TEST(service_registry, registerServiceFactoryNoProps) { TEST(service_registry, registerServiceListenerHook) { service_registry_pt registry = NULL; framework_pt framework = (framework_pt) 0x01; - serviceRegistry_create(framework,serviceRegistryTest_serviceChanged, ®istry); + serviceRegistry_create(framework, ®istry); bundle_pt bundle = (bundle_pt) 0x10; char * serviceName = my_strdup(OSGI_FRAMEWORK_LISTENER_HOOK_SERVICE_NAME); @@ -277,13 +267,7 @@ TEST(service_registry, registerServiceListenerHook) { .withParameter("dictionary", (void *) NULL) .andReturnValue(reg); - mock().expectOneCall("serviceRegistryTest_serviceChanged") - .withParameter("framework", framework) - .withParameter("eventType", OSGI_FRAMEWORK_SERVICE_EVENT_REGISTERED) - .withParameter("registration", reg) - .withParameter("oldprops", (void*)NULL); - - mock().expectOneCall("serviceRegistration_getServiceId") + mock().expectNCalls(2,"serviceRegistration_getServiceId") .withParameter("registration", reg) .andReturnValue(svcId); @@ -306,7 +290,7 @@ TEST(service_registry, registerServiceListenerHook) { TEST(service_registry, unregisterService) { service_registry_pt registry = NULL; framework_pt framework = (framework_pt) 0x01; - serviceRegistry_create(framework,serviceRegistryTest_serviceChanged, ®istry); + serviceRegistry_create(framework, ®istry); bundle_pt bundle = (bundle_pt) 0x10; service_registration_pt registration = (service_registration_pt) calloc(1,sizeof(struct serviceRegistration)); registration->serviceId = 20UL; @@ -323,6 +307,17 @@ TEST(service_registry, unregisterService) { properties_pt properties = (properties_pt) 0x40; long svcId = 12; + mock().expectNCalls(1,"framework_log"); + + char *serviceName = (char *) OSGI_FRAMEWORK_LISTENER_HOOK_SERVICE_NAME; + + + mock() + .expectOneCall("serviceRegistration_getServiceName") + .withParameter("registration", registration) + .withOutputParameterReturning("serviceName", &serviceName, sizeof(serviceName)) + .andReturnValue(CELIX_SUCCESS); + mock() .expectOneCall("serviceRegistration_getProperties") .withParameter("registration", registration) @@ -334,18 +329,11 @@ TEST(service_registry, unregisterService) { .withParameter("key", (char *)OSGI_FRAMEWORK_OBJECTCLASS) .andReturnValue((char*)OSGI_FRAMEWORK_LISTENER_HOOK_SERVICE_NAME); - mock().expectOneCall("serviceRegistration_getServiceId") + mock().expectNCalls(2,"serviceRegistration_getServiceId") .withParameter("registration", registration) .andReturnValue(svcId); mock() - .expectOneCall("serviceRegistryTest_serviceChanged") - .withParameter("framework", framework) - .withParameter("eventType", OSGI_FRAMEWORK_SERVICE_EVENT_UNREGISTERING) - .withParameter("registration", registration) - .withParameter("oldprops", (void*) NULL); - - mock() .expectOneCall("serviceReference_invalidate") .withParameter("reference", reference); @@ -368,7 +356,7 @@ TEST(service_registry, unregisterService) { TEST(service_registry, clearServiceRegistrations){ service_registry_pt registry = NULL; framework_pt framework = (framework_pt) 0x01; - serviceRegistry_create(framework,serviceRegistryTest_serviceChanged, ®istry); + serviceRegistry_create(framework, ®istry); array_list_pt registrations = NULL; arrayList_create(®istrations); service_registration_pt reg = (service_registration_pt) 0x10; @@ -424,7 +412,7 @@ TEST(service_registry, clearServiceRegistrations){ TEST(service_registry, getServiceReference){ service_registry_pt registry = NULL; framework_pt framework = (framework_pt) 0x01; - serviceRegistry_create(framework,serviceRegistryTest_serviceChanged, ®istry); + serviceRegistry_create(framework, ®istry); bundle_pt bundle = (bundle_pt) 0x10; service_registration_pt registration = (service_registration_pt) calloc(1,sizeof(struct serviceRegistration)); @@ -435,6 +423,8 @@ TEST(service_registry, getServiceReference){ hashMap_put(references, (void*)registration->serviceId, reference); hashMap_put(registry->serviceReferences, bundle, references); + mock().expectNCalls(1,"framework_log"); + mock().expectOneCall("serviceReference_retain") .withParameter("ref", reference); @@ -451,7 +441,7 @@ TEST(service_registry, getServiceReference){ TEST(service_registry, getServiceReference_unknownRef){ service_registry_pt registry = NULL; framework_pt framework = (framework_pt) 0x01; - serviceRegistry_create(framework,serviceRegistryTest_serviceChanged, ®istry); + serviceRegistry_create(framework, ®istry); bundle_pt bundle = (bundle_pt) 0x10; service_registration_pt registration = (service_registration_pt) calloc(1,sizeof(struct serviceRegistration)); @@ -485,7 +475,7 @@ TEST(service_registry, getServiceReference_unknownRef){ TEST(service_registry, getServiceReferences) { service_registry_pt registry = NULL; framework_pt framework = (framework_pt) 0x01; - serviceRegistry_create(framework,serviceRegistryTest_serviceChanged, ®istry); + serviceRegistry_create(framework, ®istry); bundle_pt bundle = (bundle_pt) 0x10; service_registration_pt registration = (service_registration_pt) calloc(1,sizeof(struct serviceRegistration)); @@ -504,6 +494,8 @@ TEST(service_registry, getServiceReferences) { hashMap_put(references, (void*)registration->serviceId, reference); hashMap_put(registry->serviceReferences, bundle, references); + mock().expectNCalls(1,"framework_log"); + mock() .expectOneCall("serviceRegistration_retain") .withParameter("registration", registration); @@ -554,7 +546,7 @@ TEST(service_registry, getServiceReferences) { TEST(service_registry, getServiceReferences_noFilterOrName) { service_registry_pt registry = NULL; framework_pt framework = (framework_pt) 0x01; - serviceRegistry_create(framework,serviceRegistryTest_serviceChanged, ®istry); + serviceRegistry_create(framework, ®istry); bundle_pt bundle = (bundle_pt) 0x10; service_registration_pt registration = (service_registration_pt) calloc(1,sizeof(struct serviceRegistration)); @@ -572,7 +564,9 @@ TEST(service_registry, getServiceReferences_noFilterOrName) { hashMap_put(references, (void*)registration->serviceId, reference); hashMap_put(registry->serviceReferences, bundle, references); - mock() + mock().expectNCalls(1,"framework_log"); + + mock() .expectOneCall("serviceRegistration_retain") .withParameter("registration", registration); @@ -611,7 +605,7 @@ TEST(service_registry, getServiceReferences_noFilterOrName) { TEST(service_registry, retainServiceReference){ service_registry_pt registry = NULL; framework_pt framework = (framework_pt) 0x01; - serviceRegistry_create(framework,serviceRegistryTest_serviceChanged, ®istry); + serviceRegistry_create(framework, ®istry); service_reference_pt reference = (service_reference_pt) 0x10; bundle_pt bundle = (bundle_pt) 0x20; @@ -651,7 +645,7 @@ TEST(service_registry, retainServiceReference){ TEST(service_registry, ungetServiceReference){ service_registry_pt registry = NULL; framework_pt framework = (framework_pt) 0x01; - serviceRegistry_create(framework,serviceRegistryTest_serviceChanged, ®istry); + serviceRegistry_create(framework, ®istry); service_registration_pt registration = (service_registration_pt) 0x10; service_registration_pt registration2 = (service_registration_pt) 0x20; @@ -661,7 +655,6 @@ TEST(service_registry, ungetServiceReference){ service_reference_pt reference3 = (service_reference_pt) 0x60; bundle_pt bundle = (bundle_pt) 0x70; bundle_pt bundle2 = (bundle_pt) 0x80; - module_pt module = (module_pt) 0x90; hash_map_pt references = hashMap_create(NULL, NULL, NULL, NULL); hashMap_put(references, registration, reference); @@ -671,9 +664,9 @@ TEST(service_registry, ungetServiceReference){ hashMap_put(references2, registration3, reference3); hashMap_put(registry->serviceReferences, bundle2, references2); - //test unknown reference (reference not present in registry->deletedServiceReferences) - mock().expectOneCall("framework_log"); + mock().expectNCalls(5,"framework_log"); + //test unknown reference (reference not present in registry->deletedServiceReferences) serviceRegistry_ungetServiceReference(registry, bundle, reference); //test known reference, but destroyed == false @@ -699,7 +692,6 @@ TEST(service_registry, ungetServiceReference){ mock().expectOneCall("serviceReference_release") .withParameter("ref", reference) .withOutputParameterReturning("destroyed", &destroyed, sizeof(destroyed)); - mock().expectOneCall("framework_log"); serviceRegistry_ungetServiceReference(registry, bundle2, reference); @@ -707,7 +699,6 @@ TEST(service_registry, ungetServiceReference){ destroyed = true; count = 5; - const char* mod_name = "mod name"; //const char* srv_name = "srv name"; mock().expectOneCall("serviceReference_getUsageCount") .withParameter("reference", reference) @@ -715,13 +706,9 @@ TEST(service_registry, ungetServiceReference){ mock().expectOneCall("serviceReference_release") .withParameter("ref", reference) .withOutputParameterReturning("destroyed", &destroyed, sizeof(destroyed)); - mock().expectNCalls(1, "bundle_getCurrentModule") - .withParameter("bundle", bundle) - .withOutputParameterReturning("module", &module, sizeof(module)); - mock().expectNCalls(1, "module_getSymbolicName") - .withParameter("module", module) - .withOutputParameterReturning("symbolicName", &mod_name, sizeof(mod_name)); - mock().expectNCalls(2, "framework_log"); + mock().expectNCalls(1, "celix_bundle_getSymbolicName") + .withConstPointerParameter("bnd", bundle) + .andReturnValue("mod name"); serviceRegistry_ungetServiceReference(registry, bundle, reference); @@ -758,7 +745,7 @@ TEST(service_registry, ungetServiceReference){ TEST(service_registry, clearReferencesFor_1){ service_registry_pt registry = NULL; framework_pt framework = (framework_pt) 0x01; - serviceRegistry_create(framework,serviceRegistryTest_serviceChanged, ®istry); + serviceRegistry_create(framework, ®istry); service_registration_pt registration = (service_registration_pt) 0x10; service_reference_pt reference = (service_reference_pt) 0x40; @@ -794,13 +781,11 @@ TEST(service_registry, clearReferencesFor_1){ TEST(service_registry, clearReferencesFor_2){ service_registry_pt registry = NULL; framework_pt framework = (framework_pt) 0x01; - serviceRegistry_create(framework,serviceRegistryTest_serviceChanged, ®istry); + serviceRegistry_create(framework, ®istry); service_registration_pt registration = (service_registration_pt) 0x10; service_reference_pt reference = (service_reference_pt) 0x40; bundle_pt bundle = (bundle_pt) 0x70; - module_pt module = (module_pt) 0x80; - const char* modName = "mod name"; const char* srvName = "srv name"; hash_map_pt references = hashMap_create(NULL, NULL, NULL, NULL); @@ -825,12 +810,9 @@ TEST(service_registry, clearReferencesFor_2){ mock().expectNCalls(1, "serviceReference_release") .withParameter("ref", reference) .withOutputParameterReturning("destroyed", &destroyed, sizeof(destroyed)); - mock().expectNCalls(2, "bundle_getCurrentModule") - .withParameter("bundle", bundle) - .withOutputParameterReturning("module", &module, sizeof(module)); - mock().expectNCalls(2, "module_getSymbolicName") - .withParameter("module", module) - .withOutputParameterReturning("symbolicName", &modName, sizeof(modName)); + mock().expectNCalls(2, "celix_bundle_getSymbolicName") + .withConstPointerParameter("bnd", bundle) + .andReturnValue("mod name"); mock().expectOneCall("serviceReference_getProperty") .withParameter("reference", reference) .withParameter("key", OSGI_FRAMEWORK_OBJECTCLASS) @@ -852,7 +834,7 @@ TEST(service_registry, clearReferencesFor_2){ TEST(service_registry, getService) { service_registry_pt registry = NULL; framework_pt framework = (framework_pt) 0x01; - serviceRegistry_create(framework,serviceRegistryTest_serviceChanged, ®istry); + serviceRegistry_create(framework, ®istry); bundle_pt bundle = (bundle_pt) 0x10; service_registration_pt registration = (service_registration_pt) 0x20; @@ -941,7 +923,7 @@ TEST(service_registry, getService) { TEST(service_registry, ungetService) { service_registry_pt registry = NULL; framework_pt framework = (framework_pt) 0x01; - serviceRegistry_create(framework,serviceRegistryTest_serviceChanged, ®istry); + serviceRegistry_create(framework, ®istry); bundle_pt bundle = (bundle_pt) 0x10; service_registration_pt registration = (service_registration_pt) 0x20; @@ -990,8 +972,7 @@ TEST(service_registry, ungetService) { hashMap_remove(registry->deletedServiceReferences, reference); hashMap_put(registry->deletedServiceReferences, reference, (void*) true); - mock() - .expectOneCall("framework_log"); + mock().expectNCalls(2,"framework_log"); status = serviceRegistry_ungetService(registry, bundle, reference, &result); LONGS_EQUAL(CELIX_BUNDLE_EXCEPTION, status); @@ -1006,7 +987,7 @@ TEST(service_registry, ungetService) { TEST(service_registry, getListenerHooks) { service_registry_pt registry = NULL; framework_pt framework = (framework_pt) 0x01; - serviceRegistry_create(framework,serviceRegistryTest_serviceChanged, ®istry); + serviceRegistry_create(framework, ®istry); bundle_pt bundle = (bundle_pt) 0x10; service_registration_pt registration = (service_registration_pt) calloc(1,sizeof(struct serviceRegistration)); registration->serviceId = 20UL; @@ -1017,6 +998,8 @@ TEST(service_registry, getListenerHooks) { hashMap_put(usages, (void*)registration->serviceId, reference); hashMap_put(registry->serviceReferences, bundle, usages); + mock().ignoreOtherCalls(); + size_t nrOfHooks = serviceRegistry_nrOfHooks(registry); LONGS_EQUAL(1, nrOfHooks); @@ -1026,28 +1009,10 @@ TEST(service_registry, getListenerHooks) { serviceRegistry_destroy(registry); } -TEST(service_registry, servicePropertiesModified) { - service_registry_pt registry = NULL; - framework_pt framework = (framework_pt) 0x01; - serviceRegistry_create(framework,serviceRegistryTest_serviceChanged, ®istry); - service_registration_pt registration = (service_registration_pt) 0x02; - properties_pt properties = (properties_pt) 0x03; - - mock().expectOneCall("serviceRegistryTest_serviceChanged") - .withParameter("framework", registry->framework) - .withParameter("eventType", OSGI_FRAMEWORK_SERVICE_EVENT_MODIFIED) - .withParameter("registration", registration) - .withParameter("oldprops", properties); - - serviceRegistry_servicePropertiesModified(registry, registration, properties); - - serviceRegistry_destroy(registry); -} - TEST(service_registry, getUsingBundles) { service_registry_pt registry = NULL; framework_pt framework = (framework_pt) 0x01; - serviceRegistry_create(framework,serviceRegistryTest_serviceChanged, ®istry); + serviceRegistry_create(framework, ®istry); service_registration_pt registration = (service_registration_pt) calloc(1,sizeof(struct serviceRegistration)); service_registration_pt registration2 = (service_registration_pt) calloc(1,sizeof(struct serviceRegistration)); @@ -1086,6 +1051,8 @@ TEST(service_registry, getUsingBundles) { hashMap_put(references3, (void*)registration4->serviceId, reference5); hashMap_put(registry->serviceReferences, bundle3, references3); + mock().ignoreOtherCalls(); + //call to getUsingBundles array_list_pt get_bundles_list = NULL; registry->callback.getUsingBundles(registry, registration, &get_bundles_list); diff --git a/libs/framework/src/bundle.c b/libs/framework/src/bundle.c index cd7db9f..f59e86a 100644 --- a/libs/framework/src/bundle.c +++ b/libs/framework/src/bundle.c @@ -637,13 +637,7 @@ const char* celix_bundle_getGroup(const celix_bundle_t *bnd) { } const char* celix_bundle_getSymbolicName(const celix_bundle_t *bnd) { - const char *result = NULL; - module_pt mod = NULL; - bundle_getCurrentModule((bundle_pt)bnd, &mod); - if (mod != NULL) { - module_getSymbolicName(mod, &result); - } - return result; + return bnd->symbolicName; } celix_array_list_t* celix_bundle_listRegisteredServices(const celix_bundle_t *bnd) { diff --git a/libs/framework/src/bundle_context.c b/libs/framework/src/bundle_context.c index 5122e30..7588970 100644 --- a/libs/framework/src/bundle_context.c +++ b/libs/framework/src/bundle_context.c @@ -502,7 +502,7 @@ void celix_bundleContext_unregisterService(bundle_context_t *ctx, long serviceId if (found != NULL) { serviceRegistration_unregister(found); } else { - framework_logIfError(logger, CELIX_ILLEGAL_ARGUMENT, NULL, "Provided service id (%li) is not used to registered using celix_bundleContext_registerCService/celix_registerServiceForLang". serviceId); + framework_logIfError(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)); } } } @@ -754,7 +754,7 @@ bool celix_bundleContext_useService( } -void celix_bundleContext_useServices( +size_t celix_bundleContext_useServices( bundle_context_t *ctx, const char* serviceName, void *callbackHandle, @@ -763,7 +763,7 @@ void celix_bundleContext_useServices( opts.filter.serviceName = serviceName; opts.callbackHandle = callbackHandle; opts.use = use; - celix_bundleContext_useServicesWithOptions(ctx, &opts); + return celix_bundleContext_useServicesWithOptions(ctx, &opts); } @@ -800,11 +800,11 @@ bool celix_bundleContext_useServiceWithOptions( -void celix_bundleContext_useServicesWithOptions( +size_t celix_bundleContext_useServicesWithOptions( celix_bundle_context_t *ctx, const celix_service_use_options_t *opts) { + size_t count = 0; celix_service_tracking_options_t trkOpts = CELIX_EMPTY_SERVICE_TRACKING_OPTIONS; - if (opts != NULL) { trkOpts.filter.serviceName = opts->filter.serviceName; trkOpts.filter.filter = opts->filter.filter; @@ -813,10 +813,11 @@ void celix_bundleContext_useServicesWithOptions( service_tracker_t *trk = celix_serviceTracker_createWithOptions(ctx, &trkOpts); if (trk != NULL) { - celix_serviceTracker_useServices(trk, opts->filter.serviceName, opts->callbackHandle, opts->use, opts->useWithProperties, opts->useWithOwner); + count = celix_serviceTracker_useServices(trk, opts->filter.serviceName, opts->callbackHandle, opts->use, opts->useWithProperties, opts->useWithOwner); celix_serviceTracker_destroy(trk); } } + return count; } diff --git a/libs/framework/src/framework.c b/libs/framework/src/framework.c index c3aef4d..be328be 100644 --- a/libs/framework/src/framework.c +++ b/libs/framework/src/framework.c @@ -32,7 +32,6 @@ #include "utils.h" #include "linked_list_iterator.h" #include "service_reference_private.h" -#include "listener_hook_service.h" #include "service_registration_private.h" #include "bundle_private.h" #include "celix_bundle_context.h" @@ -222,71 +221,6 @@ struct request { typedef struct request *request_pt; -typedef struct celix_fw_service_listener_entry { - //only set during creating - celix_bundle_t *bundle; - celix_service_listener_t *listener; - celix_filter_t *filter; - - celix_thread_mutex_t mutex; //protects retainedReferences and useCount - celix_array_list_t* retainedReferences; - celix_thread_cond_t useCond; - size_t useCount; -} celix_fw_service_listener_entry_t; - -static inline celix_fw_service_listener_entry_t* listener_create(celix_bundle_t *bnd, const char *filter, celix_service_listener_t *listener) { - celix_fw_service_listener_entry_t *entry = calloc(1, sizeof(*entry)); - entry->retainedReferences = celix_arrayList_create(); - entry->listener = listener; - entry->bundle = bnd; - if (filter != NULL) { - entry->filter = celix_filter_create(filter); - } - - entry->useCount = 1; - celixThreadMutex_create(&entry->mutex, NULL); - celixThreadCondition_init(&entry->useCond, NULL); - return entry; -} - -static inline void listener_retain(celix_fw_service_listener_entry_t *entry) { - celixThreadMutex_lock(&entry->mutex); - entry->useCount += 1; - celixThreadMutex_unlock(&entry->mutex); -} - -static inline void listener_release(celix_fw_service_listener_entry_t *entry) { - celixThreadMutex_lock(&entry->mutex); - assert(entry->useCount > 0); - entry->useCount -= 1; - celixThreadCondition_broadcast(&entry->useCond); - celixThreadMutex_unlock(&entry->mutex); -} - -static inline void listener_waitAndDestroy(celix_framework_t *framework, celix_fw_service_listener_entry_t *entry) { - celixThreadMutex_lock(&entry->mutex); - while (entry->useCount != 0) { - celixThreadCondition_wait(&entry->useCond, &entry->mutex); - } - celixThreadMutex_unlock(&entry->mutex); - - - //use count == 0 -> safe to destroy. - //destroy - int rSize = arrayList_size(entry->retainedReferences); - for (int i = 0; i < rSize; i += 1) { - service_reference_pt ref = arrayList_get(entry->retainedReferences, i); - if (ref != NULL) { - serviceRegistry_ungetServiceReference(framework->registry, entry->bundle, ref); // decrease retain counter - } - } - celix_filter_destroy(entry->filter); - celix_arrayList_destroy(entry->retainedReferences); - celixThreadMutex_destroy(&entry->mutex); - celixThreadCondition_destroy(&entry->useCond); - free(entry); -} - framework_logger_pt logger; static celix_thread_once_t loggerInit = CELIX_THREAD_ONCE_INIT; @@ -312,7 +246,6 @@ celix_status_t framework_create(framework_pt *framework, properties_pt config) { status = CELIX_DO_IF(status, celixThreadCondition_init(&(*framework)->shutdown.cond, NULL)); status = CELIX_DO_IF(status, celixThreadMutex_create(&(*framework)->shutdown.mutex, NULL)); status = CELIX_DO_IF(status, celixThreadMutex_create(&(*framework)->dispatcher.mutex, NULL)); - status = CELIX_DO_IF(status, celixThreadMutex_create(&(*framework)->serviceListenersLock, &attr)); status = CELIX_DO_IF(status, celixThreadMutex_create(&(*framework)->frameworkListenersLock, &attr)); status = CELIX_DO_IF(status, celixThreadMutex_create(&(*framework)->bundleListenerLock, NULL)); status = CELIX_DO_IF(status, celixThreadMutex_create(&(*framework)->installedBundles.mutex, NULL)); @@ -327,7 +260,6 @@ celix_status_t framework_create(framework_pt *framework, properties_pt config) { (*framework)->cache = NULL; (*framework)->installRequestMap = hashMap_create(utils_stringHash, utils_stringHash, utils_stringEquals, utils_stringEquals); (*framework)->installedBundles.entries = celix_arrayList_create(); - (*framework)->serviceListeners = NULL; (*framework)->bundleListeners = NULL; (*framework)->frameworkListeners = NULL; (*framework)->dispatcher.requests = NULL; @@ -415,15 +347,6 @@ celix_status_t framework_destroy(framework_pt framework) { serviceRegistry_destroy(framework->registry); - if (framework->serviceListeners != NULL) { - int size = celix_arrayList_size(framework->serviceListeners); - for (int i = 0; i < size; ++i) { - celix_fw_service_listener_entry_t *entry = celix_arrayList_get(framework->serviceListeners, i); - listener_release(entry); - listener_waitAndDestroy(framework, entry); - } - arrayList_destroy(framework->serviceListeners); - } if (framework->bundleListeners) { arrayList_destroy(framework->bundleListeners); } @@ -444,7 +367,6 @@ celix_status_t framework_destroy(framework_pt framework) { bundleCache_destroy(&framework->cache); celixThreadCondition_destroy(&framework->dispatcher.cond); - celixThreadMutex_destroy(&framework->serviceListenersLock); celixThreadMutex_destroy(&framework->frameworkListenersLock); celixThreadMutex_destroy(&framework->bundleListenerLock); celixThreadMutex_destroy(&framework->dispatcher.mutex); @@ -481,7 +403,6 @@ celix_status_t fw_init(framework_pt framework) { properties_set(framework->configurationMap, (char*) OSGI_FRAMEWORK_FRAMEWORK_UUID, uuid); celix_status_t status = CELIX_SUCCESS; - status = CELIX_DO_IF(status, arrayList_create(&framework->serviceListeners)); //entry is celix_fw_service_listener_entry_t status = CELIX_DO_IF(status, arrayList_create(&framework->bundleListeners)); status = CELIX_DO_IF(status, arrayList_create(&framework->frameworkListeners)); status = CELIX_DO_IF(status, arrayList_create(&framework->dispatcher.requests)); @@ -547,7 +468,7 @@ celix_status_t fw_init(framework_pt framework) { arrayList_destroy(archives); } - status = CELIX_DO_IF(status, serviceRegistry_create(framework, fw_serviceChanged, &framework->registry)); + status = CELIX_DO_IF(status, serviceRegistry_create(framework, &framework->registry)); status = CELIX_DO_IF(status, framework_setBundleStateAndNotify(framework, framework->bundle, OSGI_FRAMEWORK_BUNDLE_STARTING)); bundle_context_t *context = NULL; @@ -803,9 +724,9 @@ celix_status_t fw_installBundle2(framework_pt framework, bundle_pt * bundle, lon if (status == CELIX_SUCCESS) { long bndId = -1L; bundle_getBundleId(*bundle, &bndId); - celix_framework_bundle_entry_t *entry = fw_bundleEntry_create(*bundle); + celix_framework_bundle_entry_t *bEntry = fw_bundleEntry_create(*bundle); celixThreadMutex_lock(&framework->installedBundles.mutex); - celix_arrayList_add(framework->installedBundles.entries, entry); + celix_arrayList_add(framework->installedBundles.entries, bEntry); celixThreadMutex_unlock(&framework->installedBundles.mutex); } else { @@ -1445,7 +1366,7 @@ celix_status_t fw_populateDependentGraph(framework_pt framework, bundle_pt expor return status; } -celix_status_t fw_registerService(framework_pt framework, service_registration_pt *registration, long bndId, const char* serviceName, const void* svcObj, properties_pt properties) { +celix_status_t fw_registerService(framework_pt framework, service_registration_pt *registration, long bndId, const char* serviceName, const void* svcObj, celix_properties_t *properties) { celix_status_t status = CELIX_SUCCESS; char *error = NULL; if (serviceName == NULL || svcObj == NULL) { @@ -1454,79 +1375,9 @@ celix_status_t fw_registerService(framework_pt framework, service_registration_p } celix_framework_bundle_entry_t *entry = fw_bundleEntry_getBundleEntryAndIncreaseUseCount(framework, bndId); - - - status = CELIX_DO_IF(status, serviceRegistry_registerService(framework->registry, entry->bnd, serviceName, svcObj, properties, registration)); - - if (status == CELIX_SUCCESS) { - // If this is a listener hook, invoke the callback with all current listeners - if (strcmp(serviceName, OSGI_FRAMEWORK_LISTENER_HOOK_SERVICE_NAME) == 0) { - unsigned int i; - array_list_pt infos = NULL; - service_reference_pt ref = NULL; - listener_hook_service_pt hook = NULL; - - status = CELIX_DO_IF(status, arrayList_create(&infos)); - - if (status == CELIX_SUCCESS) { - celix_status_t subs = CELIX_SUCCESS; - - celixThreadMutex_lock(&framework->serviceListenersLock); - for (i = 0; i < arrayList_size(framework->serviceListeners); i++) { - celix_fw_service_listener_entry_t *listener = arrayList_get(framework->serviceListeners, i); - bundle_context_t *context = NULL; - listener_hook_info_pt info = NULL; - bundle_context_pt lContext = NULL; - - subs = CELIX_DO_IF(subs, bundle_getContext(entry->bnd, &context)); - if (subs == CELIX_SUCCESS) { - info = (listener_hook_info_pt) malloc(sizeof(*info)); - if (info == NULL) { - subs = CELIX_ENOMEM; - } - } - - subs = CELIX_DO_IF(subs, bundle_getContext(listener->bundle, &lContext)); - if (subs == CELIX_SUCCESS) { - info->context = lContext; - info->removed = false; - } - subs = CELIX_DO_IF(subs, filter_getString(listener->filter, &info->filter)); - - if (subs == CELIX_SUCCESS) { - arrayList_add(infos, info); - } - else{ - fw_logCode(framework->logger, OSGI_FRAMEWORK_LOG_ERROR, status, "Could not pass all listeners to the hook: %s", serviceName); - free(info); - } - } - celixThreadMutex_unlock(&framework->serviceListenersLock); - - status = CELIX_DO_IF(status, serviceRegistry_getServiceReference(framework->registry, framework->bundle, - *registration, &ref)); - status = CELIX_DO_IF(status, fw_getService(framework,framework->bundle, ref, (const void **) &hook)); - if (status == CELIX_SUCCESS) { - hook->added(hook->handle, infos); - } - status = CELIX_DO_IF(status, serviceRegistry_ungetService(framework->registry, framework->bundle, ref, NULL)); - status = CELIX_DO_IF(status, serviceRegistry_ungetServiceReference(framework->registry, framework->bundle, ref)); - - for (int j = 0; j < arrayList_size(infos); j++) { - listener_hook_info_pt info = arrayList_get(infos, j); - free(info); - } - arrayList_destroy(infos); - } - } - } - - fw_bundleEntry_decreaseUseCount(entry); - framework_logIfError(framework->logger, status, error, "Cannot register service: %s", serviceName); - return status; } @@ -1611,45 +1462,11 @@ celix_status_t framework_ungetService(framework_pt framework, bundle_pt bundle, } void fw_addServiceListener(framework_pt framework, bundle_pt bundle, celix_service_listener_t *listener, const char* sfilter) { - celix_fw_service_listener_entry_t *fwListener = listener_create(bundle, sfilter, listener); - - celixThreadMutex_lock(&framework->serviceListenersLock); - arrayList_add(framework->serviceListeners, fwListener); - celixThreadMutex_unlock(&framework->serviceListenersLock); - - serviceRegistry_callHooksForListenerFilter(framework->registry, bundle, sfilter, false); + celix_serviceRegistry_addServiceListener(framework->registry, bundle, sfilter, listener); } -void fw_removeServiceListener(framework_pt framework, bundle_pt bundle, celix_service_listener_t *listener) { - celix_fw_service_listener_entry_t *match = NULL; - - bundle_context_t *context; - bundle_getContext(bundle, &context); - - int i; - celixThreadMutex_lock(&framework->serviceListenersLock); - for (i = 0; i < arrayList_size(framework->serviceListeners); i++) { - celix_fw_service_listener_entry_t *visit = (celix_fw_service_listener_entry_t*) arrayList_get(framework->serviceListeners, i); - if (visit->listener == listener && visit->bundle == bundle) { - match = visit; - arrayList_remove(framework->serviceListeners, i); - break; - } - } - celixThreadMutex_unlock(&framework->serviceListenersLock); - - - if (match != NULL) { - //invoke listener hooks - const char *filter; - filter_getString(match->filter, &filter); - serviceRegistry_callHooksForListenerFilter(framework->registry, bundle, filter, true); - } - - if (match != NULL) { - listener_release(match); - listener_waitAndDestroy(framework, match); - } +void fw_removeServiceListener(framework_pt framework, bundle_pt bundle __attribute__((unused)), celix_service_listener_t *listener) { + celix_serviceRegistry_removeServiceListener(framework->registry, listener); } celix_status_t fw_addBundleListener(framework_pt framework, bundle_pt bundle, bundle_listener_pt listener) { @@ -1754,91 +1571,6 @@ celix_status_t fw_removeFrameworkListener(framework_pt framework, bundle_pt bund return status; } -void fw_serviceChanged(framework_pt framework, celix_service_event_type_t eventType, service_registration_pt registration, properties_pt oldprops) { - unsigned int i; - celix_fw_service_listener_entry_t *entry; - - celix_array_list_t* retainedEntries = celix_arrayList_create(); - celix_array_list_t* matchedEntries = celix_arrayList_create(); - - celixThreadMutex_lock(&framework->serviceListenersLock); - for (i = 0; i < celix_arrayList_size(framework->serviceListeners); i++) { - entry = (celix_fw_service_listener_entry_t *) celix_arrayList_get(framework->serviceListeners, i); - celix_arrayList_add(retainedEntries, entry); - listener_retain(entry); //ensure that use count > 0, so that the listener cannot be destroyed until all pending event are handled. - } - celixThreadMutex_unlock(&framework->serviceListenersLock); - - for (i = 0; i < celix_arrayList_size(retainedEntries); ++i) { - entry = (celix_fw_service_listener_entry_t *) celix_arrayList_get(retainedEntries, i); - int matched = 0; - properties_pt props = NULL; - bool matchResult = false; - serviceRegistration_getProperties(registration, &props); - if (entry->filter != NULL) { - filter_match(entry->filter, props, &matchResult); - } - matched = (entry->filter == NULL) || matchResult; - if (matched) { - celix_arrayList_add(matchedEntries, entry); - } else { - listener_release(entry); //Not a match -> release entry - } - } - celix_arrayList_destroy(retainedEntries); - - /* - * TODO FIXME, A deadlock can happen when (e.g.) a service is deregistered, triggering this fw_serviceChanged and - * one of the matching service listener callback tries to remove an other matched service listener. - * The remove service listener will call the listener_waitForDestroy and the fw_serviceChanged part keeps the - * usageCount on > 0. - * - * Not sure how to prevent/handle this. - */ - for (i = 0; i < celix_arrayList_size(matchedEntries); ++i) { - entry = (celix_fw_service_listener_entry_t *) celix_arrayList_get(matchedEntries, i); - - service_reference_pt reference = NULL; - celix_service_event_t event; - - serviceRegistry_getServiceReference(framework->registry, entry->bundle, registration, &reference); - - //NOTE: that you are never sure that the UNREGISTERED event will by handle by an service_listener. listener could be gone - //Every reference retained is therefore stored and called when a service listener is removed from the framework. - if (eventType == OSGI_FRAMEWORK_SERVICE_EVENT_REGISTERED) { - serviceRegistry_retainServiceReference(framework->registry, entry->bundle, reference); - celixThreadMutex_lock(&entry->mutex); - arrayList_add(entry->retainedReferences, reference); //TODO improve by using set (or hashmap) instead of list - celixThreadMutex_unlock(&entry->mutex); - } - - event.type = eventType; - event.reference = reference; - - entry->listener->serviceChanged(entry->listener, &event); - - serviceRegistry_ungetServiceReference(framework->registry, entry->bundle, reference); - - if (eventType == OSGI_FRAMEWORK_SERVICE_EVENT_UNREGISTERING) { - //if service listener was active when service was registered, release the retained reference - celixThreadMutex_lock(&entry->mutex); - bool removed = arrayList_removeElement(entry->retainedReferences, reference); - celixThreadMutex_unlock(&entry->mutex); - if (removed) { - serviceRegistry_ungetServiceReference(framework->registry, entry->bundle, - reference); // decrease retain counter - } - - } - - if (eventType == OSGI_FRAMEWORK_SERVICE_EVENT_MODIFIED) { - entry->listener->serviceChanged(entry->listener, &event); - } - listener_release(entry); //decrease usage, so that the listener can be destroyed (if use count is now 0) - } - celix_arrayList_destroy(matchedEntries); -} - //celix_status_t fw_isServiceAssignable(framework_pt fw, bundle_pt requester, service_reference_pt reference, bool *assignable) { // celix_status_t status = CELIX_SUCCESS; // diff --git a/libs/framework/src/framework_private.h b/libs/framework/src/framework_private.h index 537a040..fdd0132 100644 --- a/libs/framework/src/framework_private.h +++ b/libs/framework/src/framework_private.h @@ -49,9 +49,6 @@ struct celix_framework { long bundleId; //the bundle id of the framework (normally 0) hash_map_pt installRequestMap; - celix_thread_mutex_t serviceListenersLock; - array_list_pt serviceListeners; - array_list_pt frameworkListeners; celix_thread_mutex_t frameworkListenersLock; diff --git a/libs/framework/src/registry_callback_private.h b/libs/framework/src/registry_callback_private.h index f286a07..ff030bb 100644 --- a/libs/framework/src/registry_callback_private.h +++ b/libs/framework/src/registry_callback_private.h @@ -35,7 +35,6 @@ typedef struct registry_callback_struct { void *handle; celix_status_t (*getUsingBundles)(void *handle, service_registration_pt reg, array_list_pt *bundles); celix_status_t (*unregister)(void *handle, bundle_pt bundle, service_registration_pt reg); - celix_status_t (*modified)(void *handle, service_registration_pt registration, properties_pt oldProperties); } registry_callback_t; #endif /* REGISTRY_CALLBACK_H_ */ diff --git a/libs/framework/src/service_reference.c b/libs/framework/src/service_reference.c index 2394634..f7d6879 100644 --- a/libs/framework/src/service_reference.c +++ b/libs/framework/src/service_reference.c @@ -204,6 +204,18 @@ celix_status_t serviceReference_getServiceRegistration(service_reference_pt ref, } } +long serviceReference_getServiceId(service_reference_pt ref) { + long svcId = -1L; + if (ref != NULL) { + celixThreadRwlock_readLock(&ref->lock); + svcId = ref->registration->serviceId; + celixThreadRwlock_unlock(&ref->lock); + } + return svcId; +} + + + FRAMEWORK_EXPORT celix_status_t serviceReference_getPropertyWithDefault(service_reference_pt ref, const char *key, const char* def, const char **value) { celix_status_t status = CELIX_SUCCESS; diff --git a/libs/framework/src/service_registration.c b/libs/framework/src/service_registration.c index 3400134..745148e 100644 --- a/libs/framework/src/service_registration.c +++ b/libs/framework/src/service_registration.c @@ -231,19 +231,10 @@ celix_status_t serviceRegistration_getProperties(service_registration_pt registr celix_status_t serviceRegistration_setProperties(service_registration_pt registration, properties_pt properties) { celix_status_t status; - properties_pt oldProperties = NULL; - registry_callback_t callback; - celixThreadRwlock_writeLock(®istration->lock); - oldProperties = registration->properties; status = serviceRegistration_initializeProperties(registration, properties); - callback = registration->callback; celixThreadRwlock_unlock(®istration->lock); - if (status == CELIX_SUCCESS && callback.modified != NULL) { - callback.modified(callback.handle, registration, oldProperties); - } - return status; } diff --git a/libs/framework/src/service_registration_private.h b/libs/framework/src/service_registration_private.h index 6040f24..9ecb760 100644 --- a/libs/framework/src/service_registration_private.h +++ b/libs/framework/src/service_registration_private.h @@ -16,13 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -/** - * service_registration_private.h - * - * \date Feb 11, 2013 - * \author <a href="mailto:[email protected]";>Apache Celix Project Team</a> - * \copyright Apache License, Version 2.0 - */ + #ifndef SERVICE_REGISTRATION_PRIVATE_H_ #define SERVICE_REGISTRATION_PRIVATE_H_ diff --git a/libs/framework/src/service_registry.c b/libs/framework/src/service_registry.c index 135a636..7ffa0eb 100644 --- a/libs/framework/src/service_registry.c +++ b/libs/framework/src/service_registry.c @@ -49,13 +49,23 @@ static celix_status_t serviceRegistry_setReferenceStatus(service_registry_pt reg bool deleted); static celix_status_t serviceRegistry_getUsingBundles(service_registry_pt registry, service_registration_pt reg, array_list_pt *bundles); static celix_status_t serviceRegistry_getServiceReference_internal(service_registry_pt registry, bundle_pt owner, service_registration_pt registration, service_reference_pt *out); +static void celix_serviceRegistry_serviceChanged(celix_service_registry_t *registry, celix_service_event_type_t eventType, service_registration_pt registration); +static void serviceRegistry_callHooksForListenerFilter(service_registry_pt registry, celix_bundle_t *owner, const celix_filter_t *filter, bool removed); -static celix_service_registry_listener_hook_entry_t* celix_createHookEntry(long svcId, celix_listener_hook_service_t*); + static celix_service_registry_listener_hook_entry_t* celix_createHookEntry(long svcId, celix_listener_hook_service_t*); static void celix_waitAndDestroyHookEntry(celix_service_registry_listener_hook_entry_t *entry); static void celix_increaseCountHook(celix_service_registry_listener_hook_entry_t *entry); static void celix_decreaseCountHook(celix_service_registry_listener_hook_entry_t *entry); -celix_status_t serviceRegistry_create(framework_pt framework, serviceChanged_function_pt serviceChanged, service_registry_pt *out) { +static void celix_increaseCountServiceListener(celix_service_registry_service_listener_entry_t *entry); +static void celix_decreaseCountServiceListener(celix_service_registry_service_listener_entry_t *entry); +static void celix_waitAndDestroyServiceListener(celix_service_registry_service_listener_entry_t *entry); + +static void celix_increasePendingRegisteredEvent(celix_service_registry_t *registry, long svcId); +static void celix_decreasePendingRegisteredEvent(celix_service_registry_t *registry, long svcId); +static void celix_waitForPendingRegisteredEvents(celix_service_registry_t *registry, long svcId); + +celix_status_t serviceRegistry_create(framework_pt framework, service_registry_pt *out) { celix_status_t status; service_registry_pt reg = calloc(1, sizeof(*reg)); @@ -66,9 +76,7 @@ celix_status_t serviceRegistry_create(framework_pt framework, serviceChanged_fun reg->callback.handle = reg; reg->callback.getUsingBundles = (void *)serviceRegistry_getUsingBundles; reg->callback.unregister = (void *) serviceRegistry_unregisterService; - reg->callback.modified = (void *) serviceRegistry_servicePropertiesModified; - reg->serviceChanged = serviceChanged; reg->serviceRegistrations = hashMap_create(NULL, NULL, NULL, NULL); reg->framework = framework; reg->currentServiceId = 1UL; @@ -77,7 +85,12 @@ celix_status_t serviceRegistry_create(framework_pt framework, serviceChanged_fun reg->checkDeletedReferences = CHECK_DELETED_REFERENCES; reg->deletedServiceReferences = hashMap_create(NULL, NULL, NULL, NULL); - arrayList_create(®->listenerHooks); + reg->listenerHooks = celix_arrayList_create(); + reg->serviceListeners = celix_arrayList_create(); + + celixThreadMutex_create(®->pendingRegisterEvents.mutex, NULL); + celixThreadCondition_init(®->pendingRegisterEvents.cond, NULL); + reg->pendingRegisterEvents.map = hashMap_create(NULL, NULL, NULL, NULL); status = celixThreadRwlock_create(®->lock, NULL); } @@ -94,36 +107,44 @@ celix_status_t serviceRegistry_create(framework_pt framework, serviceChanged_fun celix_status_t serviceRegistry_destroy(service_registry_pt registry) { celixThreadRwlock_writeLock(®istry->lock); + //remove service listeners + int size = celix_arrayList_size(registry->serviceListeners); + if (size > 0) { + fw_log(logger, OSGI_FRAMEWORK_LOG_ERROR, "%i dangling service listeners\n", size); + } + for (int i = 0; i < size; ++i) { + celix_service_registry_service_listener_entry_t *entry = celix_arrayList_get(registry->serviceListeners, i); + celix_decreaseCountServiceListener(entry); + celix_waitAndDestroyServiceListener(entry); + } + arrayList_destroy(registry->serviceListeners); + //destroy service registration map - int size = hashMap_size(registry->serviceRegistrations); + size = hashMap_size(registry->serviceRegistrations); if (size > 0) { fw_log(logger, OSGI_FRAMEWORK_LOG_ERROR, "%i bundles with dangling service registration\n", size); - hash_map_iterator_t iter = hashMapIterator_construct(registry->serviceRegistrations); - while (hashMapIterator_hasNext(&iter)) { - hash_map_entry_t *entry = hashMapIterator_nextEntry(&iter); - bundle_t *bnd = hashMapEntry_getKey(entry); - celix_array_list_t *registrations = hashMapEntry_getValue(entry); - module_pt mod = NULL; - const char *name = NULL; - bundle_getCurrentModule(bnd, &mod); - if (mod != NULL) { - module_getSymbolicName(mod, &name); - } - for (int i = 0; i < celix_arrayList_size(registrations); ++i) { - service_registration_pt reg = celix_arrayList_get(registrations, i); - const char *svcName = NULL; - serviceRegistration_getServiceName(reg, &svcName); - fw_log(logger, OSGI_FRAMEWORK_LOG_ERROR, "Bundle %s still has a %s service registered\n", name, svcName); - } + } + hash_map_iterator_t iter = hashMapIterator_construct(registry->serviceRegistrations); + while (hashMapIterator_hasNext(&iter)) { + hash_map_entry_t *entry = hashMapIterator_nextEntry(&iter); + celix_bundle_t *bnd = hashMapEntry_getKey(entry); + celix_array_list_t *registrations = hashMapEntry_getValue(entry); + for (int i = 0; i < celix_arrayList_size(registrations); ++i) { + service_registration_pt reg = celix_arrayList_get(registrations, i); + const char *svcName = NULL; + serviceRegistration_getServiceName(reg, &svcName); + fw_log(logger, OSGI_FRAMEWORK_LOG_ERROR, "Bundle %s (bundle id: %li) still has a %s service registered\n", celix_bundle_getSymbolicName(bnd), celix_bundle_getId(bnd), svcName); } } + assert(size == 0); hashMap_destroy(registry->serviceRegistrations, false, false); //destroy service references (double) map); - //FIXME. The framework bundle does not (yet) call clearReferences, as result the size could be > 0 for test code. - //size = hashMap_size(registry->serviceReferences); - //assert(size == 0); + size = hashMap_size(registry->serviceReferences); + if (size > 0) { + fw_log(logger, OSGI_FRAMEWORK_LOG_ERROR, "Unexpected service references left in the service registry! Nr of references: %i", size); + } hashMap_destroy(registry->serviceReferences, false, false); //destroy listener hooks @@ -136,6 +157,12 @@ celix_status_t serviceRegistry_destroy(service_registry_pt registry) { hashMap_destroy(registry->deletedServiceReferences, false, false); + size = hashMap_size(registry->pendingRegisterEvents.map); + assert(size == 0); + celixThreadMutex_destroy(®istry->pendingRegisterEvents.mutex); + celixThreadCondition_destroy(®istry->pendingRegisterEvents.cond); + hashMap_destroy(registry->pendingRegisterEvents.map, false, false); + free(registry); return CELIX_SUCCESS; @@ -190,11 +217,8 @@ static celix_status_t serviceRegistry_registerServiceInternal(service_registry_p } else { //plain *registration = serviceRegistration_create(registry->callback, bundle, serviceName, ++registry->currentServiceId, serviceObject, dictionary); } - - //long id; - //bundle_getBundleId(bundle, &id); - //fprintf(stderr, "REG: Registering service '%s' for bundle id %li with reg pointer %p\n", serviceName, id, *registration); - + long svcId = serviceRegistration_getServiceId(*registration); + //printf("Registering service %li with name %s\n", svcId, serviceName); serviceRegistry_addHooks(registry, serviceName, serviceObject, *registration); @@ -206,41 +230,56 @@ static celix_status_t serviceRegistry_registerServiceInternal(service_registry_p hashMap_put(registry->serviceRegistrations, bundle, regs); } arrayList_add(regs, *registration); - celixThreadRwlock_unlock(®istry->lock); - if (registry->serviceChanged != NULL) { - registry->serviceChanged(registry->framework, OSGI_FRAMEWORK_SERVICE_EVENT_REGISTERED, *registration, NULL); - } + //update pending register event + celix_increasePendingRegisteredEvent(registry, svcId); + celixThreadRwlock_unlock(®istry->lock); + + + //NOTE there is a race condition with celix_serviceRegistry_addServiceListener, as result + //a REGISTERED event can be triggered twice instead of once. The service tracker can deal with this. + //The handling of pending registered events is to ensure that the UNREGISTERING event is always + //after the 1 or 2 REGISTERED events. + + celix_serviceRegistry_serviceChanged(registry, OSGI_FRAMEWORK_SERVICE_EVENT_REGISTERED, *registration); + //update pending register event count + celix_decreasePendingRegisteredEvent(registry, svcId); return CELIX_SUCCESS; } celix_status_t serviceRegistry_unregisterService(service_registry_pt registry, bundle_pt bundle, service_registration_pt registration) { // array_list_t clients; - array_list_pt regs; + celix_array_list_t *regs; //fprintf(stderr, "REG: Unregistering service registration with pointer %p\n", registration); + long svcId = serviceRegistration_getServiceId(registration); + const char *svcName = NULL; + serviceRegistration_getServiceName(registration, &svcName); + //printf("Unregistering service %li with name %s\n", svcId, svcName); + serviceRegistry_removeHook(registry, registration); celixThreadRwlock_writeLock(®istry->lock); - regs = (array_list_pt) hashMap_get(registry->serviceRegistrations, bundle); + regs = (celix_array_list_t*) hashMap_get(registry->serviceRegistrations, bundle); if (regs != NULL) { arrayList_removeElement(regs, registration); int size = arrayList_size(regs); if (size == 0) { - arrayList_destroy(regs); + celix_arrayList_destroy(regs); hashMap_remove(registry->serviceRegistrations, bundle); } } celixThreadRwlock_unlock(®istry->lock); - if (registry->serviceChanged != NULL) { - registry->serviceChanged(registry->framework, OSGI_FRAMEWORK_SERVICE_EVENT_UNREGISTERING, registration, NULL); - } + //check and wait for pending register events + celix_waitForPendingRegisteredEvents(registry, svcId); + + celix_serviceRegistry_serviceChanged(registry, OSGI_FRAMEWORK_SERVICE_EVENT_UNREGISTERING, registration); - celixThreadRwlock_readLock(®istry->lock); + celixThreadRwlock_readLock(®istry->lock); //invalidate service references hash_map_iterator_pt iter = hashMapIterator_create(registry->serviceReferences); while (hashMapIterator_hasNext(iter)) { @@ -308,7 +347,7 @@ celix_status_t serviceRegistry_getServiceReference(service_registry_pt registry, service_registration_pt registration, service_reference_pt *out) { celix_status_t status = CELIX_SUCCESS; - if(celixThreadRwlock_writeLock(®istry->lock) == CELIX_SUCCESS) { + if (celixThreadRwlock_writeLock(®istry->lock) == CELIX_SUCCESS) { status = serviceRegistry_getServiceReference_internal(registry, owner, registration, out); celixThreadRwlock_unlock(®istry->lock); } @@ -566,22 +605,16 @@ static void serviceRegistry_logWarningServiceReferenceUsageCount(service_registr } if(usageCount > 0 || refCount > 0) { - module_pt module_ptr = NULL; - bundle_getCurrentModule(bundle, &module_ptr); - const char* bundle_name = NULL; - module_getSymbolicName(module_ptr, &bundle_name); - + const char* bundle_name = celix_bundle_getSymbolicName(bundle); const char* service_name = "unknown"; const char* bundle_provider_name = "unknown"; if (refCount > 0 && ref != NULL) { serviceReference_getProperty(ref, OSGI_FRAMEWORK_OBJECTCLASS, &service_name); service_registration_pt reg = NULL; - bundle_pt bundle = NULL; - module_pt mod = NULL; + bundle_pt providedBnd = NULL; serviceReference_getServiceRegistration(ref, ®); - serviceRegistration_getBundle(reg, &bundle); - bundle_getCurrentModule(bundle, &mod); - module_getSymbolicName(mod, &bundle_provider_name); + serviceRegistration_getBundle(reg, &providedBnd); + bundle_provider_name = celix_bundle_getSymbolicName(providedBnd); } fw_log(logger, OSGI_FRAMEWORK_LOG_WARNING, "Previous Dangling service reference warnings caused by bundle '%s', for service '%s', provided by bundle '%s'", bundle_name, service_name, bundle_provider_name); @@ -779,14 +812,14 @@ static celix_status_t serviceRegistry_removeHook(service_registry_pt registry, s return status; } -void serviceRegistry_callHooksForListenerFilter(service_registry_pt registry, celix_bundle_t *owner, const char *filter, bool removed) { +static void serviceRegistry_callHooksForListenerFilter(service_registry_pt registry, celix_bundle_t *owner, const celix_filter_t *filter, bool removed) { celix_bundle_context_t *ctx; bundle_getContext(owner, &ctx); struct listener_hook_info info; info.context = ctx; info.removed = removed; - info.filter = filter; + info.filter = celix_filter_getFilterString(filter); celix_array_list_t *infos = celix_arrayList_create(); celix_arrayList_add(infos, &info); @@ -823,13 +856,6 @@ size_t serviceRegistry_nrOfHooks(service_registry_pt registry) { return (size_t) size; } -celix_status_t serviceRegistry_servicePropertiesModified(service_registry_pt registry, service_registration_pt registration, properties_pt oldprops) { - if (registry->serviceChanged != NULL) { - registry->serviceChanged(registry->framework, OSGI_FRAMEWORK_SERVICE_EVENT_MODIFIED, registration, oldprops); - } - return CELIX_SUCCESS; -} - static celix_status_t serviceRegistry_getUsingBundles(service_registry_pt registry, service_registration_pt registration, array_list_pt *out) { celix_status_t status; array_list_pt bundles = NULL; @@ -886,12 +912,12 @@ static void celix_waitAndDestroyHookEntry(celix_service_registry_listener_hook_e if (entry != NULL) { celixThreadMutex_lock(&entry->mutex); int waitCount = 0; - while (entry->count > 0) { + while (entry->useCount > 0) { celixThreadCondition_timedwaitRelative(&entry->cond, &entry->mutex, 1, 0); //wait for 1 second waitCount += 1; if (waitCount >= 5) { fw_log(logger, OSGI_FRAMEWORK_LOG_WARNING, - "Still waiting for service listener hook use count to become zero. Waiting for %i seconds. Use Count is %i, svc id is %li", waitCount, (int)entry->count, entry->svcId); + "Still waiting for service listener hook use count to become zero. Waiting for %i seconds. Use Count is %i, svc id is %li", waitCount, (int)entry->useCount, entry->svcId); } } celixThreadMutex_unlock(&entry->mutex); @@ -905,7 +931,7 @@ static void celix_waitAndDestroyHookEntry(celix_service_registry_listener_hook_e static void celix_increaseCountHook(celix_service_registry_listener_hook_entry_t *entry) { if (entry != NULL) { celixThreadMutex_lock(&entry->mutex); - entry->count += 1; + entry->useCount += 1; celixThreadCondition_broadcast(&entry->cond); celixThreadMutex_unlock(&entry->mutex); } @@ -913,12 +939,46 @@ static void celix_increaseCountHook(celix_service_registry_listener_hook_entry_t static void celix_decreaseCountHook(celix_service_registry_listener_hook_entry_t *entry) { if (entry != NULL) { celixThreadMutex_lock(&entry->mutex); - entry->count -= 1; + entry->useCount -= 1; + celixThreadCondition_broadcast(&entry->cond); + celixThreadMutex_unlock(&entry->mutex); + } +} + +static void celix_increaseCountServiceListener(celix_service_registry_service_listener_entry_t *entry) { + if (entry != NULL) { + celixThreadMutex_lock(&entry->mutex); + entry->useCount += 1; + celixThreadCondition_broadcast(&entry->cond); + celixThreadMutex_unlock(&entry->mutex); + } +} + +static void celix_decreaseCountServiceListener(celix_service_registry_service_listener_entry_t *entry) { + if (entry != NULL) { + celixThreadMutex_lock(&entry->mutex); + entry->useCount -= 1; celixThreadCondition_broadcast(&entry->cond); celixThreadMutex_unlock(&entry->mutex); } } +static inline void celix_waitAndDestroyServiceListener(celix_service_registry_service_listener_entry_t *entry) { + celixThreadMutex_lock(&entry->mutex); + while (entry->useCount != 0) { + celixThreadCondition_wait(&entry->cond, &entry->mutex); + } + celixThreadMutex_unlock(&entry->mutex); + + //use count == 0 -> safe to destroy. + //destroy + celixThreadMutex_destroy(&entry->mutex); + celixThreadCondition_destroy(&entry->cond); + celix_filter_destroy(entry->filter); + free(entry); +} + + celix_array_list_t* celix_serviceRegistry_listServiceIdsForOwner(celix_service_registry_t* registry, long bndId) { celix_array_list_t *result = celix_arrayList_create(); celixThreadRwlock_readLock(®istry->lock); @@ -972,4 +1032,249 @@ bool celix_serviceRegistry_getServiceInfo( celixThreadRwlock_unlock(®istry->lock); return found; -} \ No newline at end of file +} + +celix_status_t celix_serviceRegistry_addServiceListener(celix_service_registry_t *registry, celix_bundle_t *bundle, const char *stringFilter, celix_service_listener_t *listener) { + + celix_filter_t *filter = NULL; + if (stringFilter != NULL) { + filter = celix_filter_create(stringFilter); + if (filter == NULL) { + fw_log(logger, OSGI_FRAMEWORK_LOG_ERROR, "Cannot add service listener filter '%s' is invalid", stringFilter); + return CELIX_ILLEGAL_ARGUMENT; + } + } + + celix_service_registry_service_listener_entry_t *entry = calloc(1, sizeof(*entry)); + entry->bundle = bundle; + entry->filter = filter; + entry->listener = listener; + entry->useCount = 1; //new entry -> count on 1 + celixThreadMutex_create(&entry->mutex, NULL); + celixThreadCondition_init(&entry->cond, NULL); + + celix_array_list_t *registrations = celix_arrayList_create(); + + celixThreadRwlock_writeLock(®istry->lock); + celix_arrayList_add(registry->serviceListeners, entry); //use count 1 + + //find already registered services + hash_map_iterator_t iter = hashMapIterator_construct(registry->serviceRegistrations); + while (hashMapIterator_hasNext(&iter)) { + celix_array_list_t *regs = (array_list_pt) hashMapIterator_nextValue(&iter); + for (int regIdx = 0; (regs != NULL) && regIdx < celix_arrayList_size(regs); ++regIdx) { + service_registration_pt registration = celix_arrayList_get(regs, regIdx); + properties_pt props = NULL; + serviceRegistration_getProperties(registration, &props); + if (celix_filter_match(filter, props)) { + serviceRegistration_retain(registration); + long svcId = serviceRegistration_getServiceId(registration); + celix_arrayList_add(registrations, registration); + //update pending register event count + celix_increasePendingRegisteredEvent(registry, svcId); + } + } + } + celixThreadRwlock_unlock(®istry->lock); + + //NOTE there is a race condition with serviceRegistry_registerServiceInternal, as result + //a REGISTERED event can be triggered twice instead of once. The service tracker can deal with this. + //The handling of pending registered events is to ensure that the UNREGISTERING event is always + //after the 1 or 2 REGISTERED events. + + for (int i = 0; i < celix_arrayList_size(registrations); ++i) { + service_registration_pt reg = celix_arrayList_get(registrations, i); + long svcId = serviceRegistration_getServiceId(reg); + service_reference_pt ref = NULL; + serviceRegistry_getServiceReference_internal(registry, bundle, reg, &ref); + celix_service_event_t event; + event.reference = ref; + event.type = OSGI_FRAMEWORK_SERVICE_EVENT_REGISTERED; + listener->serviceChanged(listener->handle, &event); + serviceReference_release(ref, NULL); + serviceRegistration_release(reg); + + //update pending register event count + celix_decreasePendingRegisteredEvent(registry, svcId); + } + celix_arrayList_destroy(registrations); + + serviceRegistry_callHooksForListenerFilter(registry, bundle, entry->filter, false); + + celix_decreaseCountServiceListener(entry); //use count decreased, can be 0 + return CELIX_SUCCESS; +} + +celix_status_t celix_serviceRegistry_removeServiceListener(celix_service_registry_t *registry, celix_service_listener_t *listener) { + celix_service_registry_service_listener_entry_t *entry = NULL; + + celixThreadRwlock_writeLock(®istry->lock); + for (int i = 0; i < celix_arrayList_size(registry->serviceListeners); ++i) { + celix_service_registry_service_listener_entry_t *visit = celix_arrayList_get(registry->serviceListeners, i); + if (visit->listener == listener) { + entry = visit; + celix_arrayList_removeAt(registry->serviceListeners, i); + break; + } + } + celixThreadRwlock_unlock(®istry->lock); + + if (entry != NULL) { + serviceRegistry_callHooksForListenerFilter(registry, entry->bundle, entry->filter, true); + celix_waitAndDestroyServiceListener(entry); + } else { + fw_log(logger, OSGI_FRAMEWORK_LOG_ERROR, "Cannot remove service listener, listener not found"); + return CELIX_ILLEGAL_ARGUMENT; + } + return CELIX_SUCCESS; +} + +static void celix_serviceRegistry_serviceChanged(celix_service_registry_t *registry, celix_service_event_type_t eventType, service_registration_pt registration) { + celix_service_registry_service_listener_entry_t *entry; + + celix_array_list_t* retainedEntries = celix_arrayList_create(); + celix_array_list_t* matchedEntries = celix_arrayList_create(); + + celixThreadRwlock_readLock(®istry->lock); + for (int i = 0; i < celix_arrayList_size(registry->serviceListeners); ++i) { + entry = celix_arrayList_get(registry->serviceListeners, i); + celix_arrayList_add(retainedEntries, entry); + celix_increaseCountServiceListener(entry); //ensure that use count > 0, so that the listener cannot be destroyed until all pending event are handled. + } + celixThreadRwlock_unlock(®istry->lock); + + for (int i = 0; i < celix_arrayList_size(retainedEntries); ++i) { + entry = celix_arrayList_get(retainedEntries, i); + int matched = 0; + celix_properties_t *props = NULL; + bool matchResult = false; + serviceRegistration_getProperties(registration, &props); + if (entry->filter != NULL) { + filter_match(entry->filter, props, &matchResult); + } + matched = (entry->filter == NULL) || matchResult; + if (matched) { + celix_arrayList_add(matchedEntries, entry); + } else { + celix_decreaseCountServiceListener(entry); //Not a match -> release entry + } + } + celix_arrayList_destroy(retainedEntries); + + /* + * TODO FIXME, A deadlock can happen when (e.g.) a service is deregistered, triggering this fw_serviceChanged and + * one of the matching service listener callbacks tries to remove an other matched service listener. + * The remove service listener will call the listener_waitForDestroy and the fw_serviceChanged part keeps the + * usageCount on > 0. + * + * Not sure how to prevent/handle this. + */ + + for (int i = 0; i < celix_arrayList_size(matchedEntries); ++i) { + entry = celix_arrayList_get(matchedEntries, i); + service_reference_pt reference = NULL; + celix_service_event_t event; + serviceRegistry_getServiceReference(registry, entry->bundle, registration, &reference); + event.type = eventType; + event.reference = reference; + entry->listener->serviceChanged(entry->listener->handle, &event); + serviceRegistry_ungetServiceReference(registry, entry->bundle, reference); + celix_decreaseCountServiceListener(entry); //decrease usage, so that the listener can be destroyed (if use count is now 0) + } + celix_arrayList_destroy(matchedEntries); +} + + +static void celix_increasePendingRegisteredEvent(celix_service_registry_t *registry, long svcId) { + celixThreadMutex_lock(®istry->pendingRegisterEvents.mutex); + long count = (long)hashMap_get(registry->pendingRegisterEvents.map, (void*)svcId); + count += 1; + hashMap_put(registry->pendingRegisterEvents.map, (void*)svcId, (void*)count); + celixThreadMutex_unlock(®istry->pendingRegisterEvents.mutex); +} + +static void celix_decreasePendingRegisteredEvent(celix_service_registry_t *registry, long svcId) { + celixThreadMutex_lock(®istry->pendingRegisterEvents.mutex); + long count = (long)hashMap_get(registry->pendingRegisterEvents.map, (void*)svcId); + assert(count >= 1); + count -= 1; + if (count > 0) { + hashMap_put(registry->pendingRegisterEvents.map, (void *)svcId, (void *)count); + } else { + hashMap_remove(registry->pendingRegisterEvents.map, (void*)svcId); + } + celixThreadCondition_signal(®istry->pendingRegisterEvents.cond); + celixThreadMutex_unlock(®istry->pendingRegisterEvents.mutex); +} + +static void celix_waitForPendingRegisteredEvents(celix_service_registry_t *registry, long svcId) { + celixThreadMutex_lock(®istry->pendingRegisterEvents.mutex); + long count = (long)hashMap_get(registry->pendingRegisterEvents.map, (void*)svcId); + while (count > 0) { + celixThreadCondition_wait(®istry->pendingRegisterEvents.cond, ®istry->pendingRegisterEvents.mutex); + count = (long)hashMap_get(registry->pendingRegisterEvents.map, (void*)svcId); + } + celixThreadMutex_unlock(®istry->pendingRegisterEvents.mutex); +} + +//static void celix_serviceRegistry_triggerListenerHooks(celix_service_registry_t *registry, const char* serviceName, const celix_properties_t *properties) { +// //If this is a listener hook, invoke the callback with all current listeners +// if (strcmp(serviceName, OSGI_FRAMEWORK_LISTENER_HOOK_SERVICE_NAME) == 0) { +// celix_array_list_t *infos = NULL; +// service_reference_pt ref = NULL; +// listener_hook_service_pt hook = NULL; +// +// status = CELIX_DO_IF(status, arrayList_create(&infos)); +// +// if (status == CELIX_SUCCESS) { +// celix_status_t subs = CELIX_SUCCESS; +// +// celixThreadMutex_lock(&framework->serviceListenersLock); +// for (i = 0; i < arrayList_size(framework->serviceListeners); i++) { +// celix_fw_service_listener_entry_t *listener = arrayList_get(framework->serviceListeners, i); +// bundle_context_t *context = NULL; +// listener_hook_info_pt info = NULL; +// bundle_context_pt lContext = NULL; +// +// subs = CELIX_DO_IF(subs, bundle_getContext(entry->bnd, &context)); +// if (subs == CELIX_SUCCESS) { +// info = (listener_hook_info_pt) malloc(sizeof(*info)); +// if (info == NULL) { +// subs = CELIX_ENOMEM; +// } +// } +// +// subs = CELIX_DO_IF(subs, bundle_getContext(listener->bundle, &lContext)); +// if (subs == CELIX_SUCCESS) { +// info->context = lContext; +// info->removed = false; +// } +// subs = CELIX_DO_IF(subs, filter_getString(listener->filter, &info->filter)); +// +// if (subs == CELIX_SUCCESS) { +// arrayList_add(infos, info); +// } +// else{ +// fw_logCode(framework->logger, OSGI_FRAMEWORK_LOG_ERROR, status, "Could not pass all listeners to the hook: %s", serviceName); +// free(info); +// } +// } +// celixThreadMutex_unlock(&framework->serviceListenersLock); +// +// status = CELIX_DO_IF(status, serviceRegistry_getServiceReference(framework->registry, framework->bundle, +// *registration, &ref)); +// status = CELIX_DO_IF(status, fw_getService(framework,framework->bundle, ref, (const void **) &hook)); +// if (status == CELIX_SUCCESS) { +// hook->added(hook->handle, infos); +// } +// status = CELIX_DO_IF(status, serviceRegistry_ungetService(framework->registry, framework->bundle, ref, NULL)); +// status = CELIX_DO_IF(status, serviceRegistry_ungetServiceReference(framework->registry, framework->bundle, ref)); +// +// for (int j = 0; j < arrayList_size(infos); j++) { +// listener_hook_info_pt info = arrayList_get(infos, j); +// free(info); +// } +// arrayList_destroy(infos); +// } +// } +//} \ No newline at end of file diff --git a/libs/framework/src/service_registry_private.h b/libs/framework/src/service_registry_private.h index 82d719a..3a4cd01 100644 --- a/libs/framework/src/service_registry_private.h +++ b/libs/framework/src/service_registry_private.h @@ -36,18 +36,33 @@ struct celix_serviceRegistry { framework_pt framework; registry_callback_t callback; - hash_map_pt serviceRegistrations; //key = bundle (reg owner), value = list ( registration ) - hash_map_pt serviceReferences; //key = bundle, value = map (key = serviceId, value = reference) + celix_thread_rwlock_t lock; //protect below + + hash_map_t *serviceRegistrations; //key = bundle (reg owner), value = list ( registration ) + hash_map_t *serviceReferences; //key = bundle, value = map (key = serviceId, value = reference) bool checkDeletedReferences; //If enabled. check if provided service references are still valid - hash_map_pt deletedServiceReferences; //key = ref pointer, value = bool + hash_map_t *deletedServiceReferences; //key = ref pointer, value = bool - serviceChanged_function_pt serviceChanged; unsigned long currentServiceId; - array_list_pt listenerHooks; //celix_service_registry_listener_hook_entry_t* + celix_array_list_t *listenerHooks; //celix_service_registry_listener_hook_entry_t* + celix_array_list_t *serviceListeners; //celix_service_registry_service_listener_entry_t* - celix_thread_rwlock_t lock; + /** + * The pending register events are introduced to ensure UNREGISTERING events are always + * after REGISTERED events in service listeners. + * When a listener is registered and it retroactively triggers registered events, it will also + * increase the pending registered events for the matching services. + * Also when a new service is registered all matching services listeners are called. + * This is used to ensuring unregistering events can only be triggered after all registered + * events - for a given service - are triggered. + */ + struct { + celix_thread_mutex_t mutex; + celix_thread_cond_t cond; + hash_map_t *map; //key = svc id, value = long (nr of pending register events) + } pendingRegisterEvents; }; typedef struct celix_service_registry_listener_hook_entry { @@ -55,9 +70,18 @@ typedef struct celix_service_registry_listener_hook_entry { celix_listener_hook_service_t *hook; celix_thread_mutex_t mutex; //protects below celix_thread_cond_t cond; - unsigned int count; + unsigned int useCount; } celix_service_registry_listener_hook_entry_t; +typedef struct celix_service_registry_service_listener_entry { + celix_bundle_t *bundle; + celix_filter_t *filter; + celix_service_listener_t *listener; + celix_thread_mutex_t mutex; //protects below + celix_thread_cond_t cond; + unsigned int useCount; +} celix_service_registry_service_listener_entry_t; + typedef enum reference_status_enum { REF_ACTIVE, REF_DELETED, diff --git a/libs/framework/src/service_tracker.c b/libs/framework/src/service_tracker.c index d2a22f9..e421adc 100644 --- a/libs/framework/src/service_tracker.c +++ b/libs/framework/src/service_tracker.c @@ -39,12 +39,10 @@ static celix_status_t serviceTracker_untrack(celix_service_tracker_instance_t *t static void serviceTracker_untrackTracked(celix_service_tracker_instance_t *tracker, celix_tracked_entry_t *tracked); static celix_status_t serviceTracker_invokeAddingService(celix_service_tracker_instance_t *tracker, service_reference_pt ref, void **svcOut); static celix_status_t serviceTracker_invokeAddService(celix_service_tracker_instance_t *tracker, celix_tracked_entry_t *tracked); -static celix_status_t serviceTracker_invokeModifiedService(celix_service_tracker_instance_t *tracker, celix_tracked_entry_t *tracked); static celix_status_t serviceTracker_invokeRemovingService(celix_service_tracker_instance_t *tracker, celix_tracked_entry_t *tracked); static void serviceTracker_checkAndInvokeSetService(void *handle, void *highestSvc, const properties_t *props, const bundle_t *bnd); static bool serviceTracker_useHighestRankingServiceInternal(celix_service_tracker_instance_t *instance, const char *serviceName /*sanity*/, - double waitTimeoutInSeconds, void *callbackHandle, void (*use)(void *handle, void *svc), void (*useWithProperties)(void *handle, void *svc, const celix_properties_t *props), @@ -53,6 +51,8 @@ static bool serviceTracker_useHighestRankingServiceInternal(celix_service_tracke static void serviceTracker_addInstanceFromShutdownList(celix_service_tracker_instance_t *instance); static void serviceTracker_remInstanceFromShutdownList(celix_service_tracker_instance_t *instance); +static void serviceTracker_serviceChanged(void *handle, celix_service_event_t *event); + static celix_thread_once_t g_once = CELIX_THREAD_ONCE_INIT; //once for g_shutdownMutex, g_shutdownCond @@ -90,9 +90,7 @@ static inline void tracked_release(celix_tracked_entry_t *tracked) { celixThreadMutex_lock(&tracked->mutex); assert(tracked->useCount > 0); tracked->useCount -= 1; - if (tracked->useCount == 0) { - celixThreadCondition_broadcast(&tracked->useCond); - } + celixThreadCondition_signal(&tracked->useCond); celixThreadMutex_unlock(&tracked->mutex); } @@ -158,9 +156,10 @@ celix_status_t serviceTracker_destroy(service_tracker_pt tracker) { celix_status_t serviceTracker_open(service_tracker_pt tracker) { celix_service_listener_t *listener = NULL; celix_service_tracker_instance_t *instance = NULL; - array_list_pt initial = NULL; celix_status_t status = CELIX_SUCCESS; + bool addListener = false; + celixThreadRwlock_writeLock(&tracker->instanceLock); if (tracker->instance == NULL) { instance = calloc(1, sizeof(*instance)); @@ -197,36 +196,20 @@ celix_status_t serviceTracker_open(service_tracker_pt tracker) { instance->removeWithProperties = tracker->removeWithProperties; instance->removeWithOwner = tracker->removeWithOwner; - status = bundleContext_getServiceReferences(tracker->context, NULL, tracker->filter, &initial); //REF COUNT to 1 - tracker->instance = instance; + + addListener = true; } else { //already open - } - celixThreadRwlock_unlock(&tracker->instanceLock); + framework_logIfError(logger, status, NULL, "Tracker already open"); - //TODO add fw call which adds a service listener and return the then valid service references. - if (status == CELIX_SUCCESS && listener != NULL) { //register service listener - status = bundleContext_addServiceListener(tracker->context, listener, tracker->filter); - } - if (status == CELIX_SUCCESS && initial != NULL) { - service_reference_pt initial_reference; - unsigned int i; - for (i = 0; i < arrayList_size(initial); i++) { - initial_reference = (service_reference_pt) arrayList_get(initial, i); - serviceTracker_track(instance, initial_reference, NULL); //REF COUNT to 2 - bundleContext_ungetServiceReference(tracker->context, initial_reference); //REF COUNT to 1 - } - arrayList_destroy(initial); } + celixThreadRwlock_unlock(&tracker->instanceLock); - if (status != CELIX_SUCCESS && listener != NULL){ - free(listener); + if (addListener) { + bundleContext_addServiceListener(tracker->context, listener, tracker->filter); } - - framework_logIfError(logger, status, NULL, "Cannot open tracker"); - - return status; + return CELIX_SUCCESS; } static void* shutdownServiceTrackerInstanceHandler(void *data) { @@ -254,33 +237,31 @@ celix_status_t serviceTracker_close(service_tracker_pt tracker) { celixThreadRwlock_writeLock(&tracker->instanceLock); celix_service_tracker_instance_t *instance = tracker->instance; tracker->instance = NULL; - celixThreadRwlock_unlock(&tracker->instanceLock); - if (instance != NULL) { - - //prevent service listener events celixThreadMutex_lock(&instance->closingLock); + //prevent service listener events instance->closing = true; celixThreadMutex_unlock(&instance->closingLock); + } + celixThreadRwlock_unlock(&tracker->instanceLock); - int i; + if (instance != NULL) { celixThreadRwlock_writeLock(&instance->lock); - size_t size = celix_arrayList_size(instance->trackedServices); + int size = celix_arrayList_size(instance->trackedServices); celix_tracked_entry_t *trackedEntries[size]; - for (i = 0; i < arrayList_size(instance->trackedServices); i++) { + for (int i = 0; i < arrayList_size(instance->trackedServices); i++) { trackedEntries[i] = (celix_tracked_entry_t *) arrayList_get(instance->trackedServices, i); } arrayList_clear(instance->trackedServices); celixThreadRwlock_unlock(&instance->lock); //loop trough tracked entries an untrack - for (i = 0; i < size; i++) { + for (int i = 0; i < size; i++) { serviceTracker_untrackTracked(instance, trackedEntries[i]); } - //sync til all pending serviceChanged event are handled.. (TODO again a possible deadlock??) celixThreadMutex_lock(&instance->closingLock); - while(instance->activeServiceChangeCalls > 0) { + while (instance->activeServiceChangeCalls > 0) { celixThreadCondition_wait(&instance->activeServiceChangeCallsCond, &instance->closingLock); } celixThreadMutex_unlock(&instance->closingLock); @@ -292,7 +273,7 @@ celix_status_t serviceTracker_close(service_tracker_pt tracker) { // actual thread calling the removeServiceListener. // // This can be detached -> because service listener events are ignored (closing=true) and so no callbacks - //are made back to the celix framework / tracker owner. + // are made back to the celix framework / tracker owner. serviceTracker_addInstanceFromShutdownList(instance); celix_thread_t localThread; celixThread_create(&localThread, NULL, shutdownServiceTrackerInstanceHandler, instance); @@ -416,8 +397,8 @@ void *serviceTracker_getServiceByReference(service_tracker_pt tracker, service_r return service; } -void serviceTracker_serviceChanged(celix_service_listener_t *listener, celix_service_event_t *event) { - celix_service_tracker_instance_t *instance = listener->handle; +static void serviceTracker_serviceChanged(void *handle, celix_service_event_t *event) { + celix_service_tracker_instance_t *instance = handle; celixThreadMutex_lock(&instance->closingLock); bool closing = instance->closing; @@ -475,17 +456,14 @@ static celix_status_t serviceTracker_track(celix_service_tracker_instance_t *ins celix_tracked_entry_t *visit = (celix_tracked_entry_t*) arrayList_get(instance->trackedServices, i); serviceReference_equals(reference, visit->reference, &equals); if (equals) { + //NOTE it is possible to get two REGISTERED events, second one can be ignored. found = visit; - tracked_retain(found); break; } } celixThreadRwlock_unlock(&instance->lock); - if (found != NULL) { - status = serviceTracker_invokeModifiedService(instance, found); - tracked_retain(found); - } else if (status == CELIX_SUCCESS && found == NULL) { + if (found == NULL) { //NEW entry void *service = NULL; status = serviceTracker_invokeAddingService(instance, reference, &service); @@ -502,14 +480,14 @@ static celix_status_t serviceTracker_track(celix_service_tracker_instance_t *ins serviceRegistration_getProperties(reg, &props); } - celix_tracked_entry_t *tracked = tracked_create(reference, service, props, bnd); + celix_tracked_entry_t *tracked = tracked_create(reference, service, props, bnd); //use count 1 celixThreadRwlock_writeLock(&instance->lock); arrayList_add(instance->trackedServices, tracked); celixThreadRwlock_unlock(&instance->lock); serviceTracker_invokeAddService(instance, tracked); - serviceTracker_useHighestRankingServiceInternal(instance, tracked->serviceName, 0, instance, NULL, NULL, serviceTracker_checkAndInvokeSetService); + serviceTracker_useHighestRankingServiceInternal(instance, tracked->serviceName, instance, NULL, NULL, serviceTracker_checkAndInvokeSetService); } } @@ -551,31 +529,6 @@ static void serviceTracker_checkAndInvokeSetService(void *handle, void *highestS } } -static celix_status_t serviceTracker_invokeModifiedService(celix_service_tracker_instance_t *instance, celix_tracked_entry_t *tracked) { - celix_status_t status = CELIX_SUCCESS; - - void *customizerHandle = NULL; - modified_callback_pt function = NULL; - serviceTrackerCustomizer_getHandle(&instance->customizer, &customizerHandle); - serviceTrackerCustomizer_getModifiedFunction(&instance->customizer, &function); - if (function != NULL) { - function(customizerHandle, tracked->reference, tracked->service); - } - - void *handle = instance->callbackHandle; - if (instance->modified != NULL) { - instance->modified(handle, tracked->service); - } - if (instance->modifiedWithProperties != NULL) { - instance->modifiedWithProperties(handle, tracked->service, tracked->properties); - } - if (instance->modifiedWithOwner != NULL) { - instance->modifiedWithOwner(handle, tracked->service, tracked->properties, tracked->serviceOwner); - } - return status; -} - - static celix_status_t serviceTracker_invokeAddService(celix_service_tracker_instance_t *instance, celix_tracked_entry_t *tracked) { celix_status_t status = CELIX_SUCCESS; @@ -651,7 +604,7 @@ static celix_status_t serviceTracker_untrack(celix_service_tracker_instance_t* i if (size == 0) { serviceTracker_checkAndInvokeSetService(instance, NULL, NULL, NULL); } else { - serviceTracker_useHighestRankingServiceInternal(instance, serviceName, 0, instance, NULL, NULL, serviceTracker_checkAndInvokeSetService); + serviceTracker_useHighestRankingServiceInternal(instance, serviceName, instance, NULL, NULL, serviceTracker_checkAndInvokeSetService); } serviceTracker_untrackTracked(instance, remove); @@ -830,7 +783,6 @@ void celix_serviceTracker_destroy(celix_service_tracker_t *tracker) { static bool serviceTracker_useHighestRankingServiceInternal(celix_service_tracker_instance_t *instance, const char *serviceName /*sanity*/, - double waitForSvcTimeoutInSec /*0 -> do not wait */, void *callbackHandle, void (*use)(void *handle, void *svc), void (*useWithProperties)(void *handle, void *svc, const celix_properties_t *props), @@ -845,29 +797,6 @@ static bool serviceTracker_useHighestRankingServiceInternal(celix_service_tracke celixThreadRwlock_readLock(&instance->lock); unsigned int size = arrayList_size(instance->trackedServices); - if (waitForSvcTimeoutInSec > 0) { - struct timespec start; - struct timespec now; - clock_gettime(CLOCK_MONOTONIC, &start); - - double billion = 1E9; - long waitFor = (long)(waitForSvcTimeoutInSec * billion); - long diffInNs = 0L; - - while (size == 0 && diffInNs < waitFor) { - size = arrayList_size(instance->trackedServices); - if (size > 0) { - break; - } else { - celixThreadRwlock_unlock(&instance->lock); - usleep(1); - celixThreadRwlock_readLock(&instance->lock); - }; - clock_gettime(CLOCK_MONOTONIC, &now); - diffInNs = ( now.tv_nsec - start.tv_nsec ) + ( now.tv_sec - start.tv_sec ) * (long)billion; - } - } - for (i = 0; i < size; i++) { tracked = (celix_tracked_entry_t *) arrayList_get(instance->trackedServices, i); if (serviceName != NULL && tracked->serviceName != NULL && strncmp(tracked->serviceName, serviceName, 10*1024) == 0) { @@ -912,34 +841,48 @@ bool celix_serviceTracker_useHighestRankingService( void (*use)(void *handle, void *svc), void (*useWithProperties)(void *handle, void *svc, const celix_properties_t *props), void (*useWithOwner)(void *handle, void *svc, const celix_properties_t *props, const celix_bundle_t *owner)) { - celixThreadRwlock_readLock(&tracker->instanceLock); - celix_service_tracker_instance_t *instance = tracker->instance; + bool called = false; - if (instance != NULL) { - called = serviceTracker_useHighestRankingServiceInternal(instance, serviceName, waitTimeoutInSeconds, callbackHandle, use, - useWithProperties, useWithOwner); - } - celixThreadRwlock_unlock(&tracker->instanceLock); + struct timespec start, now; + clock_gettime(CLOCK_MONOTONIC, &start); + do { + celixThreadRwlock_readLock(&tracker->instanceLock); + if (tracker->instance != NULL) { + called = serviceTracker_useHighestRankingServiceInternal(tracker->instance, serviceName, callbackHandle, use, useWithProperties, useWithOwner); + } + celixThreadRwlock_unlock(&tracker->instanceLock); + + if (waitTimeoutInSeconds <= 0) { + break; + } else if (!called) { + clock_gettime(CLOCK_MONOTONIC, &now); + double diff = celix_difftime(&start, &now); + if (diff > waitTimeoutInSeconds) { + break; + } + usleep(10); + } + } while (!called); return called; } -void celix_serviceTracker_useServices( +size_t celix_serviceTracker_useServices( service_tracker_t *tracker, const char* serviceName /*sanity*/, void *callbackHandle, void (*use)(void *handle, void *svc), void (*useWithProperties)(void *handle, void *svc, const celix_properties_t *props), void (*useWithOwner)(void *handle, void *svc, const celix_properties_t *props, const celix_bundle_t *owner)) { - int i; - + size_t count = 0; celixThreadRwlock_readLock(&tracker->instanceLock); celix_service_tracker_instance_t *instance = tracker->instance; if (instance != NULL) { //first lock tracker, get tracked entries and increase use count celixThreadRwlock_readLock(&instance->lock); - size_t size = celix_arrayList_size(instance->trackedServices); + int size = celix_arrayList_size(instance->trackedServices); + count = (size_t)size; celix_tracked_entry_t *entries[size]; - for (i = 0; i < size; i++) { + for (int i = 0; i < size; i++) { celix_tracked_entry_t *tracked = (celix_tracked_entry_t *) arrayList_get(instance->trackedServices, i); tracked_retain(tracked); entries[i] = tracked; @@ -948,7 +891,7 @@ void celix_serviceTracker_useServices( celixThreadRwlock_unlock(&instance->lock); //then use entries and decrease use count - for (i = 0; i < size; i++) { + for (int i = 0; i < size; i++) { celix_tracked_entry_t *entry = entries[i]; //got service, call, decrease use count an signal useCond after. if (use != NULL) { @@ -965,6 +908,7 @@ void celix_serviceTracker_useServices( } } celixThreadRwlock_unlock(&tracker->instanceLock); + return count; } void celix_serviceTracker_syncForFramework(void *fw) {
