celix-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From pnol...@apache.org
Subject [celix] branch feature/handle_hooks_in_registry updated: CELIX-466: Adds a thread safe use count check on service listener hooks.
Date Mon, 16 Sep 2019 09:53:27 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


The following commit(s) were added to refs/heads/feature/handle_hooks_in_registry by this
push:
     new e12817a  CELIX-466: Adds a thread safe use count check on service listener hooks.
e12817a is described below

commit e12817a4f8d9c44f0dfe0ab69744bf505985b1ef
Author: Pepijn Noltes <pepijnnoltes@gmail.com>
AuthorDate: Mon Sep 16 11:52:32 2019 +0200

    CELIX-466: Adds a thread safe use count check on service listener hooks.
---
 libs/framework/include/listener_hook_service.h     |  31 +++---
 .../private/test/service_registry_test.cpp         |  15 ++-
 libs/framework/src/bundle_context_private.h        |   7 --
 libs/framework/src/service_registry.c              | 121 +++++++++++++++------
 libs/framework/src/service_registry_private.h      |  12 +-
 5 files changed, 128 insertions(+), 58 deletions(-)

diff --git a/libs/framework/include/listener_hook_service.h b/libs/framework/include/listener_hook_service.h
index ce1befe..303e15a 100644
--- a/libs/framework/include/listener_hook_service.h
+++ b/libs/framework/include/listener_hook_service.h
@@ -16,21 +16,10 @@
  *specific language governing permissions and limitations
  *under the License.
  */
-/*
- * listener_hook_service.h
- *
- *  \date       Oct 28, 2011
- *  \author    	<a href="mailto:dev@celix.apache.org">Apache Celix Project Team</a>
- *  \copyright	Apache License, Version 2.0
- */
 
 #ifndef LISTENER_HOOK_SERVICE_H_
 #define LISTENER_HOOK_SERVICE_H_
 
-
-typedef struct listener_hook_info *listener_hook_info_pt;
-typedef struct listener_hook_service *listener_hook_service_pt;
-
 #include "bundle_context.h"
 
 #define OSGI_FRAMEWORK_LISTENER_HOOK_SERVICE_NAME "listener_hook_service"
