GNOME Bugzilla – Bug 547885
ORBit2 should not call g_thread_init() itself as it is usually too late
Last modified: 2009-01-05 15:54:03 UTC
The documentation for g_thread_init() has for a long time stated: "You must call g_thread_init() before executing any other GLib functions in a threaded GLib program." Now that is no doubt a little vague. When it says "threaded GLib program", it actually means "GLib program that calls g_thread_init() at all (even if it doesn't create threads anyway)". (For ORBit2's case, though this refinement doesn't matter, as ORBit2 normally does use threads.) Unfortunately that requirement isn't enforced, and especially on Linux many programs that end up calling g_thread_init() after *tons* of other (even quite complex) GLib API work just fine anyway. Not so on Windows, though, and maybe not on various non-Linux POSIX systems either. So in general, from a correctness and pedantic point of view, I think it is very wrong for a library to call g_thread_init() at some random place. The library has no way of knowing that no GLib API has been used before. (And in most cases for instance gtk_init() has surely been called already.) Instead libraries that are going to use GLib from multiple threads should check that g_thread_init() has been called somewhere else already, and *fail* if not. (Alternatively, that g_thread_init() requirement should be removed not only de facto (as on Linux) but de jure. Then problems that occur on Windows and maybe other non-Linuxes if g_thread_init() hasn't been called first turn into bugs in GLib that have to be fixed...) Suggested patch below... Yes, this is a bit harsh, and no doubt would cause lots of essential programs to g_error() out. Obviously this would have to wait until the code freeze ends, the next GNOME stable release is out, and the next development cycle starts, but after that it could be added to the development branch. (Except that ORBit2 doesn't seem to have separate trunk and (current) stable branches, sigh.) Index: linc2/src/linc.c =================================================================== --- linc2/src/linc.c (revision 2078) +++ linc2/src/linc.c (working copy) @@ -197,7 +197,10 @@ #endif if (thread_safe && !g_thread_supported ()) - g_thread_init (NULL); + g_error ("g_thread_init() has not been called. " + "libORBit2 is going to use threads, so the application " + "should have called g_thread_init(NULL) before any other " + "GLib or GLib-using API."); link_is_thread_safe = (thread_safe && g_thread_supported()); A somewhat milder change would be: Index: linc2/src/linc.c =================================================================== --- linc2/src/linc.c (revision 2078) +++ linc2/src/linc.c (working copy) @@ -197,7 +197,12 @@ #endif if (thread_safe && !g_thread_supported ()) - g_thread_init (NULL); + g_warning ("g_thread_init() has not been called. " + "libORBit-2 would like to use threads, so the application " + "should have called g_thread_init(NULL) before any " + "GLib or GLib-using API. Calling g_thread_init() now here " + "in libORBit-2 is too late, so instead threads won't be used " + "by libORBit-2. This might have bad side-effects. Fix the application!"); link_is_thread_safe = (thread_safe && g_thread_supported());
Looks fine to me; for the next unstable branch.
We haven't had an unstable branch of ORBit2 in two years, are we planning to do one now? Or did you mean release this when the GNOME 2.25.x development cycle opens?
Patch ifnally committed to trunk: 2008-12-15 Tor Lillqvist <tml@novell.com> Bug 547885 - ORBit2 should not call g_thread_init() itself as it is usually too late * src/orb/GIOP/giop.c (giop_init): Don't assume that link_init() managed to actually take threads into use if requested to. See linc2/ChangeLog. Check with giop_thread_safe(). * test/everything/client.c (main) * test/everything/server.c (main) * test/poa/poatest-basic-shell.c (main) * test/test-corbaloc.c (main) * test/test-dynany.c (main) * test/test-giop.c (main) * test/test-mem.c (main) * test/test-performance.c (main) * test/timeout-client.c (main) * test/timeout-server.c (main): Call g_thread_init(NULL). * linc2/src/linc.c (link_init): Don't call g_thread_init() here, it's way too late. g_thread_init() must be the first GLib function called in a program if it is called at all. Instead, if thread support is requested but g_thread_init() has not been called, print a warning and continue without threads.
I'm not sure I agree with this. Apps not using threads should not care about initializing threads. If you need to, add a orbit_init() that initializes threads and have people call that. FWIW, the gnome-vfs backend to file chooser has been initializing threads and it has been working just fine. IMO, breaking a lot of applications and having them add thread initialization when they don't use threads directly just to conform to some sentence in the docs that we all know has never been truly respected is fixing problems that do not exist, and creating some new ones.
OK, so let's revert this change then and resolve this as NOTABUG? And instead just remove the silly sentence from the GLib documentation.
Right - AFAIR I pointed out on the list how the glib slab allocator can become thread-safe having initially not been so extremely trivially in terms of code changes [ which AFAIR was the main sticking point here ].
Change to link_init() has been reverted (in ORBit2 trunk). Resolving as NOTABUG.