workaround: MinGW thread_local use-after-free in ThreadContext #318

ryanofsky commented at 5:27 PM on July 22, 2026: collaborator

Work around mingw bug thread_local bug https://sourceforge.net/p/mingw-w64/bugs/527/ that causes failures in windows "test cross-built" CI jobs in https://github.com/bitcoin/bitcoin/pull/32387 by skipping thread_local destructors in mingw.

The workaround will result in resource leaks in mingw builds, that might be noticeable if a lot of threads are created and destroyed, but should not be a significant problem in practice.

This change is just a refactoring in other builds (including MSVC) not affected by this bug.

DrahtBot commented at 5:27 PM on July 22, 2026: none

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
Stale ACK	xyzconstant, Sjors

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

client disconnectss -> client disconnects [extra “s” is a typo]

ryanofsky referenced this in commit d3d74e701f on Jul 23, 2026

ryanofsky force-pushed on Jul 23, 2026

ryanofsky commented at 1:34 PM on July 23, 2026: collaborator

Updated 1243dd3309692938d6c71473da180265fe26e8d0 -> 1500701d98a72067874cb7e5b72a9c40b03fe722 (pr/win-tls.1 -> pr/win-tls.2, compare) renaming GThreadContext to CurrentThread

xyzconstant commented at 2:55 AM on July 24, 2026: contributor

In commit 1500701 "workaround: MinGW thread_local use-after-free in ThreadContext"

nit: The commit body still mentions GThreadContext() instead of CurrentThread

xyzconstant commented at 8:32 PM on July 24, 2026: contributor

Code review ACK 1500701

Compiled and ran tests on macOS. Tried to reproduce the heap corruption with a mingw-w64 cross-build run under Wine following some of the instructions in the bug report links but didn't have any success (maybe luck?).

My understanding: MinGW's machinery doesn't guarantee a safe cleanup order for thread_local variables. The idea is to skip the __cxa_thread_atexit call entirely (which registers ~ThreadContext, a non-trivial destructor to run at thread exit) so its scheduled destructor doesn't run on already-freed memory and thereby avoid heap corruption. The changes in this PR implement that by modifying the g_thread_context variable (now returned by CurrentThread()) from a value of type ThreadContext to a pointer of type ThreadContext * on MinGW. This results in only the emutls cleanup running. This frees the pointer's storage at destruction time, and the context object is, as a result, leaked.

The cost of this leakage is that exiting client threads won't release the Thread capability, which means the server threads aren't closed until the connection is closed (closing the connection destroys those OS threads).

Other target builds remain unaffected. The documentation is well-written, and it's clear to any developer why this workaround exists. This PR looks good to me.

in include/mp/proxy-io.h:979 in 1500701d98 outdated

 978 | +//! struct that meant ~ThreadContext walking the request_threads /
 979 | +//! callback_threads std::map trees through freed memory and double-freeing
 980 | +//! their nodes, corrupting the heap.
 981 | +//!
 982 | +//! This was observed in Bitcoin Core Windows CI as intermittent
 983 | +//! STATUS_HEAP_CORRUPTION (0xC0000374) exit code 3221226356 crashes of both

hebasto commented at 2:46 PM on July 27, 2026:

Is this specific to MSVCRT, or does it occur with UCRT as well?

ryanofsky commented at 4:32 PM on July 30, 2026:

re: #318 (review)

Is this specific to MSVCRT, or does it occur with UCRT as well?

I don't think it has anything do to with the C runtime, although agent noted it did not see this error under UCRT builds for some reason (see "since the underlying bug is CRT-independent" comment below)

This error is pretty easy to reproduce by disabling the #ifdef __MINGW32__ workaround in https://github.com/bitcoin/bitcoin/pull/32387 and seeing the cross-compiled CI job in interface_ipc_cli.py and interface_ipc_mining.py tests. You can see the ci errors looking at win-work branch pushes from Jul 22 in `https://github.com/ryanofsky/bitcoin/actions like https://github.com/ryanofsky/bitcoin/actions/runs/29931590196/job/88964021272

Sjors commented at 1:04 PM on July 28, 2026: member

Wrapping g_thread_context in CurrentThread(), and then dropping it entirely, is nice in general. Maybe make it a prep refactor commit(s), so the windows change is more focussed. See sjors/2026/07/mingw.

My agent also couldn't reproduce the original issue on Linux with Wine, initially not on Windows 11 either, but I'll keep trying.

I didn't verify the Windows-specific rationale, but the change itself seems simple enough is well documented.

Sjors commented at 4:05 PM on July 28, 2026: member

ACK 1500701d98a72067874cb7e5b72a9c40b03fe722

It figured it out: https://gist.github.com/Sjors/4b70c60a5d2989bf2ba4adb408d2554c

maflcko commented at 7:49 PM on July 28, 2026: contributor

Seems fine, but given that this was already fixed in https://github.com/mingw-w64/mingw-w64/commit/8e06daa36dfcea4bb491acf4b350658f40738f02 / 13.0.0 , I wonder if it would be easier to just build that version (or later).

This is with the background that the Ubuntu/Debian packages are already broken and possibly unmaintained, so we may want to consider self-building anyway? Ref: https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3748883932

Either self-building directly, or using something like https://github.com/0xB10C/bitcoind-gunix/blob/v31.1/nix/win64/toolchain.nix ? Edit: mingw in guix was bumped to v13 in https://github.com/bitcoin/bitcoin/commit/31eb46f054f8cbd62b3ac44fd98a61f6dda34879

ryanofsky referenced this in commit f5c15ce33f on Jul 30, 2026

workaround: MinGW thread_local use-after-free in ThreadContext

MinGW has a bug where thread_local destructors can run on already-freed memory
at thread exit, causing heap corruption. MSVC builds and non-Windows platforms
are unaffected.

