Skip to content

[JSC] A microtask checkpoint clears the stack that its frames are going to occupy - #681

Closed
robobun wants to merge 1 commit into
mainfrom
robobun/b42f2e8d/microtask-checkpoint-clears-stack
Closed

robobun wants to merge 1 commit into
mainfrom
robobun/b42f2e8d/microtask-checkpoint-clears-stack

Conversation

@robobun

@robobun robobun commented Sep 16, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • An object that nothing references survives every collection made from later event loop turns. The jsc shell of c28156899e keeps what the async function of the repro in the notes held for 32 turns, and for 60 of 60 turns with --useJIT=0.
  • Every turn runs its jobs from a microtask checkpoint at the same stack depth. So the frames of the drain and of runInternalMicrotask() (one switch over all job kinds) are at the same addresses each time. A job writes only some of their slots. The conservative scan reads all of them.
  • In the repro the stale word is at rbp-64 of the frame of runInternalMicrotask(). It holds the generator of the finished function. [JSC] The conservative scan does not read stack words ASan has poisoned #673 does not cover it: the word is a normal slot, not an ASan redzone.

Fix

  • performMicrotaskCheckpoint() zeroes the stack below itself before it drains a queue that has jobs: 2 KB in a release build, 32 KB with assertions or ASan.
  • Correct because memory below the stack pointer holds nothing live. [JSC] Less memory per function that never runs, and code that can be dropped and decoded again #628 does the same for the call slow paths.
  • Cost: a checkpoint with one job goes from 101 ns to 122 ns. Options::clearStackForMicrotaskCheckpoint turns the clear off.
  • Verified: two new JSTests fail in the stock shell and pass in all 28 configurations of run-javascriptcore-tests.

Background

  • JSC scans the machine stack conservatively. A word that looks like a cell pointer keeps that cell alive, also a stale word in a live frame.
  • A microtask checkpoint drains the job queue. An embedder runs one after each timer or I/O callback, so an await continues there.
  • sanitizeStackForVM() zeroes the stack below its caller. It cannot reach a live frame.
Notes

Scope. The object that stays is the last one that went through the slot. In Bun with default settings it goes away when other work overwrites the word, for example the collector's 1 s timer. So this is about collections that are deterministic (leak tests, FinalizationRegistry and WeakRef users that poll), not about unbounded growth.

Repro (jsc -m, or Bun with Bun.gc(true)):

const turn = () => new Promise(resolve => setTimeout(resolve, 0));
const refs = [];
async function makeGarbage() {
    const list = [];
    for (let i = 0; i < 2; ++i) { const object = { i }; list.push(object); refs.push(new WeakRef(object)); await turn(); }
    await Promise.all(list.map(object => Promise.resolve(object.i)));
}
await makeGarbage();
let collections = 0;
do { await turn(); fullGC(); ++collections; } while (collections < 60 && refs.some(ref => ref.deref()));
print("collections: " + collections + ", alive: " + refs.filter(ref => ref.deref()).length);
build result
jsc of autobuild-c28156899e5f (linux amd64, lto) collections: 32, alive: 0, with --useJIT=0: collections: 60, alive: 2
this branch collections: 1, alive: 0
this branch, --clearStackForMicrotaskCheckpoint=0 collections: 32, alive: 0

