Keep the idle sweep's state on the tld instead of in __thread variables (fixes the Android SIGSEGV, oven-sh/bun#38051) - #17
Merged
Jarred-Sumner merged 1 commit intoAug 13, 2026
Conversation
…read variables The hole-purging sweep kept its re-entrancy guard and per-sweep counters in __thread variables and read the guard from mi_page_free_collect_ex, inside the allocator. Where __thread is emulated (Android before API 29, bun's Android target) the first access on a thread calls malloc, so with mimalloc being malloc the allocator re-entered itself from inside a page collect and, whenever the size class emulated TLS asked for was itself on an exhausted page, recursed until the stack overflowed (oven-sh/bun#38051). The previous pin happened to write another __thread variable during thread init, which materialized each thread's emulated-TLS state while the thread had no pages yet; the dev3 sync removed it and the crash became frequent. The sweep always runs for one specific tld (the owner's own, or the parked thread's when the scavenger sweeps for it), so that tld now carries the state of the running sweep. The guard is read back through the calling thread's default theap, which every TLS model reads without allocating, and the sweep's own collects ask for no un-purging explicitly instead of relying on the guard. The only __thread variable left on the allocator's paths is the message-output guard in options.c, which is only reached when a warning is printed. test-emulated-tls builds the allocator once more with -femulated-tls, as malloc, in the pthreads TLS model, and makes a fresh thread's first page collect happen with the page of every small size bin exhausted in turn, and then has a fresh thread allocate into pages whose holes the scavenger has just purged, in every bin. Without this change it overflows the stack in optimized builds (part 1), overflows it in unoptimized builds (part 2), and trips mi_page_is_valid_init on the scavenger in assertion builds, because the emulated guard write gave the scavenger a theap of its own.
This was referenced Aug 13, 2026
Jarred-Sumner
added a commit
to oven-sh/bun
that referenced
this pull request
Aug 13, 2026
…TLS crash (#37367) Moves the mimalloc pin to oven-sh/mimalloc `bun-dev3-v2` @ `be7eb3ff1`, which is: - oven-sh/mimalloc#15 — the fork synced with upstream `dev3` (261 commits: the #1271 audit fixes, the init/sub-process restructure, reworked theap teardown), fork features re-applied on top. Includes upstream's `thread_locals_get` fix: on Linux/Windows a thread that had used a non-main heap (JSC's structure heap, every `bun_alloc::Arena`) could NULL-deref in `mi_free`/teardown after mimalloc's own thread-done ran. Regression test added here (`worker_destruction.test.ts`, fails 4/4 on a debug build of main). - oven-sh/mimalloc#17 — the idle sweep's state lives on the tld instead of `__thread` variables. On emulated-TLS targets (our Android build, API < 29) the first `__thread` access mallocs, so reading the sweep guard from inside a page collect recursed until the stack was gone. Fixes #38051. - A rate-limited park (`purge_holes_min_interval`) is now swept when its window ends instead of at the scavenger's next unrelated wake (up to its 30 s safety net) — the end-of-burst park is the one that used to be left waiting. - A `MI_DEBUG_FULL`-only assertion exemption for the detached meta theap (aborted `test-heap-churn`/`test-heap-mt` in the fork's debug suite after the sync). Replaces #38168 and #38199 (both were `patches/mimalloc/*.patch` stop-gaps against the old pin). --- ### Measured (macOS arm64, release builds of this same commit, old pin vs new; paired rounds; memory is `Bun.unsafe.memoryFootprint`) | | Old → new | |---|---| | Retained by size class, 1-in-64 survivors | Identical | | Bare startup footprint | 6.54 → 6.75 MB (+0.2 MB, 5/5 pairs) | | express settled footprint after 300k requests, read after a forced full GC | 43.4 → 43.2 MB avg, 5 pairs (equal — the unforced reading that looked ~7 MB lower was GC timing, not the allocator) | | express peak footprint | No consistent direction | | express throughput | 89.5k → 90.1k rps (parity) | The startup-snapshot branch was also built against the equivalent (#16) and its suite passes; that surfaced one snapshot-writer fix, landed on that branch separately. CI is the regression sweep for this change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fork-side version of oven-sh/bun#38168, for oven-sh/bun#38051 (intermittent SIGSEGV on Android since bun moved to this branch's
d078ad06).Problem
__threadvariables (page.c:478-489) and reads the guard frommi_page_free_collect_ex(page.c:1213), i.e. from inside the allocator.aarch64-linux-android28; for API < 29 clang lowers every__threadto emulated TLS.__emutls_get_addressmaterializes a thread's variables on first access withmalloc(), which is this allocator on that target. Clang also speculates that read ahead of the last word of themi_page_has_purgedcheck, so it fires the first time a thread collects any page with an empty free list. If the size class emulated TLS asks for (128 bytes for its per-thread array, 16 for the flag, with compiler-rt) is itself on an exhausted page at that moment, serving it collects that page, reads the still unmaterialized guard again, and recurses until the stack is gone:acd9924awrote__mi_theap_main(another__thread) in_mi_theap_default_setduring thread init, while the thread had no pages yet, which materialized each thread's emulated-TLS state at a harmless moment (breakpoint on__emutls_get_addressshowsmalloc(128)+malloc(23)from there). The sync removed that variable, so the first emulated-TLS allocation moved into the collect path. Native-TLS targets are unaffected.Fix
mi_on_thread_idle(theap.c:286), or the parked thread's when the scavenger sweeps for it. That tld now carries the state of the running sweep:holes_sweeping,holes_sweep_full,holes_sweep_skipped,holes_sweep_visited, appended to the zero-initialized tail ofmi_tld_t._mi_page_purge_holes_begin/endand_mi_page_purge_holestake it; both callers already had it._mi_page_purge_holes_in_progress()reads_mi_theap_default()->tld->holes_sweeping: the default theap is the one thread-local every TLS model reads without allocating. On the owner this is exactly the old semantics. On the scavenger it readsfalse, which is fine because its only remaining purpose is a nested allocation on the sweeping thread (reachable through an output hook on a warning), and such an allocation on the scavenger can only reach the scavenger's own pages: the parked thread's pages are owned by that thread, and every abandoned page the arena pass touches is held out of the map and owned while it is purged (arena.c:1347-1350).theap.c:174,arena.c:1357) become_mi_page_free_collect_no_unpurge, the call the pre-sweep collect already uses (theap.c:107), so "do not un-purge what we are about to purge" no longer depends on any thread-local state.__threadvariable left on the allocator's paths is the message-output guard inoptions.c(recurse), reached only when a warning or error is printed.mi_page_free_collect_excontains no emulated-TLS call;__emutls_get_addressis referenced only frommi_recurse_enter_prim/mi_recurse_exit_prim.claude/sync-dev3(Merge upstream dev3 (261 commits): #1271 audit fixes, init/subproc restructure #15), so it can go in before or after that.Test
test/test-emulated-tls.c, wired up inCMakeLists.txt: builds the allocator once more with-femulated-tls, as malloc (MI_MALLOC_OVERRIDE), in the TLS model Android uses (MI_TLS_MODEL_PTHREADS), so emulated TLS allocates from the allocator under test exactly as on Android. Skipped when the compiler cannot emulate TLS (gcc on x86-64) or under the sanitizers.mi_purge_holes_stats_getthat every bin really did hand a hole run back, so it cannot silently turn into a no-op). Covering every bin means the test does not depend on the sizes a particular emutls runtime (compiler-rt vs libgcc) asks for.bun-dev3-v2without the fix: Release dies with SIGSEGV (stack overflow) in part 1, 3/3; a-O0build without assertions gets through part 1 and dies with SIGSEGV in part 2, 2/2; Debug +MI_DEBUG_FULLaborts 3/3 inmi_page_is_valid_init(page.c:178) on the scavenger thread, because the emulated guard write in_beginmade the scavenger allocate and so gave it a theap of its own, which the sweep's validity check then compares against the parked thread's pages. With the fix: Release,-O0and Debug all printhanded back: 21 hole runs/ok, 3/3 each (nmshows onlyrecurseand the never-read__mi_thread_id_helperleft as emulated variables).ctest, Release and Debug+MI_DEBUG_FULL, clang on linux-x64: every result other than the new test is identical with and without this change. In particulartest-purge-holespasses the same 27 subtests both ways (includingsweep-does-not-unpurge-on-collectandsweep-skips-unchanged-pages, which exercise exactly the guard and counters that moved), andtest-park-handoffbehaves identically. Native TLS discard/reuse counts are unchanged (a sweep discards 6 runs, the following allocations hand 6 back, before and after).MI_DEBUG=3), where parked, inline and full sweeps on the main thread and a worker run clean.Pre-existing on this branch, seen while running the suite on linux-x64 (not touched here)
mimalloc-static, clang 21, glibc) does not compile:src/prof.cusesdl_iterate_phdr/struct dl_phdr_info, which glibc's<link.h>only declares under_GNU_SOURCE(C++ builds, which is what bun uses, define it implicitly). I ran the suite with-DMI_EXTRA_CPPDEFS=_GNU_SOURCE.test-purge-holes(2 unformed-tail subtests),test-prof-adversarial(Release:ld.soassertionnew->l_relocatedduring dlopen),test-stress-subprocs(SIGSEGV in glibc_dl_deallocate_tlswhilemi_process_donejoins the scavenger), and in Debug every test that exits a thread holding a non-main-heap theap aborts onthreadlocal.c:184tls!=NULL(MI_TLS_MODEL_LOCAL:_mi_thread_locals_thread_donesets the array pointer to NULL andmi_thread_local_get_regulardereferences it; in release this is a NULL dereference when such a thread frees a non-main-heap block from a later TLS destructor). The upstream sync in Merge upstream dev3 (261 commits): #1271 audit fixes, init/subproc restructure #15 fixes the last one (_getfalls back toinitval).