From dc3ed5f78f01bdc786a24a4e0a6772daadd7f490 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 7 Jul 2026 00:51:13 +0000 Subject: [PATCH 01/12] vm: preserve a pending exception across the GC stack-trace finalizer computeErrorInfoWrapperToString (installed via vm.setOnComputeErrorInfo) runs from ErrorInstance::finalizeUnconditionally during Heap::runEndPhase. It cleared whatever exception was pending after computing a stack string. With concurrent GC that finalizer lands at an arbitrary safepoint: if it fires while the mutator is mid vm.SourceTextModule evaluation with a module's evaluation error pending, it nulled that exception. CyclicModuleRecord::evaluate step 9.d then reached rejectWithCaughtException with no pending exception, aborting on assert builds (ASSERTION FAILED: exception, JSPromise.cpp) and segfaulting at 0x8 on release. Only swallow an exception the stack computation itself raised; leave a pre-existing one untouched so it survives the finalizer. --- src/jsc/bindings/FormatStackTraceForJS.cpp | 13 ++- .../node/vm/sourcetextmodule-link-gc.test.ts | 80 ++++++++++++++++++- 2 files changed, 89 insertions(+), 4 deletions(-) diff --git a/src/jsc/bindings/FormatStackTraceForJS.cpp b/src/jsc/bindings/FormatStackTraceForJS.cpp index b74b3a61a0ab..e08f6b5cc306 100644 --- a/src/jsc/bindings/FormatStackTraceForJS.cpp +++ b/src/jsc/bindings/FormatStackTraceForJS.cpp @@ -598,10 +598,17 @@ WTF::String computeErrorInfoWrapperToString(JSC::VM& vm, Vector& sta OrdinalNumber column = OrdinalNumber::fromOneBasedInt(column_in); auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm); + // finalizeUnconditionally can run this from Heap::runEndPhase at an + // arbitrary GC safepoint while the suspended mutator has an unrelated + // pending exception (e.g. a module evaluation error captured mid + // CyclicModuleRecord::evaluate). Only swallow an exception this + // computation itself raised; a pre-existing one belongs to the mutator + // and clearing it crashes the later reject-with-caught-exception site. + JSC::Exception* priorException = scope.exception(); WTF::String result = computeErrorInfoToString(vm, stackTrace, line, column, sourceURL); - if (scope.exception()) { - // TODO: is this correct? vm.setOnComputeErrorInfo doesnt appear to properly handle a function that can throw - // test/js/node/test/parallel/test-stream-writable-write-writev-finish.js is the one that trips the exception checker + if (scope.exception() && scope.exception() != priorException) { + // vm.setOnComputeErrorInfo doesn't handle a callback that throws. + // test/js/node/test/parallel/test-stream-writable-write-writev-finish.js trips the exception checker. (void)scope.tryClearException(); result = WTF::emptyString(); } diff --git a/test/js/node/vm/sourcetextmodule-link-gc.test.ts b/test/js/node/vm/sourcetextmodule-link-gc.test.ts index fab4c87bdd45..d20b59ee2275 100644 --- a/test/js/node/vm/sourcetextmodule-link-gc.test.ts +++ b/test/js/node/vm/sourcetextmodule-link-gc.test.ts @@ -12,7 +12,7 @@ // ~9× per link() call. import { expect, test } from "bun:test"; -import { bunEnv, bunExe, isWindows } from "harness"; +import { bunEnv, bunExe, isWindows, tempDir } from "harness"; // collectContinuously is very slow under Windows + ASAN in CI; the code path // is identical on Linux/macOS, so skip Windows to keep duration reasonable. @@ -70,3 +70,81 @@ test.skipIf(isWindows)( }, 120_000, ); + +// computeErrorInfoWrapperToString (src/jsc/bindings/FormatStackTraceForJS.cpp, +// installed via vm.setOnComputeErrorInfo) runs from ErrorInstance:: +// finalizeUnconditionally during Heap::runEndPhase. It used to clear whatever +// exception was pending after computing a stack string. With concurrent GC, +// that finalizer lands at an arbitrary safepoint: if it fires while the mutator +// is mid vm.SourceTextModule evaluation with a module's evaluation error +// pending, it nulled that exception. CyclicModuleRecord::evaluate step 9.d then +// reached rejectWithCaughtException with no pending exception and crashed +// (`ASSERTION FAILED: exception` at JSPromise.cpp, or a SEGV at 0x8 on release). +// The fix only swallows an exception the stack computation itself raised. +// +// The race depends on concurrent GC landing in that window: --smol shrinks the +// heap so collections run often enough to hit it on essentially every run of +// the unfixed assert build, while collectContinuously moves the window and +// hides it. The fixture is a real file (a -e eval script has no source URL, so +// the stack-trace finalizer takes a different path and doesn't fire). The +// processes run one at a time (concurrency starves the collector and hides the +// race); a single crash fails the test. +test.skipIf(isWindows)( + "vm.SourceTextModule evaluation error survives a concurrent-GC stack-trace finalizer", + async () => { + const fixture = ` + import * as vm from "node:vm"; + let UREJ = 0; + process.on("unhandledRejection", () => { UREJ++; }); + const sl = async () => { await new Promise(r => setTimeout(r, 12)); for (let i = 0; i < 8; i++) await null; }; + const CA = async f => { try { return await f(); } catch (e) { return e; } }; + const ni = () => { throw new Error("noimports"); }; + const M = (src, id, o) => new vm.SourceTextModule(src, { identifier: id, ...o }); + + async function fam(sd) { + { const bad = M("throw new TypeError('boom')", "s1bad" + sd), a = M("import 'bad'; export const x=1;", "s1a" + sd), b = M("import 'bad'; export const y=2;", "s1b" + sd); + await a.link(() => bad); await b.link(() => bad); + try { await a.evaluate(); } catch {} try { await b.evaluate(); } catch {} try { await a.evaluate(); } catch {} + await CA(() => bad.evaluate()); } + { const ctx = vm.createContext({ log: [] }); + const leaf = M("log.push('L0'); export const v = await Promise.resolve(7); log.push('L1');", "s2l" + sd, { context: ctx }); + const mid = M("import {v} from 'l'; export const m=v+1;", "s2m" + sd, { context: ctx }); + const root = M("import {m} from 'm'; export const r=m+1;", "s2r" + sd, { context: ctx }); + const map = { l: leaf, m: mid }; await root.link(s => map[s]); + const ep = root.evaluate(); await CA(async () => { await ep; }); await CA(() => root.evaluate()); } + { const leaf = M("await 0; throw new URIError('tla');", "s3l" + sd), a = M("import 'l'; export const x=1;", "s3a" + sd), b = M("import 'l'; export const y=1;", "s3b" + sd); + await a.link(() => leaf); await b.link(() => leaf); + try { await a.evaluate(); } catch {} try { await b.evaluate(); } catch {} try { await leaf.evaluate(); } catch {} } + { const m = M("export const a=1;", "s5" + sd); await m.link(ni); await m.evaluate(); + const bad = M("null.x", "s5b" + sd); await bad.link(ni); await bad.evaluate().catch(() => {}); } + { const val = ["str", "num", "null", "undef", "obj", "sym"][sd % 6]; + const src = { str: "throw 'x'", num: "throw 42", null: "throw null", undef: "throw undefined", obj: "throw {a:1}", sym: "throw Symbol.iterator" }[val]; + const m = M(src, "s6" + sd); await m.link(ni); await CA(() => m.evaluate()); } + await CA(async () => { await sl(); return UREJ; }); + } + for (let sd = 0; sd < 60; sd++) await fam(sd); + console.log("ok"); + `; + + using dir = tempDir("vm-module-eval-error-gc", { "fixture.mjs": fixture }); + + async function run() { + await using proc = Bun.spawn({ + cmd: [bunExe(), "--smol", "--experimental-vm-modules", "fixture.mjs"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + stdout: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout: stdout.trim(), stderr, exitCode }; + } + + for (let i = 0; i < 4; i++) { + const { stdout, stderr, exitCode } = await run(); + expect({ stdout, exitCode }).toEqual({ stdout: "ok", exitCode: 0 }); + void stderr; + } + }, + 120_000, +); From b9cd27f8d497e6607809cc67cbedd7f981125312 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 7 Jul 2026 01:08:34 +0000 Subject: [PATCH 02/12] use SuspendExceptionScope to preserve the pending exception JSC has a dedicated RAII helper for exactly this case (a callee that must run transparently to a pre-existing pending exception). It saves and nulls m_exception, m_lastException and the NeedExceptionHandling trap, then restores all three on destruction. TypeProfilerLog::processLogEntries uses the same pattern for the same reason. --- src/jsc/bindings/FormatStackTraceForJS.cpp | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/jsc/bindings/FormatStackTraceForJS.cpp b/src/jsc/bindings/FormatStackTraceForJS.cpp index e08f6b5cc306..2c80315541cf 100644 --- a/src/jsc/bindings/FormatStackTraceForJS.cpp +++ b/src/jsc/bindings/FormatStackTraceForJS.cpp @@ -6,6 +6,7 @@ #include "JavaScriptCore/ArgList.h" #include "JavaScriptCore/CallData.h" +#include "JavaScriptCore/FrameTracers.h" #include "JavaScriptCore/TopExceptionScope.h" #include "JavaScriptCore/Error.h" #include "JavaScriptCore/ErrorInstance.h" @@ -597,16 +598,19 @@ WTF::String computeErrorInfoWrapperToString(JSC::VM& vm, Vector& sta OrdinalNumber line = OrdinalNumber::fromOneBasedInt(line_in); OrdinalNumber column = OrdinalNumber::fromOneBasedInt(column_in); - auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm); // finalizeUnconditionally can run this from Heap::runEndPhase at an // arbitrary GC safepoint while the suspended mutator has an unrelated // pending exception (e.g. a module evaluation error captured mid - // CyclicModuleRecord::evaluate). Only swallow an exception this - // computation itself raised; a pre-existing one belongs to the mutator - // and clearing it crashes the later reject-with-caught-exception site. - JSC::Exception* priorException = scope.exception(); + // CyclicModuleRecord::evaluate). Suspend it so tryClearException below + // only ever swallows an exception this computation itself raised; a + // pre-existing one belongs to the mutator and clearing it crashes the + // later reject-with-caught-exception site. Same pattern as + // TypeProfilerLog::processLogEntries. + JSC::SuspendExceptionScope suspendExceptionScope(vm); + + auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm); WTF::String result = computeErrorInfoToString(vm, stackTrace, line, column, sourceURL); - if (scope.exception() && scope.exception() != priorException) { + if (scope.exception()) { // vm.setOnComputeErrorInfo doesn't handle a callback that throws. // test/js/node/test/parallel/test-stream-writable-write-writev-finish.js trips the exception checker. (void)scope.tryClearException(); From 91b46058ad68433694bb9eecd7553abac119842e Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 30 Aug 2026 04:25:15 +0000 Subject: [PATCH 03/12] test: cover the plain import() face of the GC stack-trace finalizer race A fresh on-disk ESM graph per iteration, one member throwing at top level and one using top-level await. import(a) fails on the throwing member; import(b) reaches that already-errored member, so Evaluate() rethrows the stored error and runs the step 9 -> 9.d window again. This is the path in the release crash reports (JSPromise::reject <- rejectWithCaughtException <- CyclicModuleRecord::evaluate <- dynamicImportLoadSettled, SEGV at 0x8). --- ...dynamic-import-evaluation-error-gc.test.ts | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 test/js/bun/resolve/dynamic-import-evaluation-error-gc.test.ts diff --git a/test/js/bun/resolve/dynamic-import-evaluation-error-gc.test.ts b/test/js/bun/resolve/dynamic-import-evaluation-error-gc.test.ts new file mode 100644 index 000000000000..c4df0454fbbf --- /dev/null +++ b/test/js/bun/resolve/dynamic-import-evaluation-error-gc.test.ts @@ -0,0 +1,68 @@ +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, isWindows, tempDir } from "harness"; + +// computeErrorInfoWrapperToString (src/jsc/bindings/FormatStackTraceForJS.cpp, +// installed via vm.setOnComputeErrorInfo) runs from +// ErrorInstance::reconcileWeakReferencesAtGCEnd during Heap::runEndPhase, for +// any live Error whose stack frames now reference dead code. It used to clear +// whatever exception was pending after computing the stack string. With +// concurrent GC that end phase can land while the mutator is parked at a +// safepoint inside CyclicModuleRecord::evaluate, between step 9 (the module's +// evaluation error is the pending exception) and step 9.d +// (rejectWithCaughtException), which then finds no exception and crashes +// (`ASSERTION FAILED: exception` at JSPromise.cpp on assert builds, a SEGV at +// address 0x8 on release: Sentry BUN-4R6E). +// +// This is the plain import() face of the bug. Each iteration writes a fresh +// ESM graph with one member that throws at top level and one that uses +// top-level await. import(a) fails on the throwing member; import(b) reaches +// that already-errored member, so its Evaluate() rethrows the stored error and +// runs the step 9 -> 9.d window again. The race needs the accumulated graphs +// and GC cycles of a long loop; a single iteration does not fire, 100 fire on +// about 1 run in 6, 300 on essentially every run of the unfixed assert build. +// The fixture runs twice and a single crash fails the test. +test.skipIf(isWindows)( + "import() of a module whose evaluation throws survives a GC stack-trace finalizer", + async () => { + const fixture = ` + import { mkdirSync, writeFileSync } from "node:fs"; + import { join } from "node:path"; + + const root = join(import.meta.dir, "graphs"); + for (let it = 0; it < 300; it++) { + const d = join(root, "g" + it); + mkdirSync(d, { recursive: true }); + writeFileSync(join(d, "bad.mjs"), "export const x = " + it + ";\\nthrow new Error('boom " + it + "');\\n"); + writeFileSync(join(d, "tla.mjs"), "await new Promise(r => setTimeout(r, 0));\\nexport const t = " + it + ";\\n"); + writeFileSync(join(d, "leaf.mjs"), "export const l = " + it + ";\\n"); + writeFileSync(join(d, "mid.mjs"), "import { l } from './leaf.mjs';\\nimport './bad.mjs';\\nexport const m = l + 1;\\n"); + writeFileSync(join(d, "a.mjs"), "import { m } from './mid.mjs';\\nimport { t } from './tla.mjs';\\nexport const a = m + t;\\n"); + writeFileSync(join(d, "b.mjs"), "import { t } from './tla.mjs';\\nimport './bad.mjs';\\nexport const b = t;\\n"); + writeFileSync(join(d, "c.mjs"), "import { m } from './mid.mjs';\\nexport const c = m;\\n"); + + await import(join(d, "a.mjs")).catch(() => {}); + await import(join(d, "b.mjs")).catch(() => {}); + await import(join(d, "c.mjs")).catch(() => {}); + await import("data:text/javascript,throw new Error('d" + it + "')").catch(() => {}); + } + console.log("ok"); + `; + + using dir = tempDir("dynamic-import-eval-error-gc", { "fixture.mjs": fixture }); + + for (let i = 0; i < 2; i++) { + await using proc = Bun.spawn({ + cmd: [bunExe(), "fixture.mjs"], + cwd: String(dir), + env: bunEnv, + stdio: ["ignore", "pipe", "pipe"], + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout: stdout.trim(), exitCode }).toEqual({ stdout: "ok", exitCode: 0 }); + void stderr; + } + }, + // Two runs of a 300-iteration loop on a debug+ASAN build take well over the + // default 5s. + 120_000, +); From a7637d7720ae2b1514f8493968ca0ac4ca335227 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 30 Aug 2026 04:28:49 +0000 Subject: [PATCH 04/12] trim the finalizer comments to the invariant --- src/jsc/bindings/FormatStackTraceForJS.cpp | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/jsc/bindings/FormatStackTraceForJS.cpp b/src/jsc/bindings/FormatStackTraceForJS.cpp index 2c80315541cf..88571129e9ac 100644 --- a/src/jsc/bindings/FormatStackTraceForJS.cpp +++ b/src/jsc/bindings/FormatStackTraceForJS.cpp @@ -598,21 +598,15 @@ WTF::String computeErrorInfoWrapperToString(JSC::VM& vm, Vector& sta OrdinalNumber line = OrdinalNumber::fromOneBasedInt(line_in); OrdinalNumber column = OrdinalNumber::fromOneBasedInt(column_in); - // finalizeUnconditionally can run this from Heap::runEndPhase at an - // arbitrary GC safepoint while the suspended mutator has an unrelated - // pending exception (e.g. a module evaluation error captured mid - // CyclicModuleRecord::evaluate). Suspend it so tryClearException below - // only ever swallows an exception this computation itself raised; a - // pre-existing one belongs to the mutator and clearing it crashes the - // later reject-with-caught-exception site. Same pattern as - // TypeProfilerLog::processLogEntries. + // The GC end phase calls this while the mutator may be parked with its own + // pending exception (CyclicModuleRecord::evaluate between steps 9 and 9.d). + // Keep that exception out of reach of the clear below. JSC::SuspendExceptionScope suspendExceptionScope(vm); auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm); WTF::String result = computeErrorInfoToString(vm, stackTrace, line, column, sourceURL); if (scope.exception()) { - // vm.setOnComputeErrorInfo doesn't handle a callback that throws. - // test/js/node/test/parallel/test-stream-writable-write-writev-finish.js trips the exception checker. + // The onComputeErrorInfo hook cannot propagate a throw. (void)scope.tryClearException(); result = WTF::emptyString(); } From dad596375a3531fb632282deb16002e31613e16b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 30 Aug 2026 04:30:40 +0000 Subject: [PATCH 05/12] one-line comment on the exception suspend --- src/jsc/bindings/FormatStackTraceForJS.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/jsc/bindings/FormatStackTraceForJS.cpp b/src/jsc/bindings/FormatStackTraceForJS.cpp index 88571129e9ac..ab00b5b225c8 100644 --- a/src/jsc/bindings/FormatStackTraceForJS.cpp +++ b/src/jsc/bindings/FormatStackTraceForJS.cpp @@ -598,9 +598,7 @@ WTF::String computeErrorInfoWrapperToString(JSC::VM& vm, Vector& sta OrdinalNumber line = OrdinalNumber::fromOneBasedInt(line_in); OrdinalNumber column = OrdinalNumber::fromOneBasedInt(column_in); - // The GC end phase calls this while the mutator may be parked with its own - // pending exception (CyclicModuleRecord::evaluate between steps 9 and 9.d). - // Keep that exception out of reach of the clear below. + // Runs from the GC end phase while the mutator may have its own pending exception. JSC::SuspendExceptionScope suspendExceptionScope(vm); auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm); From 4edbfd4ee4d1d64ea61cd0cd5d605e2c60e7860a Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 30 Aug 2026 04:50:34 +0000 Subject: [PATCH 06/12] test: name the Windows skips and stop masking unhandled rejections in the vm fixture The vm fixture's unhandledRejection handler counted rejections that nothing asserted. It was also hiding a separate, pre-existing bug: a throwing top-level-await SourceTextModule shared by two importers reports one unhandled rejection per graph even though every evaluate() is awaited and caught. Tolerate exactly that rejection and exit nonzero on any other, so the crash under test stays the only failure mode. Both Windows skips now say why: the fixtures are long GC-churn loops that run several times slower under Windows + ASAN, and the fixed path is not platform-specific. --- .../dynamic-import-evaluation-error-gc.test.ts | 4 ++++ .../js/node/vm/sourcetextmodule-link-gc.test.ts | 17 ++++++++++++++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/test/js/bun/resolve/dynamic-import-evaluation-error-gc.test.ts b/test/js/bun/resolve/dynamic-import-evaluation-error-gc.test.ts index c4df0454fbbf..2249180322ca 100644 --- a/test/js/bun/resolve/dynamic-import-evaluation-error-gc.test.ts +++ b/test/js/bun/resolve/dynamic-import-evaluation-error-gc.test.ts @@ -21,6 +21,10 @@ import { bunEnv, bunExe, isWindows, tempDir } from "harness"; // and GC cycles of a long loop; a single iteration does not fire, 100 fire on // about 1 run in 6, 300 on essentially every run of the unfixed assert build. // The fixture runs twice and a single crash fails the test. +// +// Skipped on Windows: two runs of a 300-iteration loop that writes 2100 files +// and evaluates 1200 module graphs are several times slower under +// Windows + ASAN in CI, and the fixed C++ path is not platform-specific. test.skipIf(isWindows)( "import() of a module whose evaluation throws survives a GC stack-trace finalizer", async () => { diff --git a/test/js/node/vm/sourcetextmodule-link-gc.test.ts b/test/js/node/vm/sourcetextmodule-link-gc.test.ts index d20b59ee2275..b1bea7066eea 100644 --- a/test/js/node/vm/sourcetextmodule-link-gc.test.ts +++ b/test/js/node/vm/sourcetextmodule-link-gc.test.ts @@ -89,13 +89,24 @@ test.skipIf(isWindows)( // the stack-trace finalizer takes a different path and doesn't fire). The // processes run one at a time (concurrency starves the collector and hides the // race); a single crash fails the test. +// +// Skipped on Windows for the same reason as the test above: four runs of a +// 60-iteration GC-churn loop are several times slower under Windows + ASAN in +// CI, and the fixed C++ path is not platform-specific. test.skipIf(isWindows)( "vm.SourceTextModule evaluation error survives a concurrent-GC stack-trace finalizer", async () => { const fixture = ` import * as vm from "node:vm"; - let UREJ = 0; - process.on("unhandledRejection", () => { UREJ++; }); + // Separate, pre-existing bug: the throwing top-level-await module below + // (shared by two importers) reports one unhandled rejection per graph even + // though every evaluate() is awaited and caught. Tolerate exactly that one + // so the crash under test stays the only way this process exits nonzero. + process.on("unhandledRejection", e => { + if (e instanceof URIError && e.message === "tla") return; + console.error("unexpected unhandled rejection:", e); + process.exit(3); + }); const sl = async () => { await new Promise(r => setTimeout(r, 12)); for (let i = 0; i < 8; i++) await null; }; const CA = async f => { try { return await f(); } catch (e) { return e; } }; const ni = () => { throw new Error("noimports"); }; @@ -120,7 +131,7 @@ test.skipIf(isWindows)( { const val = ["str", "num", "null", "undef", "obj", "sym"][sd % 6]; const src = { str: "throw 'x'", num: "throw 42", null: "throw null", undef: "throw undefined", obj: "throw {a:1}", sym: "throw Symbol.iterator" }[val]; const m = M(src, "s6" + sd); await m.link(ni); await CA(() => m.evaluate()); } - await CA(async () => { await sl(); return UREJ; }); + await sl(); } for (let sd = 0; sd < 60; sd++) await fam(sd); console.log("ok"); From d970c98c5fab8b55d8cb28812de119e993a6a43b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 30 Aug 2026 05:24:51 +0000 Subject: [PATCH 07/12] Defer termination while the stack-trace finalizer suspends the exception SuspendExceptionScope restores the saved exception slot blindly on exit. If a termination exception is thrown inside the window (a RETURN_IF_EXCEPTION in the stack computation servicing a pending terminate()), tryClearException refuses to clear it, and the restore then overwrites it with the saved value (null when nothing was pending) while the NeedExceptionHandling trap bit stays set. The next exception check asserts the exception/trap invariant: ASSERTION FAILED: !!(*scope).exception() == vm.traps().needHandling(NeedExceptionHandling) TopExceptionScope__exceptionIncludingTraps Hold termination off for the duration of the window with DeferTerminationForAWhile, which re-arms the NeedTermination trap on exit so the mutator throws it at its next check. A termination already pending on entry is suspended and rethrown by the same scope. Regression test: run the GC-churn fixture in a Worker and terminate it mid-run. Every run of the suspend-only build asserted. --- src/jsc/bindings/FormatStackTraceForJS.cpp | 6 +- .../node/vm/sourcetextmodule-link-gc.test.ts | 63 +++++++++++++++++-- 2 files changed, 62 insertions(+), 7 deletions(-) diff --git a/src/jsc/bindings/FormatStackTraceForJS.cpp b/src/jsc/bindings/FormatStackTraceForJS.cpp index ab00b5b225c8..47b6bd01eaf7 100644 --- a/src/jsc/bindings/FormatStackTraceForJS.cpp +++ b/src/jsc/bindings/FormatStackTraceForJS.cpp @@ -6,6 +6,7 @@ #include "JavaScriptCore/ArgList.h" #include "JavaScriptCore/CallData.h" +#include "JavaScriptCore/DeferTermination.h" #include "JavaScriptCore/FrameTracers.h" #include "JavaScriptCore/TopExceptionScope.h" #include "JavaScriptCore/Error.h" @@ -598,7 +599,10 @@ WTF::String computeErrorInfoWrapperToString(JSC::VM& vm, Vector& sta OrdinalNumber line = OrdinalNumber::fromOneBasedInt(line_in); OrdinalNumber column = OrdinalNumber::fromOneBasedInt(column_in); - // Runs from the GC end phase while the mutator may have its own pending exception. + // Runs from the GC end phase while the mutator may have its own pending + // exception. A termination raised inside would survive the clear below and + // be clobbered by the restore, so hold it off until the mutator's next check. + JSC::DeferTerminationForAWhile deferTermination(vm); JSC::SuspendExceptionScope suspendExceptionScope(vm); auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm); diff --git a/test/js/node/vm/sourcetextmodule-link-gc.test.ts b/test/js/node/vm/sourcetextmodule-link-gc.test.ts index b1bea7066eea..72f9268794ca 100644 --- a/test/js/node/vm/sourcetextmodule-link-gc.test.ts +++ b/test/js/node/vm/sourcetextmodule-link-gc.test.ts @@ -93,10 +93,11 @@ test.skipIf(isWindows)( // Skipped on Windows for the same reason as the test above: four runs of a // 60-iteration GC-churn loop are several times slower under Windows + ASAN in // CI, and the fixed C++ path is not platform-specific. -test.skipIf(isWindows)( - "vm.SourceTextModule evaluation error survives a concurrent-GC stack-trace finalizer", - async () => { - const fixture = ` +// +// The fixture is shared with the terminate() test below, which only needs it to +// keep the finalizer busy for as long as the worker lives. +function evaluationErrorFixture(iterations: number) { + return ` import * as vm from "node:vm"; // Separate, pre-existing bug: the throwing top-level-await module below // (shared by two importers) reports one unhandled rejection per graph even @@ -133,11 +134,15 @@ test.skipIf(isWindows)( const m = M(src, "s6" + sd); await m.link(ni); await CA(() => m.evaluate()); } await sl(); } - for (let sd = 0; sd < 60; sd++) await fam(sd); + for (let sd = 0; sd < ${iterations}; sd++) await fam(sd); console.log("ok"); `; +} - using dir = tempDir("vm-module-eval-error-gc", { "fixture.mjs": fixture }); +test.skipIf(isWindows)( + "vm.SourceTextModule evaluation error survives a concurrent-GC stack-trace finalizer", + async () => { + using dir = tempDir("vm-module-eval-error-gc", { "fixture.mjs": evaluationErrorFixture(60) }); async function run() { await using proc = Bun.spawn({ @@ -159,3 +164,49 @@ test.skipIf(isWindows)( }, 120_000, ); + +// The finalizer now sets the mutator's pending exception aside with a +// SuspendExceptionScope. That scope restores the slot blindly on exit, so a +// termination exception thrown inside the window (a RETURN_IF_EXCEPTION inside +// the stack computation services the worker's pending terminate() request) +// survived the clear and was then overwritten, leaving the NeedExceptionHandling +// trap bit set with no exception behind it. The next exception check asserted +// `!!exception == needHandling(NeedExceptionHandling)`. Termination is now +// deferred for the duration of the window (DeferTerminationForAWhile). +// +// Run the GC-churn fixture inside a Worker and terminate it mid-run, 20 times. +// Every run of the build with the suspend scope but without the deferral +// asserted; this guards that pairing rather than the original crash (it passes +// on a build with neither). Windows: same reason as above. +test.skipIf(isWindows)( + "terminate() while the stack-trace finalizer runs keeps the exception and trap state in sync", + async () => { + using dir = tempDir("vm-module-eval-error-gc-terminate", { + "fixture.mjs": evaluationErrorFixture(600), + "stress.mjs": ` + import { readFileSync } from "node:fs"; + const src = readFileSync(new URL("./fixture.mjs", import.meta.url), "utf8"); + const url = URL.createObjectURL(new Blob([src], { type: "text/javascript" })); + for (let r = 0; r < 20; r++) { + const w = new Worker(url, { type: "module" }); + await new Promise(res => setTimeout(res, 150 + ((r * 37) % 400))); + w.terminate(); + await new Promise(res => w.addEventListener("close", res, { once: true })); + } + console.log("ok"); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "--smol", "--experimental-vm-modules", "stress.mjs"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + stdout: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout: stdout.trim(), exitCode }).toEqual({ stdout: "ok", exitCode: 0 }); + void stderr; + }, + 120_000, +); From 311565fb70e11f049f183e00c4a9945132950598 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 30 Aug 2026 05:26:43 +0000 Subject: [PATCH 08/12] one-line comments per scope --- src/jsc/bindings/FormatStackTraceForJS.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/jsc/bindings/FormatStackTraceForJS.cpp b/src/jsc/bindings/FormatStackTraceForJS.cpp index 47b6bd01eaf7..5fc9bab2686d 100644 --- a/src/jsc/bindings/FormatStackTraceForJS.cpp +++ b/src/jsc/bindings/FormatStackTraceForJS.cpp @@ -599,10 +599,9 @@ WTF::String computeErrorInfoWrapperToString(JSC::VM& vm, Vector& sta OrdinalNumber line = OrdinalNumber::fromOneBasedInt(line_in); OrdinalNumber column = OrdinalNumber::fromOneBasedInt(column_in); - // Runs from the GC end phase while the mutator may have its own pending - // exception. A termination raised inside would survive the clear below and - // be clobbered by the restore, so hold it off until the mutator's next check. + // A termination thrown in here would survive the clear below and be lost to the restore. JSC::DeferTerminationForAWhile deferTermination(vm); + // Runs from the GC end phase while the mutator may have its own pending exception. JSC::SuspendExceptionScope suspendExceptionScope(vm); auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm); From d661d2b1d51238584d6ca46fe93acb000e325814 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 30 Aug 2026 05:46:05 +0000 Subject: [PATCH 09/12] test: give the terminated worker its own small heap and listen for close before yielding --smol on the parent does not reach a Worker's VM; pass smol: true so the worker's heap is the one that collects often. Register the close listener before the timer so a worker that dies early still dispatches close instead of hanging the loop until the test timeout. --- test/js/node/vm/sourcetextmodule-link-gc.test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/js/node/vm/sourcetextmodule-link-gc.test.ts b/test/js/node/vm/sourcetextmodule-link-gc.test.ts index 72f9268794ca..f10bcbfaf512 100644 --- a/test/js/node/vm/sourcetextmodule-link-gc.test.ts +++ b/test/js/node/vm/sourcetextmodule-link-gc.test.ts @@ -188,10 +188,14 @@ test.skipIf(isWindows)( const src = readFileSync(new URL("./fixture.mjs", import.meta.url), "utf8"); const url = URL.createObjectURL(new Blob([src], { type: "text/javascript" })); for (let r = 0; r < 20; r++) { - const w = new Worker(url, { type: "module" }); + // smol: the worker VM gets its own heap; --smol on the parent does not reach it. + const w = new Worker(url, { type: "module", smol: true }); + // Listen before yielding: close is only dispatched if a listener exists + // when the worker goes away, so a worker that dies early must not hang us. + const closed = new Promise(res => w.addEventListener("close", res, { once: true })); await new Promise(res => setTimeout(res, 150 + ((r * 37) % 400))); w.terminate(); - await new Promise(res => w.addEventListener("close", res, { once: true })); + await closed; } console.log("ok"); `, From 105ca04fa0f35347e4b09972c88c3f7cacb631fd Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 13 Sep 2026 15:55:41 +0000 Subject: [PATCH 10/12] test: cover the Bun.resolve() face of the GC stack-trace finalizer race Bun.resolve() allocates its rejected promise while the exception the resolve threw is still pending (JSPromise::rejected_promise_with_caught_exception). That allocation is a GC safepoint. When the concurrent collector's end phase ran there and materialized a live Error's stack, the hook cleared the pending exception and the rejection then found nothing: panic: A JavaScript exception was thrown, but it was cleared before it could be read. take_exception <- JSPromise::reject <- rejected_promise_with_caught_exception <- bun_object::resolve The unfixed debug build panics after 2500 to 8500 iterations under collectContinuously; with the finalizer fix 80000 iterations complete. --- test/js/bun/resolve/resolve-error.test.ts | 53 ++++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/test/js/bun/resolve/resolve-error.test.ts b/test/js/bun/resolve/resolve-error.test.ts index dd8be583fe5a..978f4947d126 100644 --- a/test/js/bun/resolve/resolve-error.test.ts +++ b/test/js/bun/resolve/resolve-error.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "bun:test"; -import { bunEnv, bunExe, tempDir } from "harness"; +import { bunEnv, bunExe, isWindows, tempDir } from "harness"; import path from "node:path"; describe("ResolveMessage", () => { @@ -327,3 +327,54 @@ describe.concurrent("Bun.resolve() rejections are tracked", () => { expect(exitCode).toBe(0); }); }); + +// Bun.resolve() allocates its rejected promise while the exception the resolve +// threw is still pending on the VM. That allocation is a GC safepoint, and the +// concurrent collector's end phase materializes the stack of every live Error +// whose frames died, through the onComputeErrorInfo hook. The hook used to +// clear whatever exception was pending, so the rejection then found nothing: +// `panic: A JavaScript exception was thrown, but it was cleared before it could be read.` +// +// Each call comes from a fresh closure and the reasons are kept in a ring, so +// every collection has live Errors with a dead top frame to materialize. The +// calls have to be awaited one at a time (64 per tick never fires), and +// collectContinuously keeps an end phase always imminent. The unfixed debug +// build panics after 2500 to 8500 iterations. Not concurrent with the tests +// above: a busy machine starves the collector and hides the race. +// +// Skipped on Windows: collectContinuously is several times slower under +// Windows + ASAN in CI, and the fixed C++ path is not platform-specific. +it.skipIf(isWindows)( + "Bun.resolve() rejection survives a GC stack-trace finalizer", + async () => { + using dir = tempDir("bun-resolve-rejection-gc", { + "fixture.mjs": ` + const keep = []; + let rejections = 0; + for (let i = 0; i < 20000; i++) { + const call = () => Bun.resolve(); + await call().catch(e => { + rejections++; + keep.push(e); + if (keep.length > 1024) keep.shift(); + }); + } + console.log("ok rejections=" + rejections); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "fixture.mjs"], + env: { ...bunEnv, BUN_JSC_collectContinuously: "1" }, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout: stdout.trim(), exitCode }).toEqual({ stdout: "ok rejections=20000", exitCode: 0 }); + void stderr; + }, + // 20000 awaited rejections under collectContinuously on a debug+ASAN build + // take well over the default 5s. + 120_000, +); From fadc1b2c3934f28e0fc14baa417ede3086e00407 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 13 Sep 2026 16:28:44 +0000 Subject: [PATCH 11/12] ci: retrigger From afb2e9ce6c5e3e6683659524b34cb648bb4b086d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 14 Sep 2026 13:34:43 +0000 Subject: [PATCH 12/12] test: add a deterministic check for the GC stack-trace finalizer race slowPathAllocsBetweenGCs collects every N slow-path allocations, so the end phase lands in the same place every run. With the Error built inside an eval'd arrow its frames are dead by the time the uncaught throw propagates, which gives the end phase a stack to materialize in the window where the entry module's evaluation promise is about to be rejected with the caught exception. Unlike the three concurrent-GC reproducers this needs no iteration count and finishes in about 6s. On the unfixed build all three N values abort every run; with the fix all print `error: c` and exit 1. --- .../error-stack-finalizer-exception.test.ts | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 test/js/bun/gc/error-stack-finalizer-exception.test.ts diff --git a/test/js/bun/gc/error-stack-finalizer-exception.test.ts b/test/js/bun/gc/error-stack-finalizer-exception.test.ts new file mode 100644 index 000000000000..45318d5cdf43 --- /dev/null +++ b/test/js/bun/gc/error-stack-finalizer-exception.test.ts @@ -0,0 +1,45 @@ +import { expect, test } from "bun:test"; +import { bunEnv, bunExe } from "harness"; + +// computeErrorInfoWrapperToString (src/jsc/bindings/FormatStackTraceForJS.cpp, +// the vm.setOnComputeErrorInfo hook) runs from +// ErrorInstance::reconcileWeakReferencesAtGCEnd during Heap::runEndPhase, to +// materialize the stack of a live Error whose frames have died. It used to +// clear whatever exception was pending on the VM. The entry module's +// evaluation promise is rejected with the caught exception after an allocation +// safepoint, so a collection ending in that window dropped the error: +// +// assert build: ASSERTION FAILED: exception JSPromise.cpp, rejectWithCaughtException +// release build: panic(main thread): Segmentation fault at address 0x8 +// +// Unlike the concurrent-GC reproducers (sourcetextmodule-link-gc.test.ts, +// dynamic-import-evaluation-error-gc.test.ts, the Bun.resolve() case in +// resolve-error.test.ts) this one is deterministic: slowPathAllocsBetweenGCs +// collects every N slow-path allocations, so the end phase lands in the same +// place every run. The Error is built inside an eval'd arrow so its frames are +// garbage by the time the throw propagates, which is what gives the end phase a +// stack to materialize. +// +// It is allocation-count sensitive, as that option is: on the unfixed debug +// build N of 3, 4 and 7 abort every run while 1, 2, 5, 6 and >= 8 print the +// error normally. Check the values that fire rather than one, since the counts +// shift with the build. A value that does not fire still has to print the +// error, so this cannot flake on a fixed build. +const ALLOCS_BETWEEN_GCS = [3, 4, 7]; + +test.each(ALLOCS_BETWEEN_GCS)("an uncaught Error survives a GC end phase (every %i allocations)", async n => { + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", `const e = eval("(() => Object.assign(new Error('c'), { code: 1 }))")(); throw e`], + env: { ...bunEnv, BUN_JSC_slowPathAllocsBetweenGCs: String(n) }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + // Bun prints `error: c` plus the `code: 1` property and a frame. The crash + // produces an assertion or a segfault instead, and a nonzero-but-not-1 code. + expect({ stdout, startsWithError: stderr.startsWith("error: c"), exitCode }).toEqual({ + stdout: "", + startsWithError: true, + exitCode: 1, + }); +});