Run Error.prepareStackTrace even when a GC happens before the first .stack read

[robobun]

Problem

• With a user Error.prepareStackTrace installed, an error whose frames hold a dead callee at GC time gets the default stack string. Every error thrown from an async function hits this once a GC runs before the first .stack read (the async body is a per-call closure). Node runs the formatter.
• JSC holds the frames weakly. ErrorInstance::reconcileWeakReferencesAtGCEnd renders the string in the GC end phase through computeErrorInfoWrapperToString (src/jsc/bindings/FormatStackTraceForJS.cpp:574), where no JS can run, and drops the frames. The hook that honors prepareStackTrace runs only while the frames exist.

Fix

• ErrorInstance: mark the stack frames while the embedder keeps them alive WebKit#510 adds VM::setKeepsErrorStackFramesAlive(bool). While set, ErrorInstance::visitChildren marks the frames under the cell lock, so a live error keeps them until the first .stack read.
• The Error.prepareStackTrace setter counts the realms on the VM with a callable user formatter (JSVMClientData::realmsWithUserPrepareStackTrace) and keeps the flag set while the count is non-zero. A realm leaves the count when the property is reset or its global object dies.
• This is V8's model: V8 keeps the frames alive until the stack is formatted, and prepareStackTrace exists for V8 compatibility. Programs without a formatter keep JSC's weak frames and today's memory behavior.
• Verified: test/js/node/v8/capture-stack-trace.test.js (three new tests, all fail on stock bun). Also test/js/bun/test/stack.test.ts, test/js/node/vm/vm.test.ts, test/js/node/util/util.test.js.

Background

• ErrorInstance is JSC's class behind every Error object. It captures a Vector<StackFrame> at creation and renders error.stack on first access.
• A StackFrame holds the callee function and its CodeBlock through WriteBarriers that nothing marks, so an error nobody inspects does not pin functions.
• visitChildren is the GC marking hook of a cell. Marking threads run it concurrently with JS, so the frames are read under the cell lock, as estimatedSize does.
• Bun's prepareStackTrace support is computeErrorInfoWrapperToJSValue. It builds CallSite objects from the frames and calls the user function.

Notes

