mimalloc: fix NULL deref of the thread-locals array after a thread's teardown

[robobun]

Problem

• On Linux (glibc and musl) and Windows, a thread that touched a non-main mimalloc heap crashes if anything touches a non-main heap on that thread again after mimalloc's own thread teardown has run. Release builds SIGSEGV in mi_thread_local_get_regular (vendor/mimalloc/src/threadlocal.c:186, reading tls->count through NULL); MI_DEBUG builds (bun bd) abort with:

  mimalloc: assertion failed: at "../../vendor/mimalloc/src/threadlocal.c":184, mi_thread_local_get_regular
    assertion: "tls!=NULL"
  

• Cause: _mi_thread_locals_thread_done() (threadlocal.c:205, run from _mi_thread_done, i.e. mimalloc's pthread key destructor / FLS callback) frees the thread's slot array and stores NULL into the mi_thread_locals thread local. The direct-thread-local flavour of mi_define_thread_local (threadlocal.c:54, what our glibc, musl and Windows builds compile) returns that NULL from _get(), and the three readers (mi_thread_local_get_regular, mi_thread_local_set_regular, mi_thread_locals_expand) dereference it. The pthreads flavour used on macOS and Android (threadlocal.c:45) already maps NULL back to the initial value, so those builds are not affected.
• Reached in two ways:
  • Debug builds: _mi_thread_done itself. It drops the array (init.c:866) before it abandons the thread's theaps (init.c:888 -> mi_theap_collect_ex -> mi_theap_is_valid -> _mi_heap_theap_peek -> _mi_thread_local_get), so any thread exiting while it still has a theap of a non-main heap that is not the one in _mi_theap_cached aborts. In Bun that is a Worker that called Bun.TOML.parse/Bun.YAML.parse (parks a per-thread Arena that outlives the thread, src/runtime/api.rs:272) and then used any other Arena (deterministic, see the test), and it is the threadlocal.c:184 abort quoted in bundler: join in-flight pool tasks before tearing the bundle down #37480's bundler error path (10/10 on an unpatched debug build here, 0/10 with this patch; that PR's own fix is about the teardown ordering and still stands).
  • Release builds: a later TLS destructor on the exiting thread freeing a block of a non-main heap (mi_free -> mi_free_try_collect_mt free.c:417/419 -> mi_abandoned_page_try_reclaim free.c:355 or _mi_arenas_page_try_reabandon_to_mapped arena.c:1260 -> _mi_page_associated_theap_peek prim-tls.h:475), or allocating from one (_mi_heap_theap_get_or_init heap.c:101). Bun's own TLS destructors only free main-heap memory today, so this is latent for us (JSC's structure heap and every bun_alloc::Arena are non-main heaps, so every JS thread has the NULLed array at exit); the C harness below exercises it directly.

Fix

• patches/mimalloc/threadlocal-get-initval-fallback.patch: backport of microsoft/mimalloc@6def7be ("fix thread_locals_get"), one line: the direct-thread-local _get() returns initval when the variable is NULL, which is what the pthreads flavour already does. _peek() is unchanged, so _mi_thread_locals_thread_done still sees the NULL and does not free the static empty array.
• Why this is correct: after teardown, reads now see mi_thread_locals_empty (count 0) and return NULL, which is the exact state of a thread that never used a non-main heap and which every caller already handles (_mi_page_associated_theap_peek returns NULL, the reclaim/reabandon paths bail out, _mi_heap_theap_get_or_init creates a theap). A write after teardown goes through mi_thread_locals_expand, which already treats count 0 as "allocate fresh" (threadlocal.c:105), and the re-initialised thread re-registers its key so the next destructor round frees that array again. mi_slot_fast has initval NULL, so its behaviour is unchanged; the pthreads branch is not touched. The fork pin mimalloc: sync the fork with upstream dev3, fix the Android emulated-TLS crash #37367 moves to (oven-sh/mimalloc 49182d59) already contains this commit, so this patch is the stop-gap until that lands and gets dropped with it.
• scripts/build/workarounds.ts gets an entry whose expectedToBeFixed trips as soon as MIMALLOC_COMMIT is anything other than the pin this patch was written against, so a bump (mimalloc: sync the fork with upstream dev3, fix the Android emulated-TLS crash #37367) fails configure with "delete the patch" instructions instead of merging cleanly and then failing git apply on every fresh fetch (what happened in build(mimalloc): drop strnlen-oob-read.patch, already fixed at pinned commit #34335). Verified by bumping the constant locally: configure fails with that message; with the real pin it is a no-op and build.ninja is unchanged.
• Verified:
  • test/js/node/worker_threads/worker_destruction.test.ts, "a Worker that used several allocator heaps exits cleanly": 4/4 failures on a debug build of main with the assertion above in stderr, 3/3 passes (and the whole file passes) with the patch. A release build passes it both ways, since the assertion that makes _mi_thread_done itself read the array only exists in MI_DEBUG builds; the release-only paths are covered by the C harness.
  • C harness against vendor/mimalloc/src/static.c built with exactly the flags mimalloc.ts uses for a Linux release build (-O2 -DNDEBUG -DMI_BUILD_RELEASE -DMI_MALLOC_OVERRIDE -ftls-model=initial-exec ...): threads allocate from an mi_heap_new heap and exit with a pthread key destructor that runs after mimalloc's. Unpatched: 20/20 SIGSEGV at threadlocal.c:186 for each of the three paths (free after teardown via arena.c:1260; malloc then free via free.c:355; mi_heap_malloc via heap.c:101). Patched: 0/20 for each. The same program and a two-heaps-then-exit variant built with -DMI_DEBUG=3: unpatched aborts with the tls!=NULL assertion, patched runs clean.
  • bun bd re-fetches the dep and applies the patch (vendor/mimalloc/.ref changes, threadlocal.c:57 carries the new line); heapStats-mimalloc.test.ts and worker-terminate-lifetime.test.ts still pass on the patched build.

Background

• mimalloc v3 heaps and theaps: an mi_heap_t (the main heap behind malloc, or a private one from mi_heap_new/mi_heap_new_in_arena; Bun's bun_alloc::Arena and JSC's structure heap are private heaps) owns no pages itself. Each thread that allocates from a heap gets its own mi_theap_t for it, which holds that thread's pages; allocation and most frees go through the calling thread's theap for the block's heap.
• Slot array: the main heap's theap lives in a dedicated fast thread local. The theaps of every other heap live in one per-thread array indexed by a key stored in heap->theap; mi_thread_locals is the thread local pointing at that array, mi_thread_locals_empty is the static zero-length array a fresh thread starts with, and _mi_thread_local_get/_set are the accessors _mi_heap_theap_peek, _mi_page_associated_theap_peek and _mi_heap_theap_get_or_init use.
• Thread teardown: mimalloc registers a pthread key (FLS callback on Windows) at process start, so _mi_thread_done runs among the first destructors when a thread exits. It frees the slot array, then abandons the pages of the thread's theaps (other threads can later pick them up) and frees the theaps. Destructors registered later (WTF's, libc++abi's, Rust's on musl) still run on the thread afterwards, and an allocation from one of them re-initialises the thread's main theap; neither of those restores the slot array, which is the window this patch handles.
• _mi_theap_cached: a one-entry per-thread cache of the last theap returned by the heap API, consulted before the slot array. A heap being destroyed clears it, which is why the repro needs a second heap after the parked one.
• patches/<dep>/: files listed in a dep's patches: are git apply'd over the fetched archive and hashed into the source identity (scripts/build/source.ts, fetch-cli.ts), so editing or adding one re-fetches the dep; the fork itself is not writable from here, as with mimalloc: fold every thread's theap into the subproc stats aggregate #34739. scripts/build/workarounds.ts is the build system's registry of temporary fixes: configure evaluates each entry's expectedToBeFixed and fails with the entry's cleanup text once it returns true.

C harness

// clang -x c++ -std=gnu++20 -O2 -DNDEBUG -DMI_BUILD_RELEASE -DMI_STATIC_LIB -DMI_SKIP_COLLECT_ON_EXIT=1
//       -DMI_NO_PROCESS_DETACH=1 -DMI_DEFAULT_ALLOW_THP=0 -DMI_MALLOC_OVERRIDE -fno-builtin-malloc
//       -ftls-model=initial-exec -fPIC -Ivendor/mimalloc/include -c vendor/mimalloc/src/static.c
// clang -O1 -g -pthread -fno-builtin -Ivendor/mimalloc/include -c late_dtor.c && clang++ -pthread late_dtor.o static.o
// ./a.out [free|reinit|alloc]
#include <mimalloc.h>
#include <pthread.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define NTHREADS 4
#define NBLOCKS 64
#define BLOCK_SIZE (8 * 1024)

static mi_heap_t* heap;
static pthread_key_t late_key;
static const char* mode;
typedef struct { void* blocks[NBLOCKS]; } stash_t;

static void late_dtor(void* arg) {            // runs after mimalloc's own key destructor
  stash_t* stash = (stash_t*)arg;
  if (strcmp(mode, "reinit") == 0) free(malloc(100));          // re-initialises the thread -> free.c:355
  if (strcmp(mode, "alloc") == 0) mi_free(mi_heap_malloc(heap, 64));   // heap.c:101
  else for (int i = 0; i < NBLOCKS; i++) mi_free(stash->blocks[i]);   // arena.c:1260 (pages were abandoned full)
  free(stash);
}

static void* thread_main(void* arg) {
  stash_t* stash = (stash_t*)calloc(1, sizeof(stash_t));
  for (int i = 0; i < NBLOCKS; i++) { stash->blocks[i] = mi_heap_malloc(heap, BLOCK_SIZE); memset(stash->blocks[i], 1, BLOCK_SIZE); }
  pthread_setspecific(late_key, stash);
  return NULL;
}

int main(int argc, char** argv) {
  mode = argc > 1 ? argv[1] : "free";
  free(malloc(1));                              // mimalloc's key is created before ours
  heap = mi_heap_new();
  pthread_key_create(&late_key, late_dtor);
  pthread_t t[NTHREADS];
  for (int i = 0; i < NTHREADS; i++) pthread_create(&t[i], NULL, thread_main, NULL);
  for (int i = 0; i < NTHREADS; i++) pthread_join(t[i], NULL);
  printf("ok (%s)\n", mode);
}

Unpatched stacks (release flags):

free:    mi_thread_local_get_regular threadlocal.c:186 <- _mi_page_associated_theap_peek prim-tls.h:475
         <- _mi_arenas_page_try_reabandon_to_mapped arena.c:1260 <- mi_free_try_collect_mt free.c:419 <- late_dtor
reinit:  mi_thread_local_get_regular threadlocal.c:186 <- _mi_page_associated_theap_peek prim-tls.h:475
         <- mi_abandoned_page_try_reclaim free.c:355 <- mi_free_try_collect_mt free.c:417 <- late_dtor
alloc:   mi_thread_local_get_regular threadlocal.c:186 <- _mi_heap_theap_get_or_init heap.c:101 <- mi_heap_malloc <- late_dtor

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 53 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ea4c951c-8dcd-4f06-a2de-38e9c7dbbe68

📥 Commits

Reviewing files that changed from the base of the PR and between b7a0431 and 1117d28.

📒 Files selected for processing (4)

• patches/mimalloc/threadlocal-get-initval-fallback.patch
• scripts/build/deps/mimalloc.ts
• scripts/build/workarounds.ts
• test/js/node/worker_threads/worker_destruction.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

Status: reproduced and fixed, self-review in progress.

• Reproduced on a debug build of main (bun bd) with the new case in test/js/node/worker_threads/worker_destruction.test.ts: 4/4 runs abort with threadlocal.c:184 "tls!=NULL"; 3/3 pass with the patch. The same abort is the one quoted in bundler: join in-flight pool tasks before tearing the bundle down #37480 (10/10 unpatched, 0/10 patched here).
• Release-build paths reproduced with the C harness in the description against mimalloc built with this repo's release flags: 20/20 SIGSEGV per path unpatched, 0/20 patched.
• Note for whoever reads the evidence check: the fix lives in patches/ and scripts/build/, not src/, so an automated fail-before that only strips src/ will see the test pass both ways. The fail-before runs above were done against a build of main without the patch.

[github-actions]

This PR may be a duplicate of:

1. mimalloc: sync the fork with upstream dev3, fix the Android emulated-TLS crash #37367 - Bumps MIMALLOC_COMMIT to a pin (oven-sh/mimalloc 49182d59) whose src/threadlocal.c already contains the byte-identical initval fallback this PR carries as a patch, so it delivers the same fix by other means.

🤖 Generated with Claude Code

[claude]

I reviewed this PR and didn't find any bugs. The one-line patch is a verbatim backport of an upstream mimalloc commit and the workarounds/test plumbing follows existing patterns exactly, but since it changes the global allocator's thread-teardown behavior a maintainer sign-off would still be worthwhile.

What was reviewed:

• Patch context matches the direct-thread-local mi_define_thread_local macro; _peek() is left unchanged so _mi_thread_locals_thread_done still sees NULL and won't free the static empty array; mi_slot_fast's initval is NULL so its behavior is unchanged.
• patches: field on Dependency and the workarounds.ts entry match the shape used by other deps; the new import of deps/mimalloc.ts into workarounds.ts introduces no cycle.
• New test drains stdout/stderr/exited concurrently, wires the worker error event to a nonzero exit, and asserts stderr/stdout/exitCode in that order per harness conventions.

Extended reasoning...

Overview

This PR backports microsoft/mimalloc commit 6def7be9 as a one-line patch (patches/mimalloc/threadlocal-get-initval-fallback.patch) so the direct-thread-local flavour of name##_get() returns initval instead of NULL after _mi_thread_locals_thread_done() has freed the per-thread slot array. It wires the patch into scripts/build/deps/mimalloc.ts via the existing patches: mechanism, adds a self-obsoleting entry in scripts/build/workarounds.ts that fails configure once MIMALLOC_COMMIT moves off the current pin, and adds a Worker regression test in test/js/node/worker_threads/worker_destruction.test.ts.

Security risks

None identified. The patch narrows a NULL dereference into a fallback to a static empty array, which is the same state a fresh thread starts in and which every caller already handles. No new attack surface, no user-controlled input, no auth/crypto/permissions.

Level of scrutiny

High. mimalloc is Bun's global malloc replacement on Linux and the JS heap allocator everywhere; every allocation and free routes through it, and the affected code runs during thread teardown on every worker/JS thread that ever used a non-main heap. Even though the diff itself is a verbatim upstream backport with a very thorough mechanism analysis and a C harness reproducing all three release-build crash paths, allocator changes are the definition of a critical code path and warrant a maintainer's explicit sign-off rather than an automated approval.

Other factors

• The build-system changes are purely additive and mirror existing patterns (patches: used by libarchive/highway/cares/etc.; the workarounds.ts entry mirrors the neighboring entries and imports mimalloc.source(cfg) without creating an import cycle — only configure.ts imports workarounds.ts).
• The regression test follows harness conventions (test.concurrent, bunEnv/bunExe, concurrent pipe drain, error event wired to reject via process.exit(2), exitCode asserted last). The author is transparent that the test only fails on debug (MI_DEBUG) builds without the patch — release builds need the C harness to reproduce — which is an acknowledged limitation of the automated coverage.
• No CODEOWNERS entries cover these paths and no prior human review comments exist on the PR. The bug-hunting system found nothing. Deferring solely because the change sits in the allocator, not because of any concern with the diff itself.

[robobun]

On the #37367 overlap: that PR is the real fix, and it is already described that way in the Fix section. This one is the stop-gap on the current pin. The patched line is byte-identical to the one #37367's pin carries, so there is nothing to reconcile between the two:

• If mimalloc: sync the fork with upstream dev3, fix the Android emulated-TLS crash #37367 lands first, this PR is simply closed.
• If this lands first, the workarounds.ts entry fails configure the moment MIMALLOC_COMMIT moves, so mimalloc: sync the fork with upstream dev3, fix the Android emulated-TLS crash #37367's rebase deletes the patch, its patches: line and the entry (verified locally by bumping the constant).

The reason to carry it in the meantime is that debug builds abort on the current pin today (the Worker case in the test, and the threadlocal.c:184 abort quoted in #37480), and #37367 is a 334-commit dependency sync whose landing date is not tied to this bug. #38168 takes the same approach on the same pin for #38051. If the preference is to wait for #37367 instead, closing this is fine.

Nothing to change from the review above.

[robobun]

^{Updated 12:26 PM PT - Aug 13th, 2026}

❌ @robobun, your commit 1117d28 has 1 failures in Build #94739 (All Failures):

• test/js/web/timers/setInterval.test.js - code 1 on 🐧 13 x64-asan

---

🧪   To try this PR locally:

bunx bun-pr 38199

That installs a local version of the PR into your bun-38199 executable, so you can run:

bun-38199 --bun

[Jarred-Sumner]

Folded into #37367: its pin (oven-sh/mimalloc be7eb3ff1) has this fix in the fork itself, so the .patch stop-gap isn't needed. The Worker regression test from this PR was carried over there.