Changeset 5573

Timestamp:

Mar 29, 2017 2:40:48 AM (8 years ago)

Author:

ming

Message:

Fixed #2002: Deadlock between PJSUA LOCK, transaction group lock, and UA mutex

Location:

pjproject/trunk/pjsip

Files:

• include/pjsip/sip_transaction.h (modified) (2 diffs)
• src/pjsip-ua/sip_inv.c (modified) (2 diffs)
• src/pjsip/sip_transaction.c (modified) (3 diffs)
• src/pjsip/sip_ua_layer.c (modified) (1 diff)

pjproject/trunk/pjsip/include/pjsip/sip_transaction.h

@@ -180 +180 @@
  * is created by calling #pjsip_tsx_create_key() from an incoming message.
  *
+ * IMPORTANT: To prevent deadlock, application should use
+ * #pjsip_tsx_layer_find_tsx2() instead which only adds a reference to
+ * the transaction instead of locking it.
+ *
  * @param key       The key string to find the transaction.
  * @param lock      If non-zero, transaction will be locked before the
@@ -190 +194 @@
 PJ_DECL(pjsip_transaction*) pjsip_tsx_layer_find_tsx( const pj_str_t *key,
                                                       pj_bool_t lock );
+
+/**
+ * Find a transaction with the specified key. The transaction key normally
+ * is created by calling #pjsip_tsx_create_key() from an incoming message.
+ *
+ * @param key       The key string to find the transaction.
+ * @param add_ref   If non-zero, transaction's reference will be added
+ *                  by one before the function returns, to make sure that
+ *                  it's not deleted by other threads.
+ *
+ * @return          The matching transaction instance, or NULL if transaction
+ *                  can not be found.
+ */
+PJ_DECL(pjsip_transaction*) pjsip_tsx_layer_find_tsx2( const pj_str_t *key,
+                                                       pj_bool_t add_ref );
 
 /**

pjproject/trunk/pjsip/src/pjsip-ua/sip_inv.c

@@ -3276 +3276 @@
     pjsip_tsx_create_key(rdata->tp_info.pool, &key, PJSIP_ROLE_UAS,
                          pjsip_get_invite_method(), rdata);
-    invite_tsx = pjsip_tsx_layer_find_tsx(&key, PJ_TRUE);
+    invite_tsx = pjsip_tsx_layer_find_tsx2(&key, PJ_TRUE);
 
     if (invite_tsx == NULL) {
@@ -3325 +3325 @@
 
     if (invite_tsx)
-        pj_grp_lock_release(invite_tsx->grp_lock);
+        pj_grp_lock_dec_ref(invite_tsx->grp_lock);
 }
 

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

@@ -642 +642 @@
  * Find a transaction.
  */
-PJ_DEF(pjsip_transaction*) pjsip_tsx_layer_find_tsx( const pj_str_t *key,
-                                                     pj_bool_t lock )
+static pjsip_transaction* find_tsx( const pj_str_t *key, pj_bool_t lock,
+                                    pj_bool_t add_ref )
 {
     pjsip_transaction *tsx;
@@ -655 +655 @@
     /* Prevent the transaction to get deleted before we have chance to lock it.
      */
-    if (tsx && lock)
+    if (tsx)
         pj_grp_lock_add_ref(tsx->grp_lock);
     
@@ -667 +667 @@
     PJ_RACE_ME(5);
 
-    if (tsx && lock) {
-        pj_grp_lock_acquire(tsx->grp_lock);
-        pj_grp_lock_dec_ref(tsx->grp_lock);
+    if (tsx) {
+        if (lock)
+            pj_grp_lock_acquire(tsx->grp_lock);
+
+        if (!add_ref)
+            pj_grp_lock_dec_ref(tsx->grp_lock);
     }
 
     return tsx;
+}
+
+
+PJ_DEF(pjsip_transaction*) pjsip_tsx_layer_find_tsx( const pj_str_t *key,
+                                                     pj_bool_t lock )
+{
+    return find_tsx(key, lock, PJ_FALSE);
+}
+
+
+PJ_DEF(pjsip_transaction*) pjsip_tsx_layer_find_tsx2( const pj_str_t *key,
+                                                      pj_bool_t add_ref )
+{
+    return find_tsx(key, PJ_FALSE, add_ref);
 }
 

pjproject/trunk/pjsip/src/pjsip/sip_ua_layer.c

@@ -552 +552 @@
 
         /* Lookup the INVITE transaction */
-        tsx = pjsip_tsx_layer_find_tsx(&key, PJ_TRUE);
+        tsx = pjsip_tsx_layer_find_tsx2(&key, PJ_TRUE);
 
         /* We should find the dialog attached to the INVITE transaction */
         if (tsx) {
             dlg = (pjsip_dialog*) tsx->mod_data[mod_ua.mod.id];
-            pj_grp_lock_release(tsx->grp_lock);
+            pj_grp_lock_dec_ref(tsx->grp_lock);
 
             /* Dlg may be NULL on some extreme condition