• WEBKIT_VERSION points at the preview build of ErrorInstance: mark the stack frames while the embedder keeps them alive WebKit#510 (autobuild-preview-pr-510-8f7ed779, the current pin c148a12d plus that change). Bump to the merged commit once WebSocket client messaage event is broken #510 lands.
• Error.appendStackTrace and the lazy .stack getter now move frames under the cell lock, since marking threads read them.
• Known gaps, both with today's behavior: a formatter installed inside a node:vm context has no setter hook, so errors from that realm are not covered. A formatter installed after a GC already rendered an error's string does not run for that error.
• The count exists because a VM can host several Zig::GlobalObjects (ShadowRealm, bun test --isolate), each with its own Error.prepareStackTrace. A plain flag written by the last setter call let one realm clear it for another. The third test covers the ShadowRealm case. A collected --isolate global drops its count in ~GlobalObject; that case is not asserted because whether a released callee is collected by a given GC is build dependent.
• The flag is a relaxed atomic read by marking threads. A stale read leaves that error's frames weak for the current cycle, which is today's behavior.
• Bun internals (util.getCallSites, the isInsideNodeModules check) install a formatter for one capture and restore the previous value. The count follows: one for that capture, zero after.
• Repro shapes: an async function body, a new Function callee, and Error.captureStackTrace inside a new Function callee, at the top level of a script. All three hit the GC end phase rendering on every run of stock bun 1.4.0 and of the debug ASAN build (30 and 8 runs). Shapes nested in a test function were build dependent, so the tests run the scripts in a child process.
• Whether a released callee is collected by a given Bun.gc(true) differs between the release and debug builds, so the tests do not assert it. They assert that an unread error keeps its callee alive (a WeakRef survives the GC) and that the formatter result is the same with and without a GC.
• The name/message header loss in the GC end phase rendering (Async-thrown Error loses its message from error.stack when GC runs before first .stack access #34398) is a separate path for programs without a formatter and is not changed here.

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{On-demand reviews are free for the next 27 days. After that, they cost $0.25 per reviewed file.}

Or wait 21 minutes for your next included review.

View limit details

Limit details: You’ve used all 5 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1976dff9-90ea-476b-b631-b0cd0b214db0

📥 Commits

Reviewing files that changed from the base of the PR and between 861e9ae and 64d9601.

📒 Files selected for processing (6)

• scripts/build/deps/webkit.ts
• src/jsc/bindings/BunClientData.h
• src/jsc/bindings/FormatStackTraceForJS.cpp
• src/jsc/bindings/ZigGlobalObject.cpp
• src/jsc/bindings/ZigGlobalObject.h
• test/js/node/v8/capture-stack-trace.test.js

---

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

[robobun]

Status: the fix is pushed and waits for CI and review.

Reproduced with stock bun 1.4.0 and the debug build:

Error.prepareStackTrace = (e, cs) => "custom:" + e.message;
async function boom() { throw new Error("m"); }
try { await boom(); } catch (e) { Bun.gc(true); console.log(JSON.stringify(e.stack)); }

prints the default "Error\n at boom (...)" instead of "custom:m". Node prints "custom:m". With the fix (this PR plus oven-sh/WebKit#510) it prints "custom:m".

WEBKIT_VERSION points at the preview build of oven-sh/WebKit#510 until that PR lands.

CI on d569b96: the only red tests were test/cli/run/require-cache.test.ts (x64-asan) and test/js/node/test/sequential/test-cpu-prof-name.js (Windows 2019). Both fail on main as well and are reported separately. The rest were retries that passed.

[robobun]

^{Updated 8:42 AM PT - Aug 24th, 2026}

✅ @robobun, your commit 64d960180af2e7a51f9db02625cb19b7396b6fb8 passed in Build #104940! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 40352

That installs a local version of the PR into your bun-40352 executable, so you can run:

bun-40352 --bun

[robobun]

#40354 (JSC side: oven-sh/WebKit#511) takes the same mechanism, ErrorInstance::visitChildren marking the frames under the cell lock, without the Error.prepareStackTrace gate. It also fixes the header loss of #34398 and the Error.captureStackTrace frames that turn into a fixed string, which happen with no formatter installed, and it covers a formatter installed after the GC and errors from node:vm contexts (the two gaps listed in the notes here). The cost is V8's retention: an unread error keeps its callees alive until the first .stack read.

The Bun-side cell lock changes in FormatStackTraceForJS.cpp are the same in both PRs. Whichever design is preferred, the other PR can be closed.

[claude]

Thanks — the per-VM realmsWithUserPrepareStackTrace refcount (with the ~GlobalObject decrement and the new ShadowRealm test) addresses the last-writer-wins concern I raised. I didn't find further issues in this revision, but given this touches concurrent GC marking (cell-lock discipline around ErrorInstance frames) and pins WEBKIT_VERSION to a preview build of an unmerged WebKit PR, a human should sign off before merge.

Extended reasoning...

Overview

The PR keeps ErrorInstance stack frames alive across GC while a user Error.prepareStackTrace is installed, so the formatter still runs after a GC between throw and first .stack read. It adds a per-VM refcount (JSVMClientData::realmsWithUserPrepareStackTrace) driven by Zig::GlobalObject::setHasUserPrepareStackTrace, wraps mutator-side frame-vector moves in cellLock() to match the new concurrent visitChildren reader in oven-sh/WebKit#510, and pins WEBKIT_VERSION to that PR's preview build. Three subprocess tests are added.

What changed since my last review

Commit 64d9601 replaced the unconditional VM-flag write with the refcount I suggested: each Zig::GlobalObject tracks its own m_hasUserPrepareStackTrace transition and adjusts the shared count, ~GlobalObject decrements on destruction, and a ShadowRealm test covers the cross-realm reset case. This resolves the concern.

Security risks

None identified — no untrusted input parsing, auth, or network surface.

Level of scrutiny

High. This is JSC GC/memory-safety territory (the repo's most-blocked review category): a VM-wide atomic read by concurrent marking threads, cell-lock discipline around a Vector<StackFrame> that visitChildren now walks, a writeBarrier after appendVector, and a vm()/clientData() access from ~GlobalObject (which runs during sweep). It also lands with WEBKIT_VERSION pointed at a preview autobuild of an unmerged WebKit PR — a maintainer should confirm that's acceptable to merge or should wait for #510.

Other factors

The comment-cop bot has three outstanding flags on the new multi-line comments; those are visible to the author and I'm not restating them. The refcount implementation looks balanced (early-return on no-transition prevents double-counting; destructor decrement covers --isolate teardown), but the interaction between GC-driven ~GlobalObject and JSVMClientData lifetime at VM shutdown is the kind of thing a maintainer familiar with Bun's teardown ordering should confirm.

[robobun]

On the teardown order question: ~GlobalObject runs from heap.lastChanceToFinalize() in VM::~VM, and delete clientData comes after it (vendor/WebKit/Source/JavaScriptCore/runtime/VM.cpp, the same order BunClientData.cpp documents for its own captures). So JSVMClientData is alive when the destructor drops the realm's count. During a normal run, the destructor runs from a sweep on the JS thread, where the count is only ever touched.

The WEBKIT_VERSION pin moves to the merged oven-sh/WebKit commit once #510 lands, before this merges.

[robobun]

Closing in favor of #40354. Both PRs mark the frames from ErrorInstance::visitChildren. This one gates the pin on a user Error.prepareStackTrace (with a per-VM realm count), #40354 pins unconditionally. The unconditional pin also fixes the bare Error header reported in #34398 and the Error.captureStackTrace frame loss, both of which happen with no formatter installed, and it needs no per-realm bookkeeping: a formatter installed after the GC and errors from node:vm contexts are covered too.

The three tests added here are now in #40354 (test/js/bun/test/stack.test.ts, the Error.prepareStackTrace when a GC runs before the first .stack read block): the formatter runs with call sites across an async throw, a new Function callee and Error.captureStackTrace, a WeakRef to an unread error's callee survives a GC, and a ShadowRealm resetting its own formatter does not affect the main realm. All three fail on bun 1.4.0 and pass at the oven-sh/WebKit#511 pin. The companion WebKit PR oven-sh/WebKit#510 is closed as well.