multiqueue: Fix potential deadlock with parallel release_pad calls

Commit d3a66f9851ea introduced a potential deadlock with two parallel release_pad
calls, where one could release the main multiqueue lock (qlock) while still
holding the reconf_lock and then calling other routines which in some conditions
may try to acquire qlock again. The second release_pad could already acquire the
qlock and then start waiting on reconf_lock, which may never be possible because
because the first one isn't releasing it until it can acquire qlock.

Fix it by holding reconf_lock for the whole durationg of qlock, making this
particular deadlock impossible.

Fixes #1642

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/3571>

diff --git a/subprojects/gstreamer/plugins/elements/gstmultiqueue.c b/subprojects/gstreamer/plugins/elements/gstmultiqueue.c
index 4cd4448..cf4edc8 100644 (file)
--- a/subprojects/gstreamer/plugins/elements/gstmultiqueue.c
+++ b/subprojects/gstreamer/plugins/elements/gstmultiqueue.c
@@ -1231,6 +1231,12 @@ gst_multi_queue_release_pad (GstElement * element, GstPad * pad)
 
   GST_LOG_OBJECT (element, "pad %s:%s", GST_DEBUG_PAD_NAME (pad));
 
+  /* Take the reconfiguration lock before the qlock to avoid deadlocks
+   * from two release_pad running in parallel on different mqueue slots.
+   * We need reconf_lock for removing the singlequeue from the list, to
+   * prevent overlapping release/request from causing problems */
+  g_mutex_lock (&mqueue->reconf_lock);
+
   GST_MULTI_QUEUE_MUTEX_LOCK (mqueue);
   /* Find which single queue it belongs to, knowing that it should be a sinkpad */
   for (tmp = mqueue->queues; tmp; tmp = g_list_next (tmp)) {
@@ -1250,17 +1256,13 @@ gst_multi_queue_release_pad (GstElement * element, GstPad * pad)
     gst_clear_object (&srcpad);
     GST_WARNING_OBJECT (mqueue, "That pad doesn't belong to this element ???");
     GST_MULTI_QUEUE_MUTEX_UNLOCK (mqueue);
+    g_mutex_unlock (&mqueue->reconf_lock);
     return;
   }
 
   /* FIXME: The removal of the singlequeue should probably not happen until it
    * finishes draining */
 
-  /* Take the reconfiguration lock before removing the singlequeue from
-   * the list, to prevent overlapping release/request from causing
-   * problems */
-  g_mutex_lock (&mqueue->reconf_lock);
-
   /* remove it from the list */
   mqueue->queues = g_list_delete_link (mqueue->queues, tmp);
   mqueue->queues_cookie++;