1 files changed, 77 insertions, 58 deletions

diff --git a/pjnath/src/pjnath/stun_transaction.c b/pjnath/src/pjnath/stun_transaction.c
index 8677d322..390f67f7 100644
--- a/pjnath/src/pjnath/stun_transaction.c
+++ b/pjnath/src/pjnath/stun_transaction.c
@@ -26,6 +26,8 @@
 #include <pj/timer.h>
 
 
+#define THIS_FILE		"stun_transaction.c"
+#define TIMER_INACTIVE		0
 #define TIMER_ACTIVE		1
 
 
@@ -34,6 +36,7 @@ struct pj_stun_client_tsx
     char		 obj_name[PJ_MAX_OBJ_NAME];
     pj_stun_tsx_cb	 cb;
     void		*user_data;
+    pj_grp_lock_t	*grp_lock;
 
     pj_bool_t		 complete;
 
@@ -51,18 +54,24 @@ struct pj_stun_client_tsx
 };
 
 
+#if 1
+#   define TRACE_(expr)		    PJ_LOG(5,expr)
+#else
+#   define TRACE_(expr)
+#endif
+
+
 static void retransmit_timer_callback(pj_timer_heap_t *timer_heap, 
 				      pj_timer_entry *timer);
 static void destroy_timer_callback(pj_timer_heap_t *timer_heap, 
 				   pj_timer_entry *timer);
 
