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 679950 - [pad] gst_pad_set_blocked_async in rare cases calls callback twice
[pad] gst_pad_set_blocked_async in rare cases calls callback twice
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
0.10.36
Other Linux
: Normal major
: 0.10.37
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-07-15 08:06 UTC by Marcin Lewandowski
Modified: 2013-01-13 22:47 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Log (197.35 KB, application/x-bzip)
2012-07-15 08:06 UTC, Marcin Lewandowski
Details
GStreamer pipeline dump - before block (25.01 KB, application/msword-template)
2012-07-15 08:07 UTC, Marcin Lewandowski
Details
GStreamer pipeline dump - after block callback called from thread 0x2474680 (25.01 KB, application/msword-template)
2012-07-15 08:09 UTC, Marcin Lewandowski
Details
GStreamer pipeline dump - after block callback called from thread 0x24f92d0 (25.01 KB, application/msword-template)
2012-07-15 08:09 UTC, Marcin Lewandowski
Details
GStreamer pipeline dump - before block (PNG) (409.84 KB, image/png)
2012-07-15 08:11 UTC, Marcin Lewandowski
Details
GStreamer pipeline dump - after block callback called from thread 0x24f92d0 (PNG) (409.96 KB, image/png)
2012-07-15 08:12 UTC, Marcin Lewandowski
Details
GStreamer pipeline dump - after block callback called from thread 0x2474680 (PNG) (409.96 KB, image/png)
2012-07-15 08:13 UTC, Marcin Lewandowski
Details

Description Marcin Lewandowski 2012-07-15 08:06:57 UTC
Created attachment 218841 [details]
Log

I am developing application with GStreamer 0.10.36 (default install from ubuntu 12.04) that dynamically builds and changes during runtime quite complex pipelines (30+ elements, a few adders, tee, quite a lot of queues2, a few decodebins2). Its structure is attached to this bug report as both DOT dump file and PNG.

I've run into strange situation - one of the subbins starts to dynamically unlink itself. During this process it asynchronously blocks pad to prevent flow errors.

In rare cases callback to gst_pad_set_blocked_async calls callback twice, from two different threads. Log with GST_SCHEDULING:5 is attached. I am unable to produce *:5 log, as it slow downs the application and problem disappears, propably because such slow down can affect concurrent locking issues.

The code of callback is here http://bazaar.launchpad.net/~tuned-dev/tuned-patchbay/puregst/view/head:/src/pipeline/sources/source-base.vala (please ensure that you look at rev 113), line 345. The function that is responsible for detaching is invoked by shutdown().

If you check the attached log, you can see that callback is invoked twice (lines 20613 & 20623).



I have example that triggers this bug, so I can reproduce it locally. As it is quite complex and the bug does not appear if the pipeline is simpler, I am unable to just provide code snippet. If you want, I can install it on some VPS and provide you a credentials or create virtual machine image and upload it somewhere.

For me it seems that pad locking during gst_pad_push_data() in gstpad.c is buggy, but I am not GStreamer internals expert.

I've seen that in the source code of gstpad.c there's a FIXME near 

  /* FIXME: this check can go away; pad_set_blocked could be implemented with
   * probes completely or probes with an extended pad block. */
  while (G_UNLIKELY (GST_PAD_IS_BLOCKED (pad)))
    if ((ret = handle_pad_block (pad)) != GST_FLOW_OK)

I've also seen that in GStreamer 0.11 pad blocking in gstpad.c is completely refactored.

0.10 will be in production use for quite a long time even if new version will be released, so maybe it is worth to fix that?

I can spend some time on investigating the issue, but I will appreciate any help and suggestions before I start. Maybe it is just possible to backport the code from 0.11?
Comment 1 Marcin Lewandowski 2012-07-15 08:07:58 UTC
Created attachment 218842 [details]
GStreamer pipeline dump - before block
Comment 2 Marcin Lewandowski 2012-07-15 08:09:04 UTC
Created attachment 218843 [details]
GStreamer pipeline dump - after block callback called from thread 0x2474680
Comment 3 Marcin Lewandowski 2012-07-15 08:09:43 UTC
Created attachment 218844 [details]
GStreamer pipeline dump - after block callback called from thread 0x24f92d0
Comment 4 Marcin Lewandowski 2012-07-15 08:11:22 UTC
Created attachment 218845 [details]
GStreamer pipeline dump - before block (PNG)
Comment 5 Marcin Lewandowski 2012-07-15 08:12:15 UTC
Created attachment 218846 [details]
GStreamer pipeline dump - after block callback called from thread 0x24f92d0 (PNG)
Comment 6 Marcin Lewandowski 2012-07-15 08:13:00 UTC
Created attachment 218847 [details]
GStreamer pipeline dump - after block callback called from thread 0x2474680 (PNG)
Comment 7 Wim Taymans 2012-07-16 10:49:28 UTC
Looks like concurrent pad_alloc (from flump3dec) and a push of a buffer (from the queue).

There is some code in gstpad.c to only call the callback once but it seems weird and wrong, need to take a closer look.
Comment 8 Wim Taymans 2012-07-16 13:31:01 UTC
commit cce71223d7b2f8227301f0724a18fc647f0cfd06
Author: Wim Taymans <wim.taymans@collabora.co.uk>
Date:   Mon Jul 16 15:25:46 2012 +0200

    pad: don't call pad block callbacks multiple times
    
    Ensure that when multiple threads try to block a pad, only one of the threads
    calls the callback. We do this by checking if the callback was already called.
    The previous version unconditionally called the callback and then checked if it
    was already called...
    Also only call the unblock callback once from the first thread that unblocks.
    
    Fixes https://bugzilla.gnome.org/show_bug.cgi?id=679950
Comment 9 Marcin Lewandowski 2012-07-16 18:24:32 UTC
Thank you for such quick response!
Comment 10 Marcin Lewandowski 2013-01-13 22:47:37 UTC
I've filed bug with suggestion to add this patch to ubuntu 12.04 LTS packages: https://bugs.launchpad.net/ubuntu/+source/gstreamer0.10/+bug/1099228