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:47 UTC
[celix] branch feature/handle_hooks_in_registry created (now
8751e29)
This is an automated email from the ASF dual-hosted git repository.
pnoltes pushed a change to branch feature/handle_hooks_in_registry
in repository https://gitbox.apache.org/repos/asf/celix.git.
at 8751e29 Removes some TODO and moves the hanling of listener hooks to service registry
This branch includes the following new commits:
new 8751e29 Removes some TODO and moves the hanling of listener hooks to service registry
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[celix] 01/01: Removes some TODO and moves the hanling of listener
hooks to service registry
Posted by pn...@apache.org.
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) {