After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 700868 - videomixer: Multiple locking error
videomixer: Multiple locking error
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal normal
: git master
Assigned To: Nicolas Dufresne (ndufresne)
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-05-23 00:59 UTC by Nicolas Dufresne (ndufresne)
Modified: 2014-11-01 15:36 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[PATCH] videomixer: Don't hold object lock while sending events (2.20 KB, patch)
2013-05-23 01:02 UTC, Nicolas Dufresne (ndufresne)
committed Details | Review
[PATCH] videomixer: Don't hold stream-lock while pushing non-serialized events (1.08 KB, patch)
2013-05-23 01:02 UTC, Nicolas Dufresne (ndufresne)
committed Details | Review
SVG Showing a pipeline that won't preroll (38.10 KB, image/svg)
2013-05-23 01:08 UTC, Nicolas Dufresne (ndufresne)
  Details
videomixer: Release COLLECT_PAD_STREAM lock when sending eos (1.43 KB, patch)
2013-05-23 01:15 UTC, Thibault Saunier
needs-work Details | Review

Description Nicolas Dufresne (ndufresne) 2013-05-23 00:59:16 UTC
There is few locking error in videomixer that are visible from GNonLin unit test. To produce them run:

GST_CHECKS=test_complex_operations_bis make gnl/gnloperation.gdb

I have patch coming fro two cases (running multiple time give multiple results).
Comment 1 Nicolas Dufresne (ndufresne) 2013-05-23 01:02:56 UTC
Created attachment 245093 [details] [review]
[PATCH] videomixer: Don't hold object lock while sending events


https://bugzilla.gnome.org/show_bug.cgi?id=700868
---
 gst/videomixer/videomixer2.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)
Comment 2 Nicolas Dufresne (ndufresne) 2013-05-23 01:02:59 UTC
Created attachment 245094 [details] [review]
[PATCH] videomixer: Don't hold stream-lock while pushing non-serialized events


https://bugzilla.gnome.org/show_bug.cgi?id=700868
---
 gst/videomixer/videomixer2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 3 Nicolas Dufresne (ndufresne) 2013-05-23 01:08:15 UTC
Created attachment 245095 [details]
SVG Showing a pipeline that won't preroll

There is a third case I reproduced from time to time, but can't figure-out yet. It's case where the pipeline won't preroll. Attached, you'll find an SVG showing the pipeline state.
Comment 4 Thibault Saunier 2013-05-23 01:15:26 UTC
Created attachment 245096 [details] [review]
videomixer: Release COLLECT_PAD_STREAM lock when sending eos

It create deadlock in case we get a flush_stop while we are sending it.
Comment 5 Sebastian Dröge (slomo) 2013-05-23 06:52:59 UTC
Comment on attachment 245096 [details] [review]
videomixer: Release COLLECT_PAD_STREAM lock when sending eos

What's the exact scenario here? gst_pad_push_event() should just return immediately when something downstream is flushing. And receiving flush-stop from upstream should not be relevant here
Comment 6 Nicolas Dufresne (ndufresne) 2013-05-23 13:26:37 UTC
Comment on attachment 245094 [details] [review]
[PATCH] videomixer: Don't hold stream-lock while pushing non-serialized events

commit d8c5e316576a00c2ed7b37689c87b838c16c7305
Author: Nicolas Dufresne <nicolas.dufresne@collabora.co.uk>
Date:   Wed May 22 21:01:48 2013 -0400

    videomixer: Don't hold stream-lock while pushing non-serialized events
    
    https://bugzilla.gnome.org/show_bug.cgi?id=700868
Comment 7 Nicolas Dufresne (ndufresne) 2013-05-23 13:26:57 UTC
Comment on attachment 245093 [details] [review]
[PATCH] videomixer: Don't hold object lock while sending events

commit a7e0f251cac28743d3a7e878171ebffa8c2d6fd0
Author: Nicolas Dufresne <nicolas.dufresne@collabora.co.uk>
Date:   Wed May 22 21:00:45 2013 -0400

    videomixer: Don't hold object lock while sending events
    
    https://bugzilla.gnome.org/show_bug.cgi?id=700868
Comment 8 Nicolas Dufresne (ndufresne) 2013-05-23 13:45:47 UTC
(In reply to comment #4)
> Created an attachment (id=245096) [details] [review]
> videomixer: Release COLLECT_PAD_STREAM lock when sending eos
> 
> It create deadlock in case we get a flush_stop while we are sending it.

That is strange, a flush-start event should have had unblocked that. Is it an eos sent in gnlcomposition from eos_main_thread() ?  This could explain, gnlcomp get an EOS, instead of checking if it's the end and forward it, it fire the g_idle_add, and figure-out if it's the end from there, which is racy. Other possibility is that the pipeline was not linked completly when flush-start was sent, but I don't think this patch is correct.
Comment 9 Nicolas Dufresne (ndufresne) 2013-06-25 16:42:44 UTC
Thibault, is that third patch still needed, or we can close that bug now ?
Comment 10 Sebastian Dröge (slomo) 2013-07-25 16:07:19 UTC
Thibault?
Comment 11 Thibault Saunier 2013-07-25 17:07:24 UTC
There is still a locking issue happening with videomixer in the gnl tests, I still have to investigate it.
Comment 12 Sebastian Dröge (slomo) 2013-07-26 07:39:14 UTC
Comment on attachment 245096 [details] [review]
videomixer: Release COLLECT_PAD_STREAM lock when sending eos

FLUSH_STOP and EOS are both serialized events, so they should never fight over the stream lock and deadlock. Before the FLUSH_STOP there should've been a FLUSH_START that unlocks the streaming threads everywhere and makes all pads return GST_FLOW_FLUSHING.

So this patch doesn't seem right and more like hiding a different bug.
Comment 13 Nicolas Dufresne (ndufresne) 2014-05-29 21:26:34 UTC
Anything left here ?
Comment 14 Nicolas Dufresne (ndufresne) 2014-11-01 15:36:20 UTC
There is no chance we come back to work on videomixer, the end solution endup being it's successor "compositor" base on Aggregator base class.