Fixed #1821: Remove unnecessary locking in pjsip transaction and add new API to create a group lock with handler in a single atomic instruction

git-svn-id: http://svn.pjsip.org/repos/pjproject/trunk@4992 74dad513-b988-da41-8d7b-12977e46ad98

3 files changed, 82 insertions, 23 deletions

diff --git a/pjlib/include/pj/lock.h b/pjlib/include/pj/lock.h
index b1a5d72e..415ea951 100644
--- a/pjlib/include/pj/lock.h
+++ b/pjlib/include/pj/lock.h
@@ -218,6 +218,27 @@ PJ_DECL(pj_status_t) pj_grp_lock_create(pj_pool_t *pool,
                                         pj_grp_lock_t **p_grp_lock);
 
 /**
+ * Create a group lock object, with the specified destructor handler, to be
+ * called by the group lock when it is about to be destroyed. Initially the
+ * group lock will have reference counter of one.
+ *
+ * @param pool		The group lock only uses the pool parameter to get
+ * 			the pool factory, from which it will create its own
+ * 			pool.
+ * @param cfg		Optional configuration.
+ * @param member	A pointer to be passed to the handler.
+ * @param handler	The destroy handler.
+ * @param p_grp_lock	Pointer to receive the newly created group lock.
+ *
+ * @return		PJ_SUCCESS or the appropriate error code.
+ */
+PJ_DECL(pj_status_t) pj_grp_lock_create_w_handler(pj_pool_t *pool,
+                                        	  const pj_grp_lock_config *cfg,
+                                        	  void *member,
+                                                  void (*handler)(void *member),
+                                        	  pj_grp_lock_t **p_grp_lock);
+
+/**
  * Forcibly destroy the group lock, ignoring the reference counter value.
  *
  * @param grp_lock	The group lock.
diff --git a/pjlib/src/pj/lock.c b/pjlib/src/pj/lock.c
index d7f4ffaf..15278271 100644
--- a/pjlib/src/pj/lock.c
+++ b/pjlib/src/pj/lock.c
@@ -335,6 +335,31 @@ static pj_status_t grp_lock_release(LOCK_OBJ *p)
     return pj_grp_lock_dec_ref(glock);
 }
 
+static pj_status_t grp_lock_add_handler( pj_grp_lock_t *glock,
+                             		 pj_pool_t *pool,
+                             		 void *comp,
+                             		 void (*destroy)(void *comp),
+                             		 pj_bool_t acquire_lock)
+{
+    grp_destroy_callback *cb;
+
+    if (acquire_lock)
+        grp_lock_acquire(glock);
+
+    if (pool == NULL)
+	pool = glock->pool;
+
+    cb = PJ_POOL_ZALLOC_T(pool, grp_destroy_callback);
+    cb->comp = comp;
+    cb->handler = destroy;
+    pj_list_push_back(&glock->destroy_list, cb);
+
+    if (acquire_lock)
+        grp_lock_release(glock);
+
+    return PJ_SUCCESS;
+}
+
 static pj_status_t grp_lock_destroy(LOCK_OBJ *p)
 {
     pj_grp_lock_t *glock = (pj_grp_lock_t*)p;
@@ -427,6 +452,22 @@ on_error:
     return status;
 }
 
+PJ_DEF(pj_status_t) pj_grp_lock_create_w_handler( pj_pool_t *pool,
+                                        	  const pj_grp_lock_config *cfg,
+                                        	  void *member,
+                                                  void (*handler)(void *member),
+                                        	  pj_grp_lock_t **p_grp_lock)
+{
+    pj_status_t status;
+
+    status = pj_grp_lock_create(pool, cfg, p_grp_lock);
+    if (status == PJ_SUCCESS) {
+        grp_lock_add_handler(*p_grp_lock, pool, member, handler, PJ_FALSE);
+    }
+    
+    return status;
+}
+
 PJ_DEF(pj_status_t) pj_grp_lock_destroy( pj_grp_lock_t *grp_lock)
 {
     return grp_lock_destroy(grp_lock);
@@ -476,20 +517,7 @@ PJ_DEF(pj_status_t) pj_grp_lock_add_handler( pj_grp_lock_t *glock,
                                              void *comp,
                                              void (*destroy)(void *comp))
 {
-    grp_destroy_callback *cb;
-
-    grp_lock_acquire(glock);
-
-    if (pool == NULL)
-	pool = glock->pool;
-
-    cb = PJ_POOL_ZALLOC_T(pool, grp_destroy_callback);
-    cb->comp = comp;
-    cb->handler = destroy;
-    pj_list_push_back(&glock->destroy_list, cb);
-
-    grp_lock_release(glock);
-    return PJ_SUCCESS;
+    return grp_lock_add_handler(glock, pool, comp, destroy, PJ_TRUE);
 }
 
 PJ_DEF(pj_status_t) pj_grp_lock_del_handler( pj_grp_lock_t *glock,
diff --git a/pjsip/src/pjsip/sip_transaction.c b/pjsip/src/pjsip/sip_transaction.c
index b3fb5e62..c0e57a0f 100644
--- a/pjsip/src/pjsip/sip_transaction.c
+++ b/pjsip/src/pjsip/sip_transaction.c
@@ -1007,17 +1007,20 @@ static pj_status_t tsx_create( pjsip_module *tsx_user,
     
     if (grp_lock) {
 	tsx->grp_lock = grp_lock;
+        
+        pj_grp_lock_add_ref(tsx->grp_lock);
+        pj_grp_lock_add_handler(tsx->grp_lock, tsx->pool, tsx, &tsx_on_destroy);
     } else {
-	status = pj_grp_lock_create(pool, NULL, &tsx->grp_lock);
+	status = pj_grp_lock_create_w_handler(pool, NULL, tsx, &tsx_on_destroy,
+					      &tsx->grp_lock);
 	if (status != PJ_SUCCESS) {
 	    pjsip_endpt_release_pool(mod_tsx_layer.endpt, pool);
 	    return status;
 	}
+	
+	pj_grp_lock_add_ref(tsx->grp_lock);
     }
 
-    pj_grp_lock_add_ref(tsx->grp_lock);
-    pj_grp_lock_add_handler(tsx->grp_lock, tsx->pool, tsx, &tsx_on_destroy);
-
     status = pj_mutex_create_simple(pool, tsx->obj_name, &tsx->mutex_b);
     if (status != PJ_SUCCESS) {
 	tsx_shutdown(tsx);
@@ -1308,8 +1311,12 @@ PJ_DEF(pj_status_t) pjsip_tsx_create_uac2(pjsip_module *tsx_user,
 	return status;
 
 
-    /* Lock transaction. */
-    pj_grp_lock_acquire(tsx->grp_lock);
+    /* Lock transaction.
+     * We don't need to lock the group lock if none was supplied, while the
+     * newly created group lock has not been exposed.
+     */
+    if (grp_lock)
+        pj_grp_lock_acquire(tsx->grp_lock);
 
     /* Role is UAC. */
     tsx->role = PJSIP_ROLE_UAC;
