Changeset 5851

Timestamp:

Aug 1, 2018 9:22:26 AM (6 years ago)

Author:

riza

Message:

Fixed #2137: Race condition in 183 re-transmission can result in a deadlock.

Location:

pjproject/trunk/pjsip

Files:

• include/pjsip/sip_transport.h (modified) (1 diff)
• src/pjsip-ua/sip_100rel.c (modified) (2 diffs)
• src/pjsip-ua/sip_inv.c (modified) (2 diffs)
• src/pjsip/sip_transport.c (modified) (1 diff)

pjproject/trunk/pjsip/include/pjsip/sip_transport.h

@@ -722 +722 @@
                                                  const pjsip_tpselector *sel);
 
+/**
+ * Clone pjsip_tx_data. This will duplicate the message contents of
+ * pjsip_tx_data (pjsip_tx_data.msg) and add reference count to the tdata.
+ * Once application has finished using the cloned pjsip_tx_data,
+ * it must release it by calling  #pjsip_tx_data_dec_ref().
+ * Currently, this will only clone response message.
+ *
+ * @param src       The source to be cloned.
+ * @param flags     Optional flags. Must be zero for now.
+ * @param p_rdata   Pointer to receive the cloned tdata.
+ *
+ * @return          PJ_SUCCESS on success or the appropriate error.
+ */
+PJ_DECL(pj_status_t) pjsip_tx_data_clone(const pjsip_tx_data *src,
+                                         unsigned flags,
+                                         pjsip_tx_data **p_rdata);
 
 /*****************************************************************************

pjproject/trunk/pjsip/src/pjsip-ua/sip_100rel.c

@@ -635 +635 @@
 
 
-/* Clone response. */
-static pjsip_tx_data *clone_tdata(dlg_data *dd,
-                                  const pjsip_tx_data *src)
-{
-    pjsip_tx_data *dst;
-    const pjsip_hdr *hsrc;
-    pjsip_msg *msg;
-    pj_status_t status;
-
-    status = pjsip_endpt_create_tdata(dd->inv->dlg->endpt, &dst);
-    if (status != PJ_SUCCESS)
-        return NULL;
-
-    msg = pjsip_msg_create(dst->pool, PJSIP_RESPONSE_MSG);
-    dst->msg = msg;
-    pjsip_tx_data_add_ref(dst);
-
-    /* Duplicate status line */
-    msg->line.status.code = src->msg->line.status.code;
-    pj_strdup(dst->pool, &msg->line.status.reason, 
-              &src->msg->line.status.reason);
-
-    /* Duplicate all headers */
-    hsrc = src->msg->hdr.next;
-    while (hsrc != &src->msg->hdr) {
-        pjsip_hdr *h = (pjsip_hdr*) pjsip_hdr_clone(dst->pool, hsrc);
-        pjsip_msg_add_hdr(msg, h);
-        hsrc = hsrc->next;
-    }
-
-    /* Duplicate message body */
-    if (src->msg->body)
-        msg->body = pjsip_msg_body_clone(dst->pool, src->msg->body);
-
-    PJ_LOG(5,(dd->inv->dlg->obj_name,
-             "Reliable response %s created",
-             pjsip_tx_data_get_info(dst)));
-
-    return dst;
-}
-
 
 /* Check if any pending response in transmission list has SDP */
@@ -726 +685 @@
      */
     old_tdata = tdata;
-    tdata = clone_tdata(dd, old_tdata);
+    pjsip_tx_data_clone(old_tdata, 0, &tdata);
     pjsip_tx_data_dec_ref(old_tdata);
     

pjproject/trunk/pjsip/src/pjsip-ua/sip_inv.c

@@ -2383 +2383 @@
 {
     pjsip_tx_data *last_res;
+    pjsip_tx_data *old_res;
     pj_status_t status;
 
@@ -2398 +2399 @@
     pjsip_dlg_inc_lock(inv->dlg);
 
+    /* Clone last response.
+     * The tdata (last_answer) is a shared object used by the transaction.
+     * Modifying a shared object might lead to a deadlock.
+     * Refer to ticket #2137 for more detail.
+     */
+    status = pjsip_tx_data_clone(inv->last_answer, 0, &last_res);
+    if (status != PJ_SUCCESS)
+        goto on_return;
+    old_res = inv->last_answer;
+    inv->last_answer = last_res;
+    pjsip_tx_data_dec_ref(old_res);
+
     /* Modify last response. */
-    last_res = inv->last_answer;
     status = pjsip_dlg_modify_response(inv->dlg, last_res, st_code, st_text);
     if (status != PJ_SUCCESS)

pjproject/trunk/pjsip/src/pjsip/sip_transport.c

@@ -644 +644 @@
 }
 
+/* Clone pjsip_tx_data. */
+PJ_DEF(pj_status_t) pjsip_tx_data_clone(const pjsip_tx_data *src,
+                                        unsigned flags,
+                                        pjsip_tx_data ** p_tdata)
+{
+    pjsip_tx_data *dst;
+    const pjsip_hdr *hsrc;
+    pjsip_msg *msg;
+    pj_status_t status;
+
+    PJ_UNUSED_ARG(flags);
+
+    status = pjsip_tx_data_create(src->mgr, p_tdata);
+    if (status != PJ_SUCCESS)
+        return status;
+
+    dst = *p_tdata;
+
+    msg = pjsip_msg_create(dst->pool, PJSIP_RESPONSE_MSG);
+    dst->msg = msg;
+    pjsip_tx_data_add_ref(dst);
+
+    /* Duplicate status line */
+    msg->line.status.code = src->msg->line.status.code;
+    pj_strdup(dst->pool, &msg->line.status.reason,
+              &src->msg->line.status.reason);
+
+    /* Duplicate all headers */
+    hsrc = src->msg->hdr.next;
+    while (hsrc != &src->msg->hdr) {
+        pjsip_hdr *h = (pjsip_hdr*) pjsip_hdr_clone(dst->pool, hsrc);
+        pjsip_msg_add_hdr(msg, h);
+        hsrc = hsrc->next;
+    }
+
+    /* Duplicate message body */
+    if (src->msg->body)
+        msg->body = pjsip_msg_body_clone(dst->pool, src->msg->body);
+
+    dst->is_pending = src->is_pending;
+
+    PJ_LOG(5,(THIS_FILE,
+             "Tx data %s cloned",
+             pjsip_tx_data_get_info(dst)));
+
+    return PJ_SUCCESS;
+}
 
 PJ_DEF(char*) pjsip_rx_data_get_info(pjsip_rx_data *rdata)