You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@celix.apache.org by gr...@apache.org on 2017/06/27 15:37:23 UTC

celix git commit: Fixed leaks, deadlocks and error conditions in pubsub. Fixed latest Coverity issues.

Repository: celix
Updated Branches:
  refs/heads/develop 244d1c723 -> f32232fb2


Fixed leaks,deadlocks and error conditions in pubsub. Fixed latest Coverity issues.


Project: http://git-wip-us.apache.org/repos/asf/celix/repo
Commit: http://git-wip-us.apache.org/repos/asf/celix/commit/f32232fb
Tree: http://git-wip-us.apache.org/repos/asf/celix/tree/f32232fb
Diff: http://git-wip-us.apache.org/repos/asf/celix/diff/f32232fb

Branch: refs/heads/develop
Commit: f32232fb23ddfe0a2b3632392ab42d88c90c8e01
Parents: 244d1c7
Author: gricciardi <gr...@apache.org>
Authored: Tue Jun 27 17:37:07 2017 +0200
Committer: gricciardi <gr...@apache.org>
Committed: Tue Jun 27 17:37:07 2017 +0200

----------------------------------------------------------------------
 .../private/src/pubsub_admin_impl.c                   |  4 ++--
 .../private/src/topic_publication.c                   | 14 ++++++--------
 .../private/include/pubsub_admin_impl.h               |  1 +
 .../pubsub_admin_zmq/private/src/pubsub_admin_impl.c  |  6 +++++-
 pubsub/pubsub_common/public/src/pubsub_utils.c        |  9 ++++++++-
 .../private/src/pubsub_topology_manager.c             |  7 ++++---
 6 files changed, 26 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/celix/blob/f32232fb/pubsub/pubsub_admin_udp_mc/private/src/pubsub_admin_impl.c
----------------------------------------------------------------------
diff --git a/pubsub/pubsub_admin_udp_mc/private/src/pubsub_admin_impl.c b/pubsub/pubsub_admin_udp_mc/private/src/pubsub_admin_impl.c
index cf63c25..bbb452d 100644
--- a/pubsub/pubsub_admin_udp_mc/private/src/pubsub_admin_impl.c
+++ b/pubsub/pubsub_admin_udp_mc/private/src/pubsub_admin_impl.c
@@ -314,6 +314,7 @@ celix_status_t pubsubAdmin_addSubscription(pubsub_admin_pt admin,pubsub_endpoint
 		return pubsubAdmin_addAnySubscription(admin,subEP);
 	}
 
+	celixThreadMutex_lock(&admin->subscriptionsLock);
 	/* Check if we already know some publisher about this topic, otherwise let's put the subscription in the pending hashmap */
 	celixThreadMutex_lock(&admin->localPublicationsLock);
 	celixThreadMutex_lock(&admin->externalPublicationsLock);
@@ -367,9 +368,7 @@ celix_status_t pubsubAdmin_addSubscription(pubsub_admin_pt admin,pubsub_endpoint
 			status += pubsub_topicSubscriptionStart(subscription);
 
 			if(status==CELIX_SUCCESS){
-				celixThreadMutex_lock(&admin->subscriptionsLock);
 				hashMap_put(admin->subscriptions,strdup(scope_topic),subscription);
-				celixThreadMutex_unlock(&admin->subscriptionsLock);
 			}
 		}
 		pubsub_topicIncreaseNrSubscribers(subscription);
@@ -378,6 +377,7 @@ celix_status_t pubsubAdmin_addSubscription(pubsub_admin_pt admin,pubsub_endpoint
 	free(scope_topic);
 	celixThreadMutex_unlock(&admin->externalPublicationsLock);
 	celixThreadMutex_unlock(&admin->localPublicationsLock);
+	celixThreadMutex_unlock(&admin->subscriptionsLock);
 
 	return status;
 

http://git-wip-us.apache.org/repos/asf/celix/blob/f32232fb/pubsub/pubsub_admin_udp_mc/private/src/topic_publication.c
----------------------------------------------------------------------
diff --git a/pubsub/pubsub_admin_udp_mc/private/src/topic_publication.c b/pubsub/pubsub_admin_udp_mc/private/src/topic_publication.c
index 9dcb083..bed5dfc 100644
--- a/pubsub/pubsub_admin_udp_mc/private/src/topic_publication.c
+++ b/pubsub/pubsub_admin_udp_mc/private/src/topic_publication.c
@@ -270,8 +270,10 @@ celix_status_t pubsub_topicPublicationRemoveSerializer(topic_publication_pt pub,
             celixThreadMutex_unlock(&bound->mp_lock);
 			bound->map = NULL;
         }
+
+        pub->serializerSvc = NULL;
     }