Bun at c28156899e (it has #673), six cases: the collections are made by the module or by an async function, and a turn is setTimeout, setImmediate or Bun.sleep(1). Eight turns each.

  • Release build, x86_64: both Bun.sleep cases keep both objects for 8 of 8 turns.
  • Debug ASan build: all three module cases keep both objects for 8 of 8 turns.
  • Release build against this branch: all six collect at turn 1. Same binary with BUN_JSC_clearStackForMicrotaskCheckpoint=0: all six keep both objects.
  • Which case shows it moves with the compiler. The setTimeout cases kept the objects with clang 21 and do not with clang 23.

The two artifacts (Bun release build c6b7fcb5b, BUN_GC_TIMER_DISABLE=1, the module case with Bun.sleep(1)):

  • generateHeapSnapshotForDebugging() after five collections: the objects are held by list, held by a JSLexicalEnvironment, held by the AsyncFunctionGenerator of makeGarbage(). The generator has no incoming edge, and no roots entry reaches any of them.
  • gdb, breakpoint on ConservativeRoots::genericAddSpan<CompositeMarkHook> during the next collection, search of the scanned span (10,432 bytes) for the five cell addresses: one hit, the generator. I walked the frame-pointer chain by hand. The word is at rbp-64 of the 272-byte frame of runInternalMicrotask(), under drainWithUseCallOnEachMicrotask() (624 bytes) and VM::drainMicrotasks(). The job that runs is AsyncModuleExecutionResume. With clang 21 the same word showed with the loop in an async function (AsyncFunctionResume).

Other sightings of this frame in Bun's leak tests: oven-sh/bun#37853 (the same generator word), oven-sh/bun#41607 (darwin arm64 release build, a ReadableStreamDefaultController). oven-sh/bun#42877 and #669 saw ASan redzone words of it, which #673 now covers.

How the window is cleared.

  • clearStackBelowCaller<size>() in MicrotaskQueue.cpp is NEVER_INLINE, its only local is a std::array<uint8_t, size>, and it calls zeroBytes() on it. Its frame lies right below its caller's frame, which is where the caller's next callee has its own. The compiler emits the stack probe for a large frame where a platform needs one.
  • The array's address is an input of an empty asm statement. Without it clang removes the memset() and the function is a bare ret. secureZeroBytes() does not help here: the memory clobber in secureZeroSpan() does not keep stores to a local whose address does not escape.
  • The clear does not happen if the window would reach below the soft stack limit.
  • x86_64 release: push rbx; sub rsp, 0x800; memset(rsp, 0, 0x800); add rsp, 0x800; pop rbx; ret.
  • callMicrotask() asserts (ASSERT_ENABLED) that it runs within seven eighths of the window below the checkpoint's MicrotaskCallCache, which is a local of drainImpl().

Sizes. From performMicrotaskCheckpoint() (inlined into VM::drainMicrotasks()) to the JS frame, Bun release build for x86_64, clang 23: drainWithUseCallOnEachMicrotask() 624, runInternalMicrotask() 272, asyncModuleExecutionResume() 64, JSModuleRecord::evaluate() 160, Interpreter::executeModuleProgram() 224, the entry thunk 176: 1520 in all. Debug ASan build: 12 KB, of which runInternalMicrotask() is 6320.

Why not sanitizeStackForVM() at the checkpoint.

  • It clears from the stack pointer of its last call. Nothing need have called it from below the checkpoint since then. With a plain sanitizeStackForVM() at the checkpoint the repro does not change in the jsc shell, where JSLock::didAcquireLock() resets VM::m_lastStackTop for every task.
  • The first version lowered VM::m_lastStackTop to the bottom of the window and called sanitizeStackForVMImpl(). Its loop for x86_64 stores 8 bytes per iteration: a checkpoint with one job went from 106 ns to 195 ns.

The alternative in the embedder. oven-sh/bun#37853 calls sanitizeStackForVM() once per event loop turn. Bun holds the API lock for the life of the thread, so m_lastStackTop is not reset there. I tried it on the same binary (option off, one sanitizeStackForVM() at the top of us_loop_run_bun_tick): it collects the same six cases at turn 1, and costs the same (2522 ms against 2525 ms per 1 M turns, 2504 ms with neither). Differences: it needs something to have sanitized from below the stale word since the word was written, it clears per turn and not per checkpoint, and it also covers the embedder's own frames and entries that are not checkpoints. The two do not exclude each other.

Once per checkpoint. What one job leaves is still there for the later jobs of the same checkpoint, and is gone at the next checkpoint. A clear per job would cost every job the 21 ns.

Cost. Xeon 8375C, release build, clang 23.

  • jsc: promise.then(noop); drainMicrotasks(); 5 M times: 101 ns per iteration without, 122 ns with (5 runs each, same binary, the option).
  • Bun: 1 M turns of await new Promise(r => setImmediate(r)): 2531 ms without, 2547 ms with (medians of 11). 20 M awaits in one checkpoint: 602 ms and 602 ms.

Tests.

  • JSTests/modules/microtask-checkpoint-clears-its-stack.js: an async function makes the garbage, the module collects from later turns.
  • JSTests/stress/microtask-checkpoint-clears-its-stack.js: a promise reaction that returns the object, then an async generator that yields it. An async function collects from later turns.
  • Both allow 10 turns, because the scan is conservative. What the checkpoint's frames keep, they keep in every turn.
  • Shell of c28156899e: both fail (exit 3). This branch: both pass in all 28 configurations, and fail with --clearStackForMicrotaskCheckpoint=0.
  • run-javascriptcore-tests --filter="microtask|async-func|async-gen|top-level-await|tla": 894 runs, 0 failures.

Bun PR that pins the preview build of this branch and adds the Bun test: oven-sh/bun#42975. With the preview build bun bd test (debug ASan) and a release build pass its 6 cases; the same binaries fail 3 and 2 of them with BUN_JSC_clearStackForMicrotaskCheckpoint=0.

…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).
@github-actions