@@ -38,6 +27,12 @@ typedef struct listener_hook_service *listener_hook_service_pt;
 extern "C" {
 #endif
 
+
+typedef struct listener_hook_info *listener_hook_info_pt;
+typedef struct listener_hook_info celix_listener_hook_info_t;
+typedef struct listener_hook_service *listener_hook_service_pt;
+typedef struct listener_hook_service celix_listener_hook_service_t;
+
 struct listener_hook_info {
 	celix_bundle_context_t *context;
 	const char *filter;
@@ -47,9 +42,19 @@ struct listener_hook_info {
 struct listener_hook_service {
 	void *handle;
 
-	celix_status_t (*added)(void *hook, celix_array_list_t *listeners);
+	/**
+	 * Called when a new service listener / service tracker is added.
+	 * @param handle The service handle.
+	 * @param listeners A list of celix_listener_hook_info_t* entries.
+	 */
+	celix_status_t (*added)(void *handle, celix_array_list_t *listeners);
 
-	celix_status_t (*removed)(void *hook, celix_array_list_t *listeners);
+    /**
+     * Called when a service listener / service tracker is removed.
+     * @param handle The service handle.
+     * @param listeners A list of celix_listener_hook_info_t* entries.
+     */
+	celix_status_t (*removed)(void *handle, celix_array_list_t *listeners);
 };
 
 #ifdef __cplusplus
diff --git a/libs/framework/private/test/service_registry_test.cpp b/libs/framework/private/test/service_registry_test.cpp
index cccbd0a..8830974 100644
--- a/libs/framework/private/test/service_registry_test.cpp
+++ b/libs/framework/private/test/service_registry_test.cpp
@@ -264,6 +264,7 @@ TEST(service_registry, registerServiceListenerHook) {
 	char * serviceName = my_strdup(OSGI_FRAMEWORK_LISTENER_HOOK_SERVICE_NAME);
 	void *service = (void *) 0x20;
 	service_registration_pt reg = (service_registration_pt) 0x30;
+	long svcId = 11;
 
 	mock()
 		.expectOneCall("serviceRegistration_create")
@@ -281,11 +282,17 @@ TEST(service_registry, registerServiceListenerHook) {
 			.withParameter("registration", reg)
 			.withParameter("oldprops", (void*)NULL);
 
+    mock().expectOneCall("serviceRegistration_getServiceId")
+            .withParameter("registration", reg)
+            .andReturnValue(svcId);
+
 	service_registration_pt registration = NULL;
 	serviceRegistry_registerService(registry, bundle, serviceName, service, NULL, &registration);
 	POINTERS_EQUAL(reg, registration);
 	LONGS_EQUAL(1, arrayList_size(registry->listenerHooks));
-	POINTERS_EQUAL(reg, arrayList_get(registry->listenerHooks, 0));
+
+	auto* entry = (celix_service_registry_listener_hook_entry_t*)celix_arrayList_get(registry->listenerHooks,
0);
+	POINTERS_EQUAL(svcId, entry->svcId);
 
 	//cleanup
 	array_list_pt destroy_this = (array_list_pt) hashMap_remove(registry->serviceRegistrations,
bundle);
@@ -313,6 +320,7 @@ TEST(service_registry, unregisterService) {
 	hashMap_put(references, (void*)registration->serviceId, reference);
 	hashMap_put(registry->serviceReferences, bundle, references);
 	properties_pt properties = (properties_pt) 0x40;
+	long svcId = 12;
 
 	mock()
 		.expectOneCall("serviceRegistration_getProperties")
@@ -325,12 +333,17 @@ TEST(service_registry, unregisterService) {
 		.withParameter("key", (char *)OSGI_FRAMEWORK_OBJECTCLASS)
 		.andReturnValue((char*)OSGI_FRAMEWORK_LISTENER_HOOK_SERVICE_NAME);
 
+    mock().expectOneCall("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);
diff --git a/libs/framework/src/bundle_context_private.h b/libs/framework/src/bundle_context_private.h
index 4f6baf8..309555f 100644
--- a/libs/framework/src/bundle_context_private.h
+++ b/libs/framework/src/bundle_context_private.h
@@ -16,13 +16,6 @@
  *specific language governing permissions and limitations
  *under the License.
  */
-/*
- * bundle_context_private.h
- *
- *  \date       Feb 12, 2013
- *  \author     <a href="mailto:dev@celix.apache.org">Apache Celix Project Team</a>
- *  \copyright  Apache License, Version 2.0
- */
 
 
 #ifndef BUNDLE_CONTEXT_PRIVATE_H_
diff --git a/libs/framework/src/service_registry.c b/libs/framework/src/service_registry.c
index 1d8bd2b..703b013 100644
--- a/libs/framework/src/service_registry.c
+++ b/libs/framework/src/service_registry.c
@@ -16,13 +16,7 @@
  *specific language governing permissions and limitations
  *under the License.
  */
-/*
- * service_registry.c
- *
- *  \date       Aug 6, 2010
- *  \author    	<a href="mailto:dev@celix.apache.org">Apache Celix Project Team</a>
- *  \copyright	Apache License, Version 2.0
- */
+
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
@@ -55,6 +49,11 @@ static celix_status_t serviceRegistry_setReferenceStatus(service_registry_pt
reg
 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 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) {
 	celix_status_t status;
 
@@ -127,9 +126,12 @@ celix_status_t serviceRegistry_destroy(service_registry_pt registry)
{
     hashMap_destroy(registry->serviceReferences, false, false);
 
     //destroy listener hooks
-    size = arrayList_size(registry->listenerHooks);
-    if (size == 0)
-    	arrayList_destroy(registry->listenerHooks);
+    size = celix_arrayList_size(registry->listenerHooks);
+    for (int i = 0; i < celix_arrayList_size(registry->listenerHooks); ++i) {
+        celix_service_registry_listener_hook_entry_t *entry = celix_arrayList_get(registry->listenerHooks,
i);
+        celix_waitAndDestroyHookEntry(entry);
+    }
+    celix_arrayList_destroy(registry->listenerHooks);
 
     hashMap_destroy(registry->deletedServiceReferences, false, false);
 
@@ -737,7 +739,9 @@ static celix_status_t serviceRegistry_addHooks(service_registry_pt registry,
con
 
 	if (strcmp(OSGI_FRAMEWORK_LISTENER_HOOK_SERVICE_NAME, serviceName) == 0) {
         celixThreadRwlock_writeLock(&registry->lock);
-		arrayList_add(registry->listenerHooks, registration);
+        long svcId = serviceRegistration_getServiceId(registration);
+        celix_service_registry_listener_hook_entry_t *entry = celix_createHookEntry(svcId,
(celix_listener_hook_service_t*)serviceObject);
+		arrayList_add(registry->listenerHooks, entry);
         celixThreadRwlock_unlock(&registry->lock);
 	}
 
@@ -748,15 +752,29 @@ static celix_status_t serviceRegistry_removeHook(service_registry_pt
registry, s
 	celix_status_t status = CELIX_SUCCESS;
 	const char* serviceName = NULL;
 
+	celix_service_registry_listener_hook_entry_t *removedEntry = NULL;
+
 	properties_pt props = NULL;
 	serviceRegistration_getProperties(registration, &props);
 	serviceName = properties_get(props, (char *) OSGI_FRAMEWORK_OBJECTCLASS);
+	long svcId = serviceRegistration_getServiceId(registration);
 	if (strcmp(OSGI_FRAMEWORK_LISTENER_HOOK_SERVICE_NAME, serviceName) == 0) {
         celixThreadRwlock_writeLock(&registry->lock);
-		arrayList_removeElement(registry->listenerHooks, registration);
+        for (int i = 0; i < celix_arrayList_size(registry->listenerHooks); ++i) {
+            celix_service_registry_listener_hook_entry_t *visit = celix_arrayList_get(registry->listenerHooks,
i);
+            if (visit->svcId == svcId) {
+                removedEntry = visit;
+                celix_arrayList_removeAt(registry->listenerHooks, i);
+                break;
+            }
+        }
         celixThreadRwlock_unlock(&registry->lock);
 	}
 
+	if (removedEntry != NULL) {
+        celix_waitAndDestroyHookEntry(removedEntry);
+	}
+
 	return status;
 }
 
@@ -776,37 +794,22 @@ void serviceRegistry_callHooksForListenerFilter(service_registry_pt
registry, ce
     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);
+        celix_service_registry_listener_hook_entry_t* entry = arrayList_get(registry->listenerHooks,
i);
+        if (entry != NULL) {
+            celix_increaseCountHook(entry); //increate use count to ensure the hook cannot
be removed, untill used
+            celix_arrayList_add(hookRegistrations, entry);
         }
     }
     celixThreadRwlock_unlock(&registry->lock);
 
     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);
+        celix_service_registry_listener_hook_entry_t* entry = celix_arrayList_get(hookRegistrations,
i);
+        if (removed) {
+            entry->hook->removed(entry->hook->handle, infos);
         } else {
-            fw_logCode(logger, OSGI_FRAMEWORK_LOG_ERROR, status, "Could not retrieve hook
service.");
+            entry->hook->added(entry->hook->handle, infos);
         }
-
-        serviceRegistration_release(reg);
+        celix_decreaseCountHook(entry); //done using hook. decrease count
     }
     celix_arrayList_destroy(hookRegistrations);
     celix_arrayList_destroy(infos);
@@ -868,3 +871,49 @@ celix_serviceRegistry_registerServiceFactory(
         service_registration_t **registration) {
     return serviceRegistry_registerServiceInternal(reg, (celix_bundle_t*)bnd, serviceName,
(const void *) factory, props, CELIX_FACTORY_SERVICE, registration);
 }
+
+static celix_service_registry_listener_hook_entry_t* celix_createHookEntry(long svcId, celix_listener_hook_service_t
*hook) {
+    celix_service_registry_listener_hook_entry_t* entry = calloc(1, sizeof(*entry));
+    entry->svcId = svcId;
+    entry->hook = hook;
+    celixThreadMutex_create(&entry->mutex, NULL);
+    celixThreadCondition_init(&entry->cond, NULL);
+    return entry;
+}
+
+static void celix_waitAndDestroyHookEntry(celix_service_registry_listener_hook_entry_t *entry)
{
+    if (entry != NULL) {
+        celixThreadMutex_lock(&entry->mutex);
+        int waitCount = 0;
+        while (entry->count > 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);
+            }
+        }
+        celixThreadMutex_unlock(&entry->mutex);
+
+        //done waiting, use count is on 0
+        celixThreadCondition_destroy(&entry->cond);
+        celixThreadMutex_destroy(&entry->mutex);
+        free(entry);
+    }
+}
+static void celix_increaseCountHook(celix_service_registry_listener_hook_entry_t *entry)
{
+    if (entry != NULL) {
+        celixThreadMutex_lock(&entry->mutex);
+        entry->count += 1;
+        celixThreadCondition_broadcast(&entry->cond);
+        celixThreadMutex_unlock(&entry->mutex);
+    }
+}
+static void celix_decreaseCountHook(celix_service_registry_listener_hook_entry_t *entry)
{
+    if (entry != NULL) {
+        celixThreadMutex_lock(&entry->mutex);
+        entry->count -= 1;
+        celixThreadCondition_broadcast(&entry->cond);
+        celixThreadMutex_unlock(&entry->mutex);
+    }
+}
\ 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 1bd0066..52bb1ff 100644
--- a/libs/framework/src/service_registry_private.h
+++ b/libs/framework/src/service_registry_private.h
@@ -30,6 +30,8 @@
 
 #include "registry_callback_private.h"
 #include "service_registry.h"
+#include "listener_hook_service.h"
+#include "service_reference.h"
 
 struct celix_serviceRegistry {
 	framework_pt framework;
@@ -44,11 +46,19 @@ struct celix_serviceRegistry {
 	serviceChanged_function_pt serviceChanged;
 	unsigned long currentServiceId;
 
-	array_list_pt listenerHooks;
+	array_list_pt listenerHooks; //celix_service_registry_listener_hook_entry_t*
 
 	celix_thread_rwlock_t lock;
 };
 
+typedef struct celix_service_registry_listener_hook_entry {
+    long svcId;
+    celix_listener_hook_service_t *hook;
+    celix_thread_mutex_t mutex; //protects below
+    celix_thread_cond_t cond;
+    unsigned int count;
+} celix_service_registry_listener_hook_entry_t;
+
 typedef enum reference_status_enum {
 	REF_ACTIVE,
 	REF_DELETED,


Mime
View raw message