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:
[email protected]