@@ -1375,7 +1382,8 @@ PJ_DEF(pj_status_t) pjsip_tsx_create_uac2(pjsip_module *tsx_user,
      */
     status = pjsip_get_request_dest(tdata, &dst_info);
     if (status != PJ_SUCCESS) {
-	pj_grp_lock_release(tsx->grp_lock);
+	if (grp_lock)
+	    pj_grp_lock_release(tsx->grp_lock);
 	tsx_shutdown(tsx);
 	return status;
     }
@@ -1387,14 +1395,16 @@ PJ_DEF(pj_status_t) pjsip_tsx_create_uac2(pjsip_module *tsx_user,
 	/* The assertion is removed by #1090:
 	pj_assert(!"Bug in branch_param generator (i.e. not unique)");
 	*/
-	pj_grp_lock_release(tsx->grp_lock);
+	if (grp_lock)
+	    pj_grp_lock_release(tsx->grp_lock);
 	tsx_shutdown(tsx);
 	return status;
     }
 
 
     /* Unlock transaction and return. */
-    pj_grp_lock_release(tsx->grp_lock);
+    if (grp_lock)
+        pj_grp_lock_release(tsx->grp_lock);
 
     pj_log_push_indent();
     PJ_LOG(5,(tsx->obj_name, "Transaction created for %s",