gstpad: Avoid race in (un)setting EOS flag on sinkpads

The scenario is the following:

* Thread 1 is pushing an EOS event on a sinkpad
* Thread 2 is pushing a STREAM_START event on the same sinkpad before Thread 1
returns. Note : It starts pushing the event after Thread 1 took the object lock.

There is a potential race between:

* The moment Thread 1 sets the EOS flag once it has finished sending the
event (via store_sticky_event). When it does that it has both the STREAM and
OBJECT lock

* The moment Thread 2 sends the STREAM_START event (Which should release that
EOS status), but removing the EOS flag is only done while holding the OBJECT
lock and not the STREAM_LOCK, which means it could be re-set by Thread 1 before
it then checks again the EOS flag (without the STREAM lock taken).

The EOS flag unsetting by STREAM_START should be done with the STREAM lock
taken, otherwise it will be racy.

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

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

diff --git a/subprojects/gstreamer/gst/gstpad.c b/subprojects/gstreamer/gst/gstpad.c
index 0452563..994f13f 100644 (file)
--- a/subprojects/gstreamer/gst/gstpad.c
+++ b/subprojects/gstreamer/gst/gstpad.c
@@ -5850,6 +5850,17 @@ gst_pad_send_event_unchecked (GstPad * pad, GstEvent * event,
 
       switch (event_type) {
         case GST_EVENT_STREAM_START:
+          /* Take the stream lock to unset the EOS status. This is to ensure
+           * there isn't any other serialized event passing through while this
+           * EOS status is being unset */
+          /* lock order: STREAM_LOCK, LOCK, recheck flushing. */
+          GST_OBJECT_UNLOCK (pad);
+          GST_PAD_STREAM_LOCK (pad);
+          need_unlock = TRUE;
+          GST_OBJECT_LOCK (pad);
+          if (G_UNLIKELY (GST_PAD_IS_FLUSHING (pad)))
+            goto flushing;
+
           /* Remove sticky EOS events */
           GST_LOG_OBJECT (pad, "Removing pending EOS events");
           remove_event_by_type (pad, GST_EVENT_EOS);
@@ -5858,24 +5869,22 @@ gst_pad_send_event_unchecked (GstPad * pad, GstEvent * event,
           GST_OBJECT_FLAG_UNSET (pad, GST_PAD_FLAG_EOS);
           break;
         default:
+          if (serialized) {
+            /* Take the stream lock to check the EOS status and drop the event
+             * if that is the case. */
+            /* lock order: STREAM_LOCK, LOCK, recheck flushing. */
+            GST_OBJECT_UNLOCK (pad);
+            GST_PAD_STREAM_LOCK (pad);
+            need_unlock = TRUE;
+            GST_OBJECT_LOCK (pad);
+            if (G_UNLIKELY (GST_PAD_IS_FLUSHING (pad)))
+              goto flushing;
+
+            if (G_UNLIKELY (GST_PAD_IS_EOS (pad)))
+              goto eos;
+          }
           break;
       }
-
-      if (serialized) {
-        if (G_UNLIKELY (GST_PAD_IS_EOS (pad)))
-          goto eos;
-
-        /* lock order: STREAM_LOCK, LOCK, recheck flushing. */
-        GST_OBJECT_UNLOCK (pad);
-        GST_PAD_STREAM_LOCK (pad);
-        need_unlock = TRUE;
-        GST_OBJECT_LOCK (pad);
-        if (G_UNLIKELY (GST_PAD_IS_FLUSHING (pad)))
-          goto flushing;
-
-        if (G_UNLIKELY (GST_PAD_IS_EOS (pad)))
-          goto eos;
-      }
       break;
   }