-#define stun_perror(tsx,msg,rc) pjnath_perror(tsx->obj_name, msg, rc)
-
 /*
  * Create a STUN client transaction.
  */
 PJ_DEF(pj_status_t) pj_stun_client_tsx_create(pj_stun_config *cfg,
 					      pj_pool_t *pool,
+					      pj_grp_lock_t *grp_lock,
 					      const pj_stun_tsx_cb *cb,
 					      pj_stun_client_tsx **p_tsx)
 {
@@ -74,6 +83,7 @@ PJ_DEF(pj_status_t) pj_stun_client_tsx_create(pj_stun_config *cfg,
     tsx = PJ_POOL_ZALLOC_T(pool, pj_stun_client_tsx);
     tsx->rto_msec = cfg->rto_msec;
     tsx->timer_heap = cfg->timer_heap;
+    tsx->grp_lock = grp_lock;
     pj_memcpy(&tsx->cb, cb, sizeof(*cb));
 
     tsx->retransmit_timer.cb = &retransmit_timer_callback;
@@ -82,7 +92,7 @@ PJ_DEF(pj_status_t) pj_stun_client_tsx_create(pj_stun_config *cfg,
     tsx->destroy_timer.cb = &destroy_timer_callback;
     tsx->destroy_timer.user_data = tsx;
 
-    pj_ansi_snprintf(tsx->obj_name, sizeof(tsx->obj_name), "stuntsx%p", tsx);
+    pj_ansi_snprintf(tsx->obj_name, sizeof(tsx->obj_name), "utsx%p", tsx);
 
     *p_tsx = tsx;
 
@@ -100,26 +110,30 @@ PJ_DEF(pj_status_t) pj_stun_client_tsx_schedule_destroy(
     PJ_ASSERT_RETURN(tsx && delay, PJ_EINVAL);
     PJ_ASSERT_RETURN(tsx->cb.on_destroy, PJ_EINVAL);
 
+    pj_grp_lock_acquire(tsx->grp_lock);
+
     /* Cancel previously registered timer */
-    if (tsx->destroy_timer.id != 0) {
-	pj_timer_heap_cancel(tsx->timer_heap, &tsx->destroy_timer);
-	tsx->destroy_timer.id = 0;
-    }
+    pj_timer_heap_cancel_if_active(tsx->timer_heap, &tsx->destroy_timer,
+                                   TIMER_INACTIVE);
 
     /* Stop retransmission, just in case */
-    if (tsx->retransmit_timer.id != 0) {
-	pj_timer_heap_cancel(tsx->timer_heap, &tsx->retransmit_timer);
-	tsx->retransmit_timer.id = 0;
-    }
+    pj_timer_heap_cancel_if_active(tsx->timer_heap, &tsx->retransmit_timer,
+                                   TIMER_INACTIVE);
 
-    status = pj_timer_heap_schedule(tsx->timer_heap,
-				    &tsx->destroy_timer, delay);
-    if (status != PJ_SUCCESS)
+    status = pj_timer_heap_schedule_w_grp_lock(tsx->timer_heap,
+                                               &tsx->destroy_timer, delay,
+                                               TIMER_ACTIVE, tsx->grp_lock);
+    if (status != PJ_SUCCESS) {
+	pj_grp_lock_release(tsx->grp_lock);
 	return status;
+    }
 
-    tsx->destroy_timer.id = TIMER_ACTIVE;
     tsx->cb.on_complete = NULL;
 
+    pj_grp_lock_release(tsx->grp_lock);
+
+    TRACE_((tsx->obj_name, "STUN transaction %p schedule destroy", tsx));
+
     return PJ_SUCCESS;
 }
 
@@ -127,20 +141,21 @@ PJ_DEF(pj_status_t) pj_stun_client_tsx_schedule_destroy(
 /*
  * Destroy transaction immediately.
  */
-PJ_DEF(pj_status_t) pj_stun_client_tsx_destroy(pj_stun_client_tsx *tsx)
+PJ_DEF(pj_status_t) pj_stun_client_tsx_stop(pj_stun_client_tsx *tsx)
 {
     PJ_ASSERT_RETURN(tsx, PJ_EINVAL);
 
-    if (tsx->retransmit_timer.id != 0) {
-	pj_timer_heap_cancel(tsx->timer_heap, &tsx->retransmit_timer);
-	tsx->retransmit_timer.id = 0;
-    }
-    if (tsx->destroy_timer.id != 0) {
-	pj_timer_heap_cancel(tsx->timer_heap, &tsx->destroy_timer);
-	tsx->destroy_timer.id = 0;
-    }
+    /* Don't call grp_lock_acquire() because we might be called on
+     * group lock's destructor.
+     */
+    pj_timer_heap_cancel_if_active(tsx->timer_heap, &tsx->retransmit_timer,
+                                   TIMER_INACTIVE);
+    pj_timer_heap_cancel_if_active(tsx->timer_heap, &tsx->destroy_timer,
+                                   TIMER_INACTIVE);
+
+    PJ_LOG(5,(tsx->obj_name, "STUN client transaction %p stopped, ref_cnt=%d",
+	      tsx, pj_grp_lock_get_ref(tsx->grp_lock)));
 
-    PJ_LOG(5,(tsx->obj_name, "STUN client transaction destroyed"));
     return PJ_SUCCESS;
 }
 
@@ -185,7 +200,7 @@ static pj_status_t tsx_transmit_msg(pj_stun_client_tsx *tsx,
 {
     pj_status_t status;
 
-    PJ_ASSERT_RETURN(tsx->retransmit_timer.id == 0 ||
+    PJ_ASSERT_RETURN(tsx->retransmit_timer.id == TIMER_INACTIVE ||
 		     !tsx->require_retransmit, PJ_EBUSY);
 
     if (tsx->require_retransmit && mod_count) {
@@ -211,14 +226,15 @@ static pj_status_t tsx_transmit_msg(pj_stun_client_tsx *tsx,
 	 * cancel it (as opposed to when schedule_timer() failed we cannot
 	 * cancel transmission).
 	 */;
-	status = pj_timer_heap_schedule(tsx->timer_heap, 
-					&tsx->retransmit_timer,
-					&tsx->retransmit_time);
+	status = pj_timer_heap_schedule_w_grp_lock(tsx->timer_heap,
+						   &tsx->retransmit_timer,
+						   &tsx->retransmit_time,
+						   TIMER_ACTIVE,
+						   tsx->grp_lock);
 	if (status != PJ_SUCCESS) {
-	    tsx->retransmit_timer.id = 0;
+	    tsx->retransmit_timer.id = TIMER_INACTIVE;
 	    return status;
 	}
-	tsx->retransmit_timer.id = TIMER_ACTIVE;
     }
 
 
@@ -235,12 +251,12 @@ static pj_status_t tsx_transmit_msg(pj_stun_client_tsx *tsx,
     if (status == PJNATH_ESTUNDESTROYED) {
 	/* We've been destroyed, don't access the object. */
     } else if (status != PJ_SUCCESS) {
-	if (tsx->retransmit_timer.id != 0 && mod_count) {
-	    pj_timer_heap_cancel(tsx->timer_heap, 
-				 &tsx->retransmit_timer);
-	    tsx->retransmit_timer.id = 0;
+	if (mod_count) {
+		pj_timer_heap_cancel_if_active( tsx->timer_heap,
+	                               		&tsx->retransmit_timer,
+	                               		TIMER_INACTIVE);
 	}
-	stun_perror(tsx, "STUN error sending message", status);
+	PJ_PERROR(4, (tsx->obj_name, status, "STUN error sending message"));
     }
 
     pj_log_pop_indent();
@@ -261,6 +277,8 @@ PJ_DEF(pj_status_t) pj_stun_client_tsx_send_msg(pj_stun_client_tsx *tsx,
     PJ_ASSERT_RETURN(tsx && pkt && pkt_len, PJ_EINVAL);
     PJ_ASSERT_RETURN(tsx->retransmit_timer.id == 0, PJ_EBUSY);
 
+    pj_grp_lock_acquire(tsx->grp_lock);
+
     /* Encode message */
     tsx->last_pkt = pkt;
     tsx->last_pkt_size = pkt_len;
@@ -286,27 +304,29 @@ PJ_DEF(pj_status_t) pj_stun_client_tsx_send_msg(pj_stun_client_tsx *tsx,
 	 * cancel it (as opposed to when schedule_timer() failed we cannot
 	 * cancel transmission).
 	 */;
-	status = pj_timer_heap_schedule(tsx->timer_heap, 
-					&tsx->retransmit_timer,
-					&tsx->retransmit_time);
+	status = pj_timer_heap_schedule_w_grp_lock(tsx->timer_heap,
+	                                           &tsx->retransmit_timer,
+	                                           &tsx->retransmit_time,
+	                                           TIMER_ACTIVE,
+	                                           tsx->grp_lock);
 	if (status != PJ_SUCCESS) {
-	    tsx->retransmit_timer.id = 0;
+	    tsx->retransmit_timer.id = TIMER_INACTIVE;
+	    pj_grp_lock_release(tsx->grp_lock);
 	    return status;
 	}
-	tsx->retransmit_timer.id = TIMER_ACTIVE;
     }
 
     /* Send the message */
     status = tsx_transmit_msg(tsx, PJ_TRUE);
     if (status != PJ_SUCCESS) {
-	if (tsx->retransmit_timer.id != 0) {
-	    pj_timer_heap_cancel(tsx->timer_heap, 
-				 &tsx->retransmit_timer);
-	    tsx->retransmit_timer.id = 0;
-	}
+	pj_timer_heap_cancel_if_active(tsx->timer_heap,
+	                               &tsx->retransmit_timer,
+	                               TIMER_INACTIVE);
+	pj_grp_lock_release(tsx->grp_lock);
 	return status;
     }
 
+    pj_grp_lock_release(tsx->grp_lock);
     return PJ_SUCCESS;
 }
 
@@ -319,6 +339,7 @@ static void retransmit_timer_callback(pj_timer_heap_t *timer_heap,
     pj_status_t status;
 
     PJ_UNUSED_ARG(timer_heap);
+    pj_grp_lock_acquire(tsx->grp_lock);
 
     if (tsx->transmit_count >= PJ_STUN_MAX_TRANSMIT_COUNT) {
 	/* Retransmission count exceeded. Transaction has failed */
@@ -331,6 +352,7 @@ static void retransmit_timer_callback(pj_timer_heap_t *timer_heap,
 		tsx->cb.on_complete(tsx, PJNATH_ESTUNTIMEDOUT, NULL, NULL, 0);
 	    }
 	}
+	pj_grp_lock_release(tsx->grp_lock);
 	/* We might have been destroyed, don't try to access the object */
 	pj_log_pop_indent();
 	return;
@@ -338,9 +360,7 @@ static void retransmit_timer_callback(pj_timer_heap_t *timer_heap,
 
     tsx->retransmit_timer.id = 0;
     status = tsx_transmit_msg(tsx, PJ_TRUE);
-    if (status == PJNATH_ESTUNDESTROYED) {
-	/* We've been destroyed, don't try to access the object */
-    } else if (status != PJ_SUCCESS) {
+    if (status != PJ_SUCCESS) {
 	tsx->retransmit_timer.id = 0;
 	if (!tsx->complete) {
 	    tsx->complete = PJ_TRUE;
@@ -348,8 +368,10 @@ static void retransmit_timer_callback(pj_timer_heap_t *timer_heap,
 		tsx->cb.on_complete(tsx, status, NULL, NULL, 0);
 	    }
 	}
-	/* We might have been destroyed, don't try to access the object */
     }
+
+    pj_grp_lock_release(tsx->grp_lock);
+    /* We might have been destroyed, don't try to access the object */
 }
 
 /*
@@ -362,10 +384,8 @@ PJ_DEF(pj_status_t) pj_stun_client_tsx_retransmit(pj_stun_client_tsx *tsx,
 	return PJ_SUCCESS;
     }
 
-    if (tsx->retransmit_timer.id != 0) {
-	pj_timer_heap_cancel(tsx->timer_heap, &tsx->retransmit_timer);
-	tsx->retransmit_timer.id = 0;
-    }
+    pj_timer_heap_cancel_if_active(tsx->timer_heap, &tsx->retransmit_timer,
+                                   TIMER_INACTIVE);
 
     return tsx_transmit_msg(tsx, mod_count);
 }
@@ -379,6 +399,7 @@ static void destroy_timer_callback(pj_timer_heap_t *timer_heap,
     PJ_UNUSED_ARG(timer_heap);
 
     tsx->destroy_timer.id = PJ_FALSE;
+
     tsx->cb.on_destroy(tsx);
     /* Don't access transaction after this */
 }
@@ -408,10 +429,8 @@ PJ_DEF(pj_status_t) pj_stun_client_tsx_on_rx_msg(pj_stun_client_tsx *tsx,
     /* We have a response with matching transaction ID. 
      * We can cancel retransmit timer now.
      */
-    if (tsx->retransmit_timer.id) {
-	pj_timer_heap_cancel(tsx->timer_heap, &tsx->retransmit_timer);
-	tsx->retransmit_timer.id = 0;
-    }
+    pj_timer_heap_cancel_if_active(tsx->timer_heap, &tsx->retransmit_timer,
+                                   TIMER_INACTIVE);
 
     /* Find STUN error code attribute */
     err_attr = (pj_stun_errcode_attr*)