[Bug 711138] New: Chained GstIteratorFilter objects don't release the iterator's mutex when invoking the filter function

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=711138
  GStreamer | gstreamer (core) | 1.2.0

           Summary: Chained GstIteratorFilter objects don't release the
                    iterator's mutex when invoking the filter function
    Classification: Platform
           Product: GStreamer
           Version: 1.2.0
        OS/Version: All
            Status: UNCONFIRMED
          Severity: normal
          Priority: Normal
         Component: gstreamer (core)
        AssignedTo: gstreamer-bugs at lists.freedesktop.org
        ReportedBy: stewart at eh.org
         QAContact: gstreamer-bugs at lists.freedesktop.org
     GNOME version: ---


Created an attachment (id=258558)
 --> (https://bugzilla.gnome.org/attachment.cgi?id=258558)
Larger version of the example code

The general problem is that the filter_next function in gstiterator.c carefully
releases the mutex whilst it calls the filter callback function - unless it has
been wrapped by another filter, in which case it doesn't release the mutex,
leading to possible deadlocks in the filter callback function.

The specific problem: I am trying to iterate through the caps structure of
linked pads looking for pads with caps that match a particular type.  The way I
tried to do this was to chain several filters (from gst_iterator_filter) in
series to allow the application to call gst_iterator_foreach() with the final
iterator.  However, deadlock occurs when my caps checking filter calls
gst_pad_query_caps(), as that function tries to claim the owning GstElement's
mutex that the GstIteratorFilter has still got claimed.

{
  GstIterator *it;
  it = gst_element_iterate_src_pads( element );
  it = gst_iterator_filter( it, filter_test_caps, NULL );
  it = gst_iterator_filter( it, filter_test_linked, NULL );

  while ( gst_iterator_foreach( it, ... ) != GST_ITERATOR_RESYNC ) { ... }
}

gint filter_test_caps( gconstpointer pad, gconstpointer unused )
{
    /* The following call to gst_pad_query_caps hangs */
    GstCaps *caps = gst_pad_query_caps(GST_PAD(g_value_get_object(pad)),NULL);
    ...
}

Obviously, the much-simplified example code excerpted here (copy attached)
could be easily refactored to use a single filter.  However, in the code's real
environment, that isn't really feasible.


Explanation: when gst_iterator_filter() wraps another iterator, it steals the
mutex from the wrapped iterator for itself so when filter_next of the wrapped
iterator tries to release the mutex, it can't, as it's not there any more.


A potential solution: wrapped filters must also release the mutex whilst
calling their filter functions.  I attach a small patch that would do this by
retaining a private copy of the original mutex in the private GstIteratorFilter
data structure and having filter_next() look at that mutex instead of the main
iterator mutex.  I think this is all fine and safe (and it fixes the problem
I've got) - but would appreciate review by others.  The patch is made against
1.0.7, but is the same for 1.2.0 (gstiterator.c hasn't changed between the two
versions)


Another way of solving the problem might be for all GstIterators to store a
list of filter functions that are all called in order - in other words, instead
of having multiple iterator objects, have one iterator object with a GList of
filter functions that it can run through in sequence (using an iterator!)  I've
not tried that, but it seems a sensible enough idea.

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.