Changeset 1790

Timestamp:

Feb 13, 2008 4:59:29 PM (17 years ago)

Author:

bennylp

Message:

Ticket #460: Concurrency problem when destroying stream (thanks Michael Broughton)

Location:

pjproject/trunk

Files:

• pjmedia/src/pjmedia/stream.c (modified) (1 diff)
• pjmedia/src/pjmedia/transport_ice.c (modified) (1 diff)
• pjmedia/src/pjmedia/transport_udp.c (modified) (5 diffs)
• pjnath/src/pjnath/ice_strans.c (modified) (1 diff)

pjproject/trunk/pjmedia/src/pjmedia/stream.c

@@ -1400 +1400 @@
 
 
+    /* Detach from transport 
+     * MUST NOT hold stream mutex while detaching from transport, as
+     * it may cause deadlock. See ticket #460 for the details.
+     */
+    if (stream->transport) {
+        pjmedia_transport_detach(stream->transport, stream);
+        stream->transport = NULL;
+    }
+
     /* This function may be called when stream is partly initialized. */
     if (stream->jb_mutex)
         pj_mutex_lock(stream->jb_mutex);
 
-
-    /* Detach from transport */
-    if (stream->transport) {
-        (*stream->transport->op->detach)(stream->transport, stream);
-        stream->transport = NULL;
-    }
 
     /* Free codec. */

pjproject/trunk/pjmedia/src/pjmedia/transport_ice.c

@@ -699 +699 @@
     struct transport_ice *tp_ice = (struct transport_ice*)tp;
 
+    /* TODO: need to solve ticket #460 here */
+
     tp_ice->rtp_cb = NULL;
     tp_ice->rtcp_cb = NULL;

pjproject/trunk/pjmedia/src/pjmedia/transport_udp.c

@@ -320 +320 @@
         goto on_error;
     
+    /* Disallow concurrency so that detach() and destroy() are
+     * synchronized with the callback.
+     */
+    status = pj_ioqueue_set_concurrency(tp->rtp_key, PJ_FALSE);
+    if (status != PJ_SUCCESS)
+        goto on_error;
+
     pj_ioqueue_op_key_init(&tp->rtp_read_op, sizeof(tp->rtp_read_op));
     for (i=0; i<PJ_ARRAY_SIZE(tp->rtp_pending_write); ++i)
@@ -344 +351 @@
         goto on_error;
 
+    status = pj_ioqueue_set_concurrency(tp->rtcp_key, PJ_FALSE);
+    if (status != PJ_SUCCESS)
+        goto on_error;
+
     pj_ioqueue_op_key_init(&tp->rtcp_read_op, sizeof(tp->rtcp_read_op));
     pj_ioqueue_op_key_init(&tp->rtcp_write_op, sizeof(tp->rtcp_write_op));
@@ -384 +395 @@
 
     if (udp->rtp_key) {
+        /* This will block the execution if callback is still
+         * being called.
+         */
         pj_ioqueue_unregister(udp->rtp_key);
         udp->rtp_key = NULL;
@@ -646 +660 @@
 
     if (udp->attached) {
+        /* Lock the ioqueue keys to make sure that callbacks are
+         * not executed. See ticket #460 for details.
+         */
+        pj_ioqueue_lock_key(udp->rtp_key);
+        pj_ioqueue_lock_key(udp->rtcp_key);
+
         /* User data is unreferenced on Release build */
         PJ_UNUSED_ARG(user_data);
@@ -659 +679 @@
         udp->rtcp_cb = NULL;
         udp->user_data = NULL;
+
+        /* Unlock keys */
+        pj_ioqueue_unlock_key(udp->rtcp_key);
+        pj_ioqueue_unlock_key(udp->rtp_key);
     }
 }

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

@@ -329 +329 @@
                                       comp->sock, comp, &ioqueue_cb, 
                                       &comp->key);
+    if (status != PJ_SUCCESS)
+        goto on_error;
+
+    /* Disable concurrency */
+    status = pj_ioqueue_set_concurrency(comp->key, PJ_FALSE);
     if (status != PJ_SUCCESS)
         goto on_error;