Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
14 changes: 14 additions & 0 deletions src/jsc/VirtualMachine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1059,6 +1059,20 @@ impl VirtualMachine {
|| !el.next_immediate_tasks.is_empty()
}

/// Count of ref'd handles and outstanding tasks keeping the loop alive.
/// The test runner snapshots this before loading a script-style file and
/// drains only while the count exceeds that baseline, so a prior file's
/// leaked handle (or a --parallel worker's IPC pipe) is not waited on.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
pub fn active_keepalive_count(&self) -> usize {
let el = self.event_loop_shared();
let active = self
.platform_loop_opt()
.map(|h| h.active_count() as usize)
.unwrap_or(0);
let concurrent = el.concurrent_ref.load(core::sync::atomic::Ordering::SeqCst).max(0) as usize;
active + self.active_tasks + concurrent
}

pub fn wakeup(&mut self) {
self.event_loop_mut().wakeup();
}
Expand Down
4 changes: 4 additions & 0 deletions src/libuv_sys/libuv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,10 @@ impl Loop {
unsafe { uv_loop_alive(self) != 0 }
}
#[inline]
pub fn active_count(&self) -> u32 {
self.active_handles
}
#[inline]
pub fn tick(&mut self) {
// SAFETY: self is a live loop.
let _ = unsafe { uv_run(self, RunMode::Default) };
Expand Down
34 changes: 34 additions & 0 deletions src/runtime/cli/test_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3142,7 +3142,11 @@

// need to wake up so autoTick() doesn't wait for 16-100ms after loading the entrypoint
vm.wakeup();
// Snapshot ref'd-handle count so the post-test drain (below) waits only
// on handles this file created, not a prior file's leak or a --parallel
// worker's IPC pipe.
let keepalive_baseline = script_keepalive_count(vm);
let promise = vm.load_entry_point_for_test_runner(file_path)?;

Check failure on line 3149 in src/runtime/cli/test_command.rs

View check run for this annotation

Claude / Claude Code Review

Preload handles are not in the baseline for the first file — hang regression

The baseline is snapshotted *before* `load_entry_point_for_test_runner`, but `--preload` scripts execute *inside* that call (via `reload_entry_point_for_test_runner` → `(hooks.load_preloads)(self)` at VirtualMachine.rs:4624) — so on the first test file, a preload's `setInterval`/`Bun.serve` counts *above* baseline and a script-style file hangs forever in the drain loop. The comment at 3242-3243 and the fix response both claim preload handles are "excluded by the baseline", but that's only true f
Comment thread
robobun marked this conversation as resolved.
Outdated
// Only count the file once, not once per repeat
if repeat_index == 0 {
reporter.summary().files += 1;
Expand Down Expand Up @@ -3233,6 +3237,20 @@
}
}

// A file that registered no test()/describe() is a plain script:
// drain ref'd handles it created so late uncaught errors surface,
// like `bun <file>`. Handles that predate the file (prior leak,
// preload, --parallel worker IPC) are excluded by the baseline.
if buntest.collection.root_scope.entries.is_empty() {
let drain_base = vm.unhandled_error_counter;
while drain_base == vm.unhandled_error_counter
&& script_keepalive_count(vm) > keepalive_baseline
{
vm.event_loop_ref().tick();
vm.event_loop_ref().auto_tick();
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
}

Check failure on line 3252 in src/runtime/cli/test_command.rs

View check run for this annotation

Claude / Claude Code Review

Scalar-count baseline cannot attribute post-snapshot handle changes — hang and false-pass

The scalar `count > baseline` comparison only excludes handles that exist *at* snapshot time, not ones whose state changes after it: a prior file/`--preload` callback that fires during this drain and net-creates ref'd handles (delayed `Bun.serve`, connection-pool open, retry fan-out) pushes count above baseline permanently and the loop **hangs** with no timeout — a regression from the pre-PR exit 0. Conversely, a prior file's *transient* handle completing inside the drain drops count to baseline
Comment thread
robobun marked this conversation as resolved.
Outdated

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 Expand Up @@ -3260,6 +3278,22 @@
}
}

/// Count of ref'd handles keeping the event loop alive, with JS timers counted
/// individually (they share one loop ref, so `active_count()` alone can't tell
/// a new timer from a prior file's). Used to scope the post-test drain to work
/// created by the current file.
fn script_keepalive_count(vm: &VirtualMachine) -> usize {
let state = crate::jsc_hooks::runtime_state();
let timers = if state.is_null() {
0
} else {
// SAFETY: `runtime_state()` returns the live per-thread `RuntimeState`;
// `active_timer_count` is plain data, read on the owning JS thread.
unsafe { (*state).timer.active_timer_count.max(0) as usize }
};
vm.active_keepalive_count() + timers
}

