GNOME Bugzilla – Bug 549504
Thread-safe version of gst_pad_get_internal_links()
Last modified: 2008-09-01 11:29:58 UTC
gst_pad_get_internal_links() is not thread safe. If pads are removed from a running element (like a tee), it can cause a crash when it tries to push an event onto a no-longer-existing pad. I propose adding a gst_pad_iterate_internal_links() that returns a GstIterator* and is thread-safe. I'm attaching a sample implementation.
Created attachment 117422 [details] [review] Add gst_pad_iterate_internal_links()
This looks like a good thing to do and it seems possible to implement without breaking too much API.
Created attachment 117546 [details] [review] slightly updated version to deal with parent-less pads _iterate_internal_links() returns NULL if the parent is not an element, the previous version of the patch crashed on that case, new patch doesn't.
Created attachment 117611 [details] [review] Separate first part: Implement gst_pad_iterate_internal_links()
Created attachment 117612 [details] [review] Part 2: Use _iterate_internal_links() to push events/queries As requested, here are the separated patches. The first implements the new method, with the fallback (as well as the gstghostpad part). The second uses it to dispatch events and queries. Also, I haven't implemented it in the elements that return a static list of pads on the query (I believe all elements that currently implement it do that), because there is no threading issue there.
I slightly changed the _iterate_internal_links_default() function so that it does not keep a pad-local GList. Also added some comments. Based on patch by: Olivier Crete <tester at tester dot ca> * docs/gst/gstreamer-sections.txt: * win32/common/libgstreamer.def: * gst/gstpad.c: (gst_pad_init), (gst_pad_set_iterate_internal_links_function), (int_link_iter_data_free), (iterate_pad), (gst_pad_iterate_internal_links_default), (gst_pad_iterate_internal_links), (gst_pad_get_internal_links): * gst/gstpad.h: Add threadsafe replacement functions for getting internal links of an element. Deprecate the old internal links functions. API:GstPad::gst_pad_set_iterate_internal_links_function() API:GstPad::GstPadIterIntLinkFunction API:GstPad::gst_pad_iterate_internal_links() API:GstPad::gst_pad_iterate_internal_links_default() * gst/gstghostpad.c: (gst_proxy_pad_do_iterate_internal_links), (gst_proxy_pad_init): Implement threadsafe internal links. * tests/check/elements/tee.c: (GST_START_TEST), (tee_suite): Unit test for internal links on tee. See #549504.
Patch by: Olivier Crete <tester at tester dot ca> * gst/gstpad.c: (gst_pad_iterate_internal_links_default), (gst_pad_event_default_dispatch), (gst_pad_dispatcher): Use thread-safe internal links iterator. Fixes #549504.