Changeset 4992 for pjproject

Timestamp:

Mar 6, 2015 6:09:22 AM (10 years ago)

Author:

ming

Message:

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

Location:

pjproject/trunk

Files:

• pjlib/include/pj/lock.h (modified) (1 diff)
• pjlib/src/pj/lock.c (modified) (3 diffs)
• pjsip/src/pjsip/sip_transaction.c (modified) (5 diffs)

pjproject/trunk/pjlib/include/pj/lock.h

@@ -219 +219 @@
 
 /**
+ * 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.
  *

pjproject/trunk/pjlib/src/pj/lock.c

@@ -336 +336 @@
 }
 
+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)
 {
@@ -428 +453 @@
 }
 
+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)
 {
@@ -477 +518 @@
                                              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);
 }
 

pjproject/trunk/pjsip/src/pjsip/sip_transaction.c

@@ -1008 +1008 @@
     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_handler(tsx->grp_lock, tsx->pool, tsx, &tsx_on_destroy);
+        
+        pj_grp_lock_add_ref(tsx->grp_lock);
+    }
 
     status = pj_mutex_create_simple(pool, tsx->obj_name, &tsx->mutex_b);
@@ -1309 +1312 @@
 
 
-    /* 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. */
@@ -1376 +1383 @@
     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;
@@ -1388 +1396 @@
         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;
@@ -1395 +1404 @@
 
     /* 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();