Changeset 2136

Timestamp:

Jul 14, 2008 2:14:00 PM (16 years ago)

Author:

bennylp

Message:

Ticket #567: Rare race condition causing crash in ICE stream transport when STUN Binding resolution callback is called before initialization completes

File:

• pjproject/trunk/pjnath/src/pjnath/ice_strans.c (modified) (11 diffs)

pjproject/trunk/pjnath/src/pjnath/ice_strans.c

@@ -23 +23 @@
 #include <pj/assert.h>
 #include <pj/ip_helper.h>
+#include <pj/lock.h>
 #include <pj/log.h>
 #include <pj/os.h>
@@ -156 +157 @@
     pj_ice_strans_cfg        cfg;       /**< Configuration.             */
     pj_ice_strans_cb         cb;        /**< Application callback.      */
+    pj_lock_t               *init_lock; /**< Initialization mutex.      */
 
     pj_ice_sess             *ice;       /**< ICE session.               */
@@ -269 +271 @@
             ///sess_add_ref(ice_st);
 
+            PJ_LOG(4,(ice_st->obj_name, 
+                      "Comp %d: srflx candidate starts Binding discovery",
+                      comp_id));
+
             /* Start Binding resolution */
             status = pj_stun_sock_start(comp->stun_sock, 
@@ -286 +292 @@
             }
 
-            /* Add srflx candidate with pending status */
+            /* Add srflx candidate with pending status. */
             cand = &comp->cand_list[comp->cand_cnt++];
             cand->type = PJ_ICE_CAND_TYPE_SRFLX;
@@ -297 +303 @@
             pj_ice_calc_foundation(ice_st->pool, &cand->foundation,
                                    cand->type, &cand->base_addr);
-            PJ_LOG(4,(ice_st->obj_name, 
-                      "Comp %d: srflx candidate starts Binding discovery",
-                      comp_id));
 
             /* Set default candidate to srflx */
             comp->default_cand = cand - comp->cand_list;
+
         }
 
@@ -453 +457 @@
     }
 
+    status = pj_lock_create_recursive_mutex(pool, ice_st->obj_name, 
+                                            &ice_st->init_lock);
+    if (status != PJ_SUCCESS) {
+        destroy_ice_st(ice_st);
+        return status;
+    }
+
     ice_st->comp_cnt = comp_cnt;
     ice_st->comp = (pj_ice_strans_comp**) 
                    pj_pool_calloc(pool, comp_cnt, sizeof(pj_ice_strans_comp*));
 
+    /* Acquire initialization mutex to prevent callback to be 
+     * called before we finish initialization.
+     */
+    pj_lock_acquire(ice_st->init_lock);
+
     for (i=0; i<comp_cnt; ++i) {
         status = create_comp(ice_st, i+1);
         if (status != PJ_SUCCESS) {
+            pj_lock_release(ice_st->init_lock);
             destroy_ice_st(ice_st);
             return status;
         }
     }
+
+    /* Done with initialization */
+    pj_lock_release(ice_st->init_lock);
 
     /* Check if all candidates are ready (this may call callback) */
@@ -501 +521 @@
     }
     ice_st->comp_cnt = 0;
+
+    /* Destroy mutex */
+    if (ice_st->init_lock) {
+        pj_lock_acquire(ice_st->init_lock);
+        pj_lock_release(ice_st->init_lock);
+        pj_lock_destroy(ice_st->init_lock);
+        ice_st->init_lock = NULL;
+    }
 
     /* Destroy reference counter */
@@ -1199 +1227 @@
     unsigned i;
 
+    pj_assert(status != PJ_EPENDING);
+
     comp = (pj_ice_strans_comp*) pj_stun_sock_get_user_data(stun_sock);
     ice_st = comp->ice_st;
 
     sess_add_ref(ice_st);
+
+    /* Wait until initialization completes */
+    pj_lock_acquire(ice_st->init_lock);
 
     /* Find the srflx cancidate */
@@ -1212 +1245 @@
     }
 
-    pj_assert(status != PJ_EPENDING);
+    pj_lock_release(ice_st->init_lock);
+
+    /* It is possible that we don't have srflx candidate even though this
+     * callback is called. This could happen when we cancel adding srflx
+     * candidate due to initialization error.
+     */
+    if (cand == NULL) {
+        return sess_dec_ref(ice_st);
+    }
 
     switch (op) {
@@ -1364 +1405 @@
         pj_turn_sock_get_info(turn_sock, &rel_info);
 
+        /* Wait until initialization completes */
+        pj_lock_acquire(comp->ice_st->init_lock);
+
         /* Find relayed candidate in the component */
         for (i=0; i<comp->cand_cnt; ++i) {
@@ -1372 +1416 @@
         }
         pj_assert(cand != NULL);
+
+        pj_lock_release(comp->ice_st->init_lock);
 
         /* Update candidate */