celix-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From pnol...@apache.org
Subject [celix] 01/01: Removes some TODO and moves the hanling of listener hooks to service registry
Date Thu, 12 Sep 2019 18:30:48 GMT
This is an automated email from the ASF dual-hosted git repository.

pnoltes pushed a commit to branch feature/handle_hooks_in_registry
in repository https://gitbox.apache.org/repos/asf/celix.git

commit 8751e29bc7b8482e476ba56b589b66e9f22f92a0
Author: Pepijn Noltes <pepijnnoltes@gmail.com>
AuthorDate: Thu Sep 12 20:30:08 2019 +0200

    Removes some TODO and moves the hanling of listener hooks to service registry
---
 libs/framework/include/service_registry.h          |  4 +-
 .../framework/private/mock/service_registry_mock.c | 14 +++-
 .../private/test/service_registry_test.cpp         | 17 +----
 libs/framework/src/bundle.c                        |  3 -
 libs/framework/src/bundle_context.c                |  6 +-
 libs/framework/src/bundle_revision.c               |  2 -
 libs/framework/src/dm_event.c                      |  1 -
 libs/framework/src/framework.c                     | 89 +++-------------------
 libs/framework/src/service_registry.c              | 75 +++++++++++-------
 9 files changed, 77 insertions(+), 134 deletions(-)

