Fixed race condition in ACK handling of INVITE message (#3752)

pjsip/include/pjsip/sip_transaction.h

@@ -410,11 +410,26 @@ PJ_DECL(pj_status_t) pjsip_tsx_create_key( pj_pool_t *pool,
  *
  * @param tsx       The transaction.
  * @param code      The status code to report.
+ *
+ * @return          PJ_SUCCESS or the appropriate error code.
  */
 PJ_DECL(pj_status_t) pjsip_tsx_terminate( pjsip_transaction *tsx,
                                           int code );
 
 
+/**
+ * Force terminate transaction asynchronously, using the transaction
+ * internal timer.
+ *
+ * @param tsx       The transaction.
+ * @param code      The status code to report.
+ *
+ * @return          PJ_SUCCESS or the appropriate error code.
+ */
+PJ_DECL(pj_status_t) pjsip_tsx_terminate_async(pjsip_transaction *tsx,
+                                               int code );
+
+
 /**
  * Cease retransmission on the UAC transaction. The UAC transaction is
  * still considered running, and it will complete when either final

pjsip/src/pjsip-ua/sip_inv.c

@@ -698,13 +698,25 @@ static pj_bool_t mod_inv_on_rx_request(pjsip_rx_data *rdata)
             }
 
             /* Now we can terminate the INVITE transaction */
-            pj_assert(inv->invite_tsx->status_code >= 200);
-            pjsip_tsx_terminate(inv->invite_tsx, 
-                                inv->invite_tsx->status_code);
+            if (inv->invite_tsx->status_code/100 == 2) {
+                pjsip_tsx_terminate(inv->invite_tsx,
+                                    inv->invite_tsx->status_code);
+            } else {
+                /* If the response was not 2xx, the ACK is considered part of
+                 * the INVITE transaction, so should have been handled by
+                 * the transaction.
+                 * But for best effort, we will also attempt to terminate
+                 * the tsx here. However, we need to do it asynchronously
+                 * to avoid deadlock.
+                 */
+                pjsip_tsx_terminate_async(inv->invite_tsx,
+                                          inv->invite_tsx->status_code);
+            }
             inv->invite_tsx = NULL;
+
             if (inv->last_answer) {
-                    pjsip_tx_data_dec_ref(inv->last_answer);
-                    inv->last_answer = NULL;
+                pjsip_tx_data_dec_ref(inv->last_answer);
+                inv->last_answer = NULL;
             }
         }
 

pjsip/src/pjsip/sip_transaction.c

@@ -139,6 +139,7 @@ static int max_retrans_count = -1;
 #define TIMEOUT_TIMER           2
 #define TRANSPORT_ERR_TIMER     3
 #define TRANSPORT_DISC_TIMER    4
+#define TERMINATE_TIMER         5
 
 /* Flags for tsx_set_state() */
 enum
@@ -953,6 +954,18 @@ static pj_bool_t mod_tsx_layer_on_rx_request(pjsip_rx_data *rdata)
         return PJ_FALSE;
     }
 
+    /* In the case of an INVITE transaction, if the response was a 2xx,
+     * the ACK is not considered part of the transaction.
+     * Let sip_dialog and sip_inv handle it instead.
+     */
+    if (rdata->msg_info.msg->line.req.method.id == PJSIP_ACK_METHOD &&
+        tsx->method.id == PJSIP_INVITE_METHOD &&
+        tsx->status_code/100 == 2)
+    {
+        pj_mutex_unlock( mod_tsx_layer.mutex);
+        return PJ_FALSE;
+    }
+
     /* Prevent the transaction to get deleted before we have chance to lock it
      * in pjsip_tsx_recv_msg().
      */
@@ -1253,7 +1266,13 @@ static void tsx_timer_callback( pj_timer_heap_t *theap, pj_timer_entry *entry)
         return;
     }
 
-    if (entry->id == TRANSPORT_ERR_TIMER || entry->id == TRANSPORT_DISC_TIMER)
+
+    if (entry->id == TERMINATE_TIMER) {
+        if (tsx->state < PJSIP_TSX_STATE_TERMINATED) {
+            pjsip_tsx_terminate(tsx, tsx->status_code);
+        }
+    } else if (entry->id == TRANSPORT_ERR_TIMER ||
+               entry->id == TRANSPORT_DISC_TIMER)
     {
         /* Posted transport error/disconnection event */
         pj_bool_t tp_disc = (entry->id == TRANSPORT_DISC_TIMER);
@@ -1868,6 +1887,30 @@ PJ_DEF(pj_status_t) pjsip_tsx_terminate( pjsip_transaction *tsx, int code )
 }
 
 
+/*
+ * Force terminate transaction asynchronously, using the transaction
+ * internal timer.
+ */
+PJ_DEF(pj_status_t) pjsip_tsx_terminate_async(pjsip_transaction *tsx,
+                                              int code )
+{
+    pj_time_val delay = {0, 100};
+
+    PJ_ASSERT_RETURN(tsx != NULL, PJ_EINVAL);
+
+    PJ_LOG(5,(tsx->obj_name, "Request to terminate transaction async"));
+
+    PJ_ASSERT_RETURN(code >= 200, PJ_EINVAL);
+
+    lock_timer(tsx);
+    tsx_cancel_timer(tsx, &tsx->timeout_timer);
+    tsx_schedule_timer(tsx, &tsx->timeout_timer, &delay, TERMINATE_TIMER);
+    unlock_timer(tsx);
+
+    return PJ_SUCCESS;
+}
+
+
 /*
  * Cease retransmission on the UAC transaction. The UAC transaction is
  * still considered running, and it will complete when either final

tests/pjsua/scripts-sipp/uas-reinv-with-less-media.xml

@@ -100,7 +100,7 @@
       From: sipp  <sip:sipp@[local_ip]:[local_port]>;tag=[call_number]
       To[$3]
       Call-ID: [call_id]
-      Cseq: 1 ACK
+      Cseq: 2 ACK
       Contact: sip:sipp@[local_ip]:[local_port]
       Max-Forwards: 70
       Content-Length: 0