Repository navigation
Associate the process-init thread's theap with the thread-done key - #1393
Closed
LongYinan (Brooooooklyn) wants to merge 1 commit into
Closed
LongYinan (Brooooooklyn) wants to merge 1 commit into
LongYinan (Brooooooklyn) wants to merge 1 commit into
Conversation
The thread that runs `mi_process_init` initializes its default theap before `mi_process_setup_auto_thread_done` creates the pthread thread-done key, so `_mi_prim_thread_associate_default_theap` is a no-op for it and `_mi_thread_done` never runs when that thread terminates. Up to v3.4.5, `mi_process_setup_auto_thread_done` re-associated it right after creating the key (`_mi_theap_default_set(&mi_process_theap_main)`); 5d9cb38 ("remove static main theap and tld") dropped that call, so since v3.5.0 nothing associates the first thread. When the first thread to touch mimalloc is short-lived (for example a Node.js worker thread that dlopen's an addon statically linked with mimalloc, then exits), its theap stays registered in the heap with its thread id. A later thread that reuses the same pthread TCB finds it in `mi_heap_check_for_existing_theap` and trips `mi_assert_internal(theap==NULL)` in `_mi_thread_init_with_heap` in debug builds; release builds skip the check but the dead thread's theap and pages are never abandoned or freed. Associate the current default theap with the key right after creating it, when it is already initialized. The v2 line (dev) still does the equivalent via `_mi_heap_set_default_direct(&_mi_heap_main)`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda
LongYinan (Brooooooklyn)
added a commit
to napi-rs/mimalloc-safe
that referenced
this pull request
Sep 4, 2026
…-done hook (#97) mimalloc v3.5.0 never associates the process-init thread's theap with the pthread thread-done key; when that first thread exits (a Node.js worker that loads the addon first), the next thread that reuses its pthread TCB aborts in debug builds (init.c "theap==NULL") and release builds leak the dead thread's theap and pages. Point the mimalloc3 submodule at napi-rs/mimalloc:fix/first-thread-done = upstream v3.5.1 + the fix (microsoft/mimalloc#1393). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda
LongYinan (Brooooooklyn)
added a commit
to rolldown/rolldown
that referenced
this pull request
Sep 4, 2026
0.1.66 ships mimalloc v3.5.1 plus the fix for the process-init thread's thread-done hook (see microsoft/mimalloc#1393, napi-rs/mimalloc-safe#97), so the `=0.1.64` stopgap from 4855394 is no longer needed. Cargo.lock: mimalloc-safe 0.1.64 -> 0.1.66, libmimalloc-sys2 0.1.60 -> 0.1.62. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda
Daan (daanx)
added a commit
that referenced
this pull request
Sep 11, 2026
Collaborator
|
Thank you for the PR; I fixed it in a different way as it should be platform independent -- hope that is ok. Thanks again! |
Jarred Sumner (Jarred-Sumner)
added a commit
to oven-sh/mimalloc
that referenced
this pull request
Sep 13, 2026
Takes upstream v3.5.2 (636510a, 2026-09-12): the process-init theap is associated with the thread-done key (`mi_process_setup_auto_thread_done` moves into `mi_process_init_once`, pr microsoft#1393), `mi_usable_size` always unaligns the pointer, `mi_page_ptr_unalign_ex` returns the offset to `mi_page_ptr_block_check`, `_mi_page_profile_free` becomes `_mi_page_profile_on_free`, the generic `_mi_memzero_block` (the `rep stosb/movsb` paths and `_mi_cpu_movsb_max`/ `_mi_cpu_stosb_max` are gone), small pages keep their page info in front of the slice up to `MI_SMALL_MAX_OBJ_SIZE` and not for OS-aligned blocks (636510a), the multi-config cmake generator support, the `execinfo.h` check, 32-bit warning fixes, and the action version bumps (all SHA-pinned). Conflicts: - .github/workflows/test.yaml: upstream edited the Alpine jobs (arm32 asm upload); they stay removed here (setup-alpine's nested action is not SHA-pinned). The other workflow changes are taken as they are. Everything under src/ and include/ merged without overlap with the fork's changes.
LongYinan (Brooooooklyn)
added a commit
to napi-rs/mimalloc-safe
that referenced
this pull request
Sep 14, 2026
… fix (#104) ### Background Upstream closed microsoft/mimalloc#1393 and took the fix in a different form: microsoft/mimalloc@439dc27b, shipped in **v3.5.2**. That commit works on the default TLS model (verified with the #1393 repro on Linux, debug shared build: `e35400bd` aborts, `v3.5.2` prints `OK`). But it moved `mi_process_setup_auto_thread_done()` into `mi_process_init_once()` **before** `_mi_process_is_initialized = true`. With `MI_TLS_RECURSE_GUARD` (auto-on for `MI_TLS_MODEL_LOCAL` on Apple, which is what `build.rs` forces for v3 on macOS) `_mi_theap_default()` returns `_mi_theap_empty` at that point: ``` mi_process_init_once() [v3.5.2 src/init.c:558-564] ├─ mi_thread_init() __mi_theap_default = &mi_process_theap_main ├─ _mi_tls_slots_init() ├─ _mi_thread_locals_init() ├─ mi_process_setup_auto_thread_done() │ └─ _mi_theap_default() ──► RECURSE_GUARD: _mi_process_is_initialized == false │ ──► returns &_mi_theap_empty │ ──► debug: assert fires / release: no association └─ _mi_process_is_initialized = true ← too late ``` So on plain `v3.5.2`, `cargo test --features v3` on macOS aborts at process init in every debug binary: ``` mimalloc: assertion failed: at "src/init.c":450, mi_process_setup_auto_thread_done assertion: "mi_theap_is_initialized(theap)" ``` and release builds silently skip the association, i.e. the #1393 leak is back on macOS. Reproducible with upstream alone: `cmake -DCMAKE_BUILD_TYPE=Debug -DMI_TLS_MODEL=LOCAL` on macOS, `./mimalloc-test-api` passes on v3.5.1 and aborts on v3.5.2. Same on Linux with `-DMI_TLS_RECURSE_GUARD=ON`. ### Changes - **`mimalloc`** (v2) — `v2.5.1` → `v2.5.2` (official tag, no patches). - **`mimalloc3`** — napi-rs fork `1e5d14ca` (v3.5.1 + #1393) → napi-rs fork `afa6c90d` = upstream **v3.5.2** + one-line fix: set `_mi_process_is_initialized = true` before `mi_process_setup_auto_thread_done()` (napi-rs/mimalloc@afa6c90d, branch `fix/process-init-flag-before-thread-done`). Same approach as #71 / #97. - **`.gitmodules`** — `mimalloc3` branch name updated. - No change to `build.rs` or the Rust sources. ### Verification Upstream, macOS arm64, Debug, `-DMI_TLS_MODEL=LOCAL`: | tree | `mimalloc-test-api` | |---|---| | v3.5.1 | 50/50 | | v3.5.2 | abort at process init (134) | | v3.5.2 + `afa6c90d` | 50/50 | Upstream, Linux x64 (Docker gcc:14), Debug shared, #1393 repro: | tree | `RECURSE_GUARD=OFF` | `RECURSE_GUARD=ON` | |---|---|---| | v3.5.2 | OK | abort (134) | | v3.5.2 + `afa6c90d` | OK | OK | This repo, macOS arm64, every CI command: `cargo test` / `--features secure` / `extended` / `v3` / `extended,v3` all pass; `libmimalloc-sys-test` 258/258 (v2, secure, extended) and 251/251 (v3). Pre-existing and untouched: upstream `mimalloc-test-stress-heaps` fails the `refcount == 1` assert with `MI_TLS_MODEL=LOCAL` on macOS on v3.5.1 too; `extended::tests::runtime_stable_option` fails under `cargo test --workspace` on v2.5.1 too (not in CI). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_016srh5svTkqfW21zGcU6ht9 <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes which mimalloc v3 C sources are built, affecting process/thread allocator initialization on macOS and other TLS recurse-guard configurations. > > **Overview** > Points the **`mimalloc3`** git submodule at the napi-rs fork branch **`fix/process-init-flag-before-thread-done`** instead of **`fix/first-thread-done`**, so v3 tracks upstream **v3.5.2** plus a one-line init ordering fix: set `_mi_process_is_initialized` before `mi_process_setup_auto_thread_done()`. > > That avoids macOS **`MI_TLS_MODEL_LOCAL`** / recurse-guard failures where v3.5.2 alone aborts in debug or reintroduces the #1393 leak in release. No Rust or `build.rs` changes in this diff—only the submodule branch pin in **`.gitmodules`**. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 09fc0c6. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Tyler (tyler274)
pushed a commit
to tyler274/ElyMalloc
that referenced
this pull request
Sep 17, 2026
LongYinan (Brooooooklyn)
added a commit
to rolldown/rolldown
that referenced
this pull request
Sep 20, 2026
Moves the workspace pin from mimalloc-safe 0.1.66 / libmimalloc-sys2 0.1.62 to 0.1.67 / 0.1.63, which vendors mimalloc v2.5.2 and v3.5.2 (we build the v3 tree). The fix we care about is still there, in a better shape. On 0.1.62 the fork carried an 11-line block of its own inside `mi_process_setup_auto_thread_done` that associated the process-init thread's theap with the thread-done key after the fact. Upstream v3.5.2 has now absorbed that idea (microsoft/mimalloc#1393 was closed unmerged, but the same association landed anyway): the setup call moved into `mi_process_init` and does `_mi_theap_default_set(theap)`, which associates via `_mi_prim_thread_associate_default_theap`. On top of v3.5.2 the napi-rs fork keeps a two-line reorder in `mi_process_init` (src/init.c:563-564) that sets `_mi_process_is_initialized = true` *before* `mi_process_setup_auto_thread_done()`. That ordering is load-bearing: under `MI_TLS_RECURSE_GUARD`, which is auto-defined on Apple and Android, `_mi_theap_default()` returns the empty theap while the flag is false, so with upstream's order the association is silently skipped. Diffing the vendored tree against the upstream v3.5.2 tarball shows init.c is the only file the fork touches; every other src/ and include/ file is byte-identical to upstream. Without that hook the thread that first loads the addon (e.g. a short-lived Node worker) leaves its theap registered under a thread id the OS may reuse, which is what aborted Linux debug builds on 0.1.65. Rust-side delta is nil: mimalloc-safe 0.1.66 -> 0.1.67 changes only the version and its libmimalloc-sys2 requirement; build.rs is byte-identical and still compiles through src/static.c, which upstream updated for the new sample-*.c files. Note that this cannot affect the wasm lanes at all -- rolldown_binding only depends on mimalloc-safe for non-wasm targets, and `cargo tree -p rolldown_binding --target wasm32-wasip1-threads -i mimalloc-safe` prints nothing. Gates, all on this worktree: just check-no-tokio ok just lint-rust ok (compiles libmimalloc-sys2 0.1.63) just build-rolldown ok, no generated drift just test-rust ok, 0 failed (main suite 2007 passed / 68 ignored) vp run --filter rolldown-tests test:main 1402 passed / 21 skipped worker teardown test passed on the probe binding vp run --filter rolldown-tests test:wasi 1340 passed / 83 skipped just lint-repo ok The two wasi tests that are skipped rather than passing are the pair in threaded-wasi.test.ts, gated on `capabilities.target === 'wasi-threads'`. They skip because the regen order builds the threaded flavor first and the single-thread flavor last, so the dist ends up wired to `target = 'wasi'`. That is the build order, not this change. Cargo.lock moves only libmimalloc-sys2 and mimalloc-safe. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T34fMrYCf2V13Mg8BKmnda
ByteZ1337
added a commit
to xenondevs/tessera
that referenced
this pull request
Oct 3, 2026
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.
Since v3.5.0 the thread that runs
mi_process_initnever gets its theap associated with the thread-done key._mi_thread_init_with_heapruns beforemi_process_setup_auto_thread_donecreates the key, so_mi_prim_thread_associate_default_theapis a no-op for that thread, and the call that used to fix this up afterwards (_mi_theap_default_set(&mi_process_theap_main)) was removed in 5d9cb38 ("remove static main theap and tld"). If that first thread exits,_mi_thread_donenever runs for it and its theap stays registered with its thread id.We hit this with a Node.js addon that links mimalloc statically: a worker thread loads the addon first (so the constructor runs there), exits, and the next thread that gets the same pthread TCB aborts on its first allocation in debug builds:
Release builds skip the assert and just leak the dead thread's theap and pages. v3.4.5 is fine, v3.5.1 still has it. The v2 line does the equivalent association in
_mi_heap_set_default_direct(&_mi_heap_main).The fix associates the current theap with the key right after creating it, if it is already initialized. Teardown of the static main theap already works once the destructor fires (
_mi_tld_detach_theaps, static memid skipped,thread_idset toMI_THREADID_INVALID).ctestpasses before and after; the existing tests never let the first thread exit, so they don't catch this.Standalone repro (mimalloc built with
-DCMAKE_BUILD_TYPE=Debug -DMI_BUILD_SHARED=ON)Before: T1 and T2 print the same
pthread_self, then the assertion above. After:OK.