[Bug 797223] Race condition in ipcpipelinesrc during flush

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=797223

Olivier Crête <olivier.crete at ocrete.ca> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #373802|none                        |needs-work
             status|                            |

--- Comment #1 from Olivier Crête <olivier.crete at ocrete.ca> ---
Review of attachment 373802:
 --> (https://bugzilla.gnome.org/review?bug=797223&attachment=373802)

I'm pretty sure this patch is not the right solution. I think that on
flush-stop, it should probably do whatever is needed with locking (as this is a
serialized event) to ensure that the ordering is right. Or maybe after
forwarding the flush start, it should take the STREAM lock to ensure proper
serialization after receiving the flush-start.

::: sys/ipcpipeline/gstipcpipelinesrc.c
@@ +375,3 @@
+    gst_pad_pause_task (src->srcpad);
+
+  gst_ipc_pipeline_src_cancel_queued (src);

I wonder if doing the cancel queued here before we're certain that the loop is
stopped is safe. Especially that the cancel function modifies the list without
locking, so it's not safe to run it while the loop is still running.

@@ -693,1 +692,5 @@
     case GST_EVENT_FLUSH_START:
+      GST_DEBUG_OBJECT (src,
+          "Forwarding flush-start immediately");
+      gst_element_call_async (GST_ELEMENT (src), do_oob_event, event,
+          (GDestroyNotify) gst_event_unref);

This is equally racy.. This call delegates the pyshing to another thread. So it
could get push before or after the gst_ipc_pipeline_src_stop_loop() call
unpredictably.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.