Copy link
Copy Markdown

Preview build of 9832655: autobuild-preview-pr-681-98326556

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline finding, I also checked two things and ruled them out: the GNU-style __asm__ volatile in clearStackBelowCaller builds on the Windows lanes, since Dockerfile.windows compiles with clang-cl (JSLock.cpp and JSObject.cpp already use the same form), and the 2 KB release frame is under the 4 KB page so it needs no __chkstk probe there (the 32 KB assert/ASan frame gets one from the compiler). The softStackLimit() guard in clearStackForCheckpoint keeps the window out of the reserved zone, so the clear cannot itself trip the stack guard.

Extended reasoning...

A finding is posted inline, so this body only records what else was examined. The inline-asm portability question mattered because the Windows lanes are cross-compiled and MSVC proper would reject __asm__; Dockerfile.windows shows clang-cl is the compiler and the same construct already exists in runtime/JSLock.cpp and runtime/JSObject.cpp. The soft-stack-limit check in MicrotaskQueue.cpp bounds the window before the frame is created, so a checkpoint near the end of the stack skips the clear rather than faulting. This is informational only and not a guarantee of correctness; a human should still weigh the unvalidated release window size raised inline.

#if ASSERT_ENABLED || ASAN_ENABLED
static constexpr size_t stackBytesClearedForCheckpoint = 32 * KB;
#else
static constexpr size_t stackBytesClearedForCheckpoint = 2 * KB;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟣 pre-existing, not blocking: Windows x64 release users may keep getting the leak this change fixes on Linux, if their frame chain exceeds the unchecked 2 KB window. The release window at MicrotaskQueue.h:275 is validated by nothing: the assert at JSMicrotask.cpp:102 exists only in builds that use the 32 KB window. The chain measured 1520 of 2048 bytes on Linux x64; the MS ABI adds a 32-byte home area per call, stack-passed arguments past four, and xmm6-15 saves in the entry thunk, and the Windows lane is not tested. Fix: verify the release window on every shipped ABI or size it with margin (e.g. 4 KB), and let an option run the depth check in release so the Windows lane can confirm it. [also at: Source/JavaScriptCore/runtime/MicrotaskQueueInlines.h:144 - Bun users who poll WeakRef or FinalizationRegistry after a GC still get objects that survive every turn when the stale word sits deeper than the window, and the PR presents the leak as fixed.]
A small fix can ride a push you are already making; otherwise a short reply is enough.

Extended reasoning...

The PR measured the frames from performMicrotaskCheckpoint() to the JS frame on Linux x86_64 with clang 23: 624 + 272 + 64 + 160 + 224 + 176 = 1520 bytes, a 528-byte margin under stackBytesClearedForCheckpoint = 2 * KB (MicrotaskQueue.h:275). No release build checks that figure: assertJobOfCheckpointRunsInClearedStack (JSMicrotask.cpp:97-103) is compiled only under ASSERT_ENABLED, and in every such build the window is 32 KB (MicrotaskQueue.h:272-273), so the assertion never says anything about the 2 KB constant that ships. On Windows x64 (clang-cl, MS ABI, lanes.mjs:150-173) each non-leaf frame carries a 32-byte home area, runInternalMicrotask() (7 register-sized parameters) and callMicrotask() (7 to 11) pass arguments beyond the fourth on the stack, and the entry thunk saves rsi, rdi and xmm6-xmm15 (160 bytes more than the 176-byte Linux thunk). That plausibly pushes the same chain past 2048. When it does, clearStackBelowCaller<2048>()…

