From 384a3e759f188f024aa62ddfb7558ec913a9a4c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alicia=20Boya=20Garc=C3=ADa?= Date: Tue, 19 Nov 2024 00:35:25 +0100 Subject: [PATCH] Forward port GStreamer sticky events race fix (#77) Applies https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/7632 on top of stable GStreamer. That merge request fixes a long standing race condition prone to occur in MSE. --- ...ticky-events-in-response-to-a-FLUSH_.patch | 81 +++++++++++++++++++ .../wkdev_sdk/jhbuild/webkit-sdk-deps.modules | 1 + 2 files changed, 82 insertions(+) create mode 100644 images/wkdev_sdk/jhbuild/patches/0004-pad-Never-push-sticky-events-in-response-to-a-FLUSH_.patch diff --git a/images/wkdev_sdk/jhbuild/patches/0004-pad-Never-push-sticky-events-in-response-to-a-FLUSH_.patch b/images/wkdev_sdk/jhbuild/patches/0004-pad-Never-push-sticky-events-in-response-to-a-FLUSH_.patch new file mode 100644 index 0000000..8987aa1 --- /dev/null +++ b/images/wkdev_sdk/jhbuild/patches/0004-pad-Never-push-sticky-events-in-response-to-a-FLUSH_.patch @@ -0,0 +1,81 @@ +From 59b714edc36c6d5887d54741cd628f38efe9e30c Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Alicia=20Boya=20Garc=C3=ADa?= +Date: Wed, 9 Oct 2024 13:35:33 -0400 +Subject: [PATCH] pad: Never push sticky events in response to a FLUSH_STOP + +FLUSH_STOP is meant to clear the flushing state of pads and elements +downstream, not to process data. Hence, a FLUSH_STOP should not +propagate sticky events. This is also consistent with how flushes are a +special case for probes. + +Currently this is almost always the case, since a FLUSH_STOP is +__usually__ preceded by a FLUSH_START, and events (sticky or not) are +discarded while a pad has the FLUSHING flag active (set by FLUSH_START). + +However, it is currently assumed that a FLUSH_STOP not preceded by a +FLUSH_START is correct behavior, and this will occur while autoplugging +pipelines are constructed. This leaves us with an unhandled edge case! + +This patch explicitly disables sending sticky events when pushing a +FLUSH_STOP, instead of relying on the flushing flag of the pad, which +will break in the edge case of a FLUSH_STOP not preceded by a +FLUSH_START. + +If sticky events are propagated in response to a FLUSH_STOP, the +flushing thread can end up deadlocked in blocking code of a downstream +pad, such as a blocking probe. Instead, those events should be +propagated from the streaming thread of the pad when handling a +non-flushing synchronized event or buffer. + +This fixes a deadlock found in WebKit with playbin3 when seeks occur +before preroll, where the seeking thread ended up stuck in the blocking +probe of playsink: +https://github.com/WebPlatformForEmbedded/WPEWebKit/issues/1367 + +Part-of: +--- + subprojects/gstreamer/gst/gstpad.c | 17 ++++++++++++++--- + 1 file changed, 14 insertions(+), 3 deletions(-) + +diff --git a/subprojects/gstreamer/gst/gstpad.c b/subprojects/gstreamer/gst/gstpad.c +index d8bda692b2..7c159d2816 100644 +--- a/subprojects/gstreamer/gst/gstpad.c ++++ b/subprojects/gstreamer/gst/gstpad.c +@@ -5580,8 +5580,12 @@ gst_pad_push_event_unchecked (GstPad * pad, GstEvent * event, + PROBE_PUSH (pad, type | GST_PAD_PROBE_TYPE_PUSH, event, probe_stopped); + + /* recheck sticky events because the probe might have cause a relink */ ++ /* Note: FLUSH_STOP is a serialized event, but must not propagate sticky ++ * events. FLUSH_STOP is only targeted at removing the flushing state from ++ * pads and elements, and not actually pushing data/events. */ + if (GST_PAD_HAS_PENDING_EVENTS (pad) && GST_PAD_IS_SRC (pad) +- && (GST_EVENT_IS_SERIALIZED (event))) { ++ && (GST_EVENT_IS_SERIALIZED (event)) ++ && GST_EVENT_TYPE (event) != GST_EVENT_FLUSH_STOP) { + PushStickyData data = { GST_FLOW_OK, FALSE, event }; + GST_OBJECT_FLAG_UNSET (pad, GST_PAD_FLAG_PENDING_EVENTS); + +@@ -5740,11 +5744,18 @@ gst_pad_push_event (GstPad * pad, GstEvent * event) + break; + } + } +- if (GST_PAD_IS_SRC (pad) && serialized) { ++ if (GST_PAD_IS_SRC (pad) && serialized ++ && GST_EVENT_TYPE (event) != GST_EVENT_FLUSH_STOP) { + /* All serialized events on the srcpad trigger push of sticky events. + * + * Note that we do not do this for non-serialized sticky events since it +- * could potentially block. */ ++ * could potentially block. ++ * ++ * We must NOT propagate sticky events in response to FLUSH_STOP either, as ++ * FLUSH_STOP is only targeted at removing the flushing state from pads and ++ * elements, and not actually pushing data/events. This also makes it ++ * consistent with the way flush events are handled in "blocking" pad ++ * probes. */ + res = (check_sticky (pad, event) == GST_FLOW_OK); + } + if (!serialized || !sticky) { +-- +2.47.0 + diff --git a/images/wkdev_sdk/jhbuild/webkit-sdk-deps.modules b/images/wkdev_sdk/jhbuild/webkit-sdk-deps.modules index 02252b0..0e7eb05 100644 --- a/images/wkdev_sdk/jhbuild/webkit-sdk-deps.modules +++ b/images/wkdev_sdk/jhbuild/webkit-sdk-deps.modules @@ -113,6 +113,7 @@ +