multiqueue: Protect reconfiguration with a lock

Add a lock to prevent overlapping of request and release
pads, to close a race where multiqueue might try and
add a slot with an id that hasn't quite finished being
removed yet by another thread.

Fix for https://gitlab.freedesktop.org/bilboed/gstreamer/-/issues/5
and https://gitlab.freedesktop.org/bilboed/gstreamer/-/issues/11

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

diff --git a/subprojects/gstreamer/plugins/elements/gstmultiqueue.c b/subprojects/gstreamer/plugins/elements/gstmultiqueue.c
index d23f8ec..bbe29e5 100644 (file)
--- a/subprojects/gstreamer/plugins/elements/gstmultiqueue.c
+++ b/subprojects/gstreamer/plugins/elements/gstmultiqueue.c
@@ -885,6 +885,7 @@ gst_multi_queue_init (GstMultiQueue * mqueue)
   mqueue->high_time = GST_CLOCK_STIME_NONE;
 
   g_mutex_init (&mqueue->qlock);
+  g_mutex_init (&mqueue->reconf_lock);
   g_mutex_init (&mqueue->buffering_post_lock);
 }
 
@@ -899,6 +900,7 @@ gst_multi_queue_finalize (GObject * object)
 
   /* free/unref instance data */
   g_mutex_clear (&mqueue->qlock);
+  g_mutex_clear (&mqueue->reconf_lock);
   g_mutex_clear (&mqueue->buffering_post_lock);
 
   G_OBJECT_CLASS (parent_class)->finalize (object);
@@ -1199,8 +1201,10 @@ gst_multi_queue_request_new_pad (GstElement * element, GstPadTemplate * temp,
     GST_LOG_OBJECT (element, "name : %s (id %d)", GST_STR_NULL (name), temp_id);
   }
 
+  g_mutex_lock (&mqueue->reconf_lock);
   /* Create a new single queue, add the sink and source pad and return the sink pad */
   squeue = gst_single_queue_new (mqueue, temp_id);
+  g_mutex_unlock (&mqueue->reconf_lock);
 
   new_pad = squeue ? g_weak_ref_get (&squeue->sinkpad) : NULL;
   /* request pad assumes the element is owning the ref of the pad it returns */
@@ -1247,6 +1251,11 @@ gst_multi_queue_release_pad (GstElement * element, GstPad * pad)
   /* 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++;
@@ -1263,6 +1272,8 @@ gst_multi_queue_release_pad (GstElement * element, GstPad * pad)
   gst_element_remove_pad (element, sinkpad);
   gst_object_unref (srcpad);
   gst_object_unref (sinkpad);
+
+  g_mutex_unlock (&mqueue->reconf_lock);
 }
 
 static GstStateChangeReturn

diff --git a/subprojects/gstreamer/plugins/elements/gstmultiqueue.h b/subprojects/gstreamer/plugins/elements/gstmultiqueue.h
index 8c89a17..53ceacb 100644 (file)
--- a/subprojects/gstreamer/plugins/elements/gstmultiqueue.h
+++ b/subprojects/gstreamer/plugins/elements/gstmultiqueue.h
@@ -78,6 +78,8 @@ struct _GstMultiQueue {
                        /* queues lock). Protects nbqueues, queues, global */
                        /* GstMultiQueueSize, counter and highid */
 
+  GMutex   reconf_lock;        /* Reconfiguration lock, held during request/release pads */
+
   gint numwaiting;     /* number of not-linked pads waiting */
 
   gboolean buffering_percent_changed;