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 2018/10/12 09:03:47 UTC
[30/34] celix git commit: CELIX-454: Fixes a framework shutdown issue
CELIX-454: Fixes a framework shutdown issue
Project: http://git-wip-us.apache.org/repos/asf/celix/repo
Commit: http://git-wip-us.apache.org/repos/asf/celix/commit/f94fb54e
Tree: http://git-wip-us.apache.org/repos/asf/celix/tree/f94fb54e
Diff: http://git-wip-us.apache.org/repos/asf/celix/diff/f94fb54e
Branch: refs/heads/develop
Commit: f94fb54edd30b2a0042a44b6653b5c1f9e84da4d
Parents: 4d33bcd
Author: Pepijn Noltes <pe...@gmail.com>
Authored: Thu Oct 11 17:34:46 2018 +0200
Committer: Pepijn Noltes <pe...@gmail.com>
Committed: Thu Oct 11 17:34:46 2018 +0200
----------------------------------------------------------------------
.../pubsub_admin_zmq/src/pubsub_zmq_admin.c | 9 +++-
.../src/pubsub_discovery_impl.c | 6 +--
.../private/test/bundle_cache_test.cpp | 10 +----
libs/framework/src/framework.c | 44 ++++++++++++++------
libs/framework/src/service_registry.c | 21 ++++++++++
5 files changed, 64 insertions(+), 26 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/celix/blob/f94fb54e/bundles/pubsub/pubsub_admin_zmq/src/pubsub_zmq_admin.c
----------------------------------------------------------------------
diff --git a/bundles/pubsub/pubsub_admin_zmq/src/pubsub_zmq_admin.c b/bundles/pubsub/pubsub_admin_zmq/src/pubsub_zmq_admin.c
index 90788e1..28eca77 100644
--- a/bundles/pubsub/pubsub_admin_zmq/src/pubsub_zmq_admin.c
+++ b/bundles/pubsub/pubsub_admin_zmq/src/pubsub_zmq_admin.c
@@ -466,7 +466,10 @@ celix_status_t pubsub_zmqAdmin_setupTopicReceiver(void *handle, const char *scop
hash_map_iterator_t iter = hashMapIterator_construct(psa->discoveredEndpoints.map);
while (hashMapIterator_hasNext(&iter)) {
celix_properties_t *endpoint = hashMapIterator_nextValue(&iter);
- pubsub_zmqAdmin_connectEndpointToReceiver(psa, receiver, endpoint);
+ const char *type = celix_properties_get(endpoint, PUBSUB_ENDPOINT_TYPE, NULL);
+ if (type != NULL && strncmp(PUBSUB_PUBLISHER_ENDPOINT_TYPE, type, strlen(PUBSUB_PUBLISHER_ENDPOINT_TYPE)) == 0) {
+ pubsub_zmqAdmin_connectEndpointToReceiver(psa, receiver, endpoint);
+ }
}
celixThreadMutex_unlock(&psa->discoveredEndpoints.mutex);
}
@@ -512,7 +515,9 @@ static celix_status_t pubsub_zmqAdmin_connectEndpointToReceiver(pubsub_zmq_admin
const char *url = celix_properties_get(endpoint, PUBSUB_ZMQ_URL_KEY, NULL);
if (url == NULL) {
- L_WARN("[PSA ZMQ] Error got endpoint without a zmq url");
+ const char *admin = celix_properties_get(endpoint, PUBSUB_ENDPOINT_ADMIN_TYPE);
+ const char *type = celix_properties_get(endpoint, PUBSUB_ENDPOINT_TYPE);
+ L_WARN("[PSA ZMQ] Error got endpoint without a zmq url (admin: %s, type: %s)", admin , type);
status = CELIX_BUNDLE_EXCEPTION;
} else {
if (eScope != NULL && eTopic != NULL &&
http://git-wip-us.apache.org/repos/asf/celix/blob/f94fb54e/bundles/pubsub/pubsub_discovery/src/pubsub_discovery_impl.c
----------------------------------------------------------------------
diff --git a/bundles/pubsub/pubsub_discovery/src/pubsub_discovery_impl.c b/bundles/pubsub/pubsub_discovery/src/pubsub_discovery_impl.c
index dfe8fa8..0162213 100644
--- a/bundles/pubsub/pubsub_discovery/src/pubsub_discovery_impl.c
+++ b/bundles/pubsub/pubsub_discovery/src/pubsub_discovery_impl.c
@@ -154,11 +154,11 @@ static void psd_watchForChange(pubsub_discovery_t *disc, bool *connectedPtr, lon
char *readKey = NULL;
//TODO add interruptable etcd_wait -> which returns a handle to interrupt and a can be used for a wait call
int rc = etcd_watch(disc->pubsubPath, watchIndex, &action, NULL, &value, &readKey, mIndex);
- if (rc == ETCDLIB_RC_TIMEOUT) {
- //nop
- } else if (rc == ETCDLIB_RC_ERROR || action == NULL) {
+ if (rc == ETCDLIB_RC_ERROR) {
printf("[PSD] Error communicating with etcd. rc is %i, action value is %s\n", rc, action);
*connectedPtr = false;
+ } else if (rc == ETCDLIB_RC_TIMEOUT || action == NULL) {
+ //nop
} else {
if (strncmp(ETCDLIB_ACTION_CREATE, action, strlen(ETCDLIB_ACTION_CREATE)) == 0 ||
strncmp(ETCDLIB_ACTION_SET, action, strlen(ETCDLIB_ACTION_SET)) == 0 ||
http://git-wip-us.apache.org/repos/asf/celix/blob/f94fb54e/libs/framework/private/test/bundle_cache_test.cpp
----------------------------------------------------------------------
diff --git a/libs/framework/private/test/bundle_cache_test.cpp b/libs/framework/private/test/bundle_cache_test.cpp
index 8f6926c..2c1e1c2 100644
--- a/libs/framework/private/test/bundle_cache_test.cpp
+++ b/libs/framework/private/test/bundle_cache_test.cpp
@@ -28,6 +28,7 @@
#include <sys/stat.h>
#include <string.h>
#include <unistd.h>
+#include <celix_properties.h>
#include "CppUTest/TestHarness.h"
#include "CppUTest/TestHarness_c.h"
@@ -56,14 +57,7 @@ TEST_GROUP(bundle_cache) {
};
TEST(bundle_cache, create) {
- properties_pt configuration = (properties_pt) 0x10;
-
- mock().expectOneCall("properties_get")
- .withParameter("properties", configuration)
- .withParameter("key", "org.osgi.framework.storage")
- .andReturnValue((char *) NULL);
-
- mock().ignoreOtherCalls();
+ properties_pt configuration = celix_properties_create();
bundle_cache_pt cache = NULL;
LONGS_EQUAL(CELIX_SUCCESS, bundleCache_create("test-uuid", configuration, &cache));
http://git-wip-us.apache.org/repos/asf/celix/blob/f94fb54e/libs/framework/src/framework.c
----------------------------------------------------------------------
diff --git a/libs/framework/src/framework.c b/libs/framework/src/framework.c
index 9ce3fdb..c717c17 100644
--- a/libs/framework/src/framework.c
+++ b/libs/framework/src/framework.c
@@ -81,6 +81,15 @@ static inline celix_framework_bundle_entry_t* fw_bundleEntry_create(celix_bundle
return entry;
}
+
+static inline void fw_bundleEntry_waitTillNotUsed(celix_framework_bundle_entry_t *entry) {
+ celixThreadMutex_lock(&entry->useMutex);
+ while (entry->useCount != 0) {
+ celixThreadCondition_wait(&entry->useCond, &entry->useMutex);
+ }
+ celixThreadMutex_unlock(&entry->useMutex);
+}
+
static inline void fw_bundleEntry_destroy(celix_framework_bundle_entry_t *entry, bool wait) {
celixThreadMutex_lock(&entry->useMutex);
while (wait && entry->useCount != 0) {
@@ -1065,7 +1074,6 @@ celix_status_t fw_stopBundle(framework_pt framework, bundle_pt bundle, bool reco
celix_bundle_activator_t *activator = NULL;
bundle_context_t *context = NULL;
bool wasActive = false;
- long id = 0;
char *error = NULL;
long bndId = celix_bundle_getId(bundle);
@@ -1107,9 +1115,8 @@ celix_status_t fw_stopBundle(framework_pt framework, bundle_pt bundle, bool reco
status = CELIX_DO_IF(status, framework_setBundleStateAndNotify(framework, bundle, OSGI_FRAMEWORK_BUNDLE_STOPPING));
status = CELIX_DO_IF(status, fw_fireBundleEvent(framework, OSGI_FRAMEWORK_BUNDLE_EVENT_STOPPING, bundle));
- status = CELIX_DO_IF(status, bundle_getBundleId(bundle, &id));
if (status == CELIX_SUCCESS) {
- if (wasActive || (id == 0)) {
+ if (wasActive || (bndId == 0)) {
activator = bundle_getActivator(bundle);
status = CELIX_DO_IF(status, bundle_getContext(bundle, &context));
@@ -1128,7 +1135,7 @@ celix_status_t fw_stopBundle(framework_pt framework, bundle_pt bundle, bool reco
}
}
- if (id != 0) {
+ if (bndId > 0) {
celix_serviceTracker_syncForContext(bundle->context);
status = CELIX_DO_IF(status, serviceRegistry_clearServiceRegistrations(framework->registry, bundle));
if (status == CELIX_SUCCESS) {
@@ -2110,33 +2117,46 @@ static void* framework_shutdown(void *framework) {
fw_log(fw->logger, OSGI_FRAMEWORK_LOG_INFO, "FRAMEWORK: Shutdown");
//celix_framework_bundle_entry_t *fwEntry = NULL;
- celix_array_list_t *removeEntries = celix_arrayList_create();
+ celix_array_list_t *stopEntries = celix_arrayList_create();
+ celix_framework_bundle_entry_t *fwEntry = NULL;
celixThreadMutex_lock(&fw->installedBundles.mutex);
int size = celix_arrayList_size(fw->installedBundles.entries);
for (int i = 0; i < size; ++i) {
celix_framework_bundle_entry_t *entry = celix_arrayList_get(fw->installedBundles.entries, i);
if (entry->bndId != 0) { //i.e. not framework bundle
- celix_arrayList_add(fw->installedBundles.entries, entry);
+ celix_arrayList_add(stopEntries, entry);
+ } else {
+ fwEntry = entry;
}
-
}
- celix_arrayList_clear(fw->installedBundles.entries);
+// celix_arrayList_clear(fw->installedBundles.entries);
celixThreadMutex_unlock(&fw->installedBundles.mutex);
- size = celix_arrayList_size(removeEntries);
+ size = celix_arrayList_size(stopEntries);
for (int i = size-1; i >= 0; --i) { //note loop in reverse order -> stop later installed bundle first
- celix_framework_bundle_entry_t *entry = celix_arrayList_get(removeEntries, i);
+ celix_framework_bundle_entry_t *entry = celix_arrayList_get(stopEntries, i);
//wait until entry use counts is 0
bundle_t *bnd = entry->bnd;
- fw_bundleEntry_destroy(entry, true);
+ fw_bundleEntry_waitTillNotUsed(entry);
bundle_state_e state;
bundle_getState(bnd, &state);
if (state == OSGI_FRAMEWORK_BUNDLE_ACTIVE || state == OSGI_FRAMEWORK_BUNDLE_STARTING) {
fw_stopBundle(fw, bnd, 0);
}
+ bundle_close(bnd);
+ }
+ celix_arrayList_destroy(stopEntries);
+
+
+ // 'stop' framework bundle
+ if (fwEntry != NULL) {
+ bundle_t *bnd = fwEntry->bnd;
+ fw_bundleEntry_waitTillNotUsed(fwEntry);
+
+ bundle_state_e state;
bundle_getState(bnd, &state);
if (state == OSGI_FRAMEWORK_BUNDLE_ACTIVE || state == OSGI_FRAMEWORK_BUNDLE_STARTING) {
fw_stopBundle(fw, bnd, 0);
@@ -2144,8 +2164,6 @@ static void* framework_shutdown(void *framework) {
bundle_close(bnd);
}
- //ignore fwEntry ?? TODO check
-
celixThreadMutex_lock(&fw->shutdown.mutex);
fw->shutdown.done = true;
celixThreadCondition_broadcast(&fw->shutdown.cond);
http://git-wip-us.apache.org/repos/asf/celix/blob/f94fb54e/libs/framework/src/service_registry.c
----------------------------------------------------------------------
diff --git a/libs/framework/src/service_registry.c b/libs/framework/src/service_registry.c
index 026c252..2a5ce82 100644
--- a/libs/framework/src/service_registry.c
+++ b/libs/framework/src/service_registry.c
@@ -96,6 +96,27 @@ celix_status_t serviceRegistry_destroy(service_registry_pt registry) {
//destroy service registration map
int size = hashMap_size(registry->serviceRegistrations);
+ if (size > 0) {
+ fw_log(logger, OSGI_FRAMEWORK_LOG_ERROR, "%i bundles with dangling service registration\n");
+ hash_map_iterator_t iter = hashMapIterator_construct(registry->serviceRegistrations);
+ while (hashMapIterator_nextValue(&iter)) {
+ hash_map_entry_t *entry = hashMapIterator_nextEntry(&iter);
+ bundle_t *bnd = hashMapEntry_getKey(entry);
+ celix_array_list_t *registrations = hashMapEntry_getValue(entry);
+ module_pt mod = NULL;
+ const char *name = NULL;
+ bundle_getCurrentModule(bnd, &mod);
+ if (mod != NULL) {
+ module_getSymbolicName(mod, &name);
+ }
+ for (int i = 0; i < celix_arrayList_size(registrations); ++i) {
+ service_registration_pt reg = celix_arrayList_get(registrations, i);
+ const char *svcName = NULL;
+ serviceRegistration_getServiceName(reg, &svcName);
+ fw_log(logger, OSGI_FRAMEWORK_LOG_ERROR, "Bundle %s still has a %s service registered\n", name, svcName);
+ }
+ }
+ }
assert(size == 0);
hashMap_destroy(registry->serviceRegistrations, false, false);