Changeset 4862 for pjproject/trunk/pjsip/src/pjsip/sip_transport_tls.c

Timestamp:

Jun 19, 2014 9:42:02 AM (10 years ago)

Author:

nanang

Message:

Fix #1773: Added group lock to SIP transport to avoid race condition between transport callback and destroy.

File:

• pjproject/trunk/pjsip/src/pjsip/sip_transport_tls.c (modified) (12 diffs)

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

@@ -59 +59 @@
     pj_ssl_cert_t           *cert;
     pjsip_tls_setting        tls_setting;
+
+    /* Group lock to be used by TLS transport and ioqueue key */
+    pj_grp_lock_t           *grp_lock;
 };
 
@@ -107 +110 @@
     /* Pending transmission list. */
     struct delayed_tdata     delayed_list;
+
+    /* Group lock to be used by TLS transport and ioqueue key */
+    pj_grp_lock_t           *grp_lock;
 };
 
@@ -134 +140 @@
 /* This callback is called by transport manager to destroy listener */
 static pj_status_t lis_destroy(pjsip_tpfactory *factory);
+
+/* Clean up listener resources (group lock handler) */
+static void lis_on_destroy(void *arg);
 
 /* This callback is called by transport manager to create transport */
@@ -377 +386 @@
     }
 
+    /* Create group lock */
+    status = pj_grp_lock_create(pool, NULL, &listener->grp_lock);
+    if (status != PJ_SUCCESS)
+        return status;
+
+    /* Setup group lock handler */
+    pj_grp_lock_add_ref(listener->grp_lock);
+    pj_grp_lock_add_handler(listener->grp_lock, pool, listener,
+                            &lis_on_destroy);
+
+    ssock_param.grp_lock = listener->grp_lock;
+
     /* Create SSL socket */
     status = pj_ssl_sock_create(pool, &ssock_param, &listener->ssock);
@@ -510 +531 @@
 
 
+/* Clean up listener resources */
+static void lis_on_destroy(void *arg)
+{
+    struct tls_listener *listener = (struct tls_listener*)arg;
+
+    if (listener->factory.lock) {
+        pj_lock_destroy(listener->factory.lock);
+        listener->factory.lock = NULL;
+    }
+
+    if (listener->factory.pool) {
+        pj_pool_t *pool = listener->factory.pool;
+
+        PJ_LOG(4,(listener->factory.obj_name,  "SIP TLS listener destroyed"));
+
+        listener->factory.pool = NULL;
+        pj_pool_release(pool);
+    }
+}
+
+
 /* This callback is called by transport manager to destroy listener */
 static pj_status_t lis_destroy(pjsip_tpfactory *factory)
@@ -525 +567 @@
     }
 
