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: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/7632>
This commit is contained in:
Alicia Boya García 2024-10-09 13:35:33 -04:00 committed by GStreamer Marge Bot
parent 4b8d43446e
commit 59b714edc3

View file

@ -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) {