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

/// Count of ref'd handles and outstanding tasks keeping the loop alive.
/// Snapshotted by the test runner as a per-file drain baseline so prior
/// leaks (or a --parallel worker's IPC pipe) are not waited on.
Comment thread
robobun 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 Expand Up @@ -4592,6 +4608,7 @@ impl VirtualMachine {
pub fn reload_entry_point_for_test_runner(
&mut self,
entry_path: &[u8],
after_preloads: impl FnOnce(&Self),
) -> crate::CrateResult<*mut JSInternalPromise> {
self.has_loaded = false;
self.set_main(entry_path);
Expand All @@ -4618,6 +4635,8 @@ impl VirtualMachine {
}
}

after_preloads(self);

// Note: reshaped for borrowck.
let global = self.global;
let main_str = bun_core::String::from_bytes(self.main());
Expand Down Expand Up @@ -4648,11 +4667,14 @@ impl VirtualMachine {
}

/// Loads a test-file entry point and waits for the load promise to settle.
/// `after_preloads` runs between preload completion and entry-point
/// evaluation so the caller can observe preload-created handles.
Comment thread
robobun marked this conversation as resolved.
Outdated
pub fn load_entry_point_for_test_runner(
&mut self,
entry_path: &[u8],
after_preloads: impl FnOnce(&Self),
) -> crate::CrateResult<*mut JSInternalPromise> {
let promise = self.reload_entry_point_for_test_runner(entry_path)?;
let promise = self.reload_entry_point_for_test_runner(entry_path, after_preloads)?;

// pending_internal_promise can change if hot module reloading is enabled
if self.is_watcher_enabled() {
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: 33 additions & 1 deletion src/runtime/cli/test_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3142,7 +3142,12 @@

// need to wake up so autoTick() doesn't wait for 16-100ms after loading the entrypoint
vm.wakeup();
let promise = vm.load_entry_point_for_test_runner(file_path)?;
// Record whether the loop was idle after preloads so the post-test
// drain (below) only runs when every ref'd handle is this file's.
let mut idle_after_preloads = false;
let promise = vm.load_entry_point_for_test_runner(file_path, |vm| {
idle_after_preloads = script_keepalive_count(vm) == 0;
})?;
// Only count the file once, not once per repeat
if repeat_index == 0 {
reporter.summary().files += 1;
Expand Down Expand Up @@ -3233,6 +3238,18 @@
}
}

// A file with no test()/describe() is a plain script: drain the
// loop so late errors surface (like `bun <file>`). Gated on an
// idle loop after preloads so prior-file handles can't hang it.
if buntest.collection.root_scope.entries.is_empty() && idle_after_preloads {
let drain_base = vm.unhandled_error_counter;
while drain_base == vm.unhandled_error_counter && script_keepalive_count(vm) > 0
{
vm.event_loop_ref().tick();
vm.event_loop_ref().auto_tick();
}
}
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 +3277,21 @@
}
}

/// Count of ref'd handles plus individual JS timers (timers share one loop
/// ref, so `active_count()` alone misses additional timers). Zero means the
/// file's own work is the only thing keeping the loop alive.

Check warning on line 3282 in src/runtime/cli/test_command.rs

View check run for this annotation

Claude / Claude Code Review

Misleading doc comment on script_keepalive_count: 'Zero means the file's own work is the only thing keeping the loop alive'

The third sentence is backwards: a return of zero means *nothing* is keeping the loop alive — if "the file's own work" were keeping it alive, that work would be counted and the result would be >0. The intended meaning is temporal: zero *at `after_preloads`* means the loop was idle before this file loaded, so any handle seen later during the drain must belong to it. Suggest e.g. "Zero at `after_preloads` means the loop was idle before this file loaded, so every handle observed during the drain be
Comment thread
robobun marked this conversation as resolved.
Outdated
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
155 changes: 155 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,161 @@ 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 () => {
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 drain when a prior file left a ref'd handle", async () => {
using dir = tempDir("prior-file-leak", {
"a.test.js": `
const { test } = require("bun:test");
setInterval(() => {}, 60_000);
test("noop", () => {});
`,
"b.test.js": `require("assert").strictEqual(1, 1);`,
});
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("1 pass");
expect(stderr).toContain("0 fail");
expect(exitCode).toBe(0);
});

test("does not drain when a preload left a ref'd handle", async () => {
using dir = tempDir("preload-leak", {
"setup.js": `setInterval(() => {}, 60_000);`,
"a.test.js": `require("assert").strictEqual(1, 1);`,
});
await using proc = Bun.spawn({
cmd: [bunExe(), "test", "--preload", "./setup.js", "./a.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("0 fail");
expect(exitCode).toBe(0);
});
});

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