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>
This commit is contained in:
Jan Schmidt 2022-09-28 03:08:39 +10:00 committed by GStreamer Marge Bot
parent 4b8f411c5d
commit d3a66f9851
2 changed files with 13 additions and 0 deletions

View file

@ -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

View file

@ -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;