-	pub->serializerSvc = NULL;
+
 	celixThreadMutex_unlock(&(pub->tp_lock));
 
 	return status;
@@ -502,13 +504,9 @@ static void pubsub_destroyPublishBundleBoundService(publish_bundle_bound_service
     //PRECOND lock on publish->tp_lock
 	celixThreadMutex_lock(&boundSvc->mp_lock);
 
-    if (boundSvc->map != NULL) {
-        if (boundSvc->parent->serializerSvc == NULL) {
-            printf("TP: Cannot destroy pubsub msg serializer map. No serliazer service\n");
-        } else {
-            boundSvc->parent->serializerSvc->destroySerializerMap(boundSvc->parent->serializerSvc->handle, boundSvc->map);
-            boundSvc->map = NULL;
-        }
+    if (boundSvc->map != NULL && boundSvc->parent->serializerSvc != NULL) {
+    	boundSvc->parent->serializerSvc->destroySerializerMap(boundSvc->parent->serializerSvc->handle, boundSvc->map);
+    	boundSvc->map = NULL;
     }
 
 	if (boundSvc->mp_parts!=NULL) {

http://git-wip-us.apache.org/repos/asf/celix/blob/f32232fb/pubsub/pubsub_admin_zmq/private/include/pubsub_admin_impl.h
----------------------------------------------------------------------
diff --git a/pubsub/pubsub_admin_zmq/private/include/pubsub_admin_impl.h b/pubsub/pubsub_admin_zmq/private/include/pubsub_admin_impl.h
index ccfa9e6..7e7ac42 100644
--- a/pubsub/pubsub_admin_zmq/private/include/pubsub_admin_impl.h
+++ b/pubsub/pubsub_admin_zmq/private/include/pubsub_admin_impl.h
@@ -64,6 +64,7 @@ struct pubsub_admin {
 	hash_map_pt subscriptions; //<topic(string),topic_subscription>
 
 	celix_thread_mutex_t pendingSubscriptionsLock;
+	celix_thread_mutexattr_t pendingSubscriptionsAttr;
 	hash_map_pt pendingSubscriptions; //<topic(string),List<pubsub_ep>>
 
 	char* ipAddress;

http://git-wip-us.apache.org/repos/asf/celix/blob/f32232fb/pubsub/pubsub_admin_zmq/private/src/pubsub_admin_impl.c
----------------------------------------------------------------------
diff --git a/pubsub/pubsub_admin_zmq/private/src/pubsub_admin_impl.c b/pubsub/pubsub_admin_zmq/private/src/pubsub_admin_impl.c
index 953ce16..e919c9c 100644
--- a/pubsub/pubsub_admin_zmq/private/src/pubsub_admin_impl.c
+++ b/pubsub/pubsub_admin_zmq/private/src/pubsub_admin_impl.c
@@ -98,9 +98,12 @@ celix_status_t pubsubAdmin_create(bundle_context_pt context, pubsub_admin_pt *ad
 
 		celixThreadMutex_create(&(*admin)->localPublicationsLock, NULL);
 		celixThreadMutex_create(&(*admin)->subscriptionsLock, NULL);
-		celixThreadMutex_create(&(*admin)->pendingSubscriptionsLock, NULL);
 		celixThreadMutex_create(&(*admin)->externalPublicationsLock, NULL);
 
+		celixThreadMutexAttr_create(&(*admin)->pendingSubscriptionsAttr);
+		celixThreadMutexAttr_settype(&(*admin)->pendingSubscriptionsAttr, CELIX_THREAD_MUTEX_RECURSIVE);
+		celixThreadMutex_create(&(*admin)->pendingSubscriptionsLock, &(*admin)->pendingSubscriptionsAttr);
+
 		if (logHelper_create(context, &(*admin)->loghelper) == CELIX_SUCCESS) {
 			logHelper_start((*admin)->loghelper);
 		}
@@ -225,6 +228,7 @@ celix_status_t pubsubAdmin_destroy(pubsub_admin_pt admin)
 	celixThreadMutex_unlock(&admin->externalPublicationsLock);
 
 	celixThreadMutex_destroy(&admin->pendingSubscriptionsLock);
+	celixThreadMutexAttr_destroy(&admin->pendingSubscriptionsAttr);
 	celixThreadMutex_destroy(&admin->subscriptionsLock);
 	celixThreadMutex_destroy(&admin->localPublicationsLock);
 	celixThreadMutex_destroy(&admin->externalPublicationsLock);

http://git-wip-us.apache.org/repos/asf/celix/blob/f32232fb/pubsub/pubsub_common/public/src/pubsub_utils.c
----------------------------------------------------------------------
diff --git a/pubsub/pubsub_common/public/src/pubsub_utils.c b/pubsub/pubsub_common/public/src/pubsub_utils.c
index 5f1b7ba..abc5ae6 100644
--- a/pubsub/pubsub_common/public/src/pubsub_utils.c
+++ b/pubsub/pubsub_common/public/src/pubsub_utils.c
@@ -133,11 +133,18 @@ char* pubsub_getKeysBundleDir(bundle_context_pt ctx)
 	array_list_pt bundles = NULL;
 	bundleContext_getBundles(ctx, &bundles);
 	int nrOfBundles = arrayList_size(bundles);
-
+	long bundle_id = -1;
 	char* result = NULL;
 
 	for (int i = 0; i < nrOfBundles; i++){
 		bundle_pt b = arrayList_get(bundles, i);
+
+		/* Skip bundle 0 (framework bundle) since it has no path nor revisions */
+		bundle_getBundleId(b, &bundle_id);
+		if(bundle_id==0){
+			continue;
+		}
+
 		char* dir = NULL;
 		bundle_getEntry(b, ".", &dir);
 

http://git-wip-us.apache.org/repos/asf/celix/blob/f32232fb/pubsub/pubsub_topology_manager/private/src/pubsub_topology_manager.c
----------------------------------------------------------------------
diff --git a/pubsub/pubsub_topology_manager/private/src/pubsub_topology_manager.c b/pubsub/pubsub_topology_manager/private/src/pubsub_topology_manager.c
index 6047cf8..0e7923b 100644
--- a/pubsub/pubsub_topology_manager/private/src/pubsub_topology_manager.c
+++ b/pubsub/pubsub_topology_manager/private/src/pubsub_topology_manager.c
@@ -485,8 +485,6 @@ celix_status_t pubsub_topologyManager_subscriberRemoved(void * handle, service_r
 	if(pubsubEndpoint_createFromServiceReference(reference,&subcmp) == CELIX_SUCCESS){
 
 		int j,k;
-		celixThreadMutex_lock(&manager->psaListLock);
-		celixThreadMutex_lock(&manager->subscriptionsLock);
 
 		// Inform discoveries that we not interested in the topic any more
         celixThreadMutex_lock(&manager->discoveryListLock);
@@ -501,6 +499,9 @@ celix_status_t pubsub_topologyManager_subscriberRemoved(void * handle, service_r
         hashMapIterator_destroy(iter);
         celixThreadMutex_unlock(&manager->discoveryListLock);
 
+        celixThreadMutex_lock(&manager->subscriptionsLock);
+        celixThreadMutex_lock(&manager->psaListLock);
+
 		char *sub_key = createScopeTopicKey(subcmp->scope,subcmp->topic);
 		array_list_pt sub_list_by_topic = hashMap_get(manager->subscriptions,sub_key);
 		free(sub_key);
@@ -539,8 +540,8 @@ celix_status_t pubsub_topologyManager_subscriberRemoved(void * handle, service_r
 			}
 		}
 
-		celixThreadMutex_unlock(&manager->subscriptionsLock);
 		celixThreadMutex_unlock(&manager->psaListLock);
+		celixThreadMutex_unlock(&manager->subscriptionsLock);
 
 		pubsubEndpoint_destroy(subcmp);