Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions patches/mimalloc/threadlocal-get-initval-fallback.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
Backport of microsoft/mimalloc 6def7be9 ("fix thread_locals_get", on dev3
after the pinned oven-sh/mimalloc commit). Drop this patch once the pin moves
to a revision that includes it.

mimalloc keeps the per-thread theaps of every non-main heap (mi_heap_new,
mi_heap_new_in_arena) in one per-thread slot array reached through the
mi_thread_locals thread local. The variable starts out pointing at
mi_thread_locals_empty, but _mi_thread_locals_thread_done (run from
_mi_thread_done, i.e. mimalloc's pthread key destructor at thread exit) frees
the array and stores NULL.

The pthread-keys flavour of mi_define_thread_local (macOS, Android) already
maps that NULL back to the initial value in name##_get(). The direct
thread-local flavour, which is what Bun's glibc, musl and Windows builds
compile, returned the raw NULL, so everything that reads the array on a thread
after its teardown dereferenced it: mi_thread_local_get_regular (threadlocal.c
tls->count), mi_thread_local_set_regular, and mi_thread_locals_expand.
That happens for any non-main-heap access made on a thread after its
_mi_thread_done: a later TLS destructor freeing a block of such a heap
(mi_free -> mi_free_try_collect_mt -> _mi_page_associated_theap_peek), or,
in MI_DEBUG builds, _mi_thread_done itself (mi_thread_theaps_done ->
mi_theap_is_valid -> _mi_heap_theap_peek) when the exiting thread still has a
theap of a non-main heap that is not the cached one, which aborts with
mimalloc: assertion failed: at "src/threadlocal.c":184, "tls!=NULL"

With the fallback, reads after teardown see the empty array (count 0) and
return NULL, exactly like a thread that never used a non-main heap; a write
after teardown allocates a fresh array, which the re-registered thread-done
frees again.

--- a/src/threadlocal.c
+++ b/src/threadlocal.c
@@ -54,7 +54,7 @@
#define mi_define_thread_local(tp,name,initval) \
static mi_decl_thread tp __##name = initval; \
static inline tp name##_peek(void) { return __##name; } \
- static inline tp name##_get(void) { return __##name; } \
+ static inline tp name##_get(void) { tp result = __##name; return (result!=NULL ? result : initval); } \
static inline bool name##_set(tp val) { __##name = val; return true; } \
static inline void name##_delete(void) { }
#endif
6 changes: 6 additions & 0 deletions scripts/build/deps/mimalloc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ export const mimalloc: Dependency = {
commit: MIMALLOC_COMMIT,
}),

// Backport of microsoft/mimalloc 6def7be9 (in the dev3 sync after this pin):
// the thread-locals array read after a thread's teardown. The matching entry
// in workarounds.ts fails configure once MIMALLOC_COMMIT moves, so the patch
// gets dropped with the bump instead of breaking the fetch (#34335).
patches: ["patches/mimalloc/threadlocal-get-initval-fallback.patch"],

build: cfg => {
// ─── Override behavior (global malloc replacement) ───
// ASAN: OFF — ASAN interceptors must see the real malloc.
Expand Down
25 changes: 25 additions & 0 deletions scripts/build/workarounds.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import { readFileSync } from "node:fs";
import { join } from "node:path";
import type { Config } from "./config.ts";
import { mimalloc } from "./deps/mimalloc.ts";
import { BuildError } from "./error.ts";
import { satisfiesRange } from "./tools.ts";

Expand Down Expand Up @@ -226,6 +227,30 @@ export const workarounds: Workaround[] = [
`(options.detached block), replace the local 0x80 with libc::POSIX_SPAWN_SETSID, ` +
`drop the explanatory comments, and delete this entry.`,
},
{
id: "mimalloc-threadlocal-get-initval-fallback",
issue: "https://github.com/microsoft/mimalloc/commit/6def7be9458fb8a97b8323af3fb0b0ae04387065",
description:
"mimalloc's thread teardown NULLs the per-thread array holding the theaps of non-main heaps, " +
"and the direct-thread-local build (glibc, musl, Windows) then dereferenced that NULL on any " +
"later non-main-heap access from the exiting thread (MI_DEBUG builds abort in _mi_thread_done " +
"itself). patches/mimalloc/threadlocal-get-initval-fallback.patch backports the upstream fix.",
// The patch is applied to the fetched source on every target, so a pin
// that already contains the fix breaks the fetch everywhere (#34335).
applies: () => true,
expectedToBeFixed: cfg => {
// Written against this pin. Any newer oven-sh/mimalloc pin synced with
// upstream dev3 after 2026-08-09 (e.g. the one in #37367) carries the
// commit itself; a bump that somehow doesn't can re-pin this constant.
const PATCHED_PIN = "1803341d6241d8fa4b3f65fa68cb13a32ad92f04";
const source = mimalloc.source(cfg);
return source.kind === "github-archive" && source.commit !== PATCHED_PIN;
},
cleanup:
`Check that src/threadlocal.c in the new pin returns initval from the direct-thread-local ` +
`name##_get(), then delete patches/mimalloc/threadlocal-get-initval-fallback.patch, its ` +
`entry in scripts/build/deps/mimalloc.ts, and this entry.`,
},
];

/**
Expand Down
31 changes: 31 additions & 0 deletions test/js/node/worker_threads/worker_destruction.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,35 @@ describe("Worker destruction", () => {
expect(stdout.trim()).toBe("terminated 1");
expect(exitCode).toBe(0);
});

// Bun.TOML.parse parks a private mimalloc heap on the worker thread that outlives the worker's
// JS; the Transpiler call then uses (and destroys) another one. mimalloc's own thread teardown
// still has to look at the parked heap's thread state after it has released the thread's
// heap-slot array, and used to dereference the released array there (debug builds abort with
// `threadlocal.c: assertion "tls!=NULL"`). The parent joins the thread, so that takes down the
// whole process before "worker exit" is printed.
test.concurrent("a Worker that used several allocator heaps exits cleanly", async () => {
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`
const { Worker } = require("worker_threads");
const w = new Worker(\`
Bun.TOML.parse("a = 1");
new Bun.Transpiler().transformSync("export const b = 2;");
\`, { eval: true });
w.on("error", e => { console.error(e); process.exit(2); });
w.on("exit", code => console.log("worker exit " + code));
`,
],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toBe("");
expect(stdout).toBe("worker exit 0\n");
expect(exitCode).toBe(0);
});
});