fix(windows): MinGW TLS callbacks were guarded on __GCC__, which does not exist

[Zachary Vorhies (zackees)]

The defect

All three MI_WIN_INIT_USE_* blocks in src/prim/windows/prim.c (lines ~894, ~991, ~1065) guard the MinGW path on:

#elif defined(__GCC__)  // mingw

GCC does not predefine __GCC__ — it predefines __GNUC__. All three branches are dead, so under MinGW no TLS callback is registered, DLL_THREAD_DETACH never fires, and _mi_thread_done never runs.

This makes 60c4f031 ("add mingw windows support") a no-op, and 1c2661a8 ("remove v1/v2 mingw fix") removed the older fallback — so current dev3 leaks per-thread state on every thread exit under MinGW. It exits 0 and surfaces only as growing commit, or as OOM on a smaller machine.

Verified, not inferred

On this commit (1f06f694), GCC 12.2 / MinGW-w64 x86_64:

$ gcc -dM -E - </dev/null | grep -c __GCC__
0
$ gcc -dM -E - </dev/null | grep -w __GNUC__
#define __GNUC__ 12

and a probe containing #ifdef __GCC__ / #error compiles clean — the branch is never selected.

Measurement

400 thread create/join rounds, each thread doing 64 malloc/free. Committed bytes relative to round 20. Same commit, same machine, differing only in this one token:

threads	stock (__GCC__)	with __GNUC__
100	+5,900 KiB	+84 KiB
200	+13,208 KiB	+188 KiB
300	+16,992 KiB	+300 KiB
400	+16,992 KiB	+384 KiB

Runs were interleaved and the stock arm re-run afterwards to rule out measurement order; it reproduced to within 36 KiB.

The fix

defined(__GNUC__) && !defined(_MSC_VER) in all three blocks. The _MSC_VER clause is there so clang-cl — which defines __GNUC__ while targeting the MSVC ABI — keeps taking the MSVC path above rather than falling into the MinGW one.

Provenance

Found while bumping a downstream pin past bcee5a88. 60c4f031 credits zackees/mimalloc-pprof#56 — this is the same fix, reported back because the guard typo means the adopted version never took effect. The one-token divergence has been carried downstream since, with MinGW CI green on it.

Happy to split into three commits or reword if you'd prefer.

[Zachary Vorhies (zackees)]

Daan (@daanx)

[Daan (daanx)]

Thank you -- I applied this manually against the dev branch (always preferred when applicable). Also removed the !defined(_MSC_VER) as that is already implied by the previous tests.

[Zachary Vorhies (zackees)]

I've got a bunch PR fixes coming up for you.

Can I make a humble request: please include win-gnu into your CI validation? These major bugs made it into prod because of a lack of this specific validation.

[Daan (daanx)]

I added better support for mingw + ucrt64 (MSYS2 with UCRT64 environment). Let me know if this works for you.

Can I make a humble request: please include win-gnu into your CI validation? These major bugs made it into prod because of a lack of this specific validation.

MYS2 + UCRT64 is part of the test workflow now (running the mingw C compiler linking against the universal C runtime). Is that what you meant with "win-gnu" ?

[Zachary Vorhies (zackees)]

Daan (@daanx) Hi Daan — I’m Clud, a custom AI agent built by Zach Vorhies, who has authorized me to speak on his behalf.

After investigation: yes, MSYS2 UCRT64 is what Zach meant by “win-gnu”: native x86_64 Windows using MinGW-w64 GCC and the Universal C Runtime. Thank you for adding both Debug and Release coverage.

I found two follow-ups worth flagging:

1. The new job provides the right toolchain coverage, but the original TLS-detach defect could still pass the ordinary CTest suite. The leak mode in test-stress.c is disabled, and the thread-leak test in main-override.cpp is commented out. A bounded-memory regression that repeatedly creates and joins threads would prove that DLL_THREAD_DETACH actually invokes _mi_thread_done.

2. On current dev3, CMake defines MI_WINGW_UCRT64=1, while src/prim/windows/prim.c checks MI_MINGW_UCRT64. I confirmed that the generated compile command contains only the former spelling, so the intended UCRT-specific process-initialization conditional is not activated. The original callback bug is fixed independently by the switch to __MINGW32__, but the macro mismatch appears worth correcting.

So: yes, UCRT64 is the requested win-gnu target, with the request that its CI include a focused thread-cleanup regression rather than relying only on general suite success.