GNOME Bugzilla – Bug 331853
Handle using gslice before g_thread_init() better
Last modified: 2007-07-12 15:14:13 UTC
A follow-up to bug #331367 which ultimately turned out to be mainly caused by a Win32-specific problem in the implementation of GPrivate destructor calling. There might in any case be a potential problem in gslice.c if g_slice* functions are called before g_thread_init() has been called. The attached patch attempts to handle this. I am not sure whether it is actually needed, though.
Created attachment 59752 [details] [review] Suggested patch Patch to thread_memory_from_self(): If called before g_thread_init(), save the ThreadMemory separately. If the process later calls g_thread_init(), then when called in the main thread use the saved ThreadMemory.
Tor, the docs actually say that g_thread_init() has to be the first glib function to be called it is called at all. that's exactly because of the issues you encountered with GSLice if it is used before g_thread_init() and i'm sure there are other pitfalls e.g. with gmem.c if this isn't properly adheared to. so to argue application of this patch, can you provide reasoning for why g_thread_init() would have to be called *after* using some other glib function?
I guess the problem was in the application (Evolution) that didn't call g_thread_init() itself, but relied on it being called somewhere else. If you look in libgnome/libgnome/gnome-init.c, g_thread_init() is called in bonobo_activation_pre_args_parse(). If I recall correctly, this is actually too late, gslice-using functions have already been called before that. Do you think I should open a bug report against libgnome for this? Shouldn't all applications that will use threads call g_thread_init() in main() the first thing they do, isn't it dangerous and misleading to have libbgnome do it as part of gnome_program_init()? Hmm, now I notice that I did't after all add a g_thread_init() call to Evolution's main(). Probably should do that, or at least open a bug report for it... The same problem is probably present in most GNOME apps, and they happen to work just by coincidence?
ok, according to current docs, you should file a bug against evo, yes. but as you point out, if a library uses threading, it actually forces all using apps to have g_thread_init() at the start and that may affect a bunch of gnome programs. so we should probably have an extra glib bug report that investigates deferred calling of g_thread_init() in general. since breaking the current requirement is likely to not only cause problems with GSlice, all other affected code portions will have to be carefully audited for side effects as well.
just a note, as Tor pointed out on irc, calling g_thread_init() at the start of gnome_program_init() might be the better way to fix the issue with lots of gnome programs.
Tor, do we need to keep this open any longer? i.e. do you have any more input on the issue?
I guess no need to keep this open against GLib. I probably should open a bug against libgnome instead.
Ah, I already had... bug #343490.
Reopening, since it's probably best to implement GSlice relaxization about late g_thread_init() calls, according to this thread (see in particular Owen's comments): http://mail.gnome.org/archives/gtk-devel-list/2007-January/msg00005.html (bugs regarding late g_thread_init() calls)
it looks like the test case from bug #433314 can be (ab-)used as a test for attachment #59752 [details] from Comment #1.
thanks Tor. excellent patch btw. though i reworked it a bit to not constrain thread cache retaining to the main thread (which you probably only coded in light of the early analysis of bug #331367) and added a test case that really checks for the g_thread_init()+GSlice problem. FYI, the actual problem *only* manifests itself in limited leaks (i.e. magazines of freed slices from the single-thread time could be lost), so the test case "checks" for possible leaks (hope it's not too fragile in that, time will show). Thu Jul 12 15:46:40 2007 Tim Janik <timj@imendio.com> * glib/gslice.c: migrate per-thread magazine caches from single-thread scenario to first thread using GSlice after g_thread_init(); based on a patch by Tor Lillqvist, fixes #331853. removed warning about g_thread_init() being called after other glib functions (in particular g_slice* calls), because GSlice can cope with this now and the rest of glib is believed to cope as well. * tests/slice-threadinit.c: new test program which tests GSlice working across g_thread_init() calls.