diff --git a/libs/framework/include/service_registry.h b/libs/framework/include/service_registry.h
index 81bbe3c..9150c85 100644
--- a/libs/framework/include/service_registry.h
+++ b/libs/framework/include/service_registry.h
@@ -93,7 +93,9 @@ serviceRegistry_ungetService(service_registry_pt registry, celix_bundle_t
*bundl
 
 celix_status_t serviceRegistry_clearReferencesFor(service_registry_pt registry, celix_bundle_t
*bundle);
 
-celix_status_t serviceRegistry_getListenerHooks(service_registry_pt registry, celix_bundle_t
*bundle, celix_array_list_t **hooks);
+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,
diff --git a/libs/framework/private/mock/service_registry_mock.c b/libs/framework/private/mock/service_registry_mock.c
index 9942818..52f64a2 100644
--- a/libs/framework/private/mock/service_registry_mock.c
+++ b/libs/framework/private/mock/service_registry_mock.c
@@ -181,4 +181,16 @@ celix_serviceRegistry_registerServiceFactory(
 			->withPointerParameters("props", props)
 			->withOutputParameter("registration", registration);
 	return mock_c()->returnValue().value.intValue;
-}
\ No newline at end of file
+}
+
+void serviceRegistry_callHooksForListenerFilter(
+        service_registry_pt registry,
+        celix_bundle_t *owner,
+        const char *filter,
+        bool removed) {
+    mock_c()->actualCall("serviceRegistry_callHooksForListenerFilter")
+            ->withPointerParameters("registry", registry)
+            ->withPointerParameters("owner", owner)
+            ->withStringParameters("filter", filter)
+            ->withBoolParameters("removed", removed);
+}
diff --git a/libs/framework/private/test/service_registry_test.cpp b/libs/framework/private/test/service_registry_test.cpp
index f0a4573..cccbd0a 100644
--- a/libs/framework/private/test/service_registry_test.cpp
+++ b/libs/framework/private/test/service_registry_test.cpp
@@ -1003,23 +1003,10 @@ TEST(service_registry, getListenerHooks) {
 	hashMap_put(usages, (void*)registration->serviceId, reference);
 	hashMap_put(registry->serviceReferences, bundle, usages);
 
-	mock()
-		.expectOneCall("serviceRegistration_retain")
-		.withParameter("registration", registration);
-	mock()
-		.expectOneCall("serviceReference_retain")
-		.withParameter("ref", reference);
-	mock()
-		.expectOneCall("serviceRegistration_release")
-		.withParameter("registration", registration);
-
-	array_list_pt hooks = NULL;
-	serviceRegistry_getListenerHooks(registry, bundle, &hooks);
-	LONGS_EQUAL(1, arrayList_size(hooks));
-	POINTERS_EQUAL(reference, arrayList_get(hooks, 0));
+    size_t nrOfHooks = serviceRegistry_nrOfHooks(registry);
+	LONGS_EQUAL(1, nrOfHooks);
 
 	hashMap_destroy(usages, false, false);
-	arrayList_destroy(hooks);
 	arrayList_remove(registry->listenerHooks, 0);
 	free(registration);
 	serviceRegistry_destroy(registry);
diff --git a/libs/framework/src/bundle.c b/libs/framework/src/bundle.c
index cd2610a..f05beae 100644
--- a/libs/framework/src/bundle.c
+++ b/libs/framework/src/bundle.c
@@ -274,7 +274,6 @@ celix_status_t bundle_update(bundle_pt bundle, const char *inputFile)
{
 		status = bundle_isSystemBundle(bundle, &systemBundle);
 		if (status == CELIX_SUCCESS) {
 			if (systemBundle) {
-				// #TODO: Support framework update
 				status = CELIX_BUNDLE_EXCEPTION;
 			} else {
 				status = framework_updateBundle(bundle->framework, bundle, inputFile);
@@ -461,8 +460,6 @@ celix_status_t bundle_closeAndDelete(bundle_pt bundle) {
 
 celix_status_t bundle_closeRevisions(bundle_pt bundle) {
     celix_status_t status = CELIX_SUCCESS;
-
-    // TODO implement this
     return status;
 }
 
diff --git a/libs/framework/src/bundle_context.c b/libs/framework/src/bundle_context.c
index 5f78f85..506fb1e 100644
--- a/libs/framework/src/bundle_context.c
+++ b/libs/framework/src/bundle_context.c
@@ -84,8 +84,6 @@ celix_status_t bundleContext_destroy(bundle_context_pt context) {
 	    bundleContext_cleanupServiceTrackers(context);
         bundleContext_cleanupServiceTrackerTrackers(context);
 
-        //TODO cleanup service registrations
-
 	    //NOTE still present service registrations will be cleared during bundle stop in the
 	    //service registry (serviceRegistry_clearServiceRegistrations).
         celixThreadMutex_unlock(&context->mutex);
@@ -572,8 +570,6 @@ long celix_bundleContext_trackBundlesWithOptions(
         trackerId = entry->trackerId;
 
         //loop through all already installed bundles.
-        // FIXME there is a race condition between installing the listener and looping through
the started bundles.
-        // NOTE move this to the framework, so that the framework can ensure locking to ensure
not bundles is missed.
         if (entry->opts.onStarted != NULL) {
             celix_framework_useBundles(ctx->framework, entry->opts.includeFrameworkBundle,
entry->opts.callbackHandle, entry->opts.onStarted);
         }
@@ -758,7 +754,7 @@ bool celix_bundleContext_stopBundle(celix_bundle_context_t *ctx, long
bundleId)
 
 static void bundleContext_uninstallBundleCallback(void *handle, const bundle_t *c_bnd) {
     bool *uninstalled = handle;
-    bundle_t *bnd = (bundle_t*)c_bnd; //TODO use mute-able variant ??
+    bundle_t *bnd = (bundle_t*)c_bnd;
     if (celix_bundle_getState(bnd) == OSGI_FRAMEWORK_BUNDLE_ACTIVE) {
         bundle_stop(bnd);
     }
diff --git a/libs/framework/src/bundle_revision.c b/libs/framework/src/bundle_revision.c
index 9165c7d..2f4d2f1 100644
--- a/libs/framework/src/bundle_revision.c
+++ b/libs/framework/src/bundle_revision.c
@@ -42,7 +42,6 @@ celix_status_t bundleRevision_create(const char *root, const char *location,
lon
     if (!revision) {
     	status = CELIX_ENOMEM;
     } else {
-    	// TODO: This overwrites an existing revision, is this supposed to happen?
         int state = mkdir(root, S_IRWXU);
         if ((state != 0) && (errno != EEXIST)) {
             free(revision);
@@ -51,7 +50,6 @@ celix_status_t bundleRevision_create(const char *root, const char *location,
lon
             if (inputFile != NULL) {
                 status = extractBundle(inputFile, root);
             } else if (strcmp(location, "inputstream:") != 0) {
-            	// TODO how to handle this correctly?
             	// If location != inputstream, extract it, else ignore it and assume this is
a cache entry.
                 status = extractBundle(location, root);
             }
diff --git a/libs/framework/src/dm_event.c b/libs/framework/src/dm_event.c
index 6375c4d..c1cc4ba 100644
--- a/libs/framework/src/dm_event.c
+++ b/libs/framework/src/dm_event.c
@@ -43,7 +43,6 @@ celix_status_t event_create(dm_event_type_e event_type, bundle_pt bundle,
bundle
 	serviceReference_getProperty(reference, OSGI_FRAMEWORK_SERVICE_ID, &serviceIdStr);
 	unsigned long servId = strtoul(serviceIdStr,NULL,10);
 
-	//FIXME service ranking can dynamically change, but service reference can be removed at
any time.
 	const char* rankingStr = NULL;
 	serviceReference_getProperty(reference, OSGI_FRAMEWORK_SERVICE_RANKING, &rankingStr);
 	long ranking = rankingStr == NULL ? 0 : atol(rankingStr);
diff --git a/libs/framework/src/framework.c b/libs/framework/src/framework.c
index b15dde4..51615a7 100644
--- a/libs/framework/src/framework.c
+++ b/libs/framework/src/framework.c
@@ -231,7 +231,6 @@ struct request {
 
 typedef struct request *request_pt;
 
-//TODO move to service registry
 typedef struct celix_fw_service_listener_entry {
     //only set during creating
     celix_bundle_t *bundle;
@@ -301,7 +300,6 @@ static inline void listener_waitAndDestroy(celix_framework_t *framework,
celix_f
 
 framework_logger_pt logger;
 
-//TODO introduce a counter + mutex to control the freeing of the logger when mutiple threads
are running a framework.
 static celix_thread_once_t loggerInit = CELIX_THREAD_ONCE_INIT;
 static void framework_loggerInit(void) {
     logger = malloc(sizeof(*logger));
@@ -324,11 +322,11 @@ 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));  //TODO refactor to use use count with condition (see serviceListeners)
+        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));  //TODO refactor to use use count with condition (see serviceListeners)
-        status = CELIX_DO_IF(status, celixThreadMutex_create(&(*framework)->bundleListenerLock,
NULL));  //TODO refactor to use use count with condition (see serviceListeners)
-        status = CELIX_DO_IF(status, celixThreadMutex_create(&(*framework)->installedBundles.mutex,
NULL));  //TODO refactor to use use count with condition (see serviceListeners)
+        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));
         status = CELIX_DO_IF(status, celixThreadCondition_init(&(*framework)->dispatcher.cond,
NULL));
         if (status == CELIX_SUCCESS) {
             (*framework)->bundle = NULL;
@@ -352,8 +350,7 @@ celix_status_t framework_create(framework_pt *framework, properties_pt
config) {
             status = CELIX_DO_IF(status, bundle_getBundleId((*framework)->bundle, &(*framework)->bundleId));
             status = CELIX_DO_IF(status, bundle_setFramework((*framework)->bundle, (*framework)));
             if (status == CELIX_SUCCESS) {
-                //
-                //TODO set group (*framework)->bundle
+                //nop
             } else {
                 status = CELIX_FRAMEWORK_EXCEPTION;
                 fw_logCode((*framework)->logger, OSGI_FRAMEWORK_LOG_ERROR, status, "Could
not create framework");
@@ -656,7 +653,6 @@ static void framework_autoStartConfiguredBundles(bundle_context_t *fwCtx)
{
     for (int i = 0; i < len; ++i) {
         bundleContext_getProperty(fwCtx, celixKeys[i], &autoStart);
         if (autoStart == NULL) {
-            //trying cosgi.auto.start.<level> variants. TODO make this prop deprecated
-> warning
             bundleContext_getProperty(fwCtx, cosgiKeys[i], &autoStart);
         }
         if (autoStart != NULL) {
@@ -780,7 +776,7 @@ celix_status_t fw_installBundle2(framework_pt framework, bundle_pt * bundle,
lon
         return CELIX_FILE_IO_EXCEPTION;
     }
 
-    //increase use count of framework bundle to prevent a stop. TODO is concurrent installing
of bundles supported? -> else need another lock
+    //increase use count of framework bundle to prevent a stop.
     fw_bundleEntry_increaseUseCount(framework, framework->bundleId);
 
   	status = CELIX_DO_IF(status, bundle_getState(framework->bundle, &state));
@@ -1158,7 +1154,6 @@ celix_status_t fw_stopBundle(framework_pt framework, bundle_pt bundle,
bool reco
 
                     serviceRegistry_clearReferencesFor(framework->registry, bundle);
                 }
-                // #TODO remove listeners for bundle
 
                 if (context != NULL) {
                     status = CELIX_DO_IF(status, bundleContext_destroy(context));
@@ -1230,7 +1225,6 @@ celix_status_t fw_uninstallBundle(framework_pt framework, bundle_pt
bundle) {
 
     status = CELIX_DO_IF(status, fw_stopBundle(framework, bundle, true));
 
-    // TODO Unload all libraries for transition to unresolved
     bundle_archive_t *archive = NULL;
     bundle_revision_t *revision = NULL;
 	array_list_pt handles = NULL;
@@ -1623,50 +1617,13 @@ 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) {
-	array_list_pt listenerHooks = NULL;
-	unsigned int i;
-
     celix_fw_service_listener_entry_t *fwListener = listener_create(bundle, sfilter, listener);
 
-	bundle_context_t *context = NULL;
-
     celixThreadMutex_lock(&framework->serviceListenersLock);
 	arrayList_add(framework->serviceListeners, fwListener);
     celixThreadMutex_unlock(&framework->serviceListenersLock);
 
-    //TODO lock listeners hooks?
-	celix_status_t  status = serviceRegistry_getListenerHooks(framework->registry, framework->bundle,
&listenerHooks);
-
-	if (status == CELIX_SUCCESS) {
-        struct listener_hook_info info;
-
-        bundle_getContext(bundle, &context);
-        info.context = context;
-        info.removed = false;
-        info.filter = sfilter;
-
-        for (i = 0; i < arrayList_size(listenerHooks); i++) {
-            service_reference_pt ref = (service_reference_pt) arrayList_get(listenerHooks,
i);
-            listener_hook_service_pt hook = NULL;
-            array_list_pt infos = NULL;
-            bool ungetResult = false;
-
-            status = fw_getService(framework, framework->bundle, ref, (const void **)
&hook);
-
-            if (status == CELIX_SUCCESS && hook != NULL) {
-                arrayList_create(&infos);
-                arrayList_add(infos, &info);
-                hook->added(hook->handle, infos);
-                serviceRegistry_ungetService(framework->registry, framework->bundle,
ref, &ungetResult);
-                serviceRegistry_ungetServiceReference(framework->registry, framework->bundle,
ref);
-                arrayList_destroy(infos);
-            } else {
-                fw_logCode(framework->logger, OSGI_FRAMEWORK_LOG_ERROR, status, "Could
not retrieve hook service.");
-            }
-        }
-
-        arrayList_destroy(listenerHooks);
-    }
+    serviceRegistry_callHooksForListenerFilter(framework->registry, bundle, sfilter, false);
 }
 
 void fw_removeServiceListener(framework_pt framework, bundle_pt bundle, celix_service_listener_t
*listener) {
@@ -1690,35 +1647,9 @@ void fw_removeServiceListener(framework_pt framework, bundle_pt bundle,
celix_se
 
     if (match != NULL) {
         //invoke listener hooks
-
-        bundle_context_pt lContext = NULL;
-
-        struct listener_hook_info info;
-        bundle_getContext(match->bundle, &lContext);
-        info.context = lContext;
-        filter_getString(match->filter, &info.filter);
-        info.removed = true;
-
-        array_list_pt listenerHooks = NULL;
-        serviceRegistry_getListenerHooks(framework->registry, framework->bundle, &listenerHooks);
-        for (i = 0; i < arrayList_size(listenerHooks); i++) {
-            service_reference_pt ref = (service_reference_pt) arrayList_get(listenerHooks,
i);
-            listener_hook_service_pt hook = NULL;
-            array_list_pt infos = NULL;
-            bool ungetResult;
-
-            fw_getService(framework, framework->bundle, ref, (const void **) &hook);
-
-            if (hook != NULL) {
-                arrayList_create(&infos);
-                arrayList_add(infos, &info);
-                hook->removed(hook->handle, infos);
-                serviceRegistry_ungetService(framework->registry, framework->bundle,
ref, &ungetResult);
-                serviceRegistry_ungetServiceReference(framework->registry, framework->bundle,
ref);
-                arrayList_destroy(infos);
-            }
-        }
-        arrayList_destroy(listenerHooks);
+        const char *filter;
+        filter_getString(match->filter, &filter);
+        serviceRegistry_callHooksForListenerFilter(framework->registry, bundle, filter,
true);
     }
 
     if (match != NULL) {
diff --git a/libs/framework/src/service_registry.c b/libs/framework/src/service_registry.c
index 5d2d6c5..1d8bd2b 100644
--- a/libs/framework/src/service_registry.c
+++ b/libs/framework/src/service_registry.c
@@ -760,42 +760,63 @@ static celix_status_t serviceRegistry_removeHook(service_registry_pt
registry, s
 	return status;
 }
 
-celix_status_t serviceRegistry_getListenerHooks(service_registry_pt registry, bundle_pt owner,
array_list_pt *out) {
-	celix_status_t status;
-    array_list_pt result;
+void serviceRegistry_callHooksForListenerFilter(service_registry_pt registry, celix_bundle_t
*owner, const char *filter, bool removed) {
+    celix_bundle_context_t *ctx;
+    bundle_getContext(owner, &ctx);
 
-    status = arrayList_create(&result);
-    if (status == CELIX_SUCCESS) {
-        unsigned int i;
-        unsigned size = arrayList_size(registry->listenerHooks);
+    struct listener_hook_info info;
+    info.context = ctx;
+    info.removed = removed;
+    info.filter = filter;
+    celix_array_list_t *infos = celix_arrayList_create();
+    celix_arrayList_add(infos, &info);
 
-        for (i = 0; i < size; i += 1) {
-            celixThreadRwlock_readLock(&registry->lock);
-            service_registration_pt registration = arrayList_get(registry->listenerHooks,
i);
-            if (registration != NULL) {
-                serviceRegistration_retain(registration);
-            }
-            celixThreadRwlock_unlock(&registry->lock);
+    celix_array_list_t *hookRegistrations = celix_arrayList_create();
 
-            if (registration != NULL) {
-                service_reference_pt reference = NULL;
-                serviceRegistry_getServiceReference(registry, owner, registration, &reference);
-                arrayList_add(result, reference);
-                serviceRegistration_release(registration);
-            }
+    celixThreadRwlock_readLock(&registry->lock);
+    unsigned size = arrayList_size(registry->listenerHooks);
+    for (int i = 0; i < size; ++i) {
+        service_registration_pt registration = arrayList_get(registry->listenerHooks,
i);
+        if (registration != NULL) {
+            serviceRegistration_retain(registration);
+            celix_arrayList_add(hookRegistrations, registration);
         }
     }
+    celixThreadRwlock_unlock(&registry->lock);
 
-    if (status == CELIX_SUCCESS) {
-        *out = result;
-    } else {
-        if (result != NULL) {
-            arrayList_destroy(result);
+    for (int i = 0; i < arrayList_size(hookRegistrations); ++i) {
+        service_registration_pt reg = celix_arrayList_get(hookRegistrations, i);
+        service_reference_pt ref;
+        serviceRegistry_getServiceReference(registry, owner, reg, &ref);
+
+        listener_hook_service_pt hook = NULL;
+        celix_status_t status = serviceRegistry_getService(registry, owner, ref, (const void
**) &hook);
+
+        if (status == CELIX_SUCCESS && hook != NULL) {
+            if (removed) {
+                hook->removed(hook->handle, infos);
+            } else {
+                hook->added(hook->handle, infos);
+            }
+
+            bool ungetResult = false;
+            serviceRegistry_ungetService(registry, owner, ref, &ungetResult);
+            serviceRegistry_ungetServiceReference(registry, owner, ref);
+        } else {
+            fw_logCode(logger, OSGI_FRAMEWORK_LOG_ERROR, status, "Could not retrieve hook
service.");
         }
-        framework_logIfError(logger, status, NULL, "Cannot get listener hooks");
+
+        serviceRegistration_release(reg);
     }
+    celix_arrayList_destroy(hookRegistrations);
+    celix_arrayList_destroy(infos);
+}
 
-	return status;
+size_t serviceRegistry_nrOfHooks(service_registry_pt registry) {
+    celixThreadRwlock_readLock(&registry->lock);
+    unsigned size = arrayList_size(registry->listenerHooks);
+    celixThreadRwlock_unlock(&registry->lock);
+    return (size_t) size;
 }
 
 celix_status_t serviceRegistry_servicePropertiesModified(service_registry_pt registry, service_registration_pt
registration, properties_pt oldprops) {


Mime
View raw message