[gst-devel] thread unsafety in gst_pad_get_internal_links()

[Eric Zhang]

Hi, gstreamer-devel:

    `gst_pad_get_internal_links' just call the `intlinkfunc' function 
pointer which is set by user by calling 
`gst_pad_set_internal_link_function'. And the lock is suppose to be set 
on user's `internal_link' function, so I think there is no need to set 
lock in `gst_pad_get_internal_links'.

    Here is the source code of `gst_pad_get_internal_links':

=================================
GList *
gst_pad_get_internal_links (GstPad * pad)
{
  GList *res = NULL;

  g_return_val_if_fail (GST_IS_PAD (pad), NULL);

  if (GST_PAD_INTLINKFUNC (pad))
    res = GST_PAD_INTLINKFUNC (pad) (pad);

  return res;
}

GST_PAD_INTLINKFUNC is defined as:

#define GST_PAD_INTLINKFUNC(pad)    (GST_PAD_CAST(pad)->intlinkfunc)
=================================

Eric Zhang


Olivier Crête wrote:
> Hello,
>
> I just noticed that gst_pad_get_internal_links() is not thread safe. It
> does not take the object lock while looking up the list of pads and does
> not reference the pads. So its very possible that it could happen in the
> middle of a modification of the list (lets say if I add a pad to a tee).
> Or that the pad could be gone by the time I get to do something with it.
>
> I propose deprecating it (since we can't modify it without breaking the
> ABI) and add a new call get_internal_links_safe() that will returns a
> list of refed pads. And fixing the default implementation of boths to be
> thread safe (and probably having the default implementation of the
> unsafe version call the safe version and unref the pads).
>
>   
> ------------------------------------------------------------------------
>
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
> Build the coolest Linux based applications with Moblin SDK & win great prizes
> Grand prize is a trip for two to an Open Source event anywhere in the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> ------------------------------------------------------------------------
>
> _______________________________________________
> gstreamer-devel mailing list
> gstreamer-devel at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/gstreamer-devel
>