Changeset 5991

Timestamp:

May 15, 2019 2:54:52 AM (5 years ago)

Author:

nanang

Message:

Re #2191: Fixed crash in SIP transport destroy due to bug introduced by r5971, i.e: group lock is set after registering tp to tpmgr, so tpmgr won't call pj_grp_lock_add_ref(), but in unregisteration, group lock is set, so tpmgr will call pj_grp_lock_dec_ref().

Location:

pjproject/trunk/pjsip/src/pjsip

Files:

• sip_transport_tls.c (modified) (6 diffs)
• sip_transport_udp.c (modified) (3 diffs)

pjproject/trunk/pjsip/src/pjsip/sip_transport_tls.c

@@ -163 +163 @@
                               const pj_sockaddr *remote,
                               const pj_str_t *remote_name,
+                              pj_grp_lock_t *glock,
                               struct tls_transport **p_tls);
 
@@ -787 +788 @@
                                const pj_sockaddr *remote,
                                const pj_str_t *remote_name,
+                               pj_grp_lock_t *glock,
                                struct tls_transport **p_tls)
 {
@@ -870 +872 @@
 
     tls->ssock = ssock;
+
+    /* Set up the group lock */
+    tls->grp_lock = tls->base.grp_lock = glock;
+    pj_grp_lock_add_ref(tls->grp_lock);
+    pj_grp_lock_add_handler(tls->grp_lock, pool, tls, &tls_on_destroy);
 
     /* Register transport to transport manager */
@@ -1227 +1234 @@
     /* Create the transport descriptor */
     status = tls_create(listener, pool, ssock, PJ_FALSE, &local_addr, 
-                        rem_addr, &remote_name, &tls);
-    if (status != PJ_SUCCESS) {
-        pj_grp_lock_destroy(glock);
+                        rem_addr, &remote_name, glock, &tls);
+    if (status != PJ_SUCCESS)
         return status;
-    }
 
     /* Set the "pending" SSL socket user data */
     pj_ssl_sock_set_user_data(tls->ssock, tls);
-
-    /* Set up the group lock */
-    tls->grp_lock = tls->base.grp_lock = glock;
-    pj_grp_lock_add_ref(tls->grp_lock);
-    pj_grp_lock_add_handler(tls->grp_lock, pool, tls, &tls_on_destroy);
 
     /* Start asynchronous connect() operation */
@@ -1394 +1394 @@
      */
     status = tls_create( listener, NULL, new_ssock, PJ_TRUE,
-                         &ssl_info.local_addr, &tmp_src_addr, NULL, &tls);
+                         &ssl_info.local_addr, &tmp_src_addr, NULL,
+                         ssl_info.grp_lock, &tls);
     
     if (status != PJ_SUCCESS) {
@@ -1410 +1411 @@
     /* Set the "pending" SSL socket user data */
     pj_ssl_sock_set_user_data(new_ssock, tls);
-
-    /* Set up the group lock */
-    if (ssl_info.grp_lock) {
-        tls->grp_lock = ssl_info.grp_lock;
-        pj_grp_lock_add_ref(tls->grp_lock);
-        pj_grp_lock_add_handler(tls->grp_lock, tls->base.pool, tls,
-                                &tls_on_destroy);
-    }
 
     /* Prevent immediate transport destroy as application may access it 

pjproject/trunk/pjsip/src/pjsip/sip_transport_udp.c

@@ -471 +471 @@
     }
 
+    /* When creating this transport, reference count was incremented to flag
+     * this transport as permanent so it will not be destroyed by transport
+     * manager whenever idle. Application may or may not have cleared the
+     * flag (by calling pjsip_transport_dec_ref()), so in case it has not,
+     * let's do it now, so this transport can be destroyed.
+     */
+    if (pj_atomic_get(tp->base.ref_cnt) > 0)
+        pjsip_transport_dec_ref(&tp->base);
+
+    /* Destroy transport */
     if (tp->grp_lock) {
         pj_grp_lock_t *grp_lock = tp->grp_lock;
@@ -845 +855 @@
     tp->base.destroy = &udp_destroy;
 
-    /* This is a permanent transport, so we initialize the ref count
-     * to one so that transport manager don't destroy this transport
-     * when there's no user!
-     */
-    pj_atomic_inc(tp->base.ref_cnt);
-
     /* Register to transport manager. */
     tp->base.tpmgr = pjsip_endpt_get_tpmgr(endpt);
@@ -857 +861 @@
         goto on_error;
 
+    /* This is a permanent transport, so we initialize the ref count
+     * to one so that transport manager won't destroy this transport
+     * when there's no user!
+     */
+    pjsip_transport_add_ref(&tp->base);
 
     /* Create rdata and put it in the array. */