OK the glist failure is just msvc not folding multiple constant strings into one in debug mode
l = NULL;
l = g_list_append (l, "a");
ll = g_list_find(l, "a");
that second call to find may or may not be the same address for "a", an in msvc's case it appears it's not
it's also possible that slist hack in gconstructor (calling g_slist_find on NULL in order to try and trick the linker into including our symbol) isn't needed anymore) and we could just put an INCLUDE comment pointing to _array##_func and call it a day
it was added in GNOME/glib@db4df990
I think the CRT popups are what's causing the siginvalid, glib master has actual test failures even in like, the glist tests, and even for dynamic builds, which is concerning
I ran the tests and a few things came up:
Most failures were because g_test_trap_subprocess calls g_spawn_async_with_pipes, it just passes stdin/out/err but gspawn does not know how to do that without the helper process today. There is however support for just getting the helper process from next to the test, so that's an option.
I think it's probably best to just copy the helper next to the tests for now, so that this work can be merged without also redesigning the entire gspawn infrastructure, that can come later.
Also: the tests don't set CRT error handling so when they fail the pop up the "this program had an error" window, this is also solvable separately
Do we still need this at all, even in dynamic builds.
It would be simpler to just use constructors unconditionally.
This looks much better than before, and will probably be an improvement (assuming the tests pass). I still kinda prefer directly piggybacking on the crt's special sections that are pointed to by TLS_IMAGE_DIRECTORY, since that way each "runtime" has it's own direct tls callback, and does not have to piggyback on some other runtime's linked-list of destructors (as this does, and as the strategy of compiling a C++ TU with a TLS destructor in it would). I think the only real downside is that this wastes a TLS slot (there are only a few thousand even on modern windows), but it's supported so ... that's a big upside.
Actually, I wonder what FlsAlloc actually does here, it might itself piggyback on the C++ destructor code. (one should be able to tell by looking in a debugger)
Is this true for freeing the fls index or is only the destructor associated with that fls index called?
This probably isn't good, tls slots are quite a limited resource. It's not a dealbreaker though
A great point - I've rewritten pretty much the whole fix, so the destructor only runs at most once per thread (but runs all gprivate destructors). At least, that's what I think it's doing now.
A lot of tests still don't compile (but meson compile
still exits with 0) so meson test
just fails them with exit code exit status 2147483651 or signal 2147483523 SIGinvalid
. That's not great
I've given all this some thought and changed the strategy altogether - hopefully this take addresses your concerns @barcharcraz? (It's all explained inline in a long comment)
Fixed, this function now has the exact same signature as in the main branch.
I think I just addressed that.
fasterthanlime (1f6dcf46) at 08 Dec 13:02
Fix gio test symbol visibility
fasterthanlime (3137f7d0) at 08 Dec 12:54
Add gslist.h in gconstructor.h, where G_DEFINE_CONSTRUCTOR is defined
fasterthanlime (40563297) at 08 Dec 12:22
Use a single FLS index for cleanup
Kind of a meta comment but, the real "supported" way to do this in windows (with msvc) would be to just write the windows implementation of GPrivate in C++ instead of C.
mingw may create problems for this approach I guess (I'm not sure how much they expose here, and I don't really want to read the mingw runtime code, as it could trigger a cooldown for me, I don't think they link to our startup code).
According to the docs (https://docs.microsoft.com/en-us/windows/win32/api/winnt/nc-winnt-pfls_callback_function) it's called when an FLS index is freed as well.
My understanding here is limited, but: if the first FlsAlloc call returns zero, then we call it again, but neither actually get cleaned up until the thread exits, right? (So, the answer to your question would be no?)
Actually I have a semantics question here.
Previously, unless I'm mistaken, g_thread_win32_thread_detach was called once on thread termination / detach and would run all the destructors. This new implementation using FLS seems like it's called once PER GPRIVATE on thread detach/termination. Is this a misunderstanding? If not doesn't g_thread_win32_thread_detach need modification to only run destructors associated with the specific GPrivate.
I'm beginning to think just doing it ourselves with tls callbacks as I did in GNOME/glib!1454 (closed) might be a better idea, but that's colored by the fact that I wrote that code :D.
can this codepath result in prematurely calling g_thread_win32_thread_detach if we allocated a tls index of zero?
Sidenote: the whole fix from GNOME/glib#2058 (closed) seems really suspect to me, it seems like we should just initialize key->p to TLS_OUT_OF_INDEXES and skip this nonsense with zero, but I digress.