From cec75ae5b6fce149bd8db198e687af16a5780d3d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 19 Jul 2026 16:24:39 +0000 Subject: [PATCH 1/6] fix(error): don't capture stack trace on materialized ErrorInstance in appendStackTrace Error.appendStackTrace(source, destination) called captureStackTrace on destination when it had no stack frames, without checking whether error info was already materialized. After accessing .sourceURL/.stack (which materializes and clears the frames), this left the ErrorInstance with m_errorInfoMaterialized=true and a non-null m_stackTrace. When GC's finalizeUnconditionally later found an unmarked frame, computeErrorInfo hit ASSERT(!m_errorInfoMaterialized). Skip captureStackTrace when destination is already materialized (matching errorConstructorFuncCaptureStackTrace), guard the append against a null destination trace, and skip when source and destination are the same object. --- src/jsc/bindings/FormatStackTraceForJS.cpp | 4 ++-- test/js/bun/util/error-gc-test.test.js | 28 +++++++++++++++++++++- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/src/jsc/bindings/FormatStackTraceForJS.cpp b/src/jsc/bindings/FormatStackTraceForJS.cpp index d622679f31aa..de067816b8bc 100644 --- a/src/jsc/bindings/FormatStackTraceForJS.cpp +++ b/src/jsc/bindings/FormatStackTraceForJS.cpp @@ -668,11 +668,11 @@ JSC_DEFINE_HOST_FUNCTION(errorConstructorFuncAppendStackTrace, (JSC::JSGlobalObj return {}; } - if (!destination->stackTrace()) { + if (!destination->stackTrace() && !destination->hasMaterializedErrorInfo()) { destination->captureStackTrace(vm, globalObject, 1); } - if (source->stackTrace()) { + if (source->stackTrace() && destination->stackTrace() && source != destination) { destination->stackTrace()->appendVector(*source->stackTrace()); source->stackTrace()->clear(); } diff --git a/test/js/bun/util/error-gc-test.test.js b/test/js/bun/util/error-gc-test.test.js index 19bb1210b922..a3caa64be694 100644 --- a/test/js/bun/util/error-gc-test.test.js +++ b/test/js/bun/util/error-gc-test.test.js @@ -1,6 +1,6 @@ import { expect, test } from "bun:test"; import { readFileSync } from "fs"; -import { tmpdirSync } from "harness"; +import { bunEnv, bunExe, tmpdirSync } from "harness"; import { join } from "path"; // This test checks that printing stack traces increments and decrements // reference-counted strings @@ -45,6 +45,32 @@ test("error gc test #3", () => { } }); +test("Error.appendStackTrace after materialized error info doesn't crash in GC", async () => { + const src = ` + let keep = []; + for (let i = 0; i < 200; i++) { + eval(\`(function inner\${i}() { + const a = new Error(); + const b = new Error(); + a.sourceURL; + b.sourceURL; + Error.appendStackTrace(a, b); + keep.push(b); + })();\`); + } + Bun.gc(true); + Bun.gc(true); + process.stdout.write("ok"); + `; + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", src], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, exitCode }).toEqual({ stdout: "ok", exitCode: 0 }); +}); + // This test fails if: // - it crashes // - The test failure message gets a non-sensical error From 78d6e60616ac95c944b55b8b79e2d347947f9d4b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 19 Jul 2026 16:29:04 +0000 Subject: [PATCH 2/6] test: cover self-append branch and assert stderr --- test/js/bun/util/error-gc-test.test.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/js/bun/util/error-gc-test.test.js b/test/js/bun/util/error-gc-test.test.js index a3caa64be694..89e963e42a31 100644 --- a/test/js/bun/util/error-gc-test.test.js +++ b/test/js/bun/util/error-gc-test.test.js @@ -56,6 +56,9 @@ test("Error.appendStackTrace after materialized error info doesn't crash in GC", b.sourceURL; Error.appendStackTrace(a, b); keep.push(b); + const self = new Error(); + Error.appendStackTrace(self, self); + keep.push(self); })();\`); } Bun.gc(true); @@ -68,7 +71,7 @@ test("Error.appendStackTrace after materialized error info doesn't crash in GC", stderr: "pipe", }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - expect({ stdout, exitCode }).toEqual({ stdout: "ok", exitCode: 0 }); + expect({ stdout, stderr, exitCode }).toEqual({ stdout: "ok", stderr: "", exitCode: 0 }); }); // This test fails if: From 81f515b585ada8f8c093a2f08c7c6d8d352048e9 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 19 Jul 2026 16:43:34 +0000 Subject: [PATCH 3/6] test: move appendStackTrace regression to capture-stack-trace.test.js error-gc-test.test.js has unrelated tests that time out under the ASAN debug build; moving to a file that runs clean there. --- test/js/bun/util/error-gc-test.test.js | 31 +-------------------- test/js/node/v8/capture-stack-trace.test.js | 29 +++++++++++++++++++ 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/test/js/bun/util/error-gc-test.test.js b/test/js/bun/util/error-gc-test.test.js index 89e963e42a31..19bb1210b922 100644 --- a/test/js/bun/util/error-gc-test.test.js +++ b/test/js/bun/util/error-gc-test.test.js @@ -1,6 +1,6 @@ import { expect, test } from "bun:test"; import { readFileSync } from "fs"; -import { bunEnv, bunExe, tmpdirSync } from "harness"; +import { tmpdirSync } from "harness"; import { join } from "path"; // This test checks that printing stack traces increments and decrements // reference-counted strings @@ -45,35 +45,6 @@ test("error gc test #3", () => { } }); -test("Error.appendStackTrace after materialized error info doesn't crash in GC", async () => { - const src = ` - let keep = []; - for (let i = 0; i < 200; i++) { - eval(\`(function inner\${i}() { - const a = new Error(); - const b = new Error(); - a.sourceURL; - b.sourceURL; - Error.appendStackTrace(a, b); - keep.push(b); - const self = new Error(); - Error.appendStackTrace(self, self); - keep.push(self); - })();\`); - } - Bun.gc(true); - Bun.gc(true); - process.stdout.write("ok"); - `; - await using proc = Bun.spawn({ - cmd: [bunExe(), "-e", src], - env: bunEnv, - stderr: "pipe", - }); - const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - expect({ stdout, stderr, exitCode }).toEqual({ stdout: "ok", stderr: "", exitCode: 0 }); -}); - // This test fails if: // - it crashes // - The test failure message gets a non-sensical error diff --git a/test/js/node/v8/capture-stack-trace.test.js b/test/js/node/v8/capture-stack-trace.test.js index 6cd46a1ad90a..2474e92e4962 100644 --- a/test/js/node/v8/capture-stack-trace.test.js +++ b/test/js/node/v8/capture-stack-trace.test.js @@ -1121,3 +1121,32 @@ test("lazy error-info materialization does not store an empty stack value when t }); expect(exitCode).toBe(0); }); + +test("Error.appendStackTrace on an error with materialized info does not crash in GC", async () => { + const src = ` + let keep = []; + for (let i = 0; i < 200; i++) { + eval(\`(function inner\${i}() { + const a = new Error(); + const b = new Error(); + a.sourceURL; + b.sourceURL; + Error.appendStackTrace(a, b); + keep.push(b); + const self = new Error(); + Error.appendStackTrace(self, self); + keep.push(self); + })();\`); + } + Bun.gc(true); + Bun.gc(true); + process.stdout.write("ok"); + `; + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", src], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr, exitCode }).toEqual({ stdout: "ok", stderr: "", exitCode: 0 }); +}); From fb396bee0adf31f84fe3ee752306176fe4c5cffe Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 19 Jul 2026 16:50:32 +0000 Subject: [PATCH 4/6] ci: retrigger From 801a50530d695fb1e18b61505e26401c54408657 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 19 Jul 2026 16:57:35 +0000 Subject: [PATCH 5/6] test: cover unmaterialized-source/materialized-destination append shape Exercises the destination->stackTrace() null guard; removing it now fails the test with a UBSAN null-deref. --- test/js/node/v8/capture-stack-trace.test.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/js/node/v8/capture-stack-trace.test.js b/test/js/node/v8/capture-stack-trace.test.js index 2474e92e4962..2542336de6a1 100644 --- a/test/js/node/v8/capture-stack-trace.test.js +++ b/test/js/node/v8/capture-stack-trace.test.js @@ -1133,6 +1133,11 @@ test("Error.appendStackTrace on an error with materialized info does not crash i b.sourceURL; Error.appendStackTrace(a, b); keep.push(b); + const c = new Error(); + const d = new Error(); + d.sourceURL; + Error.appendStackTrace(c, d); + keep.push(c, d); const self = new Error(); Error.appendStackTrace(self, self); keep.push(self); From dca95ca186b7d137c6eb21d8a3dde5b77a3f602b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 19 Jul 2026 17:30:02 +0000 Subject: [PATCH 6/6] ci: re-run for unrelated flakes