-    if (listener->factory.lock) {
-        pj_lock_destroy(listener->factory.lock);
-        listener->factory.lock = NULL;
-    }
-
-    if (listener->factory.pool) {
-        pj_pool_t *pool = listener->factory.pool;
-
-        PJ_LOG(4,(listener->factory.obj_name,  "SIP TLS listener destroyed"));
-
-        listener->factory.pool = NULL;
-        pj_pool_release(pool);
+    if (listener->grp_lock) {
+        pj_grp_lock_t *grp_lock = listener->grp_lock;
+        listener->grp_lock = NULL;
+        pj_grp_lock_dec_ref(grp_lock);
+        /* Listener may have been deleted at this point */
+    } else {
+        lis_on_destroy(listener);
     }
 
@@ -753 +790 @@
 
 
+/* Clean up TLS resources */
+static void tls_on_destroy(void *arg)
+{
+    struct tls_transport *tls = (struct tls_transport*)arg;
+
+    if (tls->rdata.tp_info.pool) {
+        pj_pool_release(tls->rdata.tp_info.pool);
+        tls->rdata.tp_info.pool = NULL;
+    }
+
+    if (tls->base.lock) {
+        pj_lock_destroy(tls->base.lock);
+        tls->base.lock = NULL;
+    }
+
+    if (tls->base.ref_cnt) {
+        pj_atomic_destroy(tls->base.ref_cnt);
+        tls->base.ref_cnt = NULL;
+    }
+
+    if (tls->base.pool) {
+        pj_pool_t *pool;
+
+        if (tls->close_reason != PJ_SUCCESS) {
+            char errmsg[PJ_ERR_MSG_SIZE];
+
+            pj_strerror(tls->close_reason, errmsg, sizeof(errmsg));
+            PJ_LOG(4,(tls->base.obj_name, 
+                      "TLS transport destroyed with reason %d: %s", 
+                      tls->close_reason, errmsg));
+
+        } else {
+
+            PJ_LOG(4,(tls->base.obj_name, 
+                      "TLS transport destroyed normally"));
+
+        }
+
+        pool = tls->base.pool;
+        tls->base.pool = NULL;
+        pj_pool_release(pool);
+    }
+}
+
 /* Destroy TLS transport */
 static pj_status_t tls_destroy(pjsip_transport *transport, 
@@ -794 +875 @@
     }
 
-    if (tls->rdata.tp_info.pool) {
-        pj_pool_release(tls->rdata.tp_info.pool);
-        tls->rdata.tp_info.pool = NULL;
-    }
-
     if (tls->ssock) {
         pj_ssl_sock_close(tls->ssock);
         tls->ssock = NULL;
     }
-    if (tls->base.lock) {
-        pj_lock_destroy(tls->base.lock);
-        tls->base.lock = NULL;
-    }
-
-    if (tls->base.ref_cnt) {
-        pj_atomic_destroy(tls->base.ref_cnt);
-        tls->base.ref_cnt = NULL;
-    }
-
-    if (tls->base.pool) {
-        pj_pool_t *pool;
-
-        if (reason != PJ_SUCCESS) {
-            char errmsg[PJ_ERR_MSG_SIZE];
-
-            pj_strerror(reason, errmsg, sizeof(errmsg));
-            PJ_LOG(4,(tls->base.obj_name, 
-                      "TLS transport destroyed with reason %d: %s", 
-                      reason, errmsg));
-
-        } else {
-
-            PJ_LOG(4,(tls->base.obj_name, 
-                      "TLS transport destroyed normally"));
-
-        }
-
-        pool = tls->base.pool;
-        tls->base.pool = NULL;
-        pj_pool_release(pool);
+
+    if (tls->grp_lock) {
+        pj_grp_lock_t *grp_lock = tls->grp_lock;
+        tls->grp_lock = NULL;
+        pj_grp_lock_dec_ref(grp_lock);
+        /* Transport may have been deleted at this point */
+    } else {
+        tls_on_destroy(tls);
     }
 
@@ -907 +960 @@
     struct tls_transport *tls;
     pj_pool_t *pool;
+    pj_grp_lock_t *glock;
     pj_ssl_sock_t *ssock;
     pj_ssl_sock_param ssock_param;
@@ -986 +1040 @@
     }
 
-    status = pj_ssl_sock_create(pool, &ssock_param, &ssock);
+    /* Create group lock */
+    status = pj_grp_lock_create(pool, NULL, &glock);
     if (status != PJ_SUCCESS)
         return status;
+
+    ssock_param.grp_lock = glock;
+    status = pj_ssl_sock_create(pool, &ssock_param, &ssock);
+    if (status != PJ_SUCCESS) {
+        pj_grp_lock_destroy(glock);
+        return status;
+    }
 
     /* Apply SSL certificate */
     if (listener->cert) {
         status = pj_ssl_sock_set_certificate(ssock, pool, listener->cert);
-        if (status != PJ_SUCCESS)
+        if (status != PJ_SUCCESS) {
+            pj_grp_lock_destroy(glock);
             return status;
+        }
     }
 
@@ -1005 +1069 @@
     status = tls_create(listener, pool, ssock, PJ_FALSE, &local_addr, 
                         rem_addr, &remote_name, &tls);
-    if (status != PJ_SUCCESS)
+    if (status != PJ_SUCCESS) {
+        pj_grp_lock_destroy(glock);
         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 = 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 */
@@ -1130 +1201 @@
     /* 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