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 <pepijnnol...@gmail.com>
Authored: Sun Oct 16 14:48:41 2016 +0200
Committer: Pepijn Noltes <pepijnnol...@gmail.com>
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(&registration->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, &reg);
+            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(&registry->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(&registry->lock);
 
     return status;

Reply via email to