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

diff --git a/gst/playback/gstdecodebin2.c b/gst/playback/gstdecodebin2.c
index dd7677724ff7fa500c57c9daf26645d5b7e74a9b..cf52cd6eea21c39f1d45bbb44c0a8ccd033bf21f 100644 (file)
--- 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);