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 658924 - [patch] fix gstbin's check of inner elements
[patch] fix gstbin's check of inner elements
Status: RESOLVED NOTABUG
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
0.10.31
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2011-09-13 14:14 UTC by Stas Sergeev
Modified: 2011-09-19 11:44 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
the fix (1.02 KB, patch)
2011-09-13 14:14 UTC, Stas Sergeev
rejected Details | Review

Description Stas Sergeev 2011-09-13 14:14:46 UTC
Created attachment 196380 [details] [review]
the fix

Hi.

If the hierarchy checking is bypassed with gst_pad_link_full(),
gstbin may confuse himself with his element, leading to a deadlock.
The attached patch fixes the problem.
Comment 1 Tim-Philipp Müller 2011-09-13 20:19:30 UTC
Do you have an example of how to reproduce this?
Comment 2 Stas Sergeev 2011-09-13 20:46:12 UTC
Yes, one particular revision of the
plugin I am writing here, locks up gstreamer
without this patch. The lockup happens on the
next line,
GST_OBJECT_LOCK (peer_element);
when the bin is trying to lock itself by mistake.
But later I created this patch:
https://bugzilla.gnome.org/show_bug.cgi?id=658918
and dropped the hierarchy hack from my plugin,
so the final version will not depend on that.

Suppose you have a bin N with 2 elements: A and B.
A have "src" and B have "sink".
You create a ghostpad G for A.src
You add G to N
You then link G to B.sink - hierarchy violation
N looks for the peer of B.sink, finds G, whose
parent is N - deadlock trying to lock N the second time.
Comment 3 David Schleef 2011-09-17 19:43:40 UTC
> You then link G to B.sink - hierarchy violation

Correct, because doing this is an incorrect operation.  Ghost pads are for elements outside of the bin to be connected to the bin.
Comment 4 Stas Sergeev 2011-09-17 20:30:01 UTC
> Correct, because doing this is an incorrect operation.
Yes, but since it is allowed by the
gst_pad_link_full(), why not to prevent the deadlock?
Esp when the fix is that simple.
Either the fix, or remove that possibility from gst_pad_link_full(),
but right now it is there and is deadlocking, which is not good.
And it _may_ be usefull after all: I installed the
bin's setcaps handler on that gostpad. Alternatively,
I installed the setcaps handler on the "src" pad of
the sub-element directly, as in my another report.
Comment 5 Sebastian Dröge (slomo) 2011-09-19 08:03:50 UTC
It's not allowed by gst_pad_link_full(). gst_pad_link_full() allows you to skip some checks for performance reasons if you know what you're doing is correct and will work. This situation can not happen in any valid situations and if you're using gst_pad_link_full() in the wrong situations you will get many different kinds of problems.
Comment 6 Stas Sergeev 2011-09-19 11:17:56 UTC
OK.
But this operation is not performance-critical, as linking
mostly happens only once. If there are no valid uses to _violate_
these checks, then I guess there is no reason for this function
to live at all. :)
Comment 7 Sebastian Dröge (slomo) 2011-09-19 11:44:55 UTC
Linking actually is performance-critical in many situations. By using gst_pad_link_full() with no checks, decodebin2 can autoplug it's pipeline much faster for example and you see the first video frame earlier.