From e61d594dd5c1a2fa867a257e7be5a6f7ddcbb31e Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 11:15:34 +0000 Subject: [PATCH 1/3] bun test: exit through the VM teardown path when --bail stops a run The bail triggered by a failing test called Global::exit(1) from inside handle_test_completed, skipping on_exit()/global_exit(). Under BUN_DESTRUCT_VM_ON_EXIT (the leak-check lanes) the JSC finalizers that free the test runner's Expect/ScopeFunctions/RefData boxes therefore never ran and LeakSanitizer aborted the process with 134 instead of 1. Both bail sites now go through one CommandLineReporter::bail_out, which prints the summary and reports as before and then exits the way the end of a run does: exit code 1, the gated on_exit(), deinit_for_exit(), RUNNER cleared, global_exit(). --- src/runtime/cli/test/parallel/runner.rs | 8 +- src/runtime/cli/test_command.rs | 83 ++++++------ test/cli/test/bun-test.test.ts | 165 +++++++++++++++++++++++- test/no-validate-leaksan.txt | 1 - 4 files changed, 214 insertions(+), 43 deletions(-) diff --git a/src/runtime/cli/test/parallel/runner.rs b/src/runtime/cli/test/parallel/runner.rs index e6b14c5b2100..33305d629167 100644 --- a/src/runtime/cli/test/parallel/runner.rs +++ b/src/runtime/cli/test/parallel/runner.rs @@ -306,10 +306,10 @@ fn build_worker_argv(ctx: &Command::ContextData) -> crate::Result 0 { argv.push(print_z(format_args!("--rerun-each={}", opts.repeat_count))?); } diff --git a/src/runtime/cli/test_command.rs b/src/runtime/cli/test_command.rs index 577d075d380d..6d5f6ef87a64 100644 --- a/src/runtime/cli/test_command.rs +++ b/src/runtime/cli/test_command.rs @@ -1517,16 +1517,7 @@ impl CommandLineReporter { this.summary().fail += 1; if this.summary().fail == this.jest.bail { - this.print_summary(); - pretty_error!( - "\nBailed out after {} failure{}\n", - this.jest.bail, - if this.jest.bail == 1 { "" } else { "s" } - ); - Output::flush(); - this.write_junit_report_if_needed(); - this.write_timings_if_needed(); - Global::exit(1); + this.bail_out(VirtualMachine::get().as_mut()); } } } @@ -1552,6 +1543,48 @@ impl CommandLineReporter { Output::print_start_end(bun::start_time(), bun::time::nano_timestamp()); } + /// `--bail` reached its failure count: report, then exit 1 through the same + /// teardown as the end of a normal run (`exec`). A bare `Global::exit` here + /// would skip the `BUN_DESTRUCT_VM_ON_EXIT` teardown, and the leak-check + /// lanes then report every wrapper box the JSC finalizers still own. + /// + /// Reached mid-file (`handle_test_completed`, with the file's `BunTest` + /// still held by `TestCommand::run`) and after a failed module evaluation; + /// `process.exit()` inside a test tears the VM down from the same state. + pub(crate) fn bail_out(&mut self, vm: &mut VirtualMachine) -> ! { + self.print_summary(); + pretty_error!( + "\nBailed out after {} failure{}\n", + self.jest.bail, + if self.jest.bail == 1 { "" } else { "s" } + ); + Output::flush(); + self.write_junit_report_if_needed(); + self.write_timings_if_needed(); + + vm.exit_handler.exit_code = 1; + vm.exit_handler.skip_exit_listeners = skip_exit_listeners(self); + let vm_ptr: *mut VirtualMachine = vm; + // SAFETY: `vm_ptr` reborrows the live `&mut VirtualMachine`; + // `run_with_api_lock` takes `&self` only, so the closure holds the + // unique mutable access on this single-threaded path. + vm.run_with_api_lock(|| unsafe { (*vm_ptr).on_exit() }); + // Same order as `exec`: exit listeners (user JS) ran above while the + // runner was intact; now release the `bun:test` state and unpublish + // `RUNNER` so nothing running inside the teardown GC can observe a + // half-torn-down `TestRunner`. The reporter itself stays allocated: + // both callers still borrow it. + self.jest.bun_test_root.deinit_for_exit(); + // SAFETY: `RUNNER` is a `RacyCell` touched only from the single JS + // thread; no concurrent reader exists on this shutdown path. + unsafe { + jest::Jest::RUNNER.write(None); + } + // SAFETY: as above; `global_exit()` diverges, so the closure is the + // sole mutator. + vm.run_with_api_lock(|| unsafe { (*vm_ptr).global_exit() }) + } + /// Like the JUnit report, called before every exit path (including bail) so measured durations aren't lost. pub(crate) fn write_timings_if_needed(&mut self) { if self.jest.test_options.update_timings @@ -3294,33 +3327,9 @@ impl TestCommand { reporter.summary().fail += 1; if reporter.jest.bail == reporter.summary().fail { - reporter.print_summary(); - pretty_error!( - "\nBailed out after {} failure{}\n", - reporter.jest.bail, - if reporter.jest.bail == 1 { "" } else { "s" } - ); - reporter.write_junit_report_if_needed(); - reporter.write_timings_if_needed(); - - vm.exit_handler.exit_code = 1; - vm.is_shutting_down = true; - // `global_exit()` diverges, so the `exit_file()` defer - // above never fires. Release the active file's - // `Strong`s and the preload-hook scope here so - // `Zig__GlobalObject__destructOnExit()`'s `collectNow()` can reclaim them, - // then clear `RUNNER` so finalizers can't observe a - // partially-torn-down `TestRunner`. - // SAFETY: single-threaded; raw-ptr reborrow mirrors the - // defer's escape. - unsafe { - (*bun_test_root_ptr).deinit_for_exit(); - jest::Jest::RUNNER.write(None); - } - let vm_ptr = std::ptr::from_mut::(vm); - // SAFETY: global_exit diverges; `vm_ptr` is a fresh - // raw-ptr reborrow of the exclusive `vm` borrow. - unsafe { (*vm_ptr).run_with_api_lock(|| (&mut *vm_ptr).global_exit()) }; + // Diverges, so the `exit_file()` defer above never + // fires; `bail_out` releases the active file itself. + reporter.bail_out(vm); } return Ok(()); diff --git a/test/cli/test/bun-test.test.ts b/test/cli/test/bun-test.test.ts index f582b871d8f5..8f259ea04503 100644 --- a/test/cli/test/bun-test.test.ts +++ b/test/cli/test/bun-test.test.ts @@ -1,6 +1,6 @@ import { spawnSync } from "bun"; import { beforeAll, describe, expect, it, test } from "bun:test"; -import { bunEnv, bunExe, tempDir, tempDirWithFiles, tmpdirSync } from "harness"; +import { bunEnv, bunExe, isASAN, tempDir, tempDirWithFiles, tmpdirSync } from "harness"; import { mkdirSync, rmSync, writeFileSync } from "node:fs"; import { dirname, join, resolve } from "node:path"; @@ -329,6 +329,7 @@ describe("bun test", () => { expect(true).toBe(true); }); `, + expectExitCode: 1, }); expect(stderr).toContain("Bailed out after 1 failure"); expect(stderr).not.toContain("test #2"); @@ -352,10 +353,133 @@ describe("bun test", () => { expect(true).toBe(true); }); `, + expectExitCode: 1, }); expect(stderr).toContain("Bailed out after 3 failures"); expect(stderr).not.toContain("test #4"); }); + + // The sanitizer lane (scripts/runner.node.mjs) runs every child with this environment. Bailing + // out has to exit through the same VM teardown as the end of a normal run; a bare exit leaves + // the objects the bun:test finalizers own allocated, and LeakSanitizer then aborts the process + // (exit 134) instead of it exiting 1. + describe.concurrent.skipIf(!isASAN)("exits cleanly under LeakSanitizer", () => { + const env = { + ...bunEnv, + BUN_DESTRUCT_VM_ON_EXIT: "1", + ASAN_OPTIONS: "allow_user_segv_handler=1:disable_coredump=0:detect_leaks=1:abort_on_error=1", + LSAN_OPTIONS: `malloc_context_size=30:print_suppressions=0:suppressions=${join(import.meta.dir, "../../leaksan.supp")}`, + }; + + /** Runs `bun test --bail` and returns what the test files logged to stdout. */ + async function runUntilBail(files: Record, ...args: string[]): Promise { + using dir = tempDir("bun-test-bail-leak-check", files); + await using proc = Bun.spawn({ + cmd: [bunExe(), "test", "--bail", ...args], + env, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toContain("Bailed out after 1 failure"); + expect(stderr).not.toContain("LeakSanitizer"); + expect({ exitCode, signalCode: proc.signalCode }).toEqual({ exitCode: 1, signalCode: null }); + // Everything else on stdout is the "bun test vX.Y.Z" banner. + return stdout.split("\n").filter(line => line.startsWith("ran: ")); + } + + test("after a test fails", async () => { + const ran = await runUntilBail({ + "a.test.ts": ` + import { test, expect } from "bun:test"; + test("fails", () => { + console.log("ran: fails"); + expect(1).toBe(2); + }); + test("unreachable", () => { + console.log("ran: unreachable"); + }); + `, + }); + expect(ran).toEqual(["ran: fails"]); + }); + + // The failure arrives from a promise reaction here, so the exit starts inside a JS frame. + test("after an async test fails while a timer is still pending", async () => { + const ran = await runUntilBail({ + "a.test.ts": ` + import { test, expect } from "bun:test"; + test("fails", async () => { + setTimeout(() => console.log("ran: timer"), 60_000); + await Bun.sleep(1); + console.log("ran: fails"); + expect(1).toBe(2); + }); + test("unreachable", () => { + console.log("ran: unreachable"); + }); + `, + }); + expect(ran).toEqual(["ran: fails"]); + }); + + test("after a later file fails, with preload hooks registered", async () => { + const ran = await runUntilBail( + { + "preload.ts": ` + import { afterEach, beforeAll, beforeEach } from "bun:test"; + beforeAll(() => console.log("ran: preload beforeAll")); + beforeEach(() => {}); + afterEach(() => {}); + `, + "a.test.ts": ` + import { describe, test, expect } from "bun:test"; + describe("a", () => { + test("passes", () => { + console.log("ran: a"); + expect(1).toBe(1); + }); + }); + `, + "b.test.ts": ` + import { describe, test, expect } from "bun:test"; + describe("b", () => { + test("fails", () => { + console.log("ran: b"); + expect(1).toBe(2); + }); + }); + `, + }, + "--preload=./preload.ts", + "./a.test.ts", + "./b.test.ts", + ); + expect(ran).toEqual(["ran: preload beforeAll", "ran: a", "ran: b"]); + }); + + test("after a file fails to load", async () => { + const ran = await runUntilBail( + { + "a.test.ts": ` + import { test, expect } from "bun:test"; + test("passes", () => { + console.log("ran: a"); + expect(1).toBe(1); + }); + `, + "b.test.ts": ` + console.log("ran: b (load)"); + throw new Error("b failed to load"); + `, + }, + "./a.test.ts", + "./b.test.ts", + ); + expect(ran).toEqual(["ran: a", "ran: b (load)"]); + }); + }); }); describe("--timeout", () => { test("must provide a number timeout", () => { @@ -1708,6 +1832,45 @@ describe("bun test", () => { expect(exitCode).toBe(1); }); + // --bail exits through the same on_exit() as the end of a run, so the same gate applies. + test("are not run when --bail stops a bun:test file", async () => { + const { stdout, stderr, exitCode } = await runFiles( + { + "bail.test.ts": ` + import { test, expect } from "bun:test"; + process.on("exit", () => { + console.log("exit listener ran"); + process.exit(7); + }); + test("fails", () => expect(1).toBe(2)); + `, + }, + "--bail", + "bail.test.ts", + ); + expect(stdout).not.toContain("exit listener ran"); + expect(stderr).toContain("Bailed out after 1 failure"); + expect(exitCode).toBe(1); + }); + + test("run when --bail stops a file that registered a node:test test", async () => { + const { stdout, stderr, exitCode } = await runFiles( + { + "node-bail.test.ts": ` + import { test } from "node:test"; + import assert from "node:assert"; + process.on("exit", code => console.log("exit listener ran with", code)); + test("fails", () => assert.strictEqual(1, 2)); + `, + }, + "--bail", + "node-bail.test.ts", + ); + expect(stdout).toContain("exit listener ran with 1"); + expect(stderr).toContain("Bailed out after 1 failure"); + expect(exitCode).toBe(1); + }); + test("run for a bun:test file under BUN_TEST_DRAIN_EVENT_LOOP, which the vendored node tests set", async () => { using dir = tempDir("bun-test-exit-listener", { "drain.test.ts": bunTestFile(1) }); await using proc = Bun.spawn({ diff --git a/test/no-validate-leaksan.txt b/test/no-validate-leaksan.txt index e4f53b8bfa48..d54babd66cf7 100644 --- a/test/no-validate-leaksan.txt +++ b/test/no-validate-leaksan.txt @@ -94,7 +94,6 @@ test/cli/test/bun-test.test.ts test/cli/install/bun-install-security-provider.test.ts test/js/node/test/parallel/test-tls-fast-writing.js test/js/bun/sqlite/sqlite.test.js -test/regression/issue/12250.test.ts test/js/bun/test/parallel/test-http-10177-response.write-with-non-ascii-latin1-should-not-cause-duplicated-character-or-segfault.ts test/cli/install/minimum-release-age.test.ts From d130806f50b47b20be19a1b5ee5ab4757ec6492b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 19:29:30 +0000 Subject: [PATCH 2/3] test: cover non-bail failing runs exiting cleanly under LeakSanitizer Serial and --parallel failing runs already exit through the VM teardown; pin that down next to the --bail cases, sharing one leak-check environment. --- test/cli/test/bun-test.test.ts | 62 +++++++++++++++++++++++++++------- 1 file changed, 50 insertions(+), 12 deletions(-) diff --git a/test/cli/test/bun-test.test.ts b/test/cli/test/bun-test.test.ts index 8f259ea04503..b9880b58a5e2 100644 --- a/test/cli/test/bun-test.test.ts +++ b/test/cli/test/bun-test.test.ts @@ -4,6 +4,18 @@ import { bunEnv, bunExe, isASAN, tempDir, tempDirWithFiles, tmpdirSync } from "h import { mkdirSync, rmSync, writeFileSync } from "node:fs"; import { dirname, join, resolve } from "node:path"; +// The environment the sanitizer lane (scripts/runner.node.mjs) runs test files under, which their +// bunEnv children inherit. This file is exempted in test/no-validate-leaksan.txt, so the children +// that need it get it explicitly. Under it, an exit that skips the VM teardown leaves the objects +// the bun:test finalizers own allocated, and LeakSanitizer aborts the process (exit 134) in place +// of the exit code the run meant to return. +const leakCheckEnv = { + ...bunEnv, + BUN_DESTRUCT_VM_ON_EXIT: "1", + ASAN_OPTIONS: "allow_user_segv_handler=1:disable_coredump=0:detect_leaks=1:abort_on_error=1", + LSAN_OPTIONS: `malloc_context_size=30:print_suppressions=0:suppressions=${join(import.meta.dir, "../../leaksan.supp")}`, +}; + describe("bun test", () => { test("running a non-existent absolute file path is a 1 exit code", () => { const spawn = Bun.spawnSync({ @@ -359,24 +371,15 @@ describe("bun test", () => { expect(stderr).not.toContain("test #4"); }); - // The sanitizer lane (scripts/runner.node.mjs) runs every child with this environment. Bailing - // out has to exit through the same VM teardown as the end of a normal run; a bare exit leaves - // the objects the bun:test finalizers own allocated, and LeakSanitizer then aborts the process - // (exit 134) instead of it exiting 1. + // Bailing out has to exit through the same VM teardown as the end of a normal run (see + // leakCheckEnv), not through a bare exit. describe.concurrent.skipIf(!isASAN)("exits cleanly under LeakSanitizer", () => { - const env = { - ...bunEnv, - BUN_DESTRUCT_VM_ON_EXIT: "1", - ASAN_OPTIONS: "allow_user_segv_handler=1:disable_coredump=0:detect_leaks=1:abort_on_error=1", - LSAN_OPTIONS: `malloc_context_size=30:print_suppressions=0:suppressions=${join(import.meta.dir, "../../leaksan.supp")}`, - }; - /** Runs `bun test --bail` and returns what the test files logged to stdout. */ async function runUntilBail(files: Record, ...args: string[]): Promise { using dir = tempDir("bun-test-bail-leak-check", files); await using proc = Bun.spawn({ cmd: [bunExe(), "test", "--bail", ...args], - env, + env: leakCheckEnv, cwd: String(dir), stdout: "pipe", stderr: "pipe", @@ -481,6 +484,41 @@ describe("bun test", () => { }); }); }); + + // Without --bail a failing run exits through the end of the run (and, under --parallel, through + // the worker's exit path). Printing the failure is what allocates the source map entry the bail + // exit used to leak, so these pin down that both paths tear the VM down too. + describe.concurrent.skipIf(!isASAN)("a failing run exits 1 cleanly under LeakSanitizer", () => { + test.each([ + ["serial", []], + ["--parallel", ["--parallel=2"]], + ])("%s", async (_, args) => { + using dir = tempDir("bun-test-fail-leak-check", { + "a.test.ts": ` + import { test, expect } from "bun:test"; + test("fails", () => expect(1).toBe(2)); + `, + "b.test.ts": ` + import { test, expect } from "bun:test"; + test("passes", () => expect(1).toBe(1)); + `, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "test", ...args], + env: leakCheckEnv, + cwd: String(dir), + stdout: "ignore", + stderr: "pipe", + }); + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + // A --parallel worker aborting does not change the coordinator's exit code; the workers' + // stderr is inherited, so the report check is what covers the worker exit path. + expect(stderr).not.toContain("LeakSanitizer"); + expect(stderr).toContain("Ran 2 tests across 2 files."); + expect({ exitCode, signalCode: proc.signalCode }).toEqual({ exitCode: 1, signalCode: null }); + }); + }); + describe("--timeout", () => { test("must provide a number timeout", () => { const stderr = runTest({ From 162a84298b71c6aa1be3fc6c07976ef2847db644 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 19:35:22 +0000 Subject: [PATCH 3/3] Trim teardown comments to the essentials --- src/runtime/cli/test/parallel/runner.rs | 7 +++---- src/runtime/cli/test_command.rs | 20 +++++++------------- 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/src/runtime/cli/test/parallel/runner.rs b/src/runtime/cli/test/parallel/runner.rs index 33305d629167..55bc54793b5a 100644 --- a/src/runtime/cli/test/parallel/runner.rs +++ b/src/runtime/cli/test/parallel/runner.rs @@ -306,10 +306,9 @@ fn build_worker_argv(ctx: &Command::ContextData) -> crate::Result 0 { argv.push(print_z(format_args!("--rerun-each={}", opts.repeat_count))?); } diff --git a/src/runtime/cli/test_command.rs b/src/runtime/cli/test_command.rs index 6d5f6ef87a64..32fadca9d17d 100644 --- a/src/runtime/cli/test_command.rs +++ b/src/runtime/cli/test_command.rs @@ -1543,14 +1543,10 @@ impl CommandLineReporter { Output::print_start_end(bun::start_time(), bun::time::nano_timestamp()); } - /// `--bail` reached its failure count: report, then exit 1 through the same - /// teardown as the end of a normal run (`exec`). A bare `Global::exit` here - /// would skip the `BUN_DESTRUCT_VM_ON_EXIT` teardown, and the leak-check - /// lanes then report every wrapper box the JSC finalizers still own. - /// - /// Reached mid-file (`handle_test_completed`, with the file's `BunTest` - /// still held by `TestCommand::run`) and after a failed module evaluation; - /// `process.exit()` inside a test tears the VM down from the same state. + /// `--bail` reached its failure count: report, then exit 1 through the + /// same VM teardown as the end of a normal run (`exec`). A bare + /// `Global::exit` would skip the `BUN_DESTRUCT_VM_ON_EXIT` teardown and + /// leave every JSC-finalizer-owned wrapper box for the leak checker. pub(crate) fn bail_out(&mut self, vm: &mut VirtualMachine) -> ! { self.print_summary(); pretty_error!( @@ -1569,11 +1565,9 @@ impl CommandLineReporter { // `run_with_api_lock` takes `&self` only, so the closure holds the // unique mutable access on this single-threaded path. vm.run_with_api_lock(|| unsafe { (*vm_ptr).on_exit() }); - // Same order as `exec`: exit listeners (user JS) ran above while the - // runner was intact; now release the `bun:test` state and unpublish - // `RUNNER` so nothing running inside the teardown GC can observe a - // half-torn-down `TestRunner`. The reporter itself stays allocated: - // both callers still borrow it. + // Exit listeners (user JS) ran above while the runner was intact; now + // release the `bun:test` GC roots so the teardown GC can't observe a + // half-torn-down `TestRunner`. self.jest.bun_test_root.deinit_for_exit(); // SAFETY: `RUNNER` is a `RacyCell` touched only from the single JS // thread; no concurrent reader exists on this shutdown path.