From a2ee84fa80cde8174a2e44258b3b57e94c28216e Mon Sep 17 00:00:00 2001 From: Vincent Penquerc'h Date: Thu, 5 Feb 2015 12:07:50 +0000 Subject: [PATCH] decodebin: fix deadlock between downward state change and pad addition If caps on a newly added pad are NULL, analyze_new_pad will try to acquire the chain lock to add a probe to the pad so the chain can be built later. This comes from the streaming thread, in response to headers or other buffers causing this pad to be added, so the stream lock is taken. Meanwhile, another thread might be destroying the chain from a downward state change. This will cause the chain to be freed with the chain lock taken, and some elements are set to NULL here, which can include the parser. This causes pad deactivation, which tries to take the element's pad's stream lock, deadlocking. Fix this by keeping track of which elements need setting to NULL, and only do this after the chain lock is released. Only the chain manipulation needs to be locked, not the elements' state changes. https://bugzilla.gnome.org/show_bug.cgi?id=741355 --- gst/playback/gstdecodebin2.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/gst/playback/gstdecodebin2.c b/gst/playback/gstdecodebin2.c index dd7677724f..cf52cd6eea 100644 --- a/gst/playback/gstdecodebin2.c +++ b/gst/playback/gstdecodebin2.c @@ -3149,7 +3149,7 @@ static void gst_decode_group_free_internal (GstDecodeGroup * group, static void gst_decode_chain_free_internal (GstDecodeChain * chain, gboolean hide) { - GList *l; + GList *l, *set_to_null = NULL; CHAIN_MUTEX_LOCK (chain); @@ -3209,14 +3209,15 @@ gst_decode_chain_free_internal (GstDecodeChain * chain, gboolean hide) GST_OBJECT_CAST (chain->dbin)) gst_bin_remove (GST_BIN_CAST (chain->dbin), delem->capsfilter); if (!hide) { - gst_element_set_state (delem->capsfilter, GST_STATE_NULL); + set_to_null = + g_list_append (set_to_null, gst_object_ref (delem->capsfilter)); } } if (GST_OBJECT_PARENT (element) == GST_OBJECT_CAST (chain->dbin)) gst_bin_remove (GST_BIN_CAST (chain->dbin), element); if (!hide) { - gst_element_set_state (element, GST_STATE_NULL); + set_to_null = g_list_append (set_to_null, gst_object_ref (element)); } SUBTITLE_LOCK (chain->dbin); @@ -3275,6 +3276,14 @@ gst_decode_chain_free_internal (GstDecodeChain * chain, gboolean hide) GST_DEBUG_OBJECT (chain->dbin, "%s chain %p", (hide ? "Hidden" : "Freed"), chain); CHAIN_MUTEX_UNLOCK (chain); + + while (set_to_null) { + GstElement *element = set_to_null->data; + set_to_null = g_list_delete_link (set_to_null, set_to_null); + gst_element_set_state (element, GST_STATE_NULL); + gst_object_unref (element); + } + if (!hide) { g_mutex_clear (&chain->lock); g_slice_free (GstDecodeChain, chain);