deps: bump mimalloc to dev3 (bun-dev3-v2)#29251
Conversation
Upgrades to mimalloc v3 via our bun-dev3-v2 fork which carries: - thread-safety fixes for concurrent mi_heap_delete/mi_free and mi_heap_new during another thread's mi_heap_destroy - 1025th-mi_heap_new uninitialized-chunkmap fix (threadlocal.c) - pthread_atfork-based fork handling so the child can allocate - macOS TLS slots 175/176 (avoid Swift runtime keys) - recursive mi_thread_init self-deadlock fix in _mi_os_get_aligned_hint Zig binding changes (subset of #26214): - v3 heaps are first-class; mi_heap_get_default/backing/set_default and mi_heap_check_owned/contains_block are gone. Use mi_heap_main and mi_heap_contains. - Split MimallocArena vtable into heap_allocator_vtable (mi_heap_*) and global_mimalloc_vtable (mi_malloc/mi_free) so the default allocator no longer needs a per-thread heap pointer. - Borrowed.getDefault wraps mi_heap_main (not undefined) so gc/ownsPtr remain valid; downcast only accepts the owned-heap vtable. - mi_heap_area_t gains reserved1. Build: MI_NO_OPT_ARCH=ON; Windows lib name lost the -static suffix.
|
Updated 4:47 PM PT - Apr 13th, 2026
❌ @Jarred-Sumner, your commit d811dec has 3 failures in
🧪 To try this PR locally: bunx bun-pr 29251That installs a local version of the PR into your bun-29251 --bun |
|
Found 3 issues this PR may fix:
🤖 Generated with Claude Code |
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughUpdate pinned mimalloc revision and macOS build flags, add MI_NO_OPT_ARCH, change Windows provides names, refactor allocator to use a process-wide main heap with a global mimalloc vtable and updated FFI (mi_heap_main / mi_heap_contains), expand aarch64 allowlist, update a macOS static-initializer test expectation, add heapStats mimalloc tests, and surface mimalloc stats/dumps via Changes
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/allocators/MimallocArena.zig`:
- Around line 92-94: The predicate/cast contract is inconsistent: isInstance()
currently treats allocators with &global_mimalloc_vtable as MimallocArena but
downcast() only accepts &heap_allocator_vtable, causing unsafe casts (e.g., in
LinkerGraph.takeAstOwnership calling .downcast(this.allocator)). Fix by making
isInstance() and downcast() agree — either: (A) narrow isInstance() to only
return true for &heap_allocator_vtable (owned heaps) so downcast()’s check
remains correct, or (B) broaden downcast() to accept &global_mimalloc_vtable as
well; update the functions named isInstance() and downcast() and ensure callers
like LinkerGraph.takeAstOwnership(), Default.allocator(), and
getThreadLocalDefault() use the corrected predicate or add an explicit guard
before calling downcast().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5efcb009-290d-4f0d-93fc-83efeda36b82
📒 Files selected for processing (3)
scripts/build/deps/mimalloc.tssrc/allocators/MimallocArena.zigsrc/allocators/mimalloc.zig
| std_alloc.vtable == &heap_allocator_vtable, | ||
| "not an owned MimallocArena heap (vtable is {*})", | ||
| .{std_alloc.vtable}, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== MimallocArena.isInstance call sites ==="
rg -n -C3 --glob '*.zig' 'MimallocArena\.isInstance\s*\('
echo
echo "=== Borrowed.downcast call sites ==="
rg -n -C3 --glob '*.zig' 'MimallocArena\.Borrowed\.downcast\s*\(|Borrowed\.downcast\s*\('Repository: oven-sh/bun
Length of output: 1677
🏁 Script executed:
# Check context around line 165 in src/safety/alloc.zig to see if downcast follows
cat -n src/safety/alloc.zig | sed -n '162,180p'Repository: oven-sh/bun
Length of output: 838
🏁 Script executed:
# Verify the actual vtable checks in MimallocArena.zig at the mentioned lines
echo "=== Lines 92-94 (Borrowed.downcast assertion) ==="
cat -n src/allocators/MimallocArena.zig | sed -n '90,96p'
echo ""
echo "=== Lines 264-265 (isInstance function) ==="
cat -n src/allocators/MimallocArena.zig | sed -n '263,267p'Repository: oven-sh/bun
Length of output: 670
🏁 Script executed:
# Check what vtables Default.allocator() and getThreadLocalDefault() produce
echo "=== Search for Default.allocator and getThreadLocalDefault ==="
rg -n -A5 'pub fn allocator\(\)' src/allocators/MimallocArena.zig
rg -n -A5 'pub fn getThreadLocalDefault' src/allocators/MimallocArena.zigRepository: oven-sh/bun
Length of output: 384
🏁 Script executed:
# Check if there are any other Borrowed usages beyond downcast that might have implicit safety assumptions
echo "=== Borrowed struct and all its methods ==="
rg -n -B2 -A8 'pub.*Borrowed' src/allocators/MimallocArena.zig | head -60Repository: oven-sh/bun
Length of output: 2186
🏁 Script executed:
# Look for Default.allocator() - the review mentions it produces global_mimalloc_vtable
echo "=== Default.allocator implementation ==="
rg -n -B2 -A5 'pub.*fn.*allocator.*\(\s*\)' src/allocators/MimallocArena.zig | grep -A5 "Default"
echo ""
echo "=== Check all public Default methods ==="
rg -n -B1 -A6 'pub const Default' src/allocators/MimallocArena.zigRepository: oven-sh/bun
Length of output: 467
🏁 Script executed:
# Double-check for any Borrowed.downcast calls that might exist
echo "=== Any downcast calls anywhere in codebase ==="
rg -n 'downcast\s*\(' --glob '*.zig' | grep -i mimalloc
echo ""
echo "=== Check if any code path could trigger downcast on global_mimalloc_vtable ==="
# Search for patterns that might combine isInstance check with downcast
rg -n -B5 -A5 'downcast' src/allocators/MimallocArena.zigRepository: oven-sh/bun
Length of output: 747
🏁 Script executed:
# Check the context around the downcast call in LinkerGraph.zig
echo "=== Context around downcast call in LinkerGraph.zig:420 ==="
cat -n src/bundler/LinkerGraph.zig | sed -n '410,430p'
echo ""
echo "=== Look at the class/struct definition for LinkerGraph to understand this.allocator ==="
rg -n -B5 -A15 'pub const LinkerGraph = struct' src/bundler/LinkerGraph.zig | head -40Repository: oven-sh/bun
Length of output: 1070
🏁 Script executed:
# Search for LinkerGraph's allocator field definition
echo "=== LinkerGraph allocator field ==="
rg -n 'allocator.*:' src/bundler/LinkerGraph.zig | head -20
echo ""
echo "=== Find where LinkerGraph is instantiated/created to see what allocator is passed ==="
rg -n 'LinkerGraph.*=' src/bundler/LinkerGraph.zig | head -10Repository: oven-sh/bun
Length of output: 325
🏁 Script executed:
# Search for LinkerGraph creation/init calls
echo "=== LinkerGraph creation patterns ==="
rg -n 'LinkerGraph\.init\(' --glob '*.zig' -B2 -A2
echo ""
echo "=== Alternative: search for LinkerGraph{...} construction ==="
rg -n 'LinkerGraph\s*\{' --glob '*.zig' -B2 -A2 | head -40Repository: oven-sh/bun
Length of output: 580
Fix API contract between isInstance() and downcast().
isInstance() (line 265) claims allocators with &global_mimalloc_vtable are MimallocArena instances, but downcast() (line 92) rejects them. This breaks the safety contract: a predicate should reliably guard a cast. The code in src/bundler/LinkerGraph.zig:420 calls .downcast(this.allocator) without an isInstance() guard, relying on runtime assertions to catch misuse—but isInstance() would return true for Default.allocator() or getThreadLocalDefault(), leading to a crash.
Either narrow isInstance() to only owned heaps (&heap_allocator_vtable), or split into separate predicates:
pub fn isInstance(alloc: std.mem.Allocator) bool {
- return alloc.vtable == &heap_allocator_vtable or alloc.vtable == &global_mimalloc_vtable;
+ return alloc.vtable == &heap_allocator_vtable;
}
+
+pub fn isMimallocBacked(alloc: std.mem.Allocator) bool {
+ return alloc.vtable == &heap_allocator_vtable or alloc.vtable == &global_mimalloc_vtable;
+}Alternatively, add a guard before the downcast in LinkerGraph.takeAstOwnership() or document that only owned heaps may be passed.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| std_alloc.vtable == &heap_allocator_vtable, | |
| "not an owned MimallocArena heap (vtable is {*})", | |
| .{std_alloc.vtable}, | |
| pub fn isInstance(alloc: std.mem.Allocator) bool { | |
| return alloc.vtable == &heap_allocator_vtable; | |
| } | |
| pub fn isMimallocBacked(alloc: std.mem.Allocator) bool { | |
| return alloc.vtable == &heap_allocator_vtable or alloc.vtable == &global_mimalloc_vtable; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/allocators/MimallocArena.zig` around lines 92 - 94, The predicate/cast
contract is inconsistent: isInstance() currently treats allocators with
&global_mimalloc_vtable as MimallocArena but downcast() only accepts
&heap_allocator_vtable, causing unsafe casts (e.g., in
LinkerGraph.takeAstOwnership calling .downcast(this.allocator)). Fix by making
isInstance() and downcast() agree — either: (A) narrow isInstance() to only
return true for &heap_allocator_vtable (owned heaps) so downcast()’s check
remains correct, or (B) broaden downcast() to accept &global_mimalloc_vtable as
well; update the functions named isInstance() and downcast() and ensure callers
like LinkerGraph.takeAstOwnership(), Default.allocator(), and
getThreadLocalDefault() use the corrected predicate or add an explicit guard
before calling downcast().
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| /// VTable for owned heaps created with `mi_heap_new`. | ||
| const heap_allocator_vtable = std.mem.Allocator.VTable{ | ||
| .alloc = vtable_alloc, | ||
| .resize = vtable_resize, | ||
| .remap = vtable_remap, | ||
| .free = vtable_free, | ||
| }; | ||
|
|
||
| /// VTable for the process-wide default allocator (`mi_malloc`/`mi_free`). | ||
| const global_mimalloc_vtable = std.mem.Allocator.VTable{ | ||
| .alloc = global_vtable_alloc, | ||
| .resize = global_vtable_resize, | ||
| .remap = global_vtable_remap, | ||
| .free = vtable_free, | ||
| }; | ||
|
|
There was a problem hiding this comment.
🟡 The PR splits the single mimalloc vtable into two (heap_allocator_vtable for owned heaps, global_mimalloc_vtable for the global allocator), but isInstance() returns true for both while Borrowed.downcast() only accepts heap_allocator_vtable — any code following the natural isInstance-then-downcast idiom will panic at the assertf for global-vtable allocators. No existing callers use this pattern today, so there is no immediate breakage, but the API contract is misleading since isInstance() returning true no longer implies downcast() will succeed.
Extended reasoning...
What the bug is and how it manifests
This PR intentionally splits the old single c_allocator_vtable into two vtables: heap_allocator_vtable (used by Borrowed.allocator() for per-heap mi_heap_new arenas) and global_mimalloc_vtable (used by Default.allocator() and getThreadLocalDefault() routing through mi_malloc/mi_free directly). The isInstance() function was updated to return true for both vtables. However, Borrowed.downcast() was updated to assert std_alloc.vtable == &heap_allocator_vtable exclusively, panicking for global-vtable allocators with the message "not an owned MimallocArena heap".
The specific code path that triggers it
isInstance() at line ~268 returns alloc.vtable == &heap_allocator_vtable or alloc.vtable == &global_mimalloc_vtable. Borrowed.downcast() at lines ~268-283 asserts std_alloc.vtable == &heap_allocator_vtable and then calls fromOpaque(std_alloc.ptr). For global-vtable allocators, ptr = undefined, so if the assertion were somehow bypassed, calling fromOpaque(undefined) would be UB/crash.
Why existing code does not prevent it
Before this PR, downcast() called isInstance() internally as its guard condition, so they were always in sync. After this PR, downcast() no longer uses isInstance() — it checks the vtable directly but with the narrower predicate. No existing caller currently chains isInstance into downcast: LinkerGraph.zig:420 calls downcast() directly without any isInstance guard, and the two usages in safety/alloc.zig use isInstance() without downcast(). So the divergence is latent but not currently triggered.
What the impact would be
The conventional semantic of isInstance(alloc) is that it acts as a precondition for safely casting/downcasting the allocator. A future caller writing the natural idiom if (MimallocArena.isInstance(alloc)) { const b = Borrowed.downcast(alloc); } would get a bun.assertf panic in debug/CI builds when alloc comes from Default.allocator() or getThreadLocalDefault(). In release builds without the assertion, fromOpaque(undefined) would be called, which is undefined behavior. Additionally, any code inspecting alloc.ptr after a positive isInstance check (expecting it to be a valid heap pointer) would read undefined for the global vtable case.
How to fix it
Two reasonable options: (1) Rename isInstance to isMimallocBacked or isAnyMimallocInstance to make clear it covers both vtable variants and does not imply downcast() is safe; add a separate isOwnedInstance(alloc) that checks only heap_allocator_vtable. (2) Document clearly on both isInstance and downcast that they have different scopes, so callers know not to use isInstance as a downcast guard.
Step-by-step proof
- Call
MimallocArena.getThreadLocalDefault()→ returns.{ .ptr = undefined, .vtable = &global_mimalloc_vtable } - Call
MimallocArena.isInstance(alloc)→ evaluatesalloc.vtable == &global_mimalloc_vtable→ returns true - Believing
downcastis safe (as it was before this PR), callBorrowed.downcast(alloc) downcastassertsalloc.vtable == &heap_allocator_vtable→ false →bun.assertffires: "not an owned MimallocArena heap"- In a release build without the assertion, execution continues to
fromOpaque(undefined)→ UB
| return @ptrCast(value); | ||
| } | ||
|
|
||
| fn global_vtable_alloc(_: *anyopaque, len: usize, alignment: Alignment, _: usize) ?[*]u8 { | ||
| log("Malloc: {d}\n", .{len}); | ||
| const ptr: ?*anyopaque = if (mimalloc.mustUseAlignedAlloc(alignment)) | ||
| mimalloc.mi_malloc_aligned(len, alignment.toByteUnits()) | ||
| else | ||
| mimalloc.mi_malloc(len); | ||
| return if (ptr) |p| @ptrCast(p) else null; | ||
| } | ||
|
|
||
| fn global_vtable_resize(_: *anyopaque, buf: []u8, _: Alignment, new_len: usize, _: usize) bool { |
There was a problem hiding this comment.
🟡 The new global_vtable_alloc() is missing the debug-mode usable-size assertion that Borrowed.alignedAlloc() has. In debug builds, if mimalloc returns a block smaller than requested through the global path, it will silently go undetected instead of panicking.
Extended reasoning...
What the bug is:
Borrowed.alignedAlloc() (used by heap_allocator_vtable) includes a comptime-guarded debug assertion that validates mimalloc returned a block of at least the requested size:
if (comptime bun.Environment.isDebug) {
const usable = mimalloc.mi_malloc_usable_size(ptr);
if (usable < len) {
std.debug.panic("mimalloc: allocated size is too small: {d} < {d}", .{ usable, len });
}
}The new global_vtable_alloc() introduced in this PR for global_mimalloc_vtable omits this check entirely, returning the pointer immediately after the mi_malloc/mi_malloc_aligned call.
The specific code path:
Default.allocator() and getThreadLocalDefault() now route through global_mimalloc_vtable, which calls global_vtable_alloc(). This is a new code path introduced by this PR. Previously both of these called Borrowed.getDefault().allocator(), which used heap_allocator_vtable → vtable_alloc() → alignedAlloc(), which has the check.
Why existing code doesn't prevent it:
The check in alignedAlloc() is guarded by comptime bun.Environment.isDebug so it only applies to debug builds. In production nothing changes. But global_vtable_alloc() doesn't have any equivalent guard — the assertion is simply absent.
Impact:
In debug builds, any allocation going through Default.allocator() or getThreadLocalDefault() (which is the majority of bun's allocations) will not have the invariant checked. If a mimalloc internal bug caused it to return an undersized block on the global path, it would silently proceed rather than panicking with a clear diagnostic. The scope is broad since Default.allocator() is the primary allocator throughout the codebase.
How to fix:
Add the same debug assertion in global_vtable_alloc() after the allocation:
fn global_vtable_alloc(_: *anyopaque, len: usize, alignment: Alignment, _: usize) ?[*]u8 {
log("Malloc: {d}\n", .{len});
const ptr: ?*anyopaque = if (mimalloc.mustUseAlignedAlloc(alignment))
mimalloc.mi_malloc_aligned(len, alignment.toByteUnits())
else
mimalloc.mi_malloc(len);
if (comptime bun.Environment.isDebug) {
const usable = mimalloc.mi_malloc_usable_size(ptr);
if (usable < len) {
std.debug.panic("mimalloc: allocated size is too small: {d} < {d}", .{ usable, len });
}
}
return if (ptr) |p| @ptrCast(p) else null;
}Step-by-step proof:
- Caller calls
bun.default_allocator.alloc(u8, 64)(which routes throughDefault.allocator()) Default.allocator()returns.{ .ptr = undefined, .vtable = &global_mimalloc_vtable }- The allocator calls
global_mimalloc_vtable.alloc→global_vtable_alloc(_, 64, ...) mi_malloc(64)is called; hypothetically returns a block of usable size 48 due to a mimalloc bugglobal_vtable_allocreturns the pointer with no check- In contrast, the same scenario through
heap_allocator_vtable→alignedAlloc()would callmi_malloc_usable_size, detect 48 < 64, and panic with a clear error message
… update initializer count - Pin → 09fed41b (adds purge-bit-restore on failed claim) - aarch64 allowlist: add ldclr8_relax, ldset8_relax, swp8_rel, swp8_relax — same compiler-rt outline-atomics family as the existing entries, just the relaxed/release memory-order variants that dev3 bitmap code reaches - static-initializers.test: assertion now matches the comment (was already written for v3)
Jarred-Sumner
force-pushed
the
jarred/mimalloc-dev3-v2
branch
from
1136d11 to
f91d3cf
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/verify-baseline-static/allowlist-aarch64.txt (1)
68-70:⚠️ Potential issue | 🟡 MinorUpdate the symbol count in the section header.
The header indicates "(41 symbols)" but after adding the 4 new entries (
ldclr8_relax,ldset8_relax,swp8_rel,swp8_relax), the section now contains 45 symbols.📝 Suggested fix
# ---------------------------------------------------------------------------- # compiler-rt outline atomics. Each helper has both LSE and LDXR/STXR paths; # __aarch64_have_lse_atomics (= AT_HWCAP & HWCAP_ATOMICS) picks at runtime. -# (41 symbols) +# (45 symbols) # ----------------------------------------------------------------------------🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/verify-baseline-static/allowlist-aarch64.txt` around lines 68 - 70, The section header currently reads "(41 symbols)" but four new symbols were added (ldclr8_relax, ldset8_relax, swp8_rel, swp8_relax); update the header count to "(45 symbols)" so it matches the list — locate the header text "(41 symbols)" above the compiler-rt outline atomics block and change the numeric value to 45.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/build/deps/mimalloc.ts`:
- Around line 123-124: The Windows library name in provides() (the libname
variable set in the provides() function) doesn't match the CMake targets that
build "mimalloc-static" and "mimalloc-static-debug"; update either the libname
values or the targets array so they match: change libname in provides() to
"mimalloc-static" when cfg.debug is false and "mimalloc-static-debug" when
cfg.debug is true, or alternatively remove the "-static" suffix from the targets
array so the built targets are "mimalloc" / "mimalloc-debug"; locate the
provides() function (where libname is assigned) and the targets array and make
them consistent.
---
Outside diff comments:
In `@scripts/verify-baseline-static/allowlist-aarch64.txt`:
- Around line 68-70: The section header currently reads "(41 symbols)" but four
new symbols were added (ldclr8_relax, ldset8_relax, swp8_rel, swp8_relax);
update the header count to "(45 symbols)" so it matches the list — locate the
header text "(41 symbols)" above the compiler-rt outline atomics block and
change the numeric value to 45.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: adbe3f4f-7120-4196-ad92-3a89881cdf10
📒 Files selected for processing (3)
scripts/build/deps/mimalloc.tsscripts/verify-baseline-static/allowlist-aarch64.txttest/js/bun/perf/static-initializers.test.ts
| if (cfg.windows) { | ||
| libname = cfg.debug ? "mimalloc-static-debug" : "mimalloc-static"; | ||
| libname = cfg.debug ? "mimalloc-debug" : "mimalloc"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check mimalloc CMakeLists.txt for static library naming logic
# Search for library naming patterns in mimalloc's CMake configuration
rg -n -C5 'mimalloc-static|OUTPUT_NAME|set_target_properties.*mimalloc' --type cmakeRepository: oven-sh/bun
Length of output: 37
🏁 Script executed:
cat -n scripts/build/deps/mimalloc.ts | head -50Repository: oven-sh/bun
Length of output: 2108
🌐 Web query:
mimalloc v3 Windows static library names CMake MI_BUILD_STATIC
💡 Result:
For mimalloc v3 on Windows using CMake with MI_BUILD_STATIC enabled, the static library target is named "mimalloc-static". The output library file is mimalloc-static.lib (on Windows) or libmimalloc-static.a (on Unix-like systems). The CMake option MI_BUILD_STATIC (default ON) controls whether the static library is built: option(MI_BUILD_STATIC "Build static library" ON). To link, use target_link_libraries(myapp PUBLIC mimalloc-static) after find_package(mimalloc CONFIG REQUIRED). This applies to v3.x releases (e.g., v3.2.8 as of 2026), consistent with v2 behavior. Note that Visual Studio .sln builds mimalloc-lib.lib directly, but CMake uses the -static suffix to avoid naming conflicts with DLL import libs.
Citations:
- 1: https://github.com/microsoft/mimalloc/blob/master/CMakeLists.txt
- 2: https://microsoft.github.io/mimalloc/using.html
- 3: Inconsistent output naming of
mimalloc.dllmicrosoft/mimalloc#903 - 4: https://github.com/microsoft/mimalloc/blob/dev/CMakeLists.txt
- 5: https://github.com/microsoft/mimalloc/blob/master/test/CMakeLists.txt
🏁 Script executed:
sed -n '115,130p' scripts/build/deps/mimalloc.tsRepository: oven-sh/bun
Length of output: 565
🏁 Script executed:
sed -n '115,145p' scripts/build/deps/mimalloc.tsRepository: oven-sh/bun
Length of output: 1453
🏁 Script executed:
git log --oneline -20 --all -- scripts/build/deps/mimalloc.tsRepository: oven-sh/bun
Length of output: 213
🏁 Script executed:
git show HEAD:scripts/build/deps/mimalloc.ts | sed -n '115,130p'Repository: oven-sh/bun
Length of output: 565
🏁 Script executed:
git show f91d3cf9:scripts/build/deps/mimalloc.ts | sed -n '115,130p'Repository: oven-sh/bun
Length of output: 565
🏁 Script executed:
git diff a8ee0d38^..a8ee0d38 -- scripts/build/deps/mimalloc.ts | head -60Repository: oven-sh/bun
Length of output: 2100
🏁 Script executed:
git show a8ee0d38 | grep -A 5 -B 5 "mimalloc-debug\|mimalloc-static" | head -50Repository: oven-sh/bun
Length of output: 950
🏁 Script executed:
# Check if there are any build/cmake files that reference the old naming
rg -n "mimalloc-static-debug|mimalloc-static" scripts/build/ || echo "No matches found"Repository: oven-sh/bun
Length of output: 141
🏁 Script executed:
# Check the current HEAD to see both the targets and the libname usage
sed -n '100,130p' scripts/build/deps/mimalloc.tsRepository: oven-sh/bun
Length of output: 1146
🏁 Script executed:
# Check what the actual library output files should be named
# by looking at how libname is used in the provides function
sed -n '115,160p' scripts/build/deps/mimalloc.tsRepository: oven-sh/bun
Length of output: 1708
🏁 Script executed:
# Check the commit that changed the Windows library naming
git log --all --oneline --grep="mimalloc" -- scripts/build/deps/mimalloc.ts | head -5Repository: oven-sh/bun
Length of output: 158
🏁 Script executed:
# Look for the actual commit that changed from mimalloc-static to mimalloc
git log -p --all -- scripts/build/deps/mimalloc.ts | grep -A 10 -B 10 "mimalloc-debug.*mimalloc" | head -60Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
git show f91d3cf9 --stat | head -20Repository: oven-sh/bun
Length of output: 1133
🏁 Script executed:
git show f91d3cf9 -- scripts/build/deps/mimalloc.ts | head -100Repository: oven-sh/bun
Length of output: 3941
🏁 Script executed:
# Check if there was a previous version of this file in CMake form
git log --all --oneline -- cmake/targets/BuildBun.cmake | head -5Repository: oven-sh/bun
Length of output: 332
🏁 Script executed:
# Look for any PRs or discussions about the mimalloc library naming change
git log -p --all -- scripts/build/deps/mimalloc.ts | grep -B 20 "mimalloc-debug\|mimalloc-static" | grep -A 5 -B 5 "Windows" | head -40Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Find commits that changed the Windows libname from mimalloc-static to mimalloc
git log -p --all -- scripts/build/deps/mimalloc.ts | grep -B 5 -A 5 'libname = cfg.debug ? "mimalloc' | head -50Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
# Check the commit just before f91d3cf9
git log --oneline -10 f91d3cf9^..HEAD -- scripts/build/deps/mimalloc.tsRepository: oven-sh/bun
Length of output: 94
🏁 Script executed:
# Check the CMake deletion commit to see what it was before
git show ba05a729 -- cmake/targets/BuildBun.cmake | grep -i mimalloc | head -20Repository: oven-sh/bun
Length of output: 254
🏁 Script executed:
# Look at the commit before ba05a729
git show ba05a729^:cmake/targets/BuildBun.cmake | grep -i mimalloc -A 2 -B 2 | head -40Repository: oven-sh/bun
Length of output: 560
🏁 Script executed:
# Find all commits between a8ee0d38 and f91d3cf9 that modified mimalloc.ts
git log --oneline a8ee0d38..f91d3cf9 -- scripts/build/deps/mimalloc.tsRepository: oven-sh/bun
Length of output: 158
🏁 Script executed:
# Look at the current HEAD commit message to understand the context
git show --quiet f91d3cf9Repository: oven-sh/bun
Length of output: 691
🏁 Script executed:
# Check if oven-sh/mimalloc fork has its own CMakeLists that differs from upstream
# by looking at recent changes to the mimalloc fork
cd /tmp && git clone --depth 1 https://github.com/oven-sh/mimalloc.git 2>&1 | grep -i "fatal\|clone" || echo "Clone check"Repository: oven-sh/bun
Length of output: 66
🏁 Script executed:
# Get commit info for the pinned mimalloc commit
git log --oneline --all | grep "09fed41b" || echo "Not in bun repo"Repository: oven-sh/bun
Length of output: 70
🏁 Script executed:
# Check the actual cmake/targets/BuildBun.cmake from before deletion to see how mimalloc libs were linked
git show ba05a729^:cmake/targets/BuildBun.cmake | grep -i "mimalloc" -B 3 -A 3Repository: oven-sh/bun
Length of output: 782
🏁 Script executed:
# Check how libname is used - look for references to it in the provides object
sed -n '115,154p' scripts/build/deps/mimalloc.tsRepository: oven-sh/bun
Length of output: 1708
🏁 Script executed:
# Look at the git log for the original creation to see if there's context about the naming
git show a8ee0d38:scripts/build/deps/mimalloc.ts | sed -n '100,140p'Repository: oven-sh/bun
Length of output: 1734
Fix Windows library name mismatch in CMake targets.
The CMake targets still build mimalloc-static (line 110), but the Windows provides() function looks for mimalloc and mimalloc-debug (lines 123–124). This mismatch will cause link failures. Either revert the Windows libname back to mimalloc-static-debug/mimalloc-static to match the targets, or update the targets array to remove the -static suffix to match the libname.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/build/deps/mimalloc.ts` around lines 123 - 124, The Windows library
name in provides() (the libname variable set in the provides() function) doesn't
match the CMake targets that build "mimalloc-static" and
"mimalloc-static-debug"; update either the libname values or the targets array
so they match: change libname in provides() to "mimalloc-static" when cfg.debug
is false and "mimalloc-static-debug" when cfg.debug is true, or alternatively
remove the "-static" suffix from the targets array so the built targets are
"mimalloc" / "mimalloc-debug"; locate the provides() function (where libname is
assigned) and the targets array and make them consistent.
| /// | ||
| /// This type is a `GenericAllocator`; see `src/allocators.zig`. | ||
| pub const Default = struct { | ||
| pub fn allocator(self: Default) std.mem.Allocator { | ||
| _ = self; | ||
| return Borrowed.getDefault().allocator(); | ||
| pub fn allocator(_: Default) std.mem.Allocator { | ||
| return .{ .ptr = undefined, .vtable = &global_mimalloc_vtable }; | ||
| } | ||
| }; |
There was a problem hiding this comment.
🟡 The doc comment on Default (line 7) still reads '/// Uses the default thread-local heap. This type is zero-sized.' but this PR changed Default.allocator() to return .{ .ptr = undefined, .vtable = &global_mimalloc_vtable }, routing through mi_malloc/mi_free directly with no thread-local heap pointer. The comment should be updated to reflect the new behavior (e.g. '/// Routes allocations through mi_malloc/mi_free directly; no per-thread heap pointer is cached. This type is zero-sized.').
Extended reasoning...
What the bug is and how it manifests
The doc comment on the Default struct at src/allocators/MimallocArena.zig line 7 reads:
/// Uses the default thread-local heap. This type is zero-sized.
This comment was accurate before this PR, when Default.allocator() called Borrowed.getDefault().allocator(), which in turn called getThreadHeap() → mi_heap_get_default() — a genuinely thread-local heap pointer. After this PR, Default.allocator() returns .{ .ptr = undefined, .vtable = &global_mimalloc_vtable }, which calls mi_malloc/mi_free directly through the global vtable. There is no per-thread heap pointer involved at all.
The specific code path that triggers it
Default.allocator() now routes through global_mimalloc_vtable, which dispatches to global_vtable_alloc() → mimalloc.mi_malloc(). The .ptr field is undefined — not a *mi_heap_t. The PR description itself notes: 'In v3, mi_malloc/mi_free are already thread-local-fast, there is no per-thread default heap to cache.' The code was updated; the comment was not.
Why existing code doesn't prevent it
The stale comment is purely a documentation issue — there is no runtime check or assertion that catches incorrect comments. The implementation is correct; only the doc comment describing it is wrong.
What the impact would be
A developer reading the comment on Default would believe it caches or uses a per-thread mi_heap_t pointer, which is now false. This is particularly confusing because the Borrowed struct does use a per-heap pointer (heap_allocator_vtable), and the distinction is the core architectural change of this PR. Misleading documentation could lead to incorrect reasoning about allocator identity, thread-safety properties, or whether ownsPtr/gc are meaningful on a Default-derived allocator.
How to fix it
Update line 7 to reflect the new behavior:
-/// Uses the default thread-local heap. This type is zero-sized.
+/// Routes allocations through mi_malloc/mi_free directly; no per-thread heap pointer is cached.
+/// This type is zero-sized.Step-by-step proof
- Before this PR:
Default.allocator()calledBorrowed.getDefault().allocator()→getThreadHeap()→mi_heap_get_default()→ returns current thread'smi_heap_t. Comment '/// Uses the default thread-local heap' was accurate. - After this PR:
Default.allocator()returns.{ .ptr = undefined, .vtable = &global_mimalloc_vtable }. Theptrisundefined; no heap is fetched. Calls go throughglobal_vtable_alloc→mi_malloc()directly. - Result: The comment describes v2 behavior that no longer exists in the code.
| __aarch64_ldclr2_acq_rel [LSE] | ||
| __aarch64_ldclr4_acq_rel [LSE] | ||
| __aarch64_ldclr8_acq_rel [LSE] | ||
| __aarch64_ldclr8_relax [LSE] |
There was a problem hiding this comment.
🟡 The section header comment says (41 symbols) but this PR adds 4 new LSE atomic symbols (__aarch64_ldclr8_relax, __aarch64_ldset8_relax, __aarch64_swp8_rel, __aarch64_swp8_relax), bringing the actual count to 45. Update the comment to (45 symbols) to keep it accurate.
Extended reasoning...
What the bug is
The compiler-rt outline atomics section header in scripts/verify-baseline-static/allowlist-aarch64.txt carries the comment (41 symbols). This count was accurate before this PR, but the PR adds 4 new LSE atomic symbols to that section.
The specific code path that triggers it
The diff adds these four entries to the LSE section:
__aarch64_ldclr8_relax__aarch64_ldset8_relax__aarch64_swp8_rel__aarch64_swp8_relax
Counting the actual [LSE]-tagged entries after the PR: 13 cas + 9 ldadd + 4 ldclr (incl. new) + 4 ldeor + 5 ldset (incl. new) + 10 swp (incl. 2 new) = 45. The comment was not updated to match.
Why existing code doesn't prevent it
The comment is a human-maintained annotation — there is no automated check that verifies the count in the comment matches the actual number of entries in the section. The allowlist itself works on symbol names, not on the count in the comment.
What the impact would be
Zero functional impact — the allowlist matcher uses symbol names, not the count. However, the comment exists to help maintainers quickly verify the allowlist is complete. A stale count of 41 when there are actually 45 entries could mislead a reviewer into thinking symbols are missing or that the section is incomplete, creating unnecessary confusion during audits.
How to fix it
Change the comment from (41 symbols) to (45 symbols) on the section header line.
Step-by-step proof
- Before this PR: the LSE section contained 41 entries; the comment said
(41 symbols)— accurate. - This PR adds
__aarch64_ldclr8_relax,__aarch64_ldset8_relax,__aarch64_swp8_rel,__aarch64_swp8_relax— 4 new entries. - After the PR: 41 + 4 = 45 entries; the comment still says
(41 symbols)— stale. - A maintainer auditing the section would count 45 entries but read 41 in the comment, creating a discrepancy of 4.
MI_OVERRIDE=ON + MI_OSX_ZONE=ON (interpose stays OFF since we link the .o statically). This registers mimalloc as the default malloc zone via the zone-swap constructor, so leaks/heap/vmmap/Instruments can enumerate Bun allocations. Debug/ASAN builds keep override OFF (ASAN must intercept system malloc). Pin -> 56f35a46 (adds intro_enumerator + purge-bit restore). static-initializers: 3 from mimalloc on arm64, +1 cpu_indicator on x64.
| // so UBSan doesn't false-positive on mimalloc's type punning. | ||
| args.MI_DEBUG_UBSAN = "ON"; | ||
| } else if (cfg.darwin) { | ||
| args.MI_OVERRIDE = "OFF"; | ||
| args.MI_OSX_ZONE = "OFF"; | ||
| // Register the mimalloc malloc_zone so leaks/heap/vmmap/Instruments can | ||
| // enumerate our allocations and the fork-prepare/child hooks fire. | ||
| // Interpose is OFF because we link the .o statically; the __interpose | ||
| // section only works for inserted dylibs. The zone-swap path in | ||
| // alloc-override-zone.c handles making mimalloc the default zone. | ||
| args.MI_OVERRIDE = "ON"; | ||
| args.MI_OSX_ZONE = "ON"; | ||
| args.MI_OSX_INTERPOSE = "OFF"; | ||
| } else if (cfg.linux) { | ||
| args.MI_OVERRIDE = "ON"; |
There was a problem hiding this comment.
🟡 The 'Override behavior' decision matrix comment in scripts/build/deps/mimalloc.ts still reads 'macOS: OFF — macOS's malloc zones are sufficient and overriding causes issues with system frameworks' but this PR changed the darwin block to set MI_OVERRIDE = "ON" and MI_OSX_ZONE = "ON". The decision matrix comment was not updated to reflect the new behavior and now directly contradicts the code.
Extended reasoning...
What the bug is
The decision matrix comment block (lines ~53–58) in scripts/build/deps/mimalloc.ts explicitly states:
// macOS: OFF — macOS's malloc zones are sufficient and overriding
// causes issues with system frameworks (SecureTransport, etc.)
// that have their own allocator expectations.
However, this PR changed the darwin branch of the build configuration to:
args.MI_OVERRIDE = "ON";
args.MI_OSX_ZONE = "ON";The decision matrix was not updated when the behavior changed.
The specific code path
The decision matrix at the top of the override behavior section is intended as the authoritative, at-a-glance summary of per-platform override decisions. The darwin block below it does include a new inline comment explaining the new rationale (zone registration for leaks/vmmap/Instruments and fork hooks), but the summary matrix above it still says OFF, creating a direct contradiction.
Why existing code doesn't prevent it
This is a documentation-only inconsistency with no runtime impact. There is no automated check that validates the decision matrix comment against the actual CMake argument values. The inline comment in the darwin block partially addresses this, but the authoritative matrix at the top of the section remains wrong.
Impact
A developer reading the decision matrix to understand platform behavior would conclude macOS overriding is OFF and rely on that understanding when reasoning about allocator behavior, debugging macOS-specific issues, or reviewing future changes. The actual runtime behavior is the opposite of what the matrix says. This discrepancy is particularly misleading because the old rationale ("causes issues with system frameworks") was the reason for the previous OFF setting — a future developer might even revert the change citing that comment as justification.
How to fix it
Update the macOS line in the decision matrix to reflect the new ON behavior and the new rationale, e.g.:
// macOS: ON — MI_OSX_ZONE registers the mimalloc malloc_zone so
// leaks/heap/vmmap/Instruments can enumerate allocations and
// fork-prepare/child hooks fire. Interpose remains OFF (static link).
Step-by-step proof
- Open
scripts/build/deps/mimalloc.tsand read lines ~53–58: the matrix saysmacOS: OFF. - Read the darwin block (lines ~68–78):
args.MI_OVERRIDE = "ON"andargs.MI_OSX_ZONE = "ON". - The matrix says OFF; the code sets ON. Direct contradiction.
heapStats() now includes a mimalloc property with the full mi_stats_get_json() payload: process info, page/commit counts, and a per-bin malloc_bins[] histogram. Pin -> 76101991 (fixes mi_stats_get_json returning NULL + adds the out-of-process zone enumerator).
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/build/deps/mimalloc.ts`:
- Around line 71-77: The top-level macOS decision-matrix comment is now
outdated: the code sets args.MI_OVERRIDE = "ON" and args.MI_OSX_ZONE = "ON" for
Darwin, but the comment still states the macOS override is OFF; update that
earlier comment to reflect the current behavior (mention MI_OVERRIDE and
MI_OSX_ZONE are enabled on Darwin and note alloc-override-zone.c handles
zone-swap) so readers aren’t confused when looking at the args.MI_OVERRIDE /
args.MI_OSX_ZONE settings.
In `@src/bun.js/modules/BunJSCModule.h`:
- Around line 360-366: The current code only sets the "mimalloc" property when
mi_stats_get_json(0, nullptr) returns non-null; always emit the property to keep
heapStats() shape stable by calling mi_collect(false) then calling
mi_stats_get_json and if it returns non-null parse and mi_free as before but
ensure object->putDirect(vm, Identifier::fromString(vm, "mimalloc"_s),
parsed.isEmpty() ? jsNull() : parsed) is executed in both branches; if
mi_stats_get_json returns null, call object->putDirect with jsNull() for the
"mimalloc" key so the property is always present.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e37a31bd-3299-4659-8392-2675545476fd
📒 Files selected for processing (2)
scripts/build/deps/mimalloc.tssrc/bun.js/modules/BunJSCModule.h
| // Register the mimalloc malloc_zone so leaks/heap/vmmap/Instruments can | ||
| // enumerate our allocations and the fork-prepare/child hooks fire. | ||
| // Interpose is OFF because we link the .o statically; the __interpose | ||
| // section only works for inserted dylibs. The zone-swap path in | ||
| // alloc-override-zone.c handles making mimalloc the default zone. | ||
| args.MI_OVERRIDE = "ON"; | ||
| args.MI_OSX_ZONE = "ON"; |
There was a problem hiding this comment.
Update the earlier macOS decision-matrix comment to match the new behavior.
This block now enables MI_OVERRIDE/MI_OSX_ZONE on Darwin, but the matrix comment above still says macOS override is OFF. That contradiction is easy to misread during future changes.
✏️ Proposed comment-only fix
- // macOS: OFF — macOS's malloc zones are sufficient and overriding
- // causes issues with system frameworks (SecureTransport, etc.)
- // that have their own allocator expectations.
+ // macOS: ON — register mimalloc's malloc zone so tools can enumerate
+ // allocations and fork hooks run; keep MI_OSX_INTERPOSE=OFF
+ // because we're linking statically.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/build/deps/mimalloc.ts` around lines 71 - 77, The top-level macOS
decision-matrix comment is now outdated: the code sets args.MI_OVERRIDE = "ON"
and args.MI_OSX_ZONE = "ON" for Darwin, but the comment still states the macOS
override is OFF; update that earlier comment to reflect the current behavior
(mention MI_OVERRIDE and MI_OSX_ZONE are enabled on Darwin and note
alloc-override-zone.c handles zone-swap) so readers aren’t confused when looking
at the args.MI_OVERRIDE / args.MI_OSX_ZONE settings.
| mi_collect(false); | ||
| if (char* json = mi_stats_get_json(0, nullptr)) { | ||
| JSValue parsed = JSONParse(globalObject, String::fromUTF8(json)); | ||
| mi_free(json); | ||
| object->putDirect(vm, Identifier::fromString(vm, "mimalloc"_s), | ||
| parsed.isEmpty() ? jsNull() : parsed); | ||
| } |
There was a problem hiding this comment.
Always emit the mimalloc key for a stable heapStats() shape.
When mi_stats_get_json() returns null, this currently omits the property entirely. Prefer always setting mimalloc (fallback null) so consumers can rely on a consistent object contract.
🧩 Proposed fix
- mi_collect(false);
- if (char* json = mi_stats_get_json(0, nullptr)) {
- JSValue parsed = JSONParse(globalObject, String::fromUTF8(json));
- mi_free(json);
- object->putDirect(vm, Identifier::fromString(vm, "mimalloc"_s),
- parsed.isEmpty() ? jsNull() : parsed);
- }
+ mi_collect(false);
+ JSValue mimallocStats = jsNull();
+ if (char* json = mi_stats_get_json(0, nullptr)) {
+ JSValue parsed = JSONParse(globalObject, String::fromUTF8(json));
+ mi_free(json);
+ mimallocStats = parsed.isEmpty() ? jsNull() : parsed;
+ }
+ object->putDirect(vm, Identifier::fromString(vm, "mimalloc"_s), mimallocStats);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| mi_collect(false); | |
| if (char* json = mi_stats_get_json(0, nullptr)) { | |
| JSValue parsed = JSONParse(globalObject, String::fromUTF8(json)); | |
| mi_free(json); | |
| object->putDirect(vm, Identifier::fromString(vm, "mimalloc"_s), | |
| parsed.isEmpty() ? jsNull() : parsed); | |
| } | |
| mi_collect(false); | |
| JSValue mimallocStats = jsNull(); | |
| if (char* json = mi_stats_get_json(0, nullptr)) { | |
| JSValue parsed = JSONParse(globalObject, String::fromUTF8(json)); | |
| mi_free(json); | |
| mimallocStats = parsed.isEmpty() ? jsNull() : parsed; | |
| } | |
| object->putDirect(vm, Identifier::fromString(vm, "mimalloc"_s), mimallocStats); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/bun.js/modules/BunJSCModule.h` around lines 360 - 366, The current code
only sets the "mimalloc" property when mi_stats_get_json(0, nullptr) returns
non-null; always emit the property to keep heapStats() shape stable by calling
mi_collect(false) then calling mi_stats_get_json and if it returns non-null
parse and mi_free as before but ensure object->putDirect(vm,
Identifier::fromString(vm, "mimalloc"_s), parsed.isEmpty() ? jsNull() : parsed)
is executed in both branches; if mi_stats_get_json returns null, call
object->putDirect with jsNull() for the "mimalloc" key so the property is always
present.
heapStats({dump: true}) -> mimallocDump.heaps[].pages[] with
{id, block_size, used, reserved, thread_id} per page.
heapStats({dump: "blocks"}) also includes blocks[] as [id, size] pairs.
Ids are raw addresses in debug, hashed in release. Pin -> 9a5e1f52
(adds mi_heap_dump_json + mi_heap_get_seq).
There was a problem hiding this comment.
Additional findings (outside current diff — PR may have been updated during review):
-
🟡
src/bun.js/modules/BunJSCModule.h:357-365— The new mi_stats_get_json block in functionMemoryUsageStatistics calls JSONParse() without a DECLARE_THROW_SCOPE or exception clearing. If mi_stats_get_json returns non-null but malformed JSON, JSONParse sets a pending exception on globalObject; the code checks parsed.isEmpty() and substitutes jsNull(), but the pending exception is never cleared, causing Bun.jsc.heapStats() to throw a SyntaxError instead of returning the stats object with mimalloc: null. Fix with DECLARE_THROW_SCOPE + scope.clearException() around the JSONParse call, matching the pattern used elsewhere in this file.Extended reasoning...
What the bug is and how it manifests
The new mi_stats_get_json block added to functionMemoryUsageStatistics (lines 357-365 of src/bun.js/modules/BunJSCModule.h) calls JSONParse(globalObject, String::fromUTF8(json)) without any exception handling infrastructure. Unlike every other host function in this file, this new code path has no DECLARE_THROW_SCOPE in scope. If mi_stats_get_json returns a non-null pointer but the buffer contains malformed JSON (e.g., due to a mimalloc v3 format change), JSONParse will: (1) set a pending exception on the VM via globalObject, and (2) return an empty JSValue.
The specific code path that triggers it
The code correctly detects the failure via parsed.isEmpty() ? jsNull() : parsed and places jsNull() into the result object. However, the pending exception on the VM is never cleared. The function then reaches return JSValue::encode(object). At the JSC host function boundary, the machinery checks for a pending exception, finds one, and propagates it to the caller as a SyntaxError, discarding the partially-constructed object. So Bun.jsc.heapStats() throws instead of returning the stats object with mimalloc: null.
Why existing code does not prevent it
Every other JSONParse callsite in this file that operates on trusted JSON uses scope.releaseAssertNoException() after the call (e.g., functionSamplingProfilerStackTraces at line 469), showing this pattern is well understood. The new block simply lacks the corresponding scope machinery. Without a DECLARE_THROW_SCOPE, there is no scope object on which to call clearException().
Impact
In practice, mi_stats_get_json generates JSON programmatically from internal mimalloc stats and is very unlikely to produce malformed JSON. However, the defensive code is demonstrably broken: the code intends to silently degrade (mimalloc: null) when JSON parsing fails, but instead it causes the entire heapStats() call to throw. Any future mimalloc format change or edge case that produces malformed JSON would surface as a surprising exception rather than a graceful fallback.
How to fix it
Add DECLARE_THROW_SCOPE(vm) locally, then call scope.clearException() after detecting parsed.isEmpty():
auto scope = DECLARE_THROW_SCOPE(vm); mi_collect(false); if (char* json = mi_stats_get_json(0, nullptr)) { JSValue parsed = JSONParse(globalObject, String::fromUTF8(json)); mi_free(json); if (parsed.isEmpty()) scope.clearException(); object->putDirect(vm, Identifier::fromString(vm, "mimalloc"_s), parsed.isEmpty() ? jsNull() : parsed); }Step-by-step proof
- mi_stats_get_json(0, nullptr) returns a non-null pointer containing malformed JSON (e.g., truncated output).
- JSONParse(globalObject, ...) encounters the malformed JSON, calls throwSyntaxError(globalObject, ...), and returns an empty JSValue.
- The VM now has a pending exception on globalObject.
- parsed.isEmpty() returns true; jsNull() is placed in the object. The pending exception is NOT cleared.
- The function returns JSValue::encode(object).
- JSCs JSC_DEFINE_HOST_FUNCTION wrapper checks for pending exceptions, finds the SyntaxError, and throws it to the JavaScript caller.
- Bun.jsc.heapStats() throws SyntaxError instead of returning the stats object with mimalloc: null.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/bun.js/modules/BunJSCModule.h (1)
361-367:⚠️ Potential issue | 🟡 MinorAlways populate
mimalloc, even when stats JSON is unavailable.If
mi_stats_get_json()returnsnullptr,heapStats()omits themimallockey entirely. That leaves the return shape unstable for callers and for the new test coverage that expectsstats.mimallocto exist. Initialize it tonulland overwrite it on success instead.💡 Proposed fix
- mi_collect(false); - if (char* json = mi_stats_get_json(0, nullptr)) { - JSValue parsed = JSONParse(globalObject, String::fromUTF8(json)); - mi_free(json); - object->putDirect(vm, Identifier::fromString(vm, "mimalloc"_s), - parsed.isEmpty() ? jsNull() : parsed); - } + mi_collect(false); + JSValue mimallocStats = jsNull(); + if (char* json = mi_stats_get_json(0, nullptr)) { + JSValue parsed = JSONParse(globalObject, String::fromUTF8(json)); + mi_free(json); + mimallocStats = parsed.isEmpty() ? jsNull() : parsed; + } + object->putDirect(vm, Identifier::fromString(vm, "mimalloc"_s), mimallocStats);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bun.js/modules/BunJSCModule.h` around lines 361 - 367, The heapStats path currently only sets the "mimalloc" property when mi_stats_get_json(...) returns non-null, making the return shape unstable; modify the code so you always initialize the "mimalloc" key to jsNull() via object->putDirect(vm, Identifier::fromString(vm, "mimalloc"_s), jsNull()) before calling mi_collect() / mi_stats_get_json(), then if mi_stats_get_json(...) returns a non-null json parse it and overwrite that same key (using the existing parsed.isEmpty() ? jsNull() : parsed logic) and free the json with mi_free; reference mi_stats_get_json, mi_collect, object->putDirect, Identifier::fromString, and parsed.isEmpty() to locate where to add the unconditional initialization and conditional overwrite.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/js/bun/jsc/heapStats-mimalloc.test.ts`:
- Around line 48-64: The test "dump reflects new heaps and allocations" records
a baseline in the variable before but never uses it; fix by asserting the after
dump against that baseline (e.g., change the assertion to compare after.length
to before via expect(after.length).toBeGreaterThanOrEqual(before)) so
heapStats({dump:true}) before and after transformSync() are actually compared,
or alternatively remove the unused before variable and update the test
name/comments to match that it only validates the post-transform dump shape;
look for the variables before, after, heapStats, t.transformSync and the test
block to apply the change.
---
Duplicate comments:
In `@src/bun.js/modules/BunJSCModule.h`:
- Around line 361-367: The heapStats path currently only sets the "mimalloc"
property when mi_stats_get_json(...) returns non-null, making the return shape
unstable; modify the code so you always initialize the "mimalloc" key to
jsNull() via object->putDirect(vm, Identifier::fromString(vm, "mimalloc"_s),
jsNull()) before calling mi_collect() / mi_stats_get_json(), then if
mi_stats_get_json(...) returns a non-null json parse it and overwrite that same
key (using the existing parsed.isEmpty() ? jsNull() : parsed logic) and free the
json with mi_free; reference mi_stats_get_json, mi_collect, object->putDirect,
Identifier::fromString, and parsed.isEmpty() to locate where to add the
unconditional initialization and conditional overwrite.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4715f6c9-ef76-4942-beb8-d288a0b280ed
📒 Files selected for processing (3)
scripts/build/deps/mimalloc.tssrc/bun.js/modules/BunJSCModule.htest/js/bun/jsc/heapStats-mimalloc.test.ts
| // heapStats({ dump: true | "blocks" }) -> per-heap/per-page (and per-block) live snapshot. | ||
| JSValue arg0 = callFrame->argument(0); | ||
| if (arg0.isObject()) { | ||
| JSValue dump = arg0.getObject()->get(globalObject, Identifier::fromString(vm, "dump"_s)); | ||
| if (dump.toBoolean(globalObject)) { | ||
| const bool includeBlocks = dump.isString() && dump.toWTFString(globalObject) == "blocks"_s; | ||
| #if BUN_DEBUG | ||
| const bool hashAddresses = false; | ||
| #else | ||
| const bool hashAddresses = true; | ||
| #endif | ||
| if (char* json = mi_heap_dump_json(includeBlocks, hashAddresses)) { | ||
| JSValue parsed = JSONParse(globalObject, String::fromUTF8(json)); | ||
| mi_free(json); | ||
| object->putDirect(vm, Identifier::fromString(vm, "mimallocDump"_s), | ||
| parsed.isEmpty() ? jsNull() : parsed); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that functionMemoryUsageStatistics performs JS-visible operations
# without a local throw scope / exception checks.
rg -n -C3 'functionMemoryUsageStatistics|DECLARE_THROW_SCOPE|JSONParse\(|->get\(globalObject, Identifier::fromString' src/bun.js/modules/BunJSCModule.hRepository: oven-sh/bun
Length of output: 8839
🏁 Script executed:
sed -n '223,395p' src/bun.js/modules/BunJSCModule.h | cat -nRepository: oven-sh/bun
Length of output: 8469
Add throw scope and exception checking to functionMemoryUsageStatistics.
This function performs multiple JS-visible operations without exception handling: arg0.getObject()->get(...) can execute user getters and Proxy traps, and both JSONParse(...) calls can throw. Without a throw scope, exceptions from these operations are silently dropped, leaving the result object partially constructed and returning normally.
Proposed fix
JSC_DEFINE_HOST_FUNCTION(functionMemoryUsageStatistics,
(JSGlobalObject * globalObject, CallFrame* callFrame))
{
auto& vm = JSC::getVM(globalObject);
+ auto scope = DECLARE_THROW_SCOPE(vm);
if (vm.heap.size() == 0) {
vm.heap.collectNow(Sync, CollectionScope::Full);
}
@@
mi_collect(false);
if (char* json = mi_stats_get_json(0, nullptr)) {
JSValue parsed = JSONParse(globalObject, String::fromUTF8(json));
mi_free(json);
+ RETURN_IF_EXCEPTION(scope, {});
object->putDirect(vm, Identifier::fromString(vm, "mimalloc"_s),
parsed.isEmpty() ? jsNull() : parsed);
}
// heapStats({ dump: true | "blocks" }) -> per-heap/per-page (and per-block) live snapshot.
JSValue arg0 = callFrame->argument(0);
if (arg0.isObject()) {
JSValue dump = arg0.getObject()->get(globalObject, Identifier::fromString(vm, "dump"_s));
+ RETURN_IF_EXCEPTION(scope, {});
if (dump.toBoolean(globalObject)) {
+ RETURN_IF_EXCEPTION(scope, {});
const bool includeBlocks = dump.isString() && dump.toWTFString(globalObject) == "blocks"_s;
+ RETURN_IF_EXCEPTION(scope, {});
`#if` BUN_DEBUG
const bool hashAddresses = false;
`#else`
const bool hashAddresses = true;
`#endif`
if (char* json = mi_heap_dump_json(includeBlocks, hashAddresses)) {
JSValue parsed = JSONParse(globalObject, String::fromUTF8(json));
mi_free(json);
+ RETURN_IF_EXCEPTION(scope, {});
object->putDirect(vm, Identifier::fromString(vm, "mimallocDump"_s),
parsed.isEmpty() ? jsNull() : parsed);
}
}
}
- return JSValue::encode(object);
+ RELEASE_AND_RETURN(scope, JSValue::encode(object));
}| test("dump reflects new heaps and allocations", () => { | ||
| const before = heapStats({ dump: true }).mimallocDump.heaps.length; | ||
| // MimallocArena is internal; trigger via something that creates a heap. | ||
| // Transpiler creates a per-call arena. | ||
| const t = new Bun.Transpiler(); | ||
| const out = t.transformSync("export const x = 1"); | ||
| expect(out.length).toBeGreaterThan(0); | ||
| const after = heapStats({ dump: true }).mimallocDump.heaps; | ||
| // Either a new heap was created (and may already be destroyed), or main grew. | ||
| // We assert the dump is still well-formed and >= before. | ||
| expect(after.length).toBeGreaterThanOrEqual(1); | ||
| for (const h of after) { | ||
| expect(typeof h.seq).toBe("number"); | ||
| expect(Array.isArray(h.pages)).toBe(true); | ||
| } | ||
| void before; | ||
| }); |
There was a problem hiding this comment.
This test never checks the before/after signal it describes.
before is discarded on Line 63, so the test only proves the post-transformSync() dump is still well-formed. That will not catch a regression where the allocator activity no longer changes the snapshot in the way this test name/comments claim. Either assert against before or rename/remove the dead baseline so the test contract matches what is actually being verified.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/js/bun/jsc/heapStats-mimalloc.test.ts` around lines 48 - 64, The test
"dump reflects new heaps and allocations" records a baseline in the variable
before but never uses it; fix by asserting the after dump against that baseline
(e.g., change the assertion to compare after.length to before via
expect(after.length).toBeGreaterThanOrEqual(before)) so heapStats({dump:true})
before and after transformSync() are actually compared, or alternatively remove
the unused before variable and update the test name/comments to match that it
only validates the post-transform dump shape; look for the variables before,
after, heapStats, t.transformSync and the test block to apply the change.
| expect(typeof id).toBe("number"); | ||
| expect(size).toBeGreaterThan(0); | ||
| // every block size should match some page's block_size | ||
| const pageSizes = new Set(main.pages.map((p: any) => p.block_size)); | ||
| for (const [, sz] of main.blocks.slice(0, 50)) { | ||
| expect(pageSizes.has(sz)).toBe(true); |
There was a problem hiding this comment.
🟡 The 'heapStats({dump: "blocks"})' test accesses main.blocks without first asserting main is defined, unlike the parallel 'dump: true' test which correctly guards with expect(main).toBeDefined(). If heaps.find(h => h.seq === 0) returns undefined for any reason, the test will throw a TypeError: Cannot read properties of undefined (reading 'blocks') instead of a descriptive assertion failure.
Extended reasoning...
What the bug is and how it manifests
In test/js/bun/jsc/heapStats-mimalloc.test.ts, the two dump-mode tests follow the same pattern — finding the main heap by seq === 0 and then accessing properties on it. The 'dump: true' test (lines 19–20) correctly does:
const main = s.mimallocDump.heaps.find((h: any) => h.seq === 0);
expect(main).toBeDefined(); // guard present
expect(Array.isArray(main.pages)).toBe(true);The 'dump: "blocks"' test (lines 35–36) skips the guard entirely:
const main = s.mimallocDump.heaps.find((h: any) => h.seq === 0);
// no expect(main).toBeDefined() here
expect(Array.isArray(main.blocks)).toBe(true); // will throw TypeError if main is undefinedThe specific code path that triggers it
Array.prototype.find returns undefined when no element matches the predicate. If heaps contains no entry with seq === 0, main is undefined. Accessing undefined.blocks throws TypeError: Cannot read properties of undefined (reading 'blocks') — a JavaScript runtime error rather than a Bun test assertion failure.
Why existing code doesn't prevent it
The 'dump: true' test deliberately adds expect(main).toBeDefined() as a safety guard before touching main.pages. The 'dump: "blocks"' test was written without mirroring this pattern. There is no structural mechanism that enforces the guard.
What the impact would be
In practice, the process-wide main heap (mi_heap_main(), always seq === 0) will always be present in the dump. So this bug will not cause false failures under normal conditions. However, if the mimalloc internals ever change how heaps are numbered, or if a platform-specific quirk causes the main heap to be absent or differently sequenced, the resulting error message — TypeError: Cannot read properties of undefined — is unhelpful and will mask the real failure. The 'dump: true' test would catch the same situation with a clear expect(main).toBeDefined() failure message pointing directly at the missing heap.
How to fix it
Add the missing guard at line 37 (between the find call and the blocks access):
const main = s.mimallocDump.heaps.find((h: any) => h.seq === 0);
expect(main).toBeDefined(); // add this line
expect(Array.isArray(main.blocks)).toBe(true);Step-by-step proof
heapStats({ dump: "blocks" })is called; assume hypothetically thatmimallocDump.heapscontains no entry withseq === 0const main = s.mimallocDump.heaps.find((h: any) => h.seq === 0)→mainisundefinedexpect(Array.isArray(main.blocks))→ JS evaluatesmain.blocks→TypeError: Cannot read properties of undefined (reading 'blocks')is thrown- Bun test reports an unhandled error with no actionable failure message, rather than something like
expected undefined to be defined - Compare: the 'dump: true' test would instead hit
expect(main).toBeDefined()and report a clear assertion failure at the correct line
| #endif | ||
| #endif | ||
|
|
||
| mi_collect(false); | ||
| if (char* json = mi_stats_get_json(0, nullptr)) { | ||
| JSValue parsed = JSONParse(globalObject, String::fromUTF8(json)); | ||
| mi_free(json); | ||
| object->putDirect(vm, Identifier::fromString(vm, "mimalloc"_s), | ||
| parsed.isEmpty() ? jsNull() : parsed); | ||
| } | ||
|
|
||
| // heapStats({ dump: true | "blocks" }) -> per-heap/per-page (and per-block) live snapshot. | ||
| JSValue arg0 = callFrame->argument(0); | ||
| if (arg0.isObject()) { | ||
| JSValue dump = arg0.getObject()->get(globalObject, Identifier::fromString(vm, "dump"_s)); | ||
| if (dump.toBoolean(globalObject)) { | ||
| const bool includeBlocks = dump.isString() && dump.toWTFString(globalObject) == "blocks"_s; | ||
| #if BUN_DEBUG | ||
| const bool hashAddresses = false; | ||
| #else | ||
| const bool hashAddresses = true; | ||
| #endif | ||
| if (char* json = mi_heap_dump_json(includeBlocks, hashAddresses)) { | ||
| JSValue parsed = JSONParse(globalObject, String::fromUTF8(json)); | ||
| mi_free(json); | ||
| object->putDirect(vm, Identifier::fromString(vm, "mimallocDump"_s), | ||
| parsed.isEmpty() ? jsNull() : parsed); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🟡 The new heapStats dump section in functionMemoryUsageStatistics has two issues: (1) mi_stats_get_json returning null silently omits the mimalloc property entirely, breaking the stable shape callers expect — it should always emit jsNull() as a fallback; (2) the dump section calls arg0.getObject()->get() and dump.toBoolean() — both can throw via Proxy traps or Symbol.toPrimitive — but the function has no DECLARE_THROW_SCOPE and no RETURN_IF_EXCEPTION guards, so a pending JS exception is silently swallowed while execution continues into the native mi_heap_dump_json call and returns normally.
Extended reasoning...
Bug 1 — Missing null fallback for mimalloc property
The mimalloc property is only set on the heapStats() result when mi_stats_get_json(0, nullptr) returns non-null. If it returns null (OOM allocating the internal JSON buffer), the key is simply absent from the returned object. Callers relying on a stable shape — including the new test at heapStats-mimalloc.test.ts:7 which accesses s.mimalloc.mimalloc_version — would receive a TypeError when s.mimalloc is undefined. The fix is trivial: hoist a JSValue mimallocStats = jsNull() before the if-block and move the putDirect call outside it, unconditionally emitting the property.
Bug 2 — Missing exception propagation in the dump section
Before this PR, functionMemoryUsageStatistics accepted an unnamed CallFrame* argument and performed zero JS operations on user input. This PR adds a new code path that reads callFrame->argument(0): it calls arg0.getObject()->get(globalObject, ...) and then dump.toBoolean(globalObject). Both operations take a JSGlobalObject* and can throw JS exceptions — get() via a Proxy trap or throwing getter, toBoolean() via Symbol.toPrimitive or valueOf().
The function has no DECLARE_THROW_SCOPE anywhere. Every other JS-calling host function in BunJSCModule.h (lines 79, 130, 446, 481, etc.) declares one. Neither call is followed by RETURN_IF_EXCEPTION. If an exception is raised by get(), execution falls through to dump.toBoolean() then into the native mi_heap_dump_json C function (which runs while a JS exception is pending), then JSONParse, then mi_free, then putDirect, and finally returns JSValue::encode(object) as if nothing went wrong. The pending exception is silently swallowed or overwritten.
Why existing code does not prevent this
JSC contract: any host function passing a JSGlobalObject* to a throwing operation must own a throw scope and check for exceptions before proceeding. Without a throw scope there is no compile-time enforcement. All other functions in the file follow this pattern; the new code does not.
Proof for Bug 2
- Construct: const p = new Proxy({}, { get() { throw new Error('trap') } })
- Call heapStats(p) — arg0.isObject() is true
- arg0.getObject()->get(globalObject, ...dump...) triggers the trap, sets pending exception
- No RETURN_IF_EXCEPTION check; execution continues
- dump.toBoolean(globalObject) runs with pending exception
- mi_heap_dump_json is called with a pending exception in the VM
- Function returns JSValue::encode(object) — the exception from step 3 is lost
Impact
Bug 1: Consumers see undefined for mimalloc instead of null when the stats buffer allocation fails. Extremely unlikely in practice but the API shape is unstable. The new test would also fail with a confusing TypeError rather than a clear test assertion failure.
Bug 2: Passing a Proxy with a throwing dump getter to heapStats() silently swallows the exception and returns a result object. This is an internal debug API so real-world impact is low, but the JSC invariant violation is real.
Fix
For Bug 1: move putDirect for mimalloc outside the null check, always emitting the property (jsNull() when mi_stats_get_json fails).
For Bug 2: add DECLARE_THROW_SCOPE(vm) at the top of functionMemoryUsageStatistics and add RETURN_IF_EXCEPTION(scope, {}) after the get() call and after the toBoolean() call.
| test("dump reflects new heaps and allocations", () => { | ||
| const before = heapStats({ dump: true }).mimallocDump.heaps.length; | ||
| // MimallocArena is internal; trigger via something that creates a heap. | ||
| // Transpiler creates a per-call arena. | ||
| const t = new Bun.Transpiler(); | ||
| const out = t.transformSync("export const x = 1"); | ||
| expect(out.length).toBeGreaterThan(0); | ||
| const after = heapStats({ dump: true }).mimallocDump.heaps; | ||
| // Either a new heap was created (and may already be destroyed), or main grew. | ||
| // We assert the dump is still well-formed and >= before. | ||
| expect(after.length).toBeGreaterThanOrEqual(1); | ||
| for (const h of after) { | ||
| expect(typeof h.seq).toBe("number"); | ||
| expect(Array.isArray(h.pages)).toBe(true); | ||
| } | ||
| void before; | ||
| }); |
There was a problem hiding this comment.
🟡 The test "dump reflects new heaps and allocations" records a baseline heap count in before but never uses it — the assertion is expect(after.length).toBeGreaterThanOrEqual(1) instead of toBeGreaterThanOrEqual(before), and void before on line 63 explicitly discards the variable. The test name and inline comment both claim a before/after comparison is performed, but the check only verifies the dump still has at least one heap, so a regression where allocations cause heaps to shrink would go undetected as long as one heap remains.
Extended reasoning...
What the bug is and how it manifests
The test "dump reflects new heaps and allocations" (lines 48–64) captures before = heapStats({dump:true}).mimallocDump.heaps.length before triggering allocations via Bun.Transpiler.transformSync, then captures the post-allocation dump in after. The intent — spelled out in both the test name and the inline comment "We assert the dump is still well-formed and >= before" — is to verify that the allocation activity produced at least as many heaps as were present before. However, the actual assertion is expect(after.length).toBeGreaterThanOrEqual(1), which only checks that at least one heap exists, completely ignoring before.
The specific code path that triggers it
Line 49 sets const before = heapStats(...).mimallocDump.heaps.length. Line 58 asserts expect(after.length).toBeGreaterThanOrEqual(1). Line 63 has void before — a deliberate TypeScript idiom for suppressing unused-variable lint warnings — confirming the author knew before was never referenced in any assertion. The comment between lines 56–59 says ">= before" but the code says ">= 1".
Why existing code does not prevent it
There is no mechanism that enforces a captured baseline must appear in an assertion. The test compiles and runs, and will pass as long as the dump returns any heap at all — even if allocations somehow caused the heap count to drop from 50 to 1.
What the impact would be
If a mimalloc regression caused heaps to stop being tracked after allocation (e.g., a refcount bug that destroys heaps prematurely), the heap count in the dump could shrink. The test would still pass as long as one heap remained. The invariant the test claims to enforce — that allocations produce a dump at least as large as the pre-allocation snapshot — is never actually checked.
How to fix it
Change the assertion on line 58 from toBeGreaterThanOrEqual(1) to toBeGreaterThanOrEqual(before) and remove the void before on line 63. Alternatively, if the author intentionally chose not to compare (the comment notes heaps may be destroyed between snapshots), update the test name and comment to reflect what is actually being verified.
Step-by-step proof
- Before allocation: heaps.length = 5, so before = 5
- After allocation: mimalloc regression causes heaps.length = 1
expect(after.length).toBeGreaterThanOrEqual(1)evaluates 1 >= 1, which passes- The intended check
expect(after.length).toBeGreaterThanOrEqual(before)would evaluate 1 >= 5, which would fail and catch the regression - The test passes despite the regression, providing false confidence
This reverts commit ccbaed9.
## Summary - Reverts e0cc98d ("We cannot use MI_OSX_ZONE") - Reverts ccbaed9 ("deps: bump mimalloc to dev3 (bun-dev3-v2)", #29251) We are seeing regressions with the mimalloc v3 upgrade, so this rolls back to the previous pin (`1beadf9651a7bfdec6b5367c380ecc3fe1c40d1a`) and restores the matching `MimallocArena` / `mimalloc.zig` bindings, build script, static-initializer counts, aarch64 baseline allowlist, and `process.versions` snapshot. The `heapStats({dump})` mimalloc integration and its test are removed as they depend on dev3-only APIs. The only manual conflict resolution was in `test/js/node/process/process.test.js`, where the boringssl/libarchive version lines adjacent to the mimalloc line had since changed; those were kept at their current `main` values. ## Test plan - [ ] CI builds green on all platforms - [ ] `test/js/bun/perf/static-initializers.test.ts` passes (initializer count back to 1/2) - [ ] `test/js/node/process/process.test.js` `process.versions` passes - [ ] `scripts/verify-baseline-static` aarch64 allowlist check passes
2 participants
Upgrades mimalloc to v3 via
oven-sh/mimalloc@bun-dev3-v2(2e94216f), which carries on top of upstream dev3:mi_heap_deleteracing concurrentmi_free, andmi_heap_newwhile another thread is inmi_heap_destroy(both reproduce as assertion failures / ASAN UAF under load)mi_heap_new()walking an uninitialized chunkmap → OOB writepthread_atfork-based fork handling so children of multi-threaded parents can allocatemi_thread_initself-deadlock fix in_mi_os_get_aligned_hintZig changes are a tighter subset of #26214:
mi_heap_{get,set}_default,mi_heap_get_backing,mi_heap_check_owned,mi_heap_contains_block. Addedmi_heap_main,mi_heap_contains.MimallocArenavtable split intoheap_allocator_vtable(per-mi_heap_newarena) +global_mimalloc_vtable(mi_malloc/mi_freedirectly).Default/getThreadLocalDefaultroute through global; no more cached per-thread heap pointer.Borrowed.getDefault()wrapsmi_heap_main()(notundefined) sogc()/ownsPtr()stay valid;ownsPtruses per-heapmi_heap_contains(notmi_check_owned);downcastonly accepts owned-heap vtable.Build:
MI_NO_OPT_ARCH=ON, Windows lib name dropped-staticsuffix.Fixes #26762
Closes #26214 - Also upgrades mimalloc to v3; explicitly superseded by this PR
Closes #21875 - Earlier mimalloc v3 upgrade attempt
Closes #22043 - Earlier mimalloc v2 upgrade attempt
Closes #22894 - Another mimalloc version bump