Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions src/jsc/SavedSourceMap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,28 @@ impl SavedSourceMap {
unsafe { bun_core::heap::destroy(provider) };
}
}

/// Releases every cached entry, leaving the table empty but valid (unlike
/// `Drop`, which tears the table down for good). Used by the pre-exit
/// leak-check collection: the table stores its entries as tagged pointers
/// (`Value`), which LeakSanitizer does not recognize as pointers, so any
/// entry still cached at exit is reported as a leak even though it is
/// live VM state.
pub fn clear(&mut self) {
self.lock();
// Mirror `put_value`: the caches may point at blobs freed below.
self.find_cache.invalidate_all();
self.last_ism = None;
let map = self.map_mut();
for val in map.values() {
let value = Value::from(Some(*val));
// SAFETY: values were stored by us and are live until released
// here; `clear()` below removes them so they are not released again.
unsafe { Self::release_value(value) };
}
map.clear();
self.unlock();
}
}

/// Thin forwarder to the leaf-crate state in
Expand Down
6 changes: 6 additions & 0 deletions src/jsc/VirtualMachine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1525,6 +1525,12 @@ impl VirtualMachine {
// during `destructOnExit()`'s collection after `on_exit()` ran.
self.has_run_cleanup_hooks = true;
let _ = self.garbage_collect(true);
// Remapping a stack trace (printing a test failure does this) caches
// a `ParsedSourceMap` Arc in the saved-source-map table, which stores
// entries as tagged pointers LSan cannot see through, so any cached
// entry is reported as a leak. Nothing remaps after this point on
// these exit paths; the table stays valid (empty) either way.
self.source_mappings.clear();
}

pub fn global_exit(&mut self) -> ! {
Expand Down
9 changes: 5 additions & 4 deletions src/runtime/cli/test/parallel/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,10 +352,11 @@ fn build_worker_argv(
if let Some(seed) = opts.seed {
argv.push(print_z(format_args!("--seed={}", seed))?);
}
// --bail is intentionally NOT forwarded: workers Global.exit(1) on bail
// (see test_command.rs handle_test_completed), which the coordinator would
// misread as a crash. Cross-worker bail is handled at file granularity by
// the coordinator instead.
// --bail is intentionally NOT forwarded: an in-worker bail (see
// test_command.rs handle_test_completed) would stop the worker mid
// file-list, which the coordinator would misread as a crash.
// Cross-worker bail is handled at file granularity by the coordinator
// instead, and `jest.bailed` is never set in a worker (bail stays 0).
if opts.repeat_count > 0 {
argv.push(print_z(format_args!("--rerun-each={}", opts.repeat_count))?);
}
Expand Down
75 changes: 53 additions & 22 deletions src/runtime/cli/test_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1380,7 +1380,14 @@ impl CommandLineReporter {
);
Output::flush();
this.write_junit_report_if_needed();
Global::exit(1);
// Don't `Global::exit(1)` here: the active file is
// mid-execution (`BunTest::run` frames and the run loop's
// strong ref are live), so exiting now would skip all
// teardown. Setting `bailed` stops the stepper from
// starting new entries and unwinds every run loop back to
// `TestCommand::exec`, which exits 1 through the shared
// teardown path.
this.jest.bailed = true;
}
}
}
Expand Down Expand Up @@ -2085,6 +2092,7 @@ impl TestCommand {
run_todo: ctx.test_options.run_todo,
only: ctx.test_options.only,
bail: ctx.test_options.bail,
bailed: false,
max_concurrency: ctx.test_options.max_concurrency,
// `test_filter_regex` is an erased `*mut RegularExpression` (see
// options_types::context); cast back to a typed `NonNull` —
Expand Down Expand Up @@ -2597,6 +2605,30 @@ impl TestCommand {
}
}

// --bail hit its failure threshold: the bail message and junit report
// were already written at the bail site, the run loops unwound without
// running further tests, and everything after this point in the normal
// tail (snapshot writes, coverage, summary) is intentionally skipped.
// Exit 1 through the same teardown sequence as the normal tail below
// so the exit is clean under ASAN leak checking.
if reporter.jest.bailed {
vm.exit_handler.exit_code = 1;
vm.is_shutting_down = true;
reporter.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);
}
drop(reporter);
vm.collect_for_leak_check_at_exit();
let vm_ptr: *mut VirtualMachine = vm;
// SAFETY: `vm_ptr` reborrows the live `&mut VirtualMachine`;
// `run_with_api_lock` takes `&self` only and `global_exit()`
// diverges, so the closure is the sole mutator.
vm.run_with_api_lock(|| unsafe { (*vm_ptr).global_exit() });
}

