urisourcebin: Avoid deadlock on shutdown
authorEdward Hervey <edward@centricular.com>
Mon, 16 Dec 2024 15:36:06 +0000 (16:36 +0100)
committerBackport Bot <gitlab-backport-bot@gstreamer-foundation.org>
Fri, 20 Dec 2024 11:10:17 +0000 (11:10 +0000)
The reason why the STATE lock was taken was to avoid issues where we would be
adding (and activating) elements at the same time as urisourcebin would be
brought down to READY. That would cause those new elements to potentially return
ERRORS because of not-negotiated/flushing-pads

But that creates a really bad deadlock (state lock is taken to deactivate the
streaming thread which .. is currently grabbing the state lock).

Instead, we can just ignore the warning/error messages that might occur when
shutting down.

Fixes https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/4075

Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/8187>

subprojects/gst-plugins-base/gst/playback/gsturisourcebin.c

index a4ebe9a07cf43684b28cc0918d5b294af9ba4034..cafbc691d7784cc330a81354fb0d4e3e585ba0f4 100644 (file)
@@ -2091,12 +2091,6 @@ setup_parsebin_for_slot (ChildSrcPadInfo * info, GstPad * originating_pad)
     GST_DEBUG_OBJECT (urisrc, "Shutting down, returning early");
     return FALSE;
   }
-  GST_STATE_LOCK (urisrc);
-  if (g_atomic_int_get (&urisrc->flushing)) {
-    GST_DEBUG_OBJECT (urisrc, "Shutting down, returning early");
-    GST_STATE_UNLOCK (urisrc);
-    return FALSE;
-  }
   GST_URI_SOURCE_BIN_LOCK (urisrc);
 
   /* Set up optional pre-parsebin download/ringbuffer elements */
@@ -2121,7 +2115,6 @@ setup_parsebin_for_slot (ChildSrcPadInfo * info, GstPad * originating_pad)
           "ring-buffer-max-size", urisrc->ring_buffer_max_size,
           "max-size-buffers", 0, NULL);
     }
-    gst_element_set_locked_state (info->pre_parse_queue, TRUE);
     gst_bin_add (GST_BIN_CAST (urisrc), info->pre_parse_queue);
     sinkpad = gst_element_get_static_pad (info->pre_parse_queue, "sink");
     link_res = gst_pad_link (originating_pad, sinkpad);
@@ -2136,7 +2129,6 @@ setup_parsebin_for_slot (ChildSrcPadInfo * info, GstPad * originating_pad)
     post_missing_plugin_error (GST_ELEMENT_CAST (urisrc), "parsebin");
     return FALSE;
   }
-  gst_element_set_locked_state (info->demuxer, TRUE);
   gst_bin_add (GST_BIN_CAST (urisrc), info->demuxer);
 
   info->demuxer_is_parsebin = TRUE;
@@ -2162,23 +2154,15 @@ setup_parsebin_for_slot (ChildSrcPadInfo * info, GstPad * originating_pad)
       "pad-removed", G_CALLBACK (demuxer_pad_removed_cb), info);
 
   if (info->pre_parse_queue) {
-    gst_element_set_locked_state (info->pre_parse_queue, FALSE);
     gst_element_sync_state_with_parent (info->pre_parse_queue);
   }
-  gst_element_set_locked_state (info->demuxer, FALSE);
   gst_element_sync_state_with_parent (info->demuxer);
   GST_URI_SOURCE_BIN_UNLOCK (urisrc);
-  GST_STATE_UNLOCK (urisrc);
   return TRUE;
 
 could_not_link:
   {
-    if (info->pre_parse_queue)
-      gst_element_set_locked_state (info->pre_parse_queue, FALSE);
-    if (info->demuxer)
-      gst_element_set_locked_state (info->demuxer, FALSE);
     GST_URI_SOURCE_BIN_UNLOCK (urisrc);
-    GST_STATE_UNLOCK (urisrc);
     GST_ELEMENT_ERROR (urisrc, CORE, NEGOTIATION,
         (NULL), ("Can't link to (pre-)parsebin element"));
     return FALSE;
@@ -3103,6 +3087,16 @@ handle_message (GstBin * bin, GstMessage * msg)
       handle_buffering_message (urisrc, msg);
       msg = NULL;
       break;
+    case GST_MESSAGE_ERROR:
+    case GST_MESSAGE_WARNING:
+      if (g_atomic_int_get (&urisrc->flushing)) {
+        /* Errors/warnings when shutting down are non-critical */
+        GST_DEBUG_OBJECT (urisrc, "Flushing, ignoring message %" GST_PTR_FORMAT,
+            msg);
+        gst_message_unref (msg);
+        msg = NULL;
+      }
+      break;
     default:
       break;
   }