Conversation
…body alive The abandoned-body tests collect 20 unread fetch bodies and count the aborts the origin sees. The last body's controller stays in a stale slot of the JSC::runInternalMicrotask frame that ran its final read, and the conservative stack scan keeps it alive through every Bun.gc(true) in the wait loop. The loop now reads one chunk of a throwaway body and cancels it before each collection. That runs the same code path, so the slot holds a stream nobody cares about. The assertion becomes exact.
|
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 10 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 (1)
Comment |
|
Updated 9:00 AM PT - Sep 6th, 2026
❌ @robobun, your commit e00fa6d has 3 failures in
The baseline build contains instructions not available on Static scan violations |
|
Status: the diff is ready for review. Reproduced the retention with a standalone copy of the abandoned-body shapes on darwin aarch64 (CI build of 110902) and linux x64 release: the last body stalls about 1 s in 25 to 45 % of runs. The heap snapshot and the lldb stack search in the PR body show the retainer is a stale slot in the JSC::runInternalMicrotask frame. With the scrub, 15 of 15 runs on darwin, 6 of 6 on linux and 3 of 3 under the ASAN build collect all 20 bodies. The coderabbit comment above is a rate-limit notice, nothing to address. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
Evidence for the retainer, as CLAUDE.md rule 15 asks for. Both artifacts come from the release build of CI build 110902 (the profile variant for lldb, since it carries symbols) and a standalone copy of the "dropped while a reader holds the lock" shape: 20 fetches, one 1. Heap snapshot.
2. Debugger session. The only cluster address on the stack is the controller's, at 3. The slot is what holds it. With the process stalled, one throwaway fetch body read (the scrub in this PR) is followed by the collection and the abort within 20 ms, in 12 of 12 stalled runs. A throwaway JS-source The transport was also checked during the stall: the client socket's receive queue is full, the HTTP thread does no |
|
Status: ready for a maintainer. Build 111285 ran this file on every test lane, including both darwin aarch64 jobs (the lane that went red in 110683 and 110902), darwin x64, windows x64 and aarch64, and the ASAN lane: all passed. The red jobs in that build are not touched by this diff and are red on main too: Reproduction and evidence: #41607 (comment) (heap snapshot with no retainer, lldb stack word at |
…ng to occupy A microtask checkpoint is where a turn of the embedder's event loop resumes an async function: a timer or an I/O callback settles a promise, and the queue is drained. Every turn does that from the same stack depth, so the frames of the checkpoint (MicrotaskQueue::drain*(), runInternalMicrotask(), the job's, the entry to JS) are at the same addresses in every turn. They stay for as long as the turn's JS runs, and they lie over whatever ran at that depth since the last checkpoint: that checkpoint's own frames, or the embedder's. What they do not write is in reach of the conservative scan of every collection made from under them, and sanitizeStackForVM(), which such a collection calls, clears only what is below the frame that calls it. #673 took the words that ASan poisons (redzones, locals out of scope) out of the scan. This is about the words it cannot take out: spill slots and locals of a live frame that this job's path through the function does not write. Builds without ASan have only those, and ASan builds have them too. runInternalMicrotask() is one switch over every kind of job, with callMicrotask() inlined into several arms, and a job uses one arm. Seen as: what a finished async function held is not collected by collections made from later turns. async function makeGarbage() { /* two objects in a list, one await per object */ } await makeGarbage(); for (;;) { await turn(); fullGC(); } // turn(): a promise that a timer resolves jsc shell of c281568 (linux amd64, lto): 32 turns until the objects are collected (the loop tiers up and the frames change), 60 of 60 turns not collected with --useJIT=0. Bun at that WebKit (release, x86_64): not collected in 8 of 8 turns when the timer is Bun.sleep(1); Bun's debug ASan build, which has #673: not collected in 8 of 8 turns for the module shape with any timer. A heap snapshot has the objects held by `list`, held by the lexical environment of the finished function's JSAsyncFunctionGenerator, which has no incoming edge and no root. In a debugger, the one word of the scanned span that holds the generator's address is at rbp-64 of runInternalMicrotask()'s frame (272 bytes), under drainWithUseCallOnEachMicrotask() (624 bytes). The job that runs in the later turns (AsyncModuleExecutionResume for the module, AsyncFunctionResume when the loop is in an async function of its own) does not write that slot. Which shape shows it moves with the compiler: the same script with setTimeout() kept the objects with clang 21 and does not with clang 23. Earlier sightings of this frame in Bun's leak tests: oven-sh/bun#37853 (the same generator word), oven-sh/bun#41607 (darwin arm64, release). So a checkpoint that has jobs to run first clears MicrotaskQueue::stackBytesClearedForCheckpoint bytes of the stack below performMicrotaskCheckpoint(), before the first of those frames exists: 2 KB in a release build (from there to the JS frame is 1.5 KB for x86_64), 32 KB with assertions or ASan (12.6 KB with both). It does that with a function that is never inlined and whose frame is an array of that size, which it zeroes with memset(): that frame lies exactly where the caller's next callee has its own, on every ABI, and the compiler probes the stack for it where a platform needs that. The array's address goes through an empty asm statement. Without it the stores are dead to the compiler (zeroBytes() and secureZeroBytes() both compile to a bare `ret` here: the memory clobber of secureZeroSpan() does not keep stores to a local whose address does not escape). callMicrotask() asserts that it runs within seven eighths of the window below the checkpoint's MicrotaskCallCache, which is a local of drainImpl(). Not sanitizeStackForVM() at the checkpoint: it clears from where it was last called, and nothing need have called it from below the checkpoint since. The jsc shell does not change with it (JSLock::didAcquireLock() resets VM::m_lastStackTop for every task). Not sanitizeStackForVMImpl() with m_lastStackTop lowered to the bottom of the window, which was the first version: its loop for x86_64 stores 8 bytes per iteration, and a checkpoint with one job went from 106 ns to 195 ns. Cost, Xeon 8375C, clang 23: `promise.then(noop); drainMicrotasks()` 5 M times in the jsc shell, 101 ns per iteration without the clear and 122 ns with it. Bun, 1 M turns of `await new Promise(r => setImmediate(r))`: 2531 ms without and 2547 ms with (medians of 11, same binary, the option). 20 M awaits in one checkpoint: 602 ms and 602 ms. An empty checkpoint clears nothing. Once per checkpoint and not once per job, which would cost every job that much: what one job leaves is still there for the later jobs of the same checkpoint, and is gone at the next one. Not when the window would reach below the soft stack limit. An embedder that holds the API lock for the life of its thread can get much of this from a sanitizeStackForVM() per event loop turn instead (oven-sh/bun#37853): in Bun that collects the same six cases at the same cost. It depends on something having sanitized from below the stale word since it was written, clears per turn and not per checkpoint, and covers the embedder's own frames as well. The two do not exclude each other. Options::clearStackForMicrotaskCheckpoint (default on) is for comparisons. Tests: JSTests/modules/microtask-checkpoint-clears-its-stack.js (an async function, then the module) and JSTests/stress/microtask-checkpoint-clears-its-stack.js (a promise reaction, then an async generator, each followed by an async function). Both fail in the shell of c281568 and with --clearStackForMicrotaskCheckpoint=0, and pass in the 28 configurations that run-javascriptcore-tests runs them in (release, x86_64).
…ng to occupy A microtask checkpoint is where a turn of the embedder's event loop resumes an async function: a timer or an I/O callback settles a promise, and the queue is drained. Every turn does that from the same stack depth, so the frames of the checkpoint (MicrotaskQueue::drain*(), runInternalMicrotask(), the job's, the entry to JS) are at the same addresses in every turn. They stay for as long as the turn's JS runs, and they lie over whatever ran at that depth since the last checkpoint: that checkpoint's own frames, or the embedder's. What they do not write is in reach of the conservative scan of every collection made from under them, and sanitizeStackForVM(), which such a collection calls, clears only what is below the frame that calls it. #673 took the words that ASan poisons (redzones, locals out of scope) out of the scan. This is about the words it cannot take out: spill slots and locals of a live frame that this job's path through the function does not write. Builds without ASan have only those, and ASan builds have them too. runInternalMicrotask() is one switch over every kind of job, with callMicrotask() inlined into several arms, and a job uses one arm. Seen as: what a finished async function held is not collected by collections made from later turns. async function makeGarbage() { /* two objects in a list, one await per object */ } await makeGarbage(); for (;;) { await turn(); fullGC(); } // turn(): a promise that a timer resolves jsc shell of c281568 (linux amd64, lto): 32 turns until the objects are collected (the loop tiers up and the frames change), 60 of 60 turns not collected with --useJIT=0. Bun at that WebKit (release, x86_64): not collected in 8 of 8 turns when the timer is Bun.sleep(1); Bun's debug ASan build, which has #673: not collected in 8 of 8 turns for the module shape with any timer. A heap snapshot has the objects held by `list`, held by the lexical environment of the finished function's JSAsyncFunctionGenerator, which has no incoming edge and no root. In a debugger, the one word of the scanned span that holds the generator's address is at rbp-64 of runInternalMicrotask()'s frame (272 bytes), under drainWithUseCallOnEachMicrotask() (624 bytes). The job that runs in the later turns (AsyncModuleExecutionResume for the module, AsyncFunctionResume when the loop is in an async function of its own) does not write that slot. Which shape shows it moves with the compiler: the same script with setTimeout() kept the objects with clang 21 and does not with clang 23. Earlier sightings of this frame in Bun's leak tests: oven-sh/bun#37853 (the same generator word), oven-sh/bun#41607 (darwin arm64, release). So a checkpoint that has jobs to run first clears MicrotaskQueue::stackBytesClearedForCheckpoint bytes of the stack below performMicrotaskCheckpoint(), before the first of those frames exists: 2 KB in a release build (from there to the JS frame is 1.5 KB for x86_64), 32 KB with assertions or ASan (12 KB with both). It does that with a function that is never inlined and whose frame is an array of that size, which it zeroes with memset(): that frame lies exactly where the caller's next callee has its own, on every ABI, and the compiler probes the stack for it where a platform needs that. The array's address goes through an empty asm statement. Without it the stores are dead to the compiler (zeroBytes() and secureZeroBytes() both compile to a bare `ret` here: the memory clobber of secureZeroSpan() does not keep stores to a local whose address does not escape). callMicrotask() asserts that it runs within seven eighths of the window below the checkpoint's MicrotaskCallCache, which is a local of drainImpl(). Not sanitizeStackForVM() at the checkpoint: it clears from where it was last called, and nothing need have called it from below the checkpoint since. The jsc shell does not change with it (JSLock::didAcquireLock() resets VM::m_lastStackTop for every task). Not sanitizeStackForVMImpl() with m_lastStackTop lowered to the bottom of the window, which was the first version: its loop for x86_64 stores 8 bytes per iteration, and a checkpoint with one job went from 106 ns to 195 ns. Cost, Xeon 8375C, clang 23: `promise.then(noop); drainMicrotasks()` 5 M times in the jsc shell, 101 ns per iteration without the clear and 122 ns with it. Bun, 1 M turns of `await new Promise(r => setImmediate(r))`: 2531 ms without and 2547 ms with (medians of 11, same binary, the option). 20 M awaits in one checkpoint: 602 ms and 602 ms. An empty checkpoint clears nothing. Once per checkpoint and not once per job, which would cost every job that much: what one job leaves is still there for the later jobs of the same checkpoint, and is gone at the next one. Not when the window would reach below the soft stack limit. An embedder that holds the API lock for the life of its thread can get much of this from a sanitizeStackForVM() per event loop turn instead (oven-sh/bun#37853): in Bun that collects the same six cases at the same cost. It depends on something having sanitized from below the stale word since it was written, clears per turn and not per checkpoint, and covers the embedder's own frames as well. The two do not exclude each other. Options::clearStackForMicrotaskCheckpoint (default on) is for comparisons. Tests: JSTests/modules/microtask-checkpoint-clears-its-stack.js (an async function, then the module) and JSTests/stress/microtask-checkpoint-clears-its-stack.js (a promise reaction, then an async generator, each followed by an async function). Both fail in the shell of c281568 and with --clearStackForMicrotaskCheckpoint=0, and pass in the 28 configurations that run-javascriptcore-tests runs them in (release, x86_64).
This branch has not been deployed
1 participant
Problem
test/js/web/fetch/fetch-stream-cancel-leak.test.ts> "one read() after it parked" went red on darwin aarch64 in builds 110683 and 110902:expect(N - aborted).toBeLessThan(N / 4),Expected: < 5,Received: 5. Up to 4 of the 20 abandoned bodies were allowed to survive the 3 s wait, and 5 did.read(), itsReadableStreamDefaultControllerstays in a stale slot of theJSC::runInternalMicrotaskframe that ran that read. The wait loop resumes through that same frame, so the conservative stack scan marks the controller, and through it the stream, on everyBun.gc(true). Nothing in the loop writes that slot again, so the stream lives until unrelated work does, about one second later on an idle box, and past the deadline on the CI lane.Fix
/scrubroute of the same origin and cancels it. That runs the same fetch-body code path, so the stale slot then holds a stream nobody cares about./scrubaborts do not count.expect(aborted).toBe(N). Unfixed (fetch: park an unread body stream instead of buffering it without bound #39590 reverted), none of the bodies are aborted, so the test still fails.read()after the park, unpark, re-park, collect, abort from the sweep.bun bd test(ASAN). The stall is gone in all of them.Background
runInternalMicrotaskis JSC's dispatcher for internal microtasks. Promise reactions of the stream machinery and async-function resumes both run through it, at the same depth, but they spill different values into the same frame area.Notes
How the retainer was found. A standalone copy of the "dropped while a reader holds the lock" shape stalls for about 1 s in 25 to 45 % of runs on darwin and linux release builds. During the stall: the origin's socket has a full send queue, the client socket a full receive queue (
netstat,ss -tnie:rwnd_limited), the HTTP thread does norecvfromon that socket (fs_usage), and the JS thread sends no wakeup to the HTTP thread (aDYLD_INSERT_LIBRARIESinterposer onmach_msg). So the stream is parked and the transport paused, and the only thing missing is the collection.generateHeapSnapshotForDebugging()taken afterBun.gc(true)during the stall: the stream, its controller, reader,BytesInternalReadableStreamSourceandNativeStreamSourceAdapterform a cluster with no incoming edge from outside it and norootsentry.lldbon the profile build of 110902, stopped from inside the stall: the controller's cell address0x45b98648e70is on the main thread stack at0x16fdfdc38, inside frame #10JSC::runInternalMicrotask(fp0x16fdfdd20) of the chaindrainWithUseCallOnEachMicrotask>runInternalMicrotask>asyncModuleExecutionResume> the script's continuation >Bun.gc. The fp chain was walked by hand so JIT frames do not stop it./proc/self/memon linux finds the same controller address in the[stack]mapping.Why a throwaway fetch and not a throwaway
ReadableStream. A JS-source stream or aBun.file().stream()read before each collection did not help (stalls in 16 to 19 of 20 runs, they spill into other slots). A throwaway fetch body read made 30 of 30 runs collect everything within 100 ms on darwin and 20 of 20 on linux. One throwaway fetch read in the middle of a stall collects the retained stream within 20 ms in 12 of 12 stalled runs.Why the 1 s on an idle box. Not measured to the instruction. The slot is overwritten only by the same microtask path, and the origin's own stream microtasks stop while it is backpressured. The next unrelated work on the JS thread clears it.
The CI count. 5 survivors did not reproduce locally, with or without CPU load, under the release or the profile build. The mechanism is the same for every survivor that the heap snapshot cannot explain, and the scrub re-runs the whole path (tasklet progress task,
ByteStream::on_data, the pull microtasks, the read request), so it overwrites the slots of each of those frames.Related. #41111 rewrites this file and waits on
WeakRefs with an exact count. It notes the same ~970 ms stall. The scrub applies there too.Suites run: this file under the darwin release and profile builds of 110902 (15 runs), the linux release build (6 runs), and
bun bd testASAN (3 runs).[auto-merge] gate passed · iteration 1 · 1 files touched
passes on PR (with fix)
diff hotspot
gate history · 2 passed · 0 rejected · iteration 1
evidence per changed file