celix-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [celix] pnoltes commented on a change in pull request #286: Feature/async svc registration
Date Sun, 18 Oct 2020 14:17:46 GMT

pnoltes commented on a change in pull request #286:
URL: https://github.com/apache/celix/pull/286#discussion_r507166662



##########
File path: libs/framework/src/service_tracker.c
##########
@@ -128,345 +111,285 @@ celix_status_t serviceTracker_create(bundle_context_pt context, const
char * ser
 	return status;
 }
 
-celix_status_t serviceTracker_createWithFilter(bundle_context_pt context, const char * filter,
service_tracker_customizer_pt customizer, service_tracker_pt *tracker) {
-	celix_status_t status = CELIX_SUCCESS;
-	*tracker = (service_tracker_pt) calloc(1, sizeof(**tracker));
-	if (!*tracker) {
-		status = CELIX_ENOMEM;
-	} else {
-		(*tracker)->context = context;
-		(*tracker)->filter = strdup(filter);
-        (*tracker)->customizer = customizer;
-	}
+celix_status_t serviceTracker_createWithFilter(bundle_context_pt context, const char * filter,
service_tracker_customizer_pt customizer, service_tracker_pt *out) {
+	service_tracker_t* tracker = calloc(1, sizeof(*tracker));
+	*out = tracker;
+    tracker->context = context;
+    tracker->filter = celix_utils_strdup(filter);
+    tracker->customizer = *customizer;
+    free(customizer);
 
-	framework_logIfError(celix_frameworkLogger_globalLogger(), status, NULL, "Cannot create
service tracker [filter=%s]", filter);
+    celixThreadMutex_create(&tracker->closeSync.mutex, NULL);
+    celixThreadCondition_init(&tracker->closeSync.cond, NULL);
 
-	return status;
+    celixThreadMutex_create(&tracker->mutex, NULL);
+    celixThreadCondition_init(&tracker->cond, NULL);
+    tracker->trackedServices = celix_arrayList_create();
+    tracker->untrackingServices = celix_arrayList_create();
+
+    celixThreadMutex_create(&tracker->mutex, NULL);
+    tracker->currentHighestServiceId = -1;
+
+    tracker->listener.handle = tracker;
+    tracker->listener.serviceChanged = (void *) serviceTracker_serviceChanged;
+
+    tracker->callbackHandle = tracker->callbackHandle;
+
+    tracker->add = tracker->add;
+    tracker->addWithProperties = tracker->addWithProperties;
+    tracker->addWithOwner = tracker->addWithOwner;
+    tracker->set = tracker->set;
+    tracker->setWithProperties = tracker->setWithProperties;
+    tracker->setWithOwner = tracker->setWithOwner;
+    tracker->remove = tracker->remove;
+    tracker->removeWithProperties = tracker->removeWithProperties;
+    tracker->removeWithOwner = tracker->removeWithOwner;
+
+	return CELIX_SUCCESS;
 }
 
 celix_status_t serviceTracker_destroy(service_tracker_pt tracker) {
-    if (tracker->customizer != NULL) {
-	    serviceTrackerCustomizer_destroy(tracker->customizer);
-	}
-
     free(tracker->serviceName);
 	free(tracker->filter);
-	free(tracker);
-
+    celixThreadMutex_destroy(&tracker->closeSync.mutex);
+    celixThreadCondition_destroy(&tracker->closeSync.cond);
+    celixThreadMutex_destroy(&tracker->mutex);
+    celixThreadCondition_destroy(&tracker->cond);
+    celix_arrayList_destroy(tracker->trackedServices);
+    celix_arrayList_destroy(tracker->untrackingServices);
+    free(tracker);
 	return CELIX_SUCCESS;
 }
 
 celix_status_t serviceTracker_open(service_tracker_pt tracker) {
-    celix_service_listener_t *listener = NULL;
-    celix_service_tracker_instance_t *instance = NULL;
-    celix_status_t status = CELIX_SUCCESS;
-
-    bool addListener = false;
-
-    celixThreadRwlock_writeLock(&tracker->instanceLock);
-    if (tracker->instance == NULL) {
-        instance = calloc(1, sizeof(*instance));
-        instance->context = tracker->context;
-
-        instance->closing = false;
-        instance->activeServiceChangeCalls = 0;
-        celixThreadMutex_create(&instance->closingLock, NULL);
-        celixThreadCondition_init(&instance->activeServiceChangeCallsCond, NULL);
-
-
-        celixThreadRwlock_create(&instance->lock, NULL);
-        instance->trackedServices = celix_arrayList_create();
-
-        celixThreadMutex_create(&instance->mutex, NULL);
-        instance->currentHighestServiceId = -1;
-
-        instance->listener.handle = instance;
-        instance->listener.serviceChanged = (void *) serviceTracker_serviceChanged;
-        listener = &instance->listener;
-
-        instance->callbackHandle = tracker->callbackHandle;
-        instance->filter = strdup(tracker->filter);
-        if (tracker->customizer != NULL) {
-            memcpy(&instance->customizer, tracker->customizer, sizeof(instance->customizer));
-        }
-        instance->add = tracker->add;
-        instance->addWithProperties = tracker->addWithProperties;
-        instance->addWithOwner = tracker->addWithOwner;
-        instance->set = tracker->set;
-        instance->setWithProperties = tracker->setWithProperties;
-        instance->setWithOwner = tracker->setWithOwner;
-        instance->remove = tracker->remove;
-        instance->removeWithProperties = tracker->removeWithProperties;
-        instance->removeWithOwner = tracker->removeWithOwner;
-
-        tracker->instance = instance;
-
-        addListener = true;
-    } else {
-        //already open
-        framework_logIfError(tracker->context->framework->logger, status, NULL,
"Tracker already open");
+    celixThreadMutex_lock(&tracker->mutex);
+    bool alreadyOpen = tracker->open;
+    tracker->open = true;
+    celixThreadMutex_unlock(&tracker->mutex);
 
+    if (!alreadyOpen) {
+        bundleContext_addServiceListener(tracker->context, &tracker->listener,
tracker->filter);
     }
-    celixThreadRwlock_unlock(&tracker->instanceLock);
-
-	if (addListener) {
-	    bundleContext_addServiceListener(tracker->context, listener, tracker->filter);
-	}
 	return CELIX_SUCCESS;
 }
 
-celix_status_t serviceTracker_close(service_tracker_pt tracker) {
+celix_status_t serviceTracker_close(service_tracker_t* tracker) {
 	//put all tracked entries in tmp array list, so that the untrack (etc) calls are not blocked.
     //set state to close to prevent service listener events
 
-    celixThreadRwlock_writeLock(&tracker->instanceLock);
-    celix_service_tracker_instance_t *instance = tracker->instance;
-    tracker->instance = NULL;
-    if (instance != NULL) {
-        celixThreadMutex_lock(&instance->closingLock);
-        //prevent service listener events
-        instance->closing = true;
-        celixThreadMutex_unlock(&instance->closingLock);
-    }
-    celixThreadRwlock_unlock(&tracker->instanceLock);
-
-    if (instance != NULL) {
-        celixThreadRwlock_writeLock(&instance->lock);
-        unsigned int size = celix_arrayList_size(instance->trackedServices);
-        if(size > 0) {
-            celix_tracked_entry_t *trackedEntries[size];
-            for (unsigned int i = 0u; i < size; i++) {
-                trackedEntries[i] = (celix_tracked_entry_t *) arrayList_get(instance->trackedServices,
i);
-            }
-            arrayList_clear(instance->trackedServices);
-            celixThreadRwlock_unlock(&instance->lock);
+    celixThreadMutex_lock(&tracker->mutex);
+    bool open = tracker->open;
+    tracker->open = false;
+    celixThreadMutex_unlock(&tracker->mutex);
 
-            //loop trough tracked entries an untrack
-            for (unsigned int i = 0u; i < size; i++) {
-                serviceTracker_untrackTracked(instance, trackedEntries[i]);
+    if (!open) {
+        return CELIX_SUCCESS;
+    }
+
+
+    //indicate that the service tracking is closing and wait for the still pending service
registration events.
+    celixThreadMutex_lock(&tracker->closeSync.mutex);
+    tracker->closeSync.closing = true;
+    while (tracker->closeSync.activeCalls > 0) {
+        celixThreadCondition_wait(&tracker->closeSync.cond, &tracker->closeSync.mutex);
+    }
+    celixThreadMutex_unlock(&tracker->closeSync.mutex);
+
+    int nrOfTrackedEntries;
+    do {
+        celixThreadMutex_lock(&tracker->mutex);
+        celix_tracked_entry_t* tracked = NULL;
+        nrOfTrackedEntries = celix_arrayList_size(tracker->trackedServices);
+        if (nrOfTrackedEntries > 0) {
+            tracked = celix_arrayList_get(tracker->trackedServices, 0);
+            celix_arrayList_removeAt(tracker->trackedServices, 0);
+            celix_arrayList_add(tracker->untrackingServices, tracked);
+        }
+        celixThreadMutex_unlock(&tracker->mutex);
+
+        if (tracked != NULL) {
+            int currentSize = nrOfTrackedEntries - 1;
+            if (currentSize == 0) {
+                serviceTracker_checkAndInvokeSetService(tracker, NULL, NULL, NULL);
+            } else {
+                celix_serviceTracker_useHighestRankingService(tracker, tracked->serviceName,
tracker, NULL, NULL, serviceTracker_checkAndInvokeSetService);
             }
-        } else {
-            celixThreadRwlock_unlock(&instance->lock);
-        }
 
-        celixThreadMutex_lock(&instance->closingLock);
-        while (instance->activeServiceChangeCalls > 0) {
-            celixThreadCondition_wait(&instance->activeServiceChangeCallsCond, &instance->closingLock);
+            serviceTracker_untrackTracked(tracker, tracked);
+            celixThreadMutex_lock(&tracker->mutex);
+            celix_arrayList_remove(tracker->untrackingServices, tracked);
+            celixThreadCondition_broadcast(&tracker->cond);
+            celixThreadMutex_unlock(&tracker->mutex);
         }
-        celixThreadMutex_unlock(&instance->closingLock);
 
 
-#ifdef CELIX_SERVICE_TRACKER_USE_SHUTDOWN_THREAD
-        //NOTE Separate thread is needed to prevent deadlock where closing is triggered from
a serviceChange event and the
-        // untrack -> removeServiceListener will try to remove a service listener which
is being invoked and is the
-        // 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.
-        serviceTracker_addInstanceFromShutdownList(instance);
-        celix_thread_t localThread;
-        celixThread_create(&localThread, NULL, shutdownServiceTrackerInstanceHandler,
instance);
-        celixThread_detach(localThread);
-#else
-        fw_removeServiceListener(instance->context->framework, instance->context->bundle,
&instance->listener);
-
-        celixThreadMutex_destroy(&instance->closingLock);
-        celixThreadCondition_destroy(&instance->activeServiceChangeCallsCond);
-        celixThreadMutex_destroy(&instance->mutex);
-        celixThreadRwlock_destroy(&instance->lock);
-        celix_arrayList_destroy(instance->trackedServices);
-        free(instance->filter);
-        free(instance);
-#endif
-    }
+        celixThreadMutex_lock(&tracker->mutex);
+        nrOfTrackedEntries = celix_arrayList_size(tracker->trackedServices);
+        celixThreadMutex_unlock(&tracker->mutex);
+    } while (nrOfTrackedEntries > 0);
+
+
+    fw_removeServiceListener(tracker->context->framework, tracker->context->bundle,
&tracker->listener);
 
 	return CELIX_SUCCESS;
 }
 
-service_reference_pt serviceTracker_getServiceReference(service_tracker_pt tracker) {
+service_reference_pt serviceTracker_getServiceReference(service_tracker_t* tracker) {
     //TODO deprecated warning -> not locked
-	celix_tracked_entry_t *tracked;
+
     service_reference_pt result = NULL;
-	unsigned int i;
-
-	celixThreadRwlock_readLock(&tracker->instanceLock);
-	celix_service_tracker_instance_t *instance = tracker->instance;
-	if (instance != NULL) {
-        celixThreadRwlock_readLock(&instance->lock);
-        for (i = 0; i < arrayList_size(instance->trackedServices); ++i) {
-            tracked = (celix_tracked_entry_t *) arrayList_get(instance->trackedServices,
i);
-            result = tracked->reference;
-            break;
-        }
-        celixThreadRwlock_unlock(&instance->lock);
+
+    celixThreadMutex_lock(&tracker->mutex);
+    for (int i = 0; i < celix_arrayList_size(tracker->trackedServices); ++i) {
+        celix_tracked_entry_t *tracked = celix_arrayList_get(tracker->trackedServices,
i);
+        result = tracked->reference;
+        break;
     }
-    celixThreadRwlock_unlock(&tracker->instanceLock);
+    celixThreadMutex_unlock(&tracker->mutex);
 
 	return result;
 }
 
-array_list_pt serviceTracker_getServiceReferences(service_tracker_pt tracker) {
+array_list_pt serviceTracker_getServiceReferences(service_tracker_t* tracker) {
     //TODO deprecated warning -> not locked

Review comment:
       yes most of the non celix_ prefix are deprecated and  ideally these function get a
deprecated attribute




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



Mime
View raw message