GNOME Bugzilla – Bug 698118
meta: Fix static build on Windows
Last modified: 2018-05-24 15:12:12 UTC
Created attachment 241626 [details] [review] Proposed patch win32: Prefer the use of constructors over DllMain This prevents having to depend on DllMain in static libraries Constructors are available in both the GCC build (GCC 2.7 and later) and the MSVC build (MSVC 2008 and later using _Pragma, earlier versions using #pragma) This patch was discussed in http://lists.fedoraproject.org/pipermail/mingw/2013-March/006429.html and http://lists.fedoraproject.org/pipermail/mingw/2013-March/006469.html and is confirmed to work when using GCC as compiler
*** Bug 696557 has been marked as a duplicate of this bug. ***
I'm probably the last one using msvc6, but coulD not get constructors to work with it. But even for newer versions the patch look a bit inconsitent to me. The required init calls are now under #if defined (G_HAS_CONSTRUCTORS) , so it would be symmetric to do these init call in DllMain with #ifndef G_HAS_CONSTRUCTORS
A patch along these lines is necessary for statically linking GLib on Windows with any library that also defines DLLMain inside static libraries (mistakenly or otherwise).
Created attachment 304051 [details] [review] Use constructors instead of DllMain for initialization gcc-2.7+ and msvc support C constructors/destructors, so we should prefer them over DLLMain for initialization/deinitialization on Windows. Especially since DLLMain will only work with DLLs and hence will never work if one statically links GLib on Windows. The threading/locking initialization for Windows also needs to happen during gobject initialization because the order in which constructors are called is not guaranteed, and the gobject initctor needs working locking. Hence, we make g_thread_win32_init() idempotent so it can be called multiple times, and call it from both the gobject init constructor and the glib init constructor. An alternative is to implement ordering for constructors, but that's non-trivial since the way in which ordering must be implemented for gcc, msvc, and the SUN compiler are different. 1. SUN Pro C pre-main (init/fini): Takes a list of functions which are called in order http://docs.oracle.com/cd/E18659_01/html/821-1384/bjaby.html 2. MSVC pre-main (.CRT$X?U) initialization: It is only safe to add functions to .CRT$XCU and .CRT$XIU since the other .CRT$XCA..Z are for internal use. Ordering here is slightly more complicated than the SUN compiler http://www.cnblogs.com/sunkang/archive/2011/05/24/2055635.html https://msdn.microsoft.com/en-us/library/bb918180.aspx http://wiki.osdev.org/Visual_C%2B%2B_Runtime 3. GCC __attribute__ constructor(PRIORITY)/destructor(PRIORITY) Functions are called in order of the PRIORITY number https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html ------ Without this, static linking of GLib on Windows *does not work*, and just segfaults when any code tries to use locking (such as g_print()).
(In reply to Erik van Pienbroek from comment #0) > Created attachment 241626 [details] [review] [review] > Proposed patch > > win32: Prefer the use of constructors over DllMain > > This prevents having to depend on DllMain in static libraries > > Constructors are available in both the GCC build (GCC 2.7 and later) > and the MSVC build (MSVC 2008 and later using _Pragma, earlier > versions using #pragma) > > > This patch was discussed in > http://lists.fedoraproject.org/pipermail/mingw/2013-March/006429.html and > http://lists.fedoraproject.org/pipermail/mingw/2013-March/006469.html and is > confirmed to work when using GCC as compiler This patch is incomplete since the order in which constructors is called is not guaranteed, and hence gobject_init_ctor() might be called before glib_init_ctor() is called and cause a segfault since that uses locking. It only fixes the DllMain redefinition error (which is what the mail was about), and only works when WIN32 threads aren't used.
(In reply to Nirbheek Chauhan from comment #4) > Created attachment 304051 [details] [review] [review] > Use constructors instead of DllMain for initialization > Hmm, this patch only works for static linking because depending on g_thread_win32_init() crosses the module boundary (gobject/ vs glib/), and that symbol is not exposed in libglib-2.0 since it's private.
Update after conversation with Ryan on IRC: <desrt> nirbheek: long story short is that we cannot support static builds on windows until someone comes up with an alternative for DllMain <desrt> and it's not just ctor/dtor... we also need it for notification about thread exits (in order to clean up GPrivate) [...] <nirbheek> desrt, It seems that the idiom to detect when a thread exits is to spawn a different thread that does a WaitForSingleObject() on the original thread's handle. That will return when the thread is signalled (which happens on _endthreadex()), and clean-up can be run there. <nirbheek> https://msdn.microsoft.com/en-us/library/windows/desktop/ms686915%28v=vs.85%29.aspx + https://msdn.microsoft.com/en-us/library/windows/desktop/ms687032%28v=vs.85%29.aspx <desrt> nirbheek: ouch. <desrt> what if someone else is waiting on the same handle? <nirbheek> Presumably everyone will be signalled <desrt> hm. the documents you give present another interesting idea as well <desrt> nirbheek: i'm worried about what happens if we try to go to poll a thread and someone else already freed the handle, specifically <nirbheek> desrt, If someone is polling a win32 thread using Windows API, it's unlikely they'll want to use the glib wrapper for it :) <desrt> it would probably be quite hard to protect against that when we don't own the thread in question (since it may have been created in another library, or via native calls) <desrt> anyway... one other possibility is to allocate a kernel mutex object per-thread, lock it, and poll on it <desrt> the polling thread will acquire the mutex when the previous owner thread quits <desrt> we could do that from the glib worker <desrt> and we have a guarantee that this mutex is only used for this one purpose, so it's safe <nirbheek> This is true <desrt> it's also actually better than our current approach since (as i read today) our current approach doesn't deal with abnormal thread termination ---- So, in summation the current patch is incomplete since destructors aren't called on thread exit, so these patches will leak one GPrivate per thread. The best fix right now is to detect when a thread exits by polling on a kernel mutex. Someone has to implement this. :)
I've attached a patch in bug 792297, which tries to make it at least work in the mingw/posix threads case.
Let's make this bug depend on bug #793086 and bug #792297 because they have patches too that needs to be reviewed to get a complete fix. Let's use this bug as a main/meta bug about windows static build sine it has the best description of the problems in comment #7.
Created attachment 371384 [details] [review] Meson: Early abort static build for Windows It is broken with unobvious build error since 2.48, and meson needs to support building both shared and static with different cflags.
Created attachment 371386 [details] [review] Meson: Early abort static build for Windows It is broken with unobvious build error since 2.48, and meson needs to support building both shared and static with different cflags.
Since the fix is clearly not obvious, in the meantime let's abort the build early instead of failing miserably. Not doing this in autotools build because various distro/projects seems to have half-broken patches that works for them.
Review of attachment 371386 [details] [review]: (In reply to Xavier Claessens from comment #12) > Since the fix is clearly not obvious, in the meantime let's abort the build > early instead of failing miserably. Not doing this in autotools build > because various distro/projects seems to have half-broken patches that works > for them. I’d prefer the same behaviour for autotools and meson. If distros/projects have half-broken patches already, then it should be easy enough for them to extend those patches to remove this error().
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/692.