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 585970 - gst_audioringbuffer_get_type is not thread safe
gst_audioringbuffer_get_type is not thread safe
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal normal
: 0.10.24
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2009-06-16 11:41 UTC by Philip Jägenstedt
Modified: 2009-06-16 13:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
use G_DEFINE_TYPE (3.02 KB, patch)
2009-06-16 11:42 UTC, Philip Jägenstedt
none Details | Review

Description Philip Jägenstedt 2009-06-16 11:41:22 UTC
Many _get_type functions in gstreamer (core and plugins) aren't thread safe, but this particular one I've been getting asserts on when starting several pipelines in parallel. The fix is simple, patch attached, but perhaps a larger bug to fix all of these should be opened.
Comment 1 Philip Jägenstedt 2009-06-16 11:42:18 UTC
Created attachment 136713 [details] [review]
use G_DEFINE_TYPE
Comment 2 Tim-Philipp Müller 2009-06-16 11:49:13 UTC
I fixed this yesterday actually:

 commit 3767cb60053eb12d346c815fbe695a1d2ec265b0
 Author: Tim-Philipp Müller <tim.muller@collabora.co.uk>
 Date:   Mon Jun 15 15:39:09 2009 +0100

    audiosink,audiosrc: ref the audio ring buffer class and type in class_init
    
    Hack around thread-safety issues in GObject and our racy _get_type()
    functions (we could easily fix the _get_type() functions, but we still
    need to hack around the GObject class races until we require a newer
    GLib version, I think).


If you find other cases, or want to do a general code review, please open separate bugs for those, thanks!
Comment 3 Philip Jägenstedt 2009-06-16 12:43:48 UTC
Perhaps I'm missing something, but your fix yesterday looks very circular.
gst_audioringbuffer_get_type has a race condition, but gst_audioringbuffer_class_init would only be called after gst_audioringbuffer_get_type (as it is declared static and only referenced in _get_type). How then does g_type_class_ref (GST_TYPE_AUDIORING_BUFFER) in gst_audioringbuffer_class_init help matters?

Debugging confirms that the order of calls is _get_type and then _class_init.
Comment 4 Tim-Philipp Müller 2009-06-16 13:21:04 UTC
Err, you're absolutely right of course (oops!). These refs were supposed to go into the other class_init functions :) Thanks for noticing.

Let's try this then:

 commit 70089160f84d882d1ee7cd1708df1ee87bf69b08
 Author: Tim-Philipp Müller <tim.muller@collabora.co.uk>
 Date:   Tue Jun 16 14:05:04 2009 +0100

    audiosink, audiosrc: do the class_ref()s in the right class_init functions
    
    Spotted by Philip Jägenstedt. Hopefully fixes #585970 for real.

(Btw, your patch would've been fine as well I guess, although it seems that it would have needed at least a typedef and the same fix for the source, so we end up with different GType names for the audio ring buffers in the source and the sink.)
Comment 5 Philip Jägenstedt 2009-06-16 13:53:00 UTC
OK, since GstAudioSink uses the boilerplate macro I guess this should be safe. Thanks for fixing!