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 755343 - funnel: Fix racy state change
funnel: Fix racy state change
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
unspecified
Other All
: Normal normal
: 1.6.0
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on: 755342
Blocks:
 
 
Reported: 2015-09-21 12:09 UTC by Stian Selnes (stianse)
Modified: 2015-09-24 10:15 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
funnel: Fix racy state change (3.67 KB, patch)
2015-09-21 12:09 UTC, Stian Selnes (stianse)
none Details | Review
funnel: Fix racy state change (3.50 KB, patch)
2015-09-21 12:42 UTC, Stian Selnes (stianse)
committed Details | Review

Description Stian Selnes (stianse) 2015-09-21 12:09:00 UTC
Iterator may need to be resynced, for instance if pads are released
during state change.

got_eos should be protected by the object lock of the element, not of
the pad, as is the case throughout the rest of the funnel code.
Comment 1 Stian Selnes (stianse) 2015-09-21 12:09:03 UTC
Created attachment 311748 [details] [review]
funnel: Fix racy state change
Comment 2 Stian Selnes (stianse) 2015-09-21 12:11:18 UTC
Adding dependency to bug 755342 since the added unit test may intermittently fail unless that issue is fixed.
Comment 3 Sebastian Dröge (slomo) 2015-09-21 12:14:51 UTC
Review of attachment 311748 [details] [review]:

::: plugins/elements/gstfunnel.c
@@ +454,2 @@
+      while (res == GST_ITERATOR_OK || res == GST_ITERATOR_RESYNC) {
+        res = gst_iterator_foreach (iter, reset_pad, element);

This looks wrong. Why would you get OK and then iterate again over all pads?
Comment 4 Håvard Graff (hgr) 2015-09-21 12:34:27 UTC
Hm... Where have I seen this before... http://gstconf.ubicast.tv/permalink/v124f29454991uefv57g/#start=2713
Comment 5 Stian Selnes (stianse) 2015-09-21 12:42:42 UTC
Created attachment 311749 [details] [review]
funnel: Fix racy state change

Iterator may need to be resynced, for instance if pads are released
during state change.

got_eos should be protected by the object lock of the element, not of
the pad, as is the case throughout the rest of the funnel code.
Comment 6 Sebastian Dröge (slomo) 2015-09-24 10:15:46 UTC
commit 2c60b7eb1fc8851296e7bbb209663bb0c0e53f21
Author: Stian Selnes <stian@pexip.com>
Date:   Mon Sep 21 13:58:51 2015 +0200

    funnel: Fix racy state change
    
    Iterator may need to be resynced, for instance if pads are released
    during state change.
    
    got_eos should be protected by the object lock of the element, not of
    the pad, as is the case throughout the rest of the funnel code.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=755343

commit 0fbf1d3eb7feaa753cf1d0ec03546f854dd1f037
Author: Stian Selnes <stian@pexip.com>
Date:   Mon Sep 21 15:22:19 2015 +0200

    bin: element: Ignore activate result for removed pads on state change
    
    This fixes a race where a state change may return failure if it has
    request pads that are deactivated and removed (and thus have no
    parent) at the same time as the element changes state and (de)activates
    its pads.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=755342