GNOME Bugzilla – Bug 608398
Initializing the glib thread system seems not to work
Last modified: 2010-01-30 15:40:54 UTC
Created attachment 152533 [details] Test case that initializes the glib thread system This is only filed as a blocker because I understand that a new release is coming up and bugs should be filed as blockers now that a pre-release is available. The severity can be adjusted accordingly if it is not necessarily a blocker. After initializing the glib thread system and then trying to get the GStreamer option group the following warning results (The test case is attached): (process:6132): GStreamer-WARNING **: The GStreamer function gst_init_get_option_group() was called, but the GLib threading system has not been initialised yet, something that must happen before any other GLib function is called. The application needs to be fixed so that it calls if (!g_thread_supported ()) g_thread_init(NULL); as very first thing in its main() function. Please file a bug against this application. There seems to be a problem because the g_thread_supported() macro resolves to 1 if both G_THREAD_ENABLED and G_THREADS_MANDATORY are defined. G_THREAD_ENABLED is defined in the glibconfig.h file while G_THREADS_MANDATORY comes from GStreamer's pkg-config cflags. It's hard to know exactly what's going on hence the reason for this bug. I hope it's nothing complicated.
Apparently this is related to a recent glib fix: http://git.gnome.org/browse/glib/commit/?id=de5c708e0b3d257388d3a5d09c80806d27069c88 https://bugzilla.gnome.org/show_bug.cgi?id=606775
Thanks for the bug report. Indeed it fails as you describe with glib >= 2.23.2. I guess maybe we shouldn't put -DG_THREADS_MANDATORY into our 'public' CFLAGS after all, but only use it internally to build GStreamer.
I'm not 100% convinced yet that that GLib commit is entirely right. Arguably they can define new semantics enabled by -DG_THREADS_MANDATORY as they please, but IMHO g_thread_supported() should now point to g_thread_get_initialized() and the g_thread_supported() calls around the threading API should have been changed to some other expression that either evaluates to TRUE or the old g_thread_supported().
Yes, I agree with Tim here. And we should probably just call g_thread_init() unconditionally in GLib >= 2.23.2 and remove that warning then because it's pointless now.
> Yes, I agree with Tim here. Which part? That we shouldn't put -DG_THREADS_MANDATORY in the pkg-config cflags for now, or that GLib should ideally do things slightly differently? :) > And we should probably just call g_thread_init() unconditionally in GLib >= 2.23.2 > and remove that warning then because it's pointless now. So there's no need to call g_thread_init() very early in the program any more? Before other GLib functions are called, etc.? As I see it, the assumption is sort of that g_type_init() is called first thing in main() now, more or less, and -DG_THREADS_MANDATORY only seems to be used in GObject, but not in GLib itself.
(In reply to comment #5) > > Yes, I agree with Tim here. > > Which part? That we shouldn't put -DG_THREADS_MANDATORY in the pkg-config > cflags for now, or that GLib should ideally do things slightly differently? :) That GLib should do things differently ;) > > And we should probably just call g_thread_init() unconditionally in GLib >= 2.23.2 > > and remove that warning then because it's pointless now. > > So there's no need to call g_thread_init() very early in the program any more? > Before other GLib functions are called, etc.? As I see it, the assumption is > sort of that g_type_init() is called first thing in main() now, more or less, > and -DG_THREADS_MANDATORY only seems to be used in GObject, but not in GLib > itself. Yes, either call it always (it can be called multiple times) or assume g_type_init() calls it
> > So there's no need to call g_thread_init() very early in the program any more? > > Before other GLib functions are called, etc.? As I see it, the assumption is > > sort of that g_type_init() is called first thing in main() now, more or less, > > and -DG_THREADS_MANDATORY only seems to be used in GObject, but not in GLib > > itself. > > Yes, either call it always (it can be called multiple times) or assume > g_type_init() calls it The reason we added the warning was that calling g_thread_init() too late caused mysterious issues with GSlice, and at the time the GLib people argued that that was fine because g_thread_init() was supposed to be called *first thing* in main() and behaviour was undefined if that was not the case, so it's not their problem. Has that been officially changed now to 'just call g_thread_init() before you use any threading primitives?'
(In reply to comment #7) > > > So there's no need to call g_thread_init() very early in the program any more? > > > Before other GLib functions are called, etc.? As I see it, the assumption is > > > sort of that g_type_init() is called first thing in main() now, more or less, > > > and -DG_THREADS_MANDATORY only seems to be used in GObject, but not in GLib > > > itself. > > > > Yes, either call it always (it can be called multiple times) or assume > > g_type_init() calls it > > The reason we added the warning was that calling g_thread_init() too late > caused mysterious issues with GSlice, and at the time the GLib people argued > that that was fine because g_thread_init() was supposed to be called *first > thing* in main() and behaviour was undefined if that was not the case, so it's > not their problem. > > Has that been officially changed now to 'just call g_thread_init() before you > use any threading primitives?' Yes, see the latest version of the GLib docs. It's also mentioned in the last release notes.
I see, you're right. Ok, I think what we should do is this: * for the release, don't put -DG_THREADS_MANDATORY into the pkg-config CFLAGS, just use it internally * for the release, disable the warning if runtime version of glib is >= 2.23.2, since then we know late g_thread_init will be fine and be done in g_type_init() anyway * after the release guard the warning with #ifdef GLIB_CHECK_VERSION(2,23,2) so that it is removed entirely if GStreamer is compiled against a new glib. * after the release, see if we want to put the -DG_THREADS_MANDATORY back into our pkg-config cflags (it doesn't seem 100% kosher to me) (This is all assuming GLib doesn't fix things on their end of course...)
(In reply to comment #9) > I see, you're right. > > Ok, I think what we should do is this: > > * for the release, don't put -DG_THREADS_MANDATORY > into the pkg-config CFLAGS, just use it internally > > * for the release, disable the warning if runtime version > of glib is >= 2.23.2, since then we know late g_thread_init > will be fine and be done in g_type_init() anyway Ok, sounds good to me. > * after the release guard the warning with > #ifdef GLIB_CHECK_VERSION(2,23,2) so > that it is removed entirely if GStreamer is compiled > against a new glib. I'd leave it as a runtime check, that's safer ;) > * after the release, see if we want to put the > -DG_THREADS_MANDATORY back into our pkg-config > cflags (it doesn't seem 100% kosher to me) Well, maybe it's not nice if we add it to our pkg-config files. We could add it to GLIB_CFLAGS in common/m4/gst-glib2.m4 and then it's at least used for all of our code and other applications are not affected. What do you think?
commit 6c6f20e0b6fdcce6c4248b063598c98ec91d1838 Author: Tim-Philipp Müller <tim.muller@collabora.co.uk> Date: Sat Jan 30 13:45:58 2010 +0000 init: don't spew warning about late g_thread_init()s if GLib >= 2.23.2 Late g_thread_init() is fine with newer GLib versions and done automatically from g_type_init() there, so don't warn if the application hasn't called g_thread_init() yet when gst_init() is called with new GLib versions. Fixes #608398. commit 79d3f1a2c102fe0f0ee6a1f1980943528b46c5c3 Author: Tim-Philipp Müller <tim.muller@collabora.co.uk> Date: Fri Jan 29 09:41:30 2010 +0000 pkgconfig: don't put -DG_THREADS_MANDATORY into our pkg-config CFLAGS If we force -DG_THREADS_MANDATORY onto apps, then g_thread_supported() will always evaluate to TRUE, so the typical thread initialisation boilerplate code if (!g_thread_supported()) g_thread_init(NULL); will no longer work, and the threading system not be initialised and us printing a warning in gst_init. This may be fine in most cases, since late initialisation is allowed and automatically done in g_type_init() since GLib 2.23.2, but let's be cautious and only use this define when compiling GStreamer itself. See #608398. commit 96dc7930768997da39cd1e6b80485bc69b421601 Author: Tim-Philipp Müller <tim.muller@collabora.co.uk> Date: Sat Jan 30 13:56:42 2010 +0000 gst-glib2.m4: add -DG_THREADS_MANDATORY to GLIB_CFLAGS Threading is always enabled in GStreamer code, so we may just as well add -DG_THREADS_MANDATORY to GLIB_CFLAGS so that all GStreamer modules are automatically compiled with it (optimisation).