Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 8891A200BA2 for ; Sun, 16 Oct 2016 14:47:04 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id 87603160AFB; Sun, 16 Oct 2016 12:47:04 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 7938F160AF8 for ; Sun, 16 Oct 2016 14:47:03 +0200 (CEST) Received: (qmail 4554 invoked by uid 500); 16 Oct 2016 12:47:02 -0000 Mailing-List: contact commits-help@celix.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@celix.apache.org Delivered-To: mailing list commits@celix.apache.org Received: (qmail 4464 invoked by uid 99); 16 Oct 2016 12:47:02 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Sun, 16 Oct 2016 12:47:02 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 710C8E69A3; Sun, 16 Oct 2016 12:47:01 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: pnoltes@apache.org To: commits@celix.apache.org Date: Sun, 16 Oct 2016 12:47:08 -0000 Message-Id: <081f7445a87f47069d105d0e75fab71a@git.apache.org> In-Reply-To: <8845fbcb792b49179f9a8a7323393d88@git.apache.org> References: <8845fbcb792b49179f9a8a7323393d88@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [8/8] celix git commit: CELIX-376: Fixes an error for wronly printing about dandling references, not sure when it was introduced archived-at: Sun, 16 Oct 2016 12:47:04 -0000 CELIX-376: Fixes an error for wronly printing about dandling references, not sure when it was introduced Project: http://git-wip-us.apache.org/repos/asf/celix/repo Commit: http://git-wip-us.apache.org/repos/asf/celix/commit/5e2d7907 Tree: http://git-wip-us.apache.org/repos/asf/celix/tree/5e2d7907 Diff: http://git-wip-us.apache.org/repos/asf/celix/diff/5e2d7907 Branch: refs/heads/develop Commit: 5e2d7907f22613ce3e033b3cb8e8d99075ab096e Parents: 505f6a8 Author: Pepijn Noltes Authored: Sun Oct 16 14:48:41 2016 +0200 Committer: Pepijn Noltes Committed: Sun Oct 16 14:48:41 2016 +0200 ---------------------------------------------------------------------- .../private/src/dm_service_dependency.c | 47 +++++++-------- framework/private/src/bundle.c | 3 +- framework/private/src/module.c | 2 +- framework/private/src/service_reference.c | 12 ++-- framework/private/src/service_registration.c | 5 +- framework/private/src/service_registry.c | 61 ++++++++++++-------- 6 files changed, 71 insertions(+), 59 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/celix/blob/5e2d7907/dependency_manager/private/src/dm_service_dependency.c ---------------------------------------------------------------------- diff --git a/dependency_manager/private/src/dm_service_dependency.c b/dependency_manager/private/src/dm_service_dependency.c index 3c5d2a0..233593c 100644 --- a/dependency_manager/private/src/dm_service_dependency.c +++ b/dependency_manager/private/src/dm_service_dependency.c @@ -396,7 +396,6 @@ celix_status_t serviceDependency_start(dm_service_dependency_pt dependency) { celix_status_t serviceDependency_stop(dm_service_dependency_pt dependency) { celix_status_t status = CELIX_SUCCESS; - celix_status_t tmp_status; if (!dependency) { status = CELIX_ILLEGAL_ARGUMENT; @@ -406,16 +405,11 @@ celix_status_t serviceDependency_stop(dm_service_dependency_pt dependency) { dependency->isStarted = false; } - if (status == CELIX_SUCCESS) { - if (dependency->tracker) { - tmp_status = serviceTracker_close(dependency->tracker); - if (tmp_status != CELIX_SUCCESS) { - status = tmp_status; - } - tmp_status = serviceTracker_destroy(dependency->tracker); - if (tmp_status != CELIX_SUCCESS && status == CELIX_SUCCESS) { - status = tmp_status; - } + if (status == CELIX_SUCCESS && dependency->tracker) { + status = serviceTracker_close(dependency->tracker); + if (status == CELIX_SUCCESS) { + serviceTracker_destroy(dependency->tracker); + dependency->tracker = NULL; } } @@ -485,29 +479,30 @@ celix_status_t serviceDependency_invokeSet(dm_service_dependency_pt dependency, curServRef = serviceReference; } } else { - arrayList_destroy(serviceReferences); - return status; + break; } } arrayList_destroy(serviceReferences); - if (curServRef) { - status = bundleContext_getService(event->context, curServRef, &service); - } else { - service = NULL; - } + if (status == CELIX_SUCCESS) { + if (curServRef) { + status = bundleContext_getService(event->context, curServRef, &service); + } else { + service = NULL; + } - if (dependency->set) { - dependency->set(serviceDependency_getCallbackHandle(dependency), service); - } - if (dependency->set_with_ref) { - dependency->set_with_ref(serviceDependency_getCallbackHandle(dependency), curServRef, service); - } + if (dependency->set) { + dependency->set(serviceDependency_getCallbackHandle(dependency), service); + } + if (dependency->set_with_ref) { + dependency->set_with_ref(serviceDependency_getCallbackHandle(dependency), curServRef, service); + } - if (curServRef) { - bundleContext_ungetService(event->context, curServRef, NULL); + if (curServRef) { + bundleContext_ungetService(event->context, curServRef, NULL); + } } return status; http://git-wip-us.apache.org/repos/asf/celix/blob/5e2d7907/framework/private/src/bundle.c ---------------------------------------------------------------------- diff --git a/framework/private/src/bundle.c b/framework/private/src/bundle.c index 9e957d6..a0e6b3d 100644 --- a/framework/private/src/bundle.c +++ b/framework/private/src/bundle.c @@ -142,13 +142,12 @@ celix_status_t bundle_getArchive(bundle_pt bundle, bundle_archive_pt *archive) { celix_status_t bundle_getCurrentModule(bundle_pt bundle, module_pt *module) { celix_status_t status = CELIX_SUCCESS; - if (bundle == NULL || *module != NULL || arrayList_size(bundle->modules)==0 ) { + if (bundle == NULL || arrayList_size(bundle->modules)==0 ) { status = CELIX_ILLEGAL_ARGUMENT; } else { *module = arrayList_get(bundle->modules, arrayList_size(bundle->modules) - 1); } - return status; } http://git-wip-us.apache.org/repos/asf/celix/blob/5e2d7907/framework/private/src/module.c ---------------------------------------------------------------------- diff --git a/framework/private/src/module.c b/framework/private/src/module.c index 6d64f9f..e81a1ee 100644 --- a/framework/private/src/module.c +++ b/framework/private/src/module.c @@ -177,7 +177,7 @@ version_pt module_getVersion(module_pt module) { celix_status_t module_getSymbolicName(module_pt module, const char **symbolicName) { celix_status_t status = CELIX_SUCCESS; - if (module == NULL || *symbolicName != NULL) { + if (module == NULL) { status = CELIX_ILLEGAL_ARGUMENT; } else { *symbolicName = module->symbolicName; http://git-wip-us.apache.org/repos/asf/celix/blob/5e2d7907/framework/private/src/service_reference.c ---------------------------------------------------------------------- diff --git a/framework/private/src/service_reference.c b/framework/private/src/service_reference.c index b89aaf2..545426d 100644 --- a/framework/private/src/service_reference.c +++ b/framework/private/src/service_reference.c @@ -193,10 +193,14 @@ celix_status_t serviceReference_getOwner(service_reference_pt ref, bundle_pt *ow } celix_status_t serviceReference_getServiceRegistration(service_reference_pt ref, service_registration_pt *out) { - celixThreadRwlock_readLock(&ref->lock); - *out = ref->registration; - celixThreadRwlock_unlock(&ref->lock); - return CELIX_SUCCESS; + if (ref != NULL) { + celixThreadRwlock_readLock(&ref->lock); + *out = ref->registration; + celixThreadRwlock_unlock(&ref->lock); + return CELIX_SUCCESS; + } else { + return CELIX_ILLEGAL_ARGUMENT; + } } celix_status_t serviceReference_getProperty(service_reference_pt ref, const char* key, const char** value) { http://git-wip-us.apache.org/repos/asf/celix/blob/5e2d7907/framework/private/src/service_registration.c ---------------------------------------------------------------------- diff --git a/framework/private/src/service_registration.c b/framework/private/src/service_registration.c index e2932f9..e916457 100644 --- a/framework/private/src/service_registration.c +++ b/framework/private/src/service_registration.c @@ -255,7 +255,10 @@ celix_status_t serviceRegistration_setProperties(service_registration_pt registr celix_status_t serviceRegistration_getBundle(service_registration_pt registration, bundle_pt *bundle) { - celix_status_t status = CELIX_SUCCESS; + celix_status_t status = CELIX_SUCCESS; + if (registration == NULL) { + return CELIX_ILLEGAL_ARGUMENT; + } if (registration != NULL && *bundle == NULL) { celixThreadRwlock_readLock(®istration->lock); http://git-wip-us.apache.org/repos/asf/celix/blob/5e2d7907/framework/private/src/service_registry.c ---------------------------------------------------------------------- diff --git a/framework/private/src/service_registry.c b/framework/private/src/service_registry.c index 139ee70..c0dafc7 100644 --- a/framework/private/src/service_registry.c +++ b/framework/private/src/service_registry.c @@ -44,7 +44,7 @@ static celix_status_t serviceRegistry_registerServiceInternal(service_registry_pt registry, bundle_pt bundle, const char* serviceName, const void * serviceObject, properties_pt dictionary, bool isFactory, service_registration_pt *registration); static celix_status_t serviceRegistry_addHooks(service_registry_pt registry, const char* serviceName, const void *serviceObject, service_registration_pt registration); static celix_status_t serviceRegistry_removeHook(service_registry_pt registry, service_registration_pt registration); -static void serviceRegistry_logWarningServiceReferenceUsageCount(service_registry_pt registry, size_t usageCount, size_t refCount); +static void serviceRegistry_logWarningServiceReferenceUsageCount(service_registry_pt registry, bundle_pt bundle, service_reference_pt ref, size_t usageCount, size_t refCount); static void serviceRegistry_logWarningServiceRegistration(service_registry_pt registry, service_registration_pt reg); static celix_status_t serviceRegistry_checkReference(service_registry_pt registry, service_reference_pt ref, reference_status_t *refStatus); @@ -448,30 +448,30 @@ celix_status_t serviceRegistry_ungetServiceReference(service_registry_pt registr if (destroyed) { if (count > 0) { - serviceRegistry_logWarningServiceReferenceUsageCount(registry, count, 0); + serviceRegistry_logWarningServiceReferenceUsageCount(registry, bundle, reference, count, 0); } hash_map_pt refsMap = hashMap_get(registry->serviceReferences, bundle); - unsigned long reg = 0UL; + unsigned long refId = 0UL; service_reference_pt ref = NULL; hash_map_iterator_pt iter = hashMapIterator_create(refsMap); while (hashMapIterator_hasNext(iter)) { hash_map_entry_pt entry = hashMapIterator_nextEntry(iter); - reg = (unsigned long)hashMapEntry_getKey(entry); //note could be invalid e.g. freed + refId = (unsigned long)hashMapEntry_getKey(entry); //note could be invalid e.g. freed ref = hashMapEntry_getValue(entry); if (ref == reference) { break; } else { ref = NULL; - reg = 0UL; + refId = 0UL; } } hashMapIterator_destroy(iter); if (ref != NULL) { - hashMap_remove(refsMap, (void*)reg); + hashMap_remove(refsMap, (void*)refId); int size = hashMap_size(refsMap); if (size == 0) { hashMap_destroy(refsMap, false, false); @@ -527,12 +527,35 @@ static celix_status_t serviceRegistry_checkReference(service_registry_pt registr return status; } -static void serviceRegistry_logWarningServiceReferenceUsageCount(service_registry_pt registry __attribute__((unused)), size_t usageCount, size_t refCount) { +static void serviceRegistry_logWarningServiceReferenceUsageCount(service_registry_pt registry __attribute__((unused)), bundle_pt bundle, service_reference_pt ref, size_t usageCount, size_t refCount) { if (usageCount > 0) { - fw_log(logger, OSGI_FRAMEWORK_LOG_WARNING, "Service Reference destroyed with usage count is %zu. Look for missing bundleContext_ungetService calls.", usageCount); + fw_log(logger, OSGI_FRAMEWORK_LOG_WARNING, "Service Reference destroyed with usage count is %zu, expected 0. Look for missing bundleContext_ungetService calls.", usageCount); } - if (refCount > 0) { - fw_log(logger, OSGI_FRAMEWORK_LOG_WARNING, "Dangling service reference. Reference count is %zu. Look for missing bundleContext_ungetServiceReference calls.", refCount); + if (refCount > 1) { + fw_log(logger, OSGI_FRAMEWORK_LOG_WARNING, "Dangling service reference. Reference count is %zu, expected 1. Look for missing bundleContext_ungetServiceReference calls.", refCount); + } + + if(usageCount > 0 || refCount > 1) { + module_pt module_ptr = NULL; + bundle_getCurrentModule(bundle, &module_ptr); + const char* bundle_name = NULL; + module_getSymbolicName(module_ptr, &bundle_name); + + const char* service_name = "unknown"; + const char* bundle_provider_name = "unknown"; + if (ref != NULL) { + serviceReference_getProperty(ref, OSGI_FRAMEWORK_OBJECTCLASS, &service_name); + + service_registration_pt reg = NULL; + bundle_pt bundle = NULL; + module_pt mod = NULL; + serviceReference_getServiceRegistration(ref, ®); + serviceRegistration_getBundle(reg, &bundle); + bundle_getCurrentModule(bundle, &mod); + module_getSymbolicName(mod, &bundle_provider_name); + } + + fw_log(logger, OSGI_FRAMEWORK_LOG_WARNING, "Previous Dangling service reference warnings caused by bundle '%s', for service '%s', provided by bundle '%s'", bundle_name, service_name, bundle_provider_name); } } @@ -540,7 +563,6 @@ static void serviceRegistry_logWarningServiceReferenceUsageCount(service_registr celix_status_t serviceRegistry_clearReferencesFor(service_registry_pt registry, bundle_pt bundle) { celix_status_t status = CELIX_SUCCESS; - int echoName =0; celixThreadRwlock_writeLock(®istry->lock); hash_map_pt refsMap = hashMap_remove(registry->serviceReferences, bundle); @@ -553,12 +575,10 @@ celix_status_t serviceRegistry_clearReferencesFor(service_registry_pt registry, serviceReference_getUsageCount(ref, &usageCount); serviceReference_getReferenceCount(ref, &refCount); - if(refCount>0) - { - echoName++; - } - serviceRegistry_logWarningServiceReferenceUsageCount(registry, usageCount, refCount); + if (refCount > 1 || usageCount > 0) { + serviceRegistry_logWarningServiceReferenceUsageCount(registry, bundle, ref, usageCount, refCount); + } while (usageCount > 0) { serviceReference_decreaseUsage(ref, &usageCount); @@ -575,15 +595,6 @@ celix_status_t serviceRegistry_clearReferencesFor(service_registry_pt registry, hashMap_destroy(refsMap, false, false); } - if(echoName >0) - { - module_pt module_ptr = NULL; - bundle_getCurrentModule(bundle, &module_ptr); - const char *name_str = NULL; - module_getSymbolicName(module_ptr, &name_str); - fw_log(logger, OSGI_FRAMEWORK_LOG_WARNING,"Previous Dangling service reference warnings caused by bundle: %s",name_str); - } - celixThreadRwlock_unlock(®istry->lock); return status;