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 608398 - Initializing the glib thread system seems not to work
Initializing the glib thread system seems not to work
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal blocker
: 0.10.26
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-01-29 00:12 UTC by José Alburquerque
Modified: 2010-01-30 15:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Test case that initializes the glib thread system (180 bytes, text/x-csrc)
2010-01-29 00:12 UTC, José Alburquerque
Details

Description José Alburquerque 2010-01-29 00:12:07 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.
Comment 1 José Alburquerque 2010-01-29 02:31:34 UTC
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
Comment 2 Tim-Philipp Müller 2010-01-29 09:37:47 UTC
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.
Comment 3 Tim-Philipp Müller 2010-01-29 09:52:07 UTC
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().
Comment 4 Sebastian Dröge (slomo) 2010-01-29 10:05:26 UTC
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.
Comment 5 Tim-Philipp Müller 2010-01-29 10:24:14 UTC
> 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.
Comment 6 Sebastian Dröge (slomo) 2010-01-29 10:51:00 UTC
(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
Comment 7 Tim-Philipp Müller 2010-01-29 11:00:38 UTC
> > 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?'
Comment 8 Sebastian Dröge (slomo) 2010-01-29 15:59:41 UTC
(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.
Comment 9 Tim-Philipp Müller 2010-01-29 17:38:31 UTC
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...)
Comment 10 Sebastian Dröge (slomo) 2010-01-29 19:41:21 UTC
(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?
Comment 11 Tim-Philipp Müller 2010-01-30 15:40:54 UTC
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).