pub(crate) fn handle_top_level_test_error_before_javascript_start(err: &crate::Error) -> ! {
if cfg!(debug_assertions) {
if !matches!(err, crate::Error::ModuleNotFound) {
Expand Down
5 changes: 5 additions & 0 deletions src/uws_sys/Loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,11 @@ impl PosixLoop {
self.active > 0
}

#[inline]
pub fn active_count(&self) -> u32 {
self.active
}

// This exists as a method so that we can stick a debugger in here
pub fn add_active(&mut self, value: u32) {
bun_core::scoped_log!(
Expand Down
147 changes: 147 additions & 0 deletions test/cli/test/bun-test.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1458,6 +1458,153 @@ describe("bun test", () => {
});
});

// https://github.com/oven-sh/bun/issues/34859
describe.concurrent("script files with no test() registrations", () => {
test("fails on a delayed unhandled rejection from an async IIFE", async () => {
using dir = tempDir("script-delayed-rejection", {
"delayed.test.js": `
'use strict';
(async () => {
await new Promise(r => setTimeout(r, 20));
throw new Error("delayed rejection should fail the test run");
})();
`,
});
await using proc = Bun.spawn({
cmd: [bunExe(), "test", "./delayed.test.js"],
env: bunEnv,
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("delayed rejection should fail the test run");
expect(stderr).toContain("Unhandled error between tests");
expect(stderr).toContain("1 error");
expect(exitCode).toBe(1);
});

test("fails on a delayed uncaught exception from a timer", async () => {
using dir = tempDir("script-delayed-throw", {
"delayed.test.js": `setTimeout(() => { throw new Error("boom from timer"); }, 20);`,
});
await using proc = Bun.spawn({
cmd: [bunExe(), "test", "./delayed.test.js"],
env: bunEnv,
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("boom from timer");
expect(stderr).toContain("Unhandled error between tests");
expect(exitCode).toBe(1);
});

test("waits for ref'd async work to complete", async () => {
using dir = tempDir("script-drain", {
"drain.test.js": `
const assert = require("assert");
(async () => {
await new Promise(r => setTimeout(r, 20));
assert.strictEqual(1, 1);
console.log("reached end of async iife");
})();
`,
});
await using proc = Bun.spawn({
cmd: [bunExe(), "test", "./drain.test.js"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stdout).toContain("reached end of async iife");
expect(stderr).toContain("0 fail");
expect(exitCode).toBe(0);
});

test("drains per file so a later script file's delayed rejection is caught", async () => {
using dir = tempDir("script-two-files", {
"a.test.js": `
(async () => {
await Promise.resolve();
throw new Error("file A microtask error");
})();
`,
"b.test.js": `
(async () => {
await new Promise(r => setTimeout(r, 20));
throw new Error("file B delayed error");
})();
`,
});
await using proc = Bun.spawn({
cmd: [bunExe(), "test", "./a.test.js", "./b.test.js"],
env: bunEnv,
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("file A microtask error");
expect(stderr).toContain("file B delayed error");
expect(exitCode).toBe(1);
});

test("does not drain when the file registered a test()", async () => {
// Files that use bun:test keep today's behavior: the runner does not
// block on leaked handles after the last test finishes.
using dir = tempDir("registered-test-no-drain", {
"with.test.js": `
const { test } = require("bun:test");
setInterval(() => {}, 60_000);
test("noop", () => {});
`,
});
await using proc = Bun.spawn({
cmd: [bunExe(), "test", "./with.test.js"],
env: bunEnv,
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("1 pass");
expect(exitCode).toBe(0);
});

test("does not wait on a handle leaked by a prior file", async () => {
using dir = tempDir("prior-file-leak", {
"a.test.js": `
const { test } = require("bun:test");
setInterval(() => {}, 60_000);
test("noop", () => {});
`,
"b.test.js": `
(async () => {
await new Promise(r => setTimeout(r, 20));
throw new Error("file B delayed error");
})();
`,
});
await using proc = Bun.spawn({
cmd: [bunExe(), "test", "./a.test.js", "./b.test.js"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
// a passes; b's own delayed error is still surfaced; a's leaked interval
// is not waited on so the run does not hang.
expect(stderr).toContain("1 pass");
expect(stderr).toContain("file B delayed error");
expect(exitCode).toBe(1);
});
});

function createTest(input?: string | (string | { filename: string; contents: string })[], filename?: string): string {
const cwd = tmpdirSync();
const inputs = Array.isArray(input) ? input : [input ?? ""];
Expand Down
Loading