From 3db0222b32f711e62e798335a94a532e969d3b09 Mon Sep 17 00:00:00 2001 From: Jan Schmidt Date: Tue, 26 Jul 2016 01:17:11 +1000 Subject: 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 | 56 +++++++++++++++++++++++---- 1 file changed, 48 insertions(+), 8 deletions(-) diff --git a/gst-libs/gst/adaptivedemux/gstadaptivedemux.c b/gst-libs/gst/adaptivedemux/gstadaptivedemux.c index 8b4426754..729935457 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)); @@ -798,6 +806,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; @@ -842,7 +851,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 */ @@ -1012,9 +1026,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. @@ -1028,6 +1045,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; @@ -1092,6 +1110,7 @@ gst_adaptive_demux_find_stream_for_pad (GstAdaptiveDemux * demux, GstPad * pad) return stream; } } + return NULL; } @@ -1160,9 +1179,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); @@ -1240,7 +1261,7 @@ gst_adaptive_demux_src_event (GstPad * pad, GstObject * parent, gint64 start, stop; guint32 seqnum; gboolean update; - gboolean ret = TRUE; + gboolean ret; GstSegment oldsegment; GstAdaptiveDemuxStream *stream = NULL; @@ -1344,8 +1365,10 @@ gst_adaptive_demux_src_event (GstPad * pad, GstObject * parent, } } - 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); @@ -1432,8 +1455,8 @@ gst_adaptive_demux_src_event (GstPad * pad, GstObject * parent, gst_adaptive_demux_start_tasks (demux); GST_MANIFEST_UNLOCK (demux); GST_API_UNLOCK (demux); - gst_event_unref (event); + return ret; } case GST_EVENT_RECONFIGURE:{ @@ -1623,6 +1646,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; @@ -1941,6 +1966,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; @@ -3121,13 +3147,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; } -- cgit v1.2.1