You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@celix.apache.org by pn...@apache.org on 2019/09/12 18:30:48 UTC
[celix] 01/01: Removes some TODO and moves the hanling of listener
hooks to service registry
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 <pe...@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(®istry->lock);
- service_registration_pt registration = arrayList_get(registry->listenerHooks, i);
- if (registration != NULL) {
- serviceRegistration_retain(registration);
- }
- celixThreadRwlock_unlock(®istry->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(®istry->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(®istry->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(®istry->lock);
+ unsigned size = arrayList_size(registry->listenerHooks);
+ celixThreadRwlock_unlock(®istry->lock);
+ return (size_t) size;
}
celix_status_t serviceRegistry_servicePropertiesModified(service_registry_pt registry, service_registration_pt registration, properties_pt oldprops) {