[Bug 755169] dashdemux: can we have multiple seek events at the same time?

[GStreamer (GNOME Bugzilla)]

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

--- Comment #25 from Vincent Penquerc'h <vincent.penquerch at collabora.co.uk> ---
(In reply to Florin Apostol from comment #23)
> gst_adaptive_demux_stream_download_loop can call
> gst_adaptive_demux_advance_period which calls
> gst_adaptive_demux_expose_streams which sets old_streams = demux->streams;
> demux->streams = demux->next_streams;
> If you release the lock in gst_adaptive_demux_stop_tasks, when you join
> tasks you are killing streams that were moved to old_streams!

OK, I see how that can happen, because _download_loop earlies out on cancelled
just before thw switch, while locking it with the object lock, but not the
manifest lock, so there's an opening left.

I'm not sure adding yet another lock is a good idea though. I understand your
idea, but I'm concerned it will make future changes even harder.

What do you think about the patch below (on top of my last one here), which
takes care of the issue you mentioned I think, since the manifest lock is held
all the time between checking cancelled and calling _expose_streams) and then
changing the use of the object lock with the manifest lock (I've not tried it
yet, but I think this'd be needed because I'm locking manifest-then-object
below, which is the wrong order, and thus possibly racy, but I can't use the
other order since I need to unlock object and keep manifest - something which
would be solved by merging both and would also get rid of a lock, making things
a bit simpler).


diff --git a/gst-libs/gst/adaptivedemux/gstadaptivedemux.c
b/gst-libs/gst/adaptivedemux/gstadaptivedemux.c
index a3f708a..4237262 100644
--- a/gst-libs/gst/adaptivedemux/gstadaptivedemux.c
+++ b/gst-libs/gst/adaptivedemux/gstadaptivedemux.c
@@ -2296,14 +2296,16 @@ gst_adaptive_demux_stream_download_loop
(GstAdaptiveDemuxStream * stream)
     next_download = g_get_monotonic_time ();
     ret = gst_adaptive_demux_stream_download_fragment (stream);

+    GST_MANIFEST_LOCK (demux);
     GST_OBJECT_LOCK (demux);
     if (demux->cancelled) {
       stream->last_ret = GST_FLOW_FLUSHING;
-      goto cancelled;
+      GST_OBJECT_UNLOCK (demux);
+      GST_MANIFEST_UNLOCK (demux);
+      goto cancelled_unlocked;
     }
     GST_OBJECT_UNLOCK (demux);

-    GST_MANIFEST_LOCK (demux);
   } else {
     stream->last_ret = ret;
   }

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