Verification: pre-existing (nit): on a platform/compiler whose frames from performMicrotaskCheckpoint() to the JS entry exceed 2 KB, the lower frames stay uncleared and the base's stale-word leak persists there; nothing regresses against the base. Mechanism verified: Source/JavaScriptCore/runtime/MicrotaskQueue.h:272-276 sets the window to 32 KB under ASSERT_ENABLED || ASAN_ENABLED and 2 KB otherwise, and…

@coderabbitai

coderabbitai Bot commented Sep 16, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 2e8ed779-045e-4e3f-a4de-0540f2fabec4

📥 Commits

Reviewing files that changed from the base of the PR and between c281568 and 9832655.

📒 Files selected for processing (7)
  • JSTests/modules/microtask-checkpoint-clears-its-stack.js
  • JSTests/stress/microtask-checkpoint-clears-its-stack.js
  • Source/JavaScriptCore/runtime/JSMicrotask.cpp
  • Source/JavaScriptCore/runtime/MicrotaskQueue.cpp
  • Source/JavaScriptCore/runtime/MicrotaskQueue.h
  • Source/JavaScriptCore/runtime/MicrotaskQueueInlines.h
  • Source/JavaScriptCore/runtime/OptionsList.h

Included review availability: Your plan provides up to 5 included reviews per hour; 1 remains after this review.


Walkthrough

Changes

Microtask checkpoint stack clearing

Layer / File(s) Summary
Checkpoint stack-clearing contract
Source/JavaScriptCore/runtime/MicrotaskQueue.h, Source/JavaScriptCore/runtime/OptionsList.h
Defines the stack-clearing size, declares clearStackForCheckpoint(VM&), and adds the default-enabled clearStackForMicrotaskCheckpoint option.
Checkpoint stack clearing integration
Source/JavaScriptCore/runtime/MicrotaskQueue.cpp, Source/JavaScriptCore/runtime/MicrotaskQueueInlines.h
Clears a bounded stack region before draining a non-empty microtask queue when the option and stack-limit checks allow it.
Microtask stack validation
Source/JavaScriptCore/runtime/JSMicrotask.cpp, JSTests/modules/microtask-checkpoint-clears-its-stack.js, JSTests/stress/microtask-checkpoint-clears-its-stack.js
Adds assertion-build stack-bound checks and tests collection of weakly referenced objects after multiple turns and garbage collections.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 98326

No merge-blocking issue was identified from the available evidence.

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description gives a detailed and relevant explanation of the problem, fix, performance impact, testing, and changed behavior. It does not include the required Bugzilla bug title and link, the "Rev… Add the associated Bugzilla URL and bug title, include the required review line, and list the changed files with relevant functions or classes using the repository template.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly describes the primary change: clearing stack space during a microtask checkpoint to prevent stale stack references.
Full details: Description check

Explanation

The description gives a detailed and relevant explanation of the problem, fix, performance impact, testing, and changed behavior. It does not include the required Bugzilla bug title and link, the "Reviewed by NOBODY (OOPS!)." line, or the template-style file and function change list.

  • Fix all pre-merge checks with AI

Warning

Git: CodeRabbit could not clone the repository, so clone-backed analysis was skipped and this review may be incomplete. Verify repository clone access, such as SSH credentials, before requesting another full review. If clone access is intentionally unavailable, use path_filters to narrow the review scope.


Comment @coderabbitai help to get the list of available commands.

robobun added a commit to oven-sh/bun that referenced this pull request Sep 16, 2026
A microtask checkpoint that has jobs to run first clears the stack that
the frames of the drain, of runInternalMicrotask() and of the job are
going to occupy. What a job of an earlier event loop turn left in those
frames is then no longer a root of a collection made from a later turn.

The pin is autobuild-preview-pr-681-98326556: c28156899e plus the one
commit of that PR.
@robobun

robobun commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator Author

Closing this PR.

The branch stays, in case a clear inside JSC is wanted later. The repro, the two artifacts and the measurements are in the description above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant