Repository navigation
Conversation
A synchronous collection runs on the calling thread, and the marker's frames hold the address of every cell it visits. When those frames are gone the memory below the caller keeps the addresses. A later callee whose frame has a slot it never writes exposes one of them to the next conservative stack scan, and a dead object survives an explicit gc(). VM::run_gc now zeroes 32 KiB of stack below its frame once the collection returns.
|
Warning Review limit reached
On-demand reviews are free for the next 14 days. After that, they cost $0.25 per reviewed file. Or wait 12 minutes for your next included review. View limit detailsLimit details: You’ve used all 10 included reviews currently available. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (9)
Comment |
|
Updated 12:05 PM PT - Sep 6th, 2026
❌ @robobun, your commit 7facfd3 has 2 failures in
🧪 To try this PR locally: bunx bun-pr 41669That installs a local version of the PR into your bun-41669 --bun |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Move the scrub into C++ as Bun::zeroCollectorStack and call it from each place that runs a synchronous collection on the calling thread: the runGC shim behind VM::run_gc, the --hot reload collection, bun:jsc's gcAndSweep, fullGC, edenGC and memoryUsageStatistics, the V8 LowMemoryNotification shim, the JSC testing helper, and the inspector close path.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Status Reproduced on the canary profile build at d316760: run Head 7facfd3: the new |
This branch has not been deployed
1 participant
Problem
test-process-finalization.mjsfails on main at random:should exit file gc-not-close.mjs with code=0,AssertionError: true !== false. An object survivedgc(): four CI sightings since Build JavaScriptCore and ICU from source in bun's own build graph on every target #41330, none in the 500 builds before it.src/runtime/cli/run_command.rs:1475) marks on the main thread and leaves the object's address in dead stack belowRun::start. ThesetImmediatecallback'sInterpreter::executeCallframe covers that spot with a slot it never writes (rbp-0x110), so the conservative scan ingc()keeps the object. Build JavaScriptCore and ICU from source in bun's own build graph on every target #41330 changed frame layouts so the two line up.Fix
Bun::zeroCollectorStack()(src/jsc/bindings/ZeroCollectorStack.cpp) zeroes 32 KiB of stack below its caller. Each synchronous collection site calls it afterwards:JSC__VM__runGC(post-entry,Bun.gc(true),gc()),--hotreload, thebun:jsccollectors,Isolate::LowMemoryNotification, the JSC testing helper, the inspector close path.reservedZoneSize).bindings.cpp), 3 addressed.test/js/bun/gc/collector-stack-scrub.test.ts(new, fails on main). Alsotest-process-finalization.mjs,ffi/cc.test.ts,test/js/bun/gc/,test/v8.Background
sanitizeStackForVMzeroes dead stack only down to its last recorded point, which sits above the marking frames.Notes
Reproduction: with 8 loops of the fixture in parallel, the canary profile build (d316760) fails 27 to 31 of 2000 runs and the build before #41330 fails 0 of 2000. Alone it does not fail.
How the cause was found, on that canary build:
require("bun:jsc").generateHeapSnapshotForDebugging()taken in the immediate aftergc(): the{ foo: 'bar' }node has no incoming edge and norootsentry./proc/pid/mem: the cell address sits at exactly one live slot,rbp-0x110inJSC::Interpreter::executeCall(the frame that runs the immediate callback, called fromBun__JSTimeout__call).objdumpofexecuteCallshows no instruction touches-0x110(%rbp).bun:ffi) on 1600 runs under load: 22 of 22 survivors had the address at that slot beforegc(), 0 of 1578 non-survivors did. At module end no copy of the address existed anywhere on the live stack.SlotVisitor::drain/drainFromShared/MarkingConstraintSolverunderHeap::collectInMutatorThread←JSC__VM__runGC←Run::start, that is, the post-entry collection.BUN_JSC_logGC=1on the fixture: exactly two collections, the post-entry one andgc().Bun.gc(true)andBun.gc(false): the collector's frames reach 19.4 to 20.0 KiB below the caller.Safety of the 32 KiB frame: main thread stacks are 8 MB or more, Bun Worker threads get 4 MB (18 MB reserve on Windows), the inspector thread 16 MB, bundler macro threads the pool default. A host function called from JS at maximum recursion still has
softReservedZoneSize(128 KiB, tripled under ASAN) of native stack. clang emits__chkstkprobes for the frame on Windows. Bun's__asan_default_optionspinsdetect_stack_use_after_return=0, so the array stays on the real stack under ASAN. The empty__asm__ __volatile__with the array as an"r"input and amemoryclobber is the idiom WTF'ssecureZeroSpanuses.NEVER_INLINEkeeps the frame below the caller under ThinLTO.Not covered, on purpose:
destroyVMcollects right before the VM is torn down, so nothing scans afterwards.Bun.shrink()collects inside JSC'swhenIdlecallback, and a concurrent collection whose conn the mutator takes also marks on the mutator stack fromstopIfNecessary. Both are inside JavaScriptCore. A JSC-side variant of this change (zero belowHeap::collectInMutatorThreadwhen it finishes) would cover them too, at the cost of a WebKit commit. This PR keeps the change in Bun.Review: the first push scrubbed only in Rust
VM::run_gc. The review pointed out the C++ sites that callcollectNow/collectSyncdirectly, so the scrub moved to C++ and is called from each of them, and the Rust copy was dropped.Why the test checks zeros and not object survival: the survival depends on the frame layout of the binary. The debug build does not reproduce the fixture failure in 480 runs under load without the fix. The scrub is the mechanism, and the test checks it directly: after
Bun.gc(true), the window 8 to 28 KiB below the caller is zero. It scans from acc()-compiled C function, so Windows is skipped. On CI it passes on debian x64, x64-asan, alpine x64 and aarch64, and darwin aarch64. Without the fix it reports 28672 (debug) and 27800 (release) bytes of residue.Culprit: #41330 (frame layouts). The post-entry collection itself dates from the Zig runtime.