From d3a66f9851eacc8dc2ac3e6d13ad139952134400 Mon Sep 17 00:00:00 2001 From: Jan Schmidt Date: Wed, 28 Sep 2022 03:08:39 +1000 Subject: [PATCH] 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: --- subprojects/gstreamer/plugins/elements/gstmultiqueue.c | 11 +++++++++++ subprojects/gstreamer/plugins/elements/gstmultiqueue.h | 2 ++ 2 files changed, 13 insertions(+) diff --git a/subprojects/gstreamer/plugins/elements/gstmultiqueue.c b/subprojects/gstreamer/plugins/elements/gstmultiqueue.c index d23f8ec..bbe29e5 100644 --- 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 --- 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; -- 2.7.4