This caused intermittent STATUS_HEAP_CORRUPTION in MinGW CI builds when
~ThreadContext walked freed memory at thread exit. See CurrentThread() in
proxy-io.h for the full explanation and upstream bug references.

Workaround: change CurrentThread() accessor in MinGW builds to return a
heap-allocated object held by a trivially-destructible pointer, skipping
destructor registration at thread exit. The object is deliberately
leaked.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

608d09a8fe

ryanofsky force-pushed on Jul 30, 2026

ryanofsky commented at 4:46 PM on July 30, 2026: collaborator

re: #318 (comment)

mingw in guix was bumped to v13 in bitcoin/bitcoin@31eb46f

Thanks for these details. Would have to look into this more but would be nice if this is already fixed in guix builds and the workaround is unnecessary there.

I think we still might want this code change (maybe narrowed to depend on mingw version or whether the fix is present) if it is easier than self-building mingw in CI. A motivation for this change is getting interface_ipc_cli.py and interface_ipc_mining.py tests to pass in https://github.com/bitcoin/bitcoin/pull/32387 cross-compiled CI jobs

Rebased 1500701d98a72067874cb7e5b72a9c40b03fe722 -> 608d09a8fec7f65cbe3a2c4b2afec69dfca32c15 (pr/win-tls.2 -> pr/win-tls.3, compare) due to silent conflicts with #323

maflcko commented at 12:39 PM on July 31, 2026: contributor

Thanks for these details. Would have to look into this more but would be nice if this is already fixed in guix builds and the workaround is unnecessary there.

It is just a guess that this is already fixed, since I don't have Windows to test. One could check by using ubuntu:resolute in the CI config, maybe?

The general idea is that we are having problems cross-compiling with the apt system packages, so using guix or nix instead could help with that, and possibly also help with making the CI closer to the release bins, and possibly even align the cross-compile docs to produce bins identical to the ones from contrib/guix.

So trying to use a mingw from nix (possibly in combination with a apt system gcc, or so) would be interesting and could possibly fix a bunch of issues (or introduce more issues, heh).

Kino1994 referenced this in commit 6101a2e2e8 on Aug 2, 2026

maflcko commented at 2:51 PM on August 4, 2026: contributor

I think we still might want this code change (maybe narrowed to depend on mingw version or whether the fix is present) if it is easier than self-building mingw in CI. A motivation for this change is getting interface_ipc_cli.py and interface_ipc_mining.py tests to pass in bitcoin/bitcoin#32387 cross-compiled CI jobs

If you want, you can merge https://github.com/bitcoin/bitcoin/pull/35877 (mingw 13) into 32387, to see if it passes CI.

ryanofsky commented at 2:10 AM on August 5, 2026: collaborator

re: #318 (comment)

If you want, you can merge bitcoin/bitcoin#35877 (mingw 13) into 32387, to see if it passes CI.

Thanks I confirmed bitcoin/bitcoin#35877 works and fixes the CI jobs. I'll convert this PR to a draft assuming https://github.com/bitcoin/bitcoin/pull/35877 can be merged and the workaround leaking memory with mingw is no longer necessary.

I still think some change to libmultiprocess is needed to trigger an error if it is built with older versions of mingw that would be unstable without the bugfix. So I will keep this PR open, but it should be less important now

Separately it might also make sense to disable the nontrivial-threadlocal.h check at some point if the underlying mingw bug is fixed.

Test 1: Reverting current workaround

I tested this with 3 pushes. The first push reverted the workaround in this PR and showed the cross-built msvcrt and ucrt jobs failing:

https://github.com/ryanofsky/bitcoin/commits/pr/ipc-win.27-revert-mingw https://github.com/bitcoin/bitcoin/actions/runs/30943797734/job/92119010339?pr=32387 https://github.com/bitcoin/bitcoin/actions/runs/30943797734/job/92119010280?pr=32387

It was actually a little surprising to see the ucrt job failing since it didn't look like UCRT jobs were failing previously, but it does make sense for them to fail.

The msvcrt job showed both interface_ipc_cli.py and interface_ipc_mining.py failing while the ucrt job showed only interface_ipc_cli.py failing. In the mining test the node failed with 3221226356 exit code which is the the heap corruption error described by Claude. And in the cli tests bitcoin-cli failed with the same 3221226356 exit code.

Test 2: Applying fix

The next push added cherry-picked commits from bitcoin/bitcoin#35877 but these failed to compile with error: ‘pthread_self’ was not declared in this scope caused by HAVE_PTHREAD_GETNAME_NP check being true for the nix build of mingw when it was false in the debian build.

https://github.com/ryanofsky/bitcoin/commits/pr/pr/ipc-win.27-nix-mingw https://github.com/bitcoin/bitcoin/actions/runs/30952272732/job/92139206608?pr=32387 https://github.com/bitcoin/bitcoin/actions/runs/30952272732/job/92139206619?pr=32387

Test 3: Verifying fix

The last push fixed the compile error and confirmed the fixes with all jobs now passing:

https://github.com/ryanofsky/bitcoin/commits/pr/pr/ipc-win.27-nix-mingw2 https://github.com/bitcoin/bitcoin/actions/runs/30959207156/job/92159561789?pr=32387 https://github.com/bitcoin/bitcoin/actions/runs/30959207156/job/92159561813?pr=32387

ryanofsky marked this as a draft on Aug 5, 2026

maflcko commented at 4:44 AM on August 5, 2026: contributor

Separately it might also make sense to disable the nontrivial-threadlocal.h check at some point if the underlying mingw bug is fixed.

Maybe, but see https://github.com/bitcoin/bitcoin/pull/30095#discussion_r1608289449