// With --changed, only a subset of test files (possibly none) runs,
// so the module loader won't naturally add every source file to the
// watcher. Seed it from the module graph so editing any local source
Expand Down Expand Up @@ -3008,6 +3040,9 @@ impl TestCommand {
) {
handle_top_level_test_error_before_javascript_start(err);
}
if reporter.jest.bailed {
return;
}
reporter.jest.default_timeout_override = u32::MAX;
Global::mimalloc_cleanup(false);
if isolate {
Expand Down Expand Up @@ -3184,25 +3219,11 @@ impl TestCommand {
if reporter.jest.bail == 1 { "" } else { "s" }
);
reporter.write_junit_report_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
// `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::<VirtualMachine>(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()) };
// Unwind to `TestCommand::exec`'s bail exit: the
// `exit_file()` defer above releases the active file's
// `Strong`s on the way out, and exec tears down the
// rest (`RUNNER`, pre-exit collection) before exiting 1.
reporter.jest.bailed = true;
}

return Ok(());
Expand Down Expand Up @@ -3230,10 +3251,12 @@ impl TestCommand {
bun_test::BunTest::run(&buntest_strong, vm.global())?;

// Process event loop while bun_test tests are running
vm.event_loop_ref().tick();
if !reporter.jest.bailed {
vm.event_loop_ref().tick();
}

let mut prev_unhandled_count = vm.unhandled_error_counter;
while buntest.phase != bun_test::Phase::Done {
while !reporter.jest.bailed && buntest.phase != bun_test::Phase::Done {
if buntest.wants_wakeup {
buntest.wants_wakeup = false;
vm.wakeup();
Expand All @@ -3250,6 +3273,14 @@ impl TestCommand {
}
}

if reporter.jest.bailed {
// Bail message already printed. Stop ticking the event
// loop so nothing else runs or prints, and unwind to
// `TestCommand::exec`'s bail exit (the defers above
// release the active file on the way out).
return Ok(());
}

let el = vm.event_loop();
// SAFETY: el is the VM-owned event loop; vm is passed back as *mut.
unsafe { (*el).tick_immediate_tasks(vm) };
Expand Down
14 changes: 14 additions & 0 deletions src/runtime/test_runner/Execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -957,6 +957,20 @@ fn step_sequence_one(
) -> JsResult<Option<AdvanceSequenceStatus>> {
let _g = group_begin!();
let buntest = buntest_strong.get();

// Once --bail has hit its threshold, start nothing further — no test
// callbacks, no remaining hooks. Reporting Done lets the group loops
// drain without executing anything so the run loop can unwind to the
// bail exit in `TestCommand::exec`.
if let Some(reporter) = buntest.reporter {
// SAFETY: the reporter outlives every BunTest (owned by
// `test_command::exec`); `exit_file()` nulls this field before the
// file is dropped.
if unsafe { reporter.as_ref() }.jest.bailed {
return Ok(Some(AdvanceSequenceStatus::Done));
}
}

let buntest_ptr = NonNull::from(&mut *buntest);
let this = &mut buntest.execution;

Expand Down
9 changes: 9 additions & 0 deletions src/runtime/test_runner/bun_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -953,6 +953,15 @@ impl BunTest {
// SAFETY: see block-SAFETY above. `step()` may call `.get()` internally;
// no outer `&mut` overlaps because we only touch `*this` between calls.
while let Some(result) = unsafe { (*this).result_queue.read_item() } {
// Once --bail has hit its threshold, discard queued results
// instead of stepping them: stepping a queued completion would
// report another test result after the bail message. The drive
// loop in `TestCommand::run` unwinds to the bail exit.
// SAFETY: short-lived reborrow; the reporter outlives every
// BunTest (owned by `test_command::exec`).
if unsafe { (*this).reporter.is_some_and(|r| r.as_ref().jest.bailed) } {
break;
}
global_this.clear_termination_exception();
// SAFETY: `UnsafeCell`-derived `*mut`; short-lived field read between re-entrant calls.
let step_result: StepResult = match unsafe { (*this).phase } {
Expand Down
7 changes: 7 additions & 0 deletions src/runtime/test_runner/jest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,13 @@ pub struct TestRunner<'a> {
pub concurrent_test_glob: Option<&'a [&'a [u8]]>,
pub last_file: u64,
pub bail: u32,
/// Set when `--bail`'s failure threshold is reached (after the bail
/// message has been printed). Execution stops starting new entries, the
/// run loops unwind without ticking the event loop further, and
/// `TestCommand::exec` exits 1 through the shared teardown path instead
/// of `Global::exit(1)` mid-run (which would skip the pre-exit
/// collection that keeps ASAN leak-check exits clean).
pub bailed: bool,
pub max_concurrency: u32,

pub drainer: jsc::AnyTask::AnyTask,
Expand Down
109 changes: 96 additions & 13 deletions test/cli/test/isolation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -870,6 +870,19 @@ test.concurrent("--isolate: require(esm) caches a BunTranspiledModule SourceProv
}
});

// LeakSanitizer only exists in ASAN builds, and `detect_leaks=1` is a
// startup error on platforms without LSan (macOS arm64); the CI lane
// that leak-checks is linux x64-asan.
const leakCheckEnv = {
...bunEnv,
// The CI runner sets this for outer test processes; the exit path
// must be leak-clean without the full destruct-on-exit teardown too.
BUN_DESTRUCT_VM_ON_EXIT: undefined,
BUN_TEST_PARALLEL_SCALE_MS: "0",
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")}`,
};

// At exit, the final file's `expect()` wrapper boxes (and the `RefData` /
// `BunTestCell` they pin) are freed only by GC finalizers, and no collection
// used to run between the last test and `exit()`. With LeakSanitizer active
Expand All @@ -886,19 +899,6 @@ describe.concurrent("exit is leak-clean under LeakSanitizer", () => {
`;
}

// LeakSanitizer only exists in ASAN builds, and `detect_leaks=1` is a
// startup error on platforms without LSan (macOS arm64); the CI lane
// that leak-checks is linux x64-asan.
const leakCheckEnv = {
...bunEnv,
// The CI runner sets this for outer test processes; the exit path
// must be leak-clean without the full destruct-on-exit teardown too.
BUN_DESTRUCT_VM_ON_EXIT: undefined,
BUN_TEST_PARALLEL_SCALE_MS: "0",
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")}`,
};

test.skipIf(!isASAN || !isLinux).each([
["serial", []],
["--isolate", ["--isolate"]],
Expand All @@ -922,3 +922,86 @@ describe.concurrent("exit is leak-clean under LeakSanitizer", () => {
expect(exitCode).toBe(0);
});
});

// https://github.com/oven-sh/bun/issues/32183
// Failing runs must exit 1 (not SIGABRT from LeakSanitizer's exit scan):
// printing a failure caches a ParsedSourceMap in the saved-source-map table,
// and --bail used to Global::exit(1) mid-run with no teardown at all.
describe.concurrent("failing exits are leak-clean under LeakSanitizer", () => {
const runLeakChecked = async (args: string[], files: Record<string, string>) => {
using dir = tempDir("test-fail-exit-lsan", files);
await using proc = Bun.spawn({
cmd: [bunExe(), "test", ...args, "."],
env: leakCheckEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
return { stdout, stderr, exitCode };
};

test.skipIf(!isASAN || !isLinux)("failing test, serial", async () => {
const { stderr, exitCode } = await runLeakChecked([], {
"a.test.js": `
import { test, expect } from "bun:test";
test("fails", () => { expect(1 + 1).toBe(3); });
`,
});
expect(stderr).not.toContain("LeakSanitizer");
expect(stderr).toContain("1 fail");
expect(exitCode).toBe(1);
});

test.skipIf(!isASAN || !isLinux)("failing test, --parallel", async () => {
// The failure prints (and caches a source map) inside the worker, whose
// abort would not change the coordinator's exit code, so the stderr
// assertion is the load-bearing one.
const { stderr, exitCode } = await runLeakChecked(["--parallel=2"], {
"a.test.js": `
import { test, expect } from "bun:test";
test("fails", () => { expect(1 + 1).toBe(3); });
`,
"b.test.js": `
import { test, expect } from "bun:test";
test("passes", () => { expect(1 + 1).toBe(2); });
`,
});
expect(stderr).not.toContain("LeakSanitizer");
expect(stderr).toContain("1 fail");
expect(exitCode).toBe(1);
});

test.skipIf(!isASAN || !isLinux)("--bail mid-file", async () => {
const { stdout, stderr, exitCode } = await runLeakChecked(["--bail"], {
"a.test.js": `
import { test, expect } from "bun:test";
test("fails", () => { expect(1 + 1).toBe(3); });
test("after bail", () => { console.log("MARKER_AFTER_BAIL"); });
`,
"b.test.js": `
import { test, expect } from "bun:test";
test("also fails", () => { expect(1 + 1).toBe(3); });
`,
});
expect(stderr).not.toContain("LeakSanitizer");
expect(stderr).toContain("Bailed out after 1 failure");
// Nothing may run after the bail: not the rest of the bailing file...
expect(stdout).not.toContain("MARKER_AFTER_BAIL");
// ...and not the other file (whichever file ran first, it failed).
expect(stderr).toContain("Ran 1 test across 1 file.");
expect(exitCode).toBe(1);
});

test.skipIf(!isASAN || !isLinux)("--bail on module evaluation failure", async () => {
const { stderr, exitCode } = await runLeakChecked(["--bail"], {
"a.test.js": `
import { expect } from "bun:test";
expect(1 + 1).toBe(3);
`,
});
expect(stderr).not.toContain("LeakSanitizer");
expect(stderr).toContain("Bailed out after 1 failure");
expect(exitCode).toBe(1);
});
});
Loading