Re #1918:

 - Fixed issue of cannot make/receive call after previous call initialization fails due to STUN error, reproducing steps:
   1. Configure an account with acc->cfg.media_stun_use set PJSUA_STUN_RETRY_ON_FAILURE.
   2. Start pjsua with STUN servers A and B configured. On startup, both STUN servers A and B are available, so PJSIP will use STUN server A
   3. Both STUN server A and B become unavailable
   4. Make an outgoing call.
   5. Pjsua first tries with STUN server A, fails
   6. Pjsua then retry with STUN server B, still fails
   7. PJSIP then aborts the call with error (which is desired!)
   8. Both STUN server A and B become available again
   9. User tries to dial or receive an incoming call, but cannot because the last STUN server status is stored and used in making/receiving call without retrying to resolve STUN server.
 - Fixed deadlock issues.



git-svn-id: http://svn.pjsip.org/repos/pjproject/trunk@5326 74dad513-b988-da41-8d7b-12977e46ad98

4 files changed, 108 insertions, 40 deletions

diff --git a/pjsip/include/pjsua-lib/pjsua.h b/pjsip/include/pjsua-lib/pjsua.h
index b88aa462..e75636ca 100644
--- a/pjsip/include/pjsua-lib/pjsua.h
+++ b/pjsip/include/pjsua-lib/pjsua.h
@@ -3325,7 +3325,7 @@ typedef struct pjsua_acc_config
     /**
      * Control the use of STUN for the media transports.
      *
-     * Default: PJSUA_STUN_USE_DEFAULT
+     * Default: PJSUA_STUN_RETRY_ON_FAILURE
      */
     pjsua_stun_use 		media_stun_use;
 
diff --git a/pjsip/include/pjsua-lib/pjsua_internal.h b/pjsip/include/pjsua-lib/pjsua_internal.h
index 24a87fc0..8a4f6c84 100644
--- a/pjsip/include/pjsua-lib/pjsua_internal.h
+++ b/pjsip/include/pjsua-lib/pjsua_internal.h
@@ -367,6 +367,8 @@ typedef struct pjsua_stun_resolve
     void		*token;	    /**< App token	    */
     pj_stun_resolve_cb	 cb;	    /**< App callback	    */
     pj_bool_t		 blocking;  /**< Blocking?	    */
+    pj_thread_t		*waiter;    /**< Waiting thread	    */
+    pj_timer_entry	 timer;	    /**< Destroy timer	    */
     pj_status_t		 status;    /**< Session status	    */
     pj_sockaddr		 addr;	    /**< Result		    */
     pj_stun_sock	*stun_sock; /**< Testing STUN sock  */
@@ -608,7 +610,7 @@ void pjsua_set_state(pjsua_state new_state);
  * STUN resolution
  */
 /* Resolve the STUN server */
-pj_status_t resolve_stun_server(pj_bool_t wait);
+pj_status_t resolve_stun_server(pj_bool_t wait, pj_bool_t retry_if_cur_error);
 
 /** 
  * Normalize route URI (check for ";lr" and append one if it doesn't
diff --git a/pjsip/src/pjsua-lib/pjsua_core.c b/pjsip/src/pjsua-lib/pjsua_core.c
index a5dee5a5..1c9c7fef 100644
--- a/pjsip/src/pjsua-lib/pjsua_core.c
+++ b/pjsip/src/pjsua-lib/pjsua_core.c
@@ -302,6 +302,8 @@ PJ_DEF(void) pjsua_acc_config_default(pjsua_acc_config *cfg)
     cfg->call_hold_type = PJSUA_CALL_HOLD_TYPE_DEFAULT;
     cfg->register_on_acc_add = PJ_TRUE;
     cfg->mwi_expires = PJSIP_MWI_DEFAULT_EXPIRES;
+
+    cfg->media_stun_use = PJSUA_STUN_RETRY_ON_FAILURE;
 }
 
 PJ_DEF(void) pjsua_buddy_config_default(pjsua_buddy_config *cfg)
@@ -1049,7 +1051,7 @@ PJ_DEF(pj_status_t) pjsua_init( const pjsua_config *ua_cfg,
     }
 
     /* Start resolving STUN server */
-    status = resolve_stun_server(PJ_FALSE);
+    status = resolve_stun_server(PJ_FALSE, PJ_FALSE);
     if (status != PJ_SUCCESS && status != PJ_EPENDING) {
 	pjsua_perror(THIS_FILE, "Error resolving STUN server", status);
 	goto on_error;
@@ -1156,13 +1158,40 @@ static void stun_resolve_add_ref(pjsua_stun_resolve *sess)
     ++sess->ref_cnt;
 }
 
+
+static void destroy_stun_resolve_cb(pj_timer_heap_t *t, pj_timer_entry *e)
+{
+    pjsua_stun_resolve *sess = (pjsua_stun_resolve*)e->user_data;
+    PJ_UNUSED_ARG(t);
+
+    PJSUA_LOCK();
+    pj_list_erase(sess);
+    PJSUA_UNLOCK();
+
+    pj_assert(sess->stun_sock==NULL);
+    pj_pool_release(sess->pool);
+}
+
+
 static void destroy_stun_resolve(pjsua_stun_resolve *sess)
 {
+    pj_time_val timeout = {0, 0};
+
     sess->destroy_flag = PJ_TRUE;
-    if (sess->ref_cnt > 0)
-	return;
 
-    PJSUA_LOCK();
+    /* If the STUN resolution session is blocking, only the waiting thread
+     * is allowed to destroy the session, otherwise it may cause deadlock.
+     */
+    if (sess->blocking) {
+	if (sess->waiter != pj_thread_this())
+	    return;
+
+	/* Before destroying, make sure ref count is zero. */
+	while (sess->ref_cnt > 0)
+	    pj_thread_sleep(10);
+
+    } else if (sess->ref_cnt > 0)
+	return;
 
     if (sess->stun_sock) {
         pj_stun_sock_destroy(sess->stun_sock);
@@ -1175,12 +1204,14 @@ static void destroy_stun_resolve(pjsua_stun_resolve *sess)
         pjsua_var.stun_status = PJNATH_ESTUNDESTROYED;
     }
     
-    pj_list_erase(sess);
-
-    PJSUA_UNLOCK();
-
-    pj_assert(sess->stun_sock==NULL);
-    pj_pool_release(sess->pool);
+    /* Schedule session clean up, it needs PJSUA lock and locking it here
+     * may cause deadlock as this function may be called by STUN socket
+     * while holding STUN socket lock, while application may wait for STUN
+     * resolution while holding PJSUA lock.
+     */
+    pj_timer_entry_init(&sess->timer, 0, (void*)sess,
+			&destroy_stun_resolve_cb);
+    pjsua_schedule_timer(&sess->timer, &timeout);
 }
 
 static void stun_resolve_dec_ref(pjsua_stun_resolve *sess)
@@ -1223,9 +1254,7 @@ static void stun_resolve_complete(pjsua_stun_resolve *sess)
 	PJ_LOG(1,(THIS_FILE, "STUN resolution failed: %s", errmsg));
     }
 
-    stun_resolve_add_ref(sess);
     sess->cb(&result);
-    stun_resolve_dec_ref(sess);
 
 on_return:
     if (!sess->blocking) {
@@ -1257,12 +1286,16 @@ static pj_bool_t test_stun_on_status(pj_stun_sock *stun_sock,
 	pj_stun_sock_destroy(stun_sock);
 	sess->stun_sock = NULL;
 
+	stun_resolve_add_ref(sess);
+
 	++sess->idx;
 	if (sess->idx >= sess->count)
             sess->status = status;
 
 	resolve_stun_entry(sess);
 
+	stun_resolve_dec_ref(sess);
+
 	return PJ_FALSE;
 
     } else if (op == PJ_STUN_SOCK_BINDING_OP) {
@@ -1271,12 +1304,16 @@ static pj_bool_t test_stun_on_status(pj_stun_sock *stun_sock,
 	pj_stun_sock_get_info(stun_sock, &ssi);
 	pj_memcpy(&sess->addr, &ssi.srv_addr, sizeof(sess->addr));
 
+	stun_resolve_add_ref(sess);
+
 	sess->status = PJ_SUCCESS;
 	pj_stun_sock_destroy(stun_sock);
 	sess->stun_sock = NULL;
 
 	stun_resolve_complete(sess);
 
+	stun_resolve_dec_ref(sess);
+
 	return PJ_FALSE;
 
     } else
@@ -1292,8 +1329,6 @@ static void resolve_stun_entry(pjsua_stun_resolve *sess)
 {
     pj_status_t status = PJ_EUNKNOWN;
 
-    stun_resolve_add_ref(sess);
-
     /* Loop while we have entry to try */
     for (; sess->idx < sess->count; ++sess->idx) {
 	const int af = pj_AF_INET();
@@ -1360,19 +1395,18 @@ static void resolve_stun_entry(pjsua_stun_resolve *sess)
 	/* Done for now, testing will resume/complete asynchronously in
 	 * stun_sock_cb()
 	 */
-	goto on_return;
+	return;
     }
 
     if (sess->idx >= sess->count) {
 	/* No more entries to try */
+	stun_resolve_add_ref(sess);
 	pj_assert(status != PJ_SUCCESS || sess->status != PJ_EPENDING);
         if (sess->status == PJ_EPENDING)
             sess->status = status;
 	stun_resolve_complete(sess);
+	stun_resolve_dec_ref(sess);
     }
-
-on_return:
-    stun_resolve_dec_ref(sess);
 }
 
 
@@ -1396,12 +1430,12 @@ PJ_DEF(pj_status_t) pjsua_update_stun_servers(unsigned count, pj_str_t srv[],
     }
     pjsua_var.stun_status = PJ_EUNKNOWN;
 
-    status = resolve_stun_server(wait);
+    PJSUA_UNLOCK();
+    
+    status = resolve_stun_server(wait, PJ_FALSE);
     if (wait == PJ_FALSE && status == PJ_EPENDING)
         status = PJ_SUCCESS;
 
-    PJSUA_UNLOCK();
-    
     return status;
 }
 
@@ -1418,7 +1452,8 @@ PJ_DEF(pj_status_t) pjsua_resolve_stun_servers( unsigned count,
     pj_pool_t *pool;
     pjsua_stun_resolve *sess;
     pj_status_t status;
-    unsigned i;
+    unsigned i, max_wait_ms;
+    pj_timestamp start, now;
 
     PJ_ASSERT_RETURN(count && srv && cb, PJ_EINVAL);
 
@@ -1432,6 +1467,7 @@ PJ_DEF(pj_status_t) pjsua_resolve_stun_servers( unsigned count,
     sess->cb = cb;
     sess->count = count;
     sess->blocking = wait;
+    sess->waiter = pj_thread_this();
     sess->status = PJ_EPENDING;
     sess->srv = (pj_str_t*) pj_pool_calloc(pool, count, sizeof(pj_str_t));
     for (i=0; i<count; ++i) {
@@ -1447,6 +1483,13 @@ PJ_DEF(pj_status_t) pjsua_resolve_stun_servers( unsigned count,
     if (!wait)
 	return PJ_SUCCESS;
 
+    /* Should limit the wait time to avoid deadlock. For example,
+     * if app holds dlg/tsx lock, pjsua worker thread will block on
+     * any dlg/tsx state change.
+     */
+    max_wait_ms = count * pjsua_var.stun_cfg.rto_msec * (1 << 7);
+    pj_get_timestamp(&start);
+
     while (sess->status == PJ_EPENDING) {
         /* If there is no worker thread or
          * the function is called from the only worker thread,
@@ -1460,6 +1503,10 @@ PJ_DEF(pj_status_t) pjsua_resolve_stun_servers( unsigned count,
         } else {
             pj_thread_sleep(20);
         }
+
+	pj_get_timestamp(&now);
+	if (pj_elapsed_msec(&start, &now) > max_wait_ms)
+	    sess->status = PJ_ETIMEDOUT;
     }
 
     status = sess->status;
@@ -1527,8 +1574,16 @@ static void internal_stun_resolve_cb(const pj_stun_resolve_result *result)
 /*
  * Resolve STUN server.
  */
-pj_status_t resolve_stun_server(pj_bool_t wait)
+pj_status_t resolve_stun_server(pj_bool_t wait, pj_bool_t retry_if_cur_error)
 {
+    /* Retry resolving if currently the STUN status is error */
+    if (pjsua_var.stun_status != PJ_EPENDING &&
+	pjsua_var.stun_status != PJ_SUCCESS &&
+	retry_if_cur_error)
+    {
+	pjsua_var.stun_status = PJ_EUNKNOWN;
+    }
+
     if (pjsua_var.stun_status == PJ_EUNKNOWN) {
 	pj_status_t status;
 
@@ -1556,10 +1611,16 @@ pj_status_t resolve_stun_server(pj_bool_t wait)
 	 * result.
 	 */
 	if (wait) {
-	    pj_bool_t has_pjsua_lock = PJSUA_LOCK_IS_LOCKED();
+	    unsigned max_wait_ms;
+	    pj_timestamp start, now;
 
-	    if (has_pjsua_lock)
-		PJSUA_UNLOCK();
+	    /* Should limit the wait time to avoid deadlock. For example,
+	     * if app holds dlg/tsx lock, pjsua worker thread will block on
+	     * any dlg/tsx state change.
+	     */
+	    max_wait_ms = pjsua_var.ua_cfg.stun_srv_cnt *
+			  pjsua_var.stun_cfg.rto_msec * (1 << 7);
+	    pj_get_timestamp(&start);
 
 	    while (pjsua_var.stun_status == PJ_EPENDING) {		
                 /* If there is no worker thread or
@@ -1574,9 +1635,11 @@ pj_status_t resolve_stun_server(pj_bool_t wait)
                 } else {
 		    pj_thread_sleep(10);
                 }
+
+		pj_get_timestamp(&now);
+		if (pj_elapsed_msec(&start, &now) > max_wait_ms)
+		    return PJ_ETIMEDOUT;
 	    }
-	    if (has_pjsua_lock)
-		PJSUA_LOCK();
 	}
     }
 
@@ -1997,7 +2060,7 @@ static pj_status_t create_sip_udp_sock(int af,
     pj_status_t status;
 
     /* Make sure STUN server resolution has completed */
-    status = resolve_stun_server(PJ_TRUE);
+    status = resolve_stun_server(PJ_TRUE, PJ_TRUE);
     if (status != PJ_SUCCESS) {
 	pjsua_perror(THIS_FILE, "Error resolving STUN server", status);
 	return status;
@@ -2784,7 +2847,7 @@ PJ_DEF(pj_status_t) pjsua_detect_nat_type()
 	return PJ_SUCCESS;
 
     /* Make sure STUN server resolution has completed */
-    status = resolve_stun_server(PJ_TRUE);
+    status = resolve_stun_server(PJ_TRUE, PJ_TRUE);
     if (status != PJ_SUCCESS) {
 	pjsua_var.nat_status = status;
 	pjsua_var.nat_type = PJ_STUN_NAT_TYPE_ERR_UNKNOWN;
diff --git a/pjsip/src/pjsua-lib/pjsua_media.c b/pjsip/src/pjsua-lib/pjsua_media.c
index bb8dc49b..cab67ff2 100644
--- a/pjsip/src/pjsua-lib/pjsua_media.c
+++ b/pjsip/src/pjsua-lib/pjsua_media.c
@@ -254,7 +254,10 @@ static pj_status_t create_rtp_rtcp_sock(pjsua_call_media *call_med,
 
     /* Make sure STUN server resolution has completed */
     if (!use_ipv6 && pjsua_media_acc_is_using_stun(call_med->call->acc_id)) {
-	status = resolve_stun_server(PJ_TRUE);
+	pj_bool_t retry_stun = (acc->cfg.media_stun_use &
+				PJSUA_STUN_RETRY_ON_FAILURE) ==
+				PJSUA_STUN_RETRY_ON_FAILURE;
+	status = resolve_stun_server(PJ_TRUE, retry_stun);
 	if (status != PJ_SUCCESS) {
 	    pjsua_perror(THIS_FILE, "Error resolving STUN server", status);
 	    return status;
@@ -390,7 +393,8 @@ static pj_status_t create_rtp_rtcp_sock(pjsua_call_media *call_med,
 #endif
 
 	    if (status != PJ_SUCCESS && pjsua_var.ua_cfg.stun_srv_cnt > 1 &&
-	        ((acc->cfg.media_stun_use & PJSUA_STUN_RETRY_ON_FAILURE)!=0))
+	        ((acc->cfg.media_stun_use & PJSUA_STUN_RETRY_ON_FAILURE)==
+		  PJSUA_STUN_RETRY_ON_FAILURE))
 	    {
 		pj_str_t srv = 
 			     pjsua_var.ua_cfg.stun_srv[pjsua_var.stun_srv_idx];
@@ -418,11 +422,7 @@ static pj_status_t create_rtp_rtcp_sock(pjsua_call_media *call_med,
 		}
 	    	status=pjsua_update_stun_servers(pjsua_var.ua_cfg.stun_srv_cnt,
 	    			   	    	 pjsua_var.ua_cfg.stun_srv,
-	    			   	    	 PJ_FALSE);
-
-		if (status == PJ_SUCCESS)
-		    status = resolve_stun_server(PJ_TRUE);
-
+	    			   	    	 PJ_TRUE);
 	    	if (status == PJ_SUCCESS) {
 		    if (pjsua_var.stun_srv.addr.sa_family != 0) {
     			pj_sockaddr_print(&pjsua_var.stun_srv,
@@ -828,7 +828,10 @@ static pj_status_t create_ice_media_transport(
 
     /* Make sure STUN server resolution has completed */
     if (pjsua_media_acc_is_using_stun(call_med->call->acc_id)) {
-	status = resolve_stun_server(PJ_TRUE);
+	pj_bool_t retry_stun = (acc_cfg->media_stun_use &
+				PJSUA_STUN_RETRY_ON_FAILURE) ==
+				PJSUA_STUN_RETRY_ON_FAILURE;
+	status = resolve_stun_server(PJ_TRUE, retry_stun);
 	if (status != PJ_SUCCESS) {
 	    pjsua_perror(THIS_FILE, "Error resolving STUN server", status);
 	    return status;