Changeset 1401

Timestamp:

Jun 28, 2007 7:20:26 AM (17 years ago)

Author:

bennylp

Message:

Fixed ticket #351: Possible deadlock in pjsua-api presence subscription (thanks Paul Levin)

Location:

pjproject/trunk/pjsip

Files:

• include/pjsua-lib/pjsua_internal.h (modified) (1 diff)
• src/pjsua-lib/pjsua_pres.c (modified) (16 diffs)

pjproject/trunk/pjsip/include/pjsua-lib/pjsua_internal.h

@@ -134 +134 @@
     unsigned             port;      /**< Buddy port.                    */
     pj_bool_t            monitor;   /**< Should we monitor?             */
+    pjsip_dialog        *dlg;       /**< The underlying dialog.         */
     pjsip_evsub         *sub;       /**< Buddy presence subscription    */
     pjsip_pres_status    status;    /**< Buddy presence status.         */

pjproject/trunk/pjsip/src/pjsua-lib/pjsua_pres.c

@@ -24 +24 @@
 
 #ifndef PJSUA_PRES_TIMER
-#   define PJSUA_PRES_TIMER     120
+#   define PJSUA_PRES_TIMER     2
 #endif
 
@@ -71 +71 @@
 
     return PJ_SUCCESS;
-
 }
 
@@ -208 +207 @@
     pjsua_var.buddy_cnt++;
 
+    PJSUA_UNLOCK();
+
     pjsua_buddy_subscribe_pres(index, cfg->subscribe);
-
-    PJSUA_UNLOCK();
 
     return PJ_SUCCESS;
@@ -225 +224 @@
                      PJ_EINVAL);
 
-    PJSUA_LOCK();
-
     if (pjsua_var.buddy[buddy_id].uri.slen == 0) {
-        PJSUA_UNLOCK();
         return PJ_SUCCESS;
     }
@@ -234 +230 @@
     /* Unsubscribe presence */
     pjsua_buddy_subscribe_pres(buddy_id, PJ_FALSE);
+
+    PJSUA_LOCK();
 
     /* Remove buddy */
@@ -263 +261 @@
     buddy = &pjsua_var.buddy[buddy_id];
     buddy->monitor = subscribe;
+
+    PJSUA_UNLOCK();
+
     pjsua_pres_refresh();
-
-    PJSUA_UNLOCK();
 
     return PJ_SUCCESS;
@@ -1015 +1014 @@
     pjsua_acc *acc;
     pj_str_t contact;
-    pjsip_dialog *dlg;
     pjsip_tx_data *tdata;
     pj_status_t status;
@@ -1040 +1038 @@
                                    &contact,
                                    &buddy->uri,
-                                   NULL, &dlg);
+                                   NULL, &buddy->dlg);
     if (status != PJ_SUCCESS) {
         pjsua_perror(THIS_FILE, "Unable to create dialog", 
@@ -1047 +1045 @@
     }
 
-    status = pjsip_pres_create_uac( dlg, &pres_callback, 
+    status = pjsip_pres_create_uac( buddy->dlg, &pres_callback, 
                                     PJSIP_EVSUB_NO_EVENT_ID, &buddy->sub);
     if (status != PJ_SUCCESS) {
@@ -1053 +1051 @@
         pjsua_perror(THIS_FILE, "Unable to create presence client", 
                      status);
-        pjsip_dlg_terminate(dlg);
+        pjsip_dlg_terminate(buddy->dlg);
         return;
     }
@@ -1064 +1062 @@
 
         pjsua_init_tpselector(acc->cfg.transport_id, &tp_sel);
-        pjsip_dlg_set_transport(dlg, &tp_sel);
+        pjsip_dlg_set_transport(buddy->dlg, &tp_sel);
     }
 
     /* Set route-set */
     if (!pj_list_empty(&acc->route_set)) {
-        pjsip_dlg_set_route_set(dlg, &acc->route_set);
+        pjsip_dlg_set_route_set(buddy->dlg, &acc->route_set);
     }
 
     /* Set credentials */
     if (acc->cred_cnt) {
-        pjsip_auth_clt_set_credentials( &dlg->auth_sess, 
+        pjsip_auth_clt_set_credentials( &buddy->dlg->auth_sess, 
                                         acc->cred_cnt, acc->cred);
     }
@@ -1138 +1136 @@
 
 
+/* Lock all buddies */
+#define LOCK_BUDDIES    unsigned cnt_ = 0; \
+                        pjsip_dialog *dlg_list_[PJSUA_MAX_BUDDIES]; \
+                        unsigned i_; \
+                        for (i_=0; i_<PJ_ARRAY_SIZE(pjsua_var.buddy);++i_) { \
+                            if (pjsua_var.buddy[i_].sub) { \
+                                dlg_list_[cnt_++] = pjsua_var.buddy[i_].dlg; \
+                                pjsip_dlg_inc_lock(pjsua_var.buddy[i_].dlg); \
+                            } \
+                        } \
+                        PJSUA_LOCK();
+
+/* Unlock all buddies */
+#define UNLOCK_BUDDIES  PJSUA_UNLOCK(); \
+                        for (i_=0; i_<cnt_; ++i_) { \
+                            pjsip_dlg_dec_lock(dlg_list_[i_]); \
+                        }
+                        
+
+
 /* It does what it says.. */
 static void refresh_client_subscriptions(void)
@@ -1143 +1161 @@
     unsigned i;
 
+    LOCK_BUDDIES;
+
     for (i=0; i<PJ_ARRAY_SIZE(pjsua_var.buddy); ++i) {
 
@@ -1156 +1176 @@
         }
     }
+
+    UNLOCK_BUDDIES;
 }
 
@@ -1164 +1186 @@
     pj_time_val delay = { PJSUA_PRES_TIMER, 0 };
 
-    PJ_UNUSED_ARG(th);
-
-    PJSUA_LOCK();
-
     entry->id = PJ_FALSE;
     refresh_client_subscriptions();
@@ -1174 +1192 @@
     entry->id = PJ_TRUE;
 
-    PJSUA_UNLOCK();
+    PJ_UNUSED_ARG(th);
 }