From 62ca319927c15c984fb587e357cfbbf994eca717 Mon Sep 17 00:00:00 2001 From: Jan Schmidt Date: Wed, 13 Jul 2016 23:02:10 +1000 Subject: [PATCH] adaptivedemux: Drop the manifest lock during outbound actions Drop the manifest lock when performing actions that might call back into adaptivedemux and trigger deadlocks, such as adding/removing pads or sending in-band events (EOS). Unlock the manifest lock when changing the child bin state to NULL, as it might call back to acquire the manifest lock when shutting down pads. Drop the manifest lock while pushing events. --- gst-libs/gst/adaptivedemux/gstadaptivedemux.c | 50 +++++++++++++++++-- 1 file changed, 45 insertions(+), 5 deletions(-) diff --git a/gst-libs/gst/adaptivedemux/gstadaptivedemux.c b/gst-libs/gst/adaptivedemux/gstadaptivedemux.c index 51418d32fe..fc82be2da6 100644 --- a/gst-libs/gst/adaptivedemux/gstadaptivedemux.c +++ b/gst-libs/gst/adaptivedemux/gstadaptivedemux.c @@ -138,8 +138,16 @@ GST_DEBUG_CATEGORY (adaptivedemux_debug); #define NUM_LOOKBACK_FRAGMENTS 3 #define GST_MANIFEST_GET_LOCK(d) (&(GST_ADAPTIVE_DEMUX_CAST(d)->priv->manifest_lock)) -#define GST_MANIFEST_LOCK(d) g_rec_mutex_lock (GST_MANIFEST_GET_LOCK (d)); -#define GST_MANIFEST_UNLOCK(d) g_rec_mutex_unlock (GST_MANIFEST_GET_LOCK (d)); +#define GST_MANIFEST_LOCK(d) G_STMT_START { \ + GST_TRACE("Locking from thread %p", g_thread_self()); \ + g_rec_mutex_lock (GST_MANIFEST_GET_LOCK (d)); \ + GST_TRACE("Locked from thread %p", g_thread_self()); \ + } G_STMT_END + +#define GST_MANIFEST_UNLOCK(d) G_STMT_START { \ + GST_TRACE("Unlocking from thread %p", g_thread_self()); \ + g_rec_mutex_unlock (GST_MANIFEST_GET_LOCK (d)); \ + } G_STMT_END #define GST_API_GET_LOCK(d) (&(GST_ADAPTIVE_DEMUX_CAST(d)->priv->api_lock)) #define GST_API_LOCK(d) g_mutex_lock (GST_API_GET_LOCK (d)); @@ -839,6 +847,7 @@ static gboolean gst_adaptive_demux_expose_stream (GstAdaptiveDemux * demux, GstAdaptiveDemuxStream * stream) { + gboolean ret; GstPad *pad = stream->pad; gchar *name = gst_pad_get_name (pad); GstEvent *event; @@ -883,7 +892,12 @@ gst_adaptive_demux_expose_stream (GstAdaptiveDemux * demux, gst_object_ref (pad); - return gst_element_add_pad (GST_ELEMENT_CAST (demux), pad); + /* Don't hold the manifest lock while exposing a pad */ + GST_MANIFEST_UNLOCK (demux); + ret = gst_element_add_pad (GST_ELEMENT_CAST (demux), pad); + GST_MANIFEST_LOCK (demux); + + return ret; } /* must be called with manifest_lock taken */ @@ -1053,9 +1067,12 @@ gst_adaptive_demux_expose_streams (GstAdaptiveDemux * demux, GstAdaptiveDemuxStream *stream = iter->data; GST_LOG_OBJECT (stream->pad, "Removing stream"); + GST_MANIFEST_UNLOCK (demux); + gst_pad_push_event (stream->pad, gst_event_ref (eos)); gst_pad_set_active (stream->pad, FALSE); gst_element_remove_pad (GST_ELEMENT (demux), stream->pad); + GST_MANIFEST_LOCK (demux); /* ask the download task to stop. * We will not join it now, because our thread can be one of these tasks. @@ -1069,6 +1086,7 @@ gst_adaptive_demux_expose_streams (GstAdaptiveDemux * demux, * Even if it doesn't do that, we will change its state later in * gst_adaptive_demux_stop_tasks. */ + GST_LOG_OBJECT (stream, "Marking stream as cancelled"); gst_task_stop (stream->download_task); g_mutex_lock (&stream->fragment_download_lock); stream->cancelled = TRUE; @@ -1132,6 +1150,7 @@ gst_adaptive_demux_find_stream_for_pad (GstAdaptiveDemux * demux, GstPad * pad) return stream; } } + return NULL; } @@ -1200,9 +1219,11 @@ gst_adaptive_demux_stream_free (GstAdaptiveDemuxStream * stream) } if (stream->src) { + GST_MANIFEST_UNLOCK (demux); gst_element_set_state (stream->src, GST_STATE_NULL); gst_bin_remove (GST_BIN_CAST (demux), stream->src); stream->src = NULL; + GST_MANIFEST_LOCK (demux); } g_cond_clear (&stream->fragment_download_cond); @@ -1374,7 +1395,9 @@ gst_adaptive_demux_handle_seek_event (GstAdaptiveDemux * demux, GstPad * pad, } } - demux_class->stream_seek (stream, rate >= 0, stream_seek_flags, ts, &ts); + if (stream) { + demux_class->stream_seek (stream, rate >= 0, stream_seek_flags, ts, &ts); + } /* replace event with a new one without snaping to seek on all streams */ gst_event_unref (event); @@ -1668,6 +1691,8 @@ gst_adaptive_demux_stop_tasks (GstAdaptiveDemux * demux) gst_uri_downloader_cancel (demux->downloader); + GST_LOG_OBJECT (demux, "Stopping tasks"); + for (iter = demux->streams; iter; iter = g_list_next (iter)) { GstAdaptiveDemuxStream *stream = iter->data; @@ -1985,6 +2010,7 @@ gst_adaptive_demux_stream_push_buffer (GstAdaptiveDemuxStream * stream, g_mutex_lock (&stream->fragment_download_lock); if (G_UNLIKELY (stream->cancelled)) { + GST_LOG_OBJECT (stream, "Stream was cancelled"); ret = stream->last_ret = GST_FLOW_FLUSHING; g_mutex_unlock (&stream->fragment_download_lock); return ret; @@ -3182,13 +3208,27 @@ gst_adaptive_demux_stream_push_event (GstAdaptiveDemuxStream * stream, GstEvent * event) { gboolean ret; + GstPad *pad; + GstAdaptiveDemux *demux = stream->demux; if (GST_EVENT_TYPE (event) == GST_EVENT_EOS) { stream->eos = TRUE; } + + pad = gst_object_ref (GST_ADAPTIVE_DEMUX_STREAM_PAD (stream)); + + /* Can't push events holding the manifest lock */ + GST_MANIFEST_UNLOCK (demux); + GST_DEBUG_OBJECT (GST_ADAPTIVE_DEMUX_STREAM_PAD (stream), "Pushing event %" GST_PTR_FORMAT, event); - ret = gst_pad_push_event (GST_ADAPTIVE_DEMUX_STREAM_PAD (stream), event); + + ret = gst_pad_push_event (pad, event); + + gst_object_unref (pad); + + GST_MANIFEST_LOCK (demux); + return ret; }