Skip to content
Open
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
11 changes: 11 additions & 0 deletions docs/test/writing-tests.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,17 @@ test(

When a test times out, Bun kills any still-running processes that the test spawned with `Bun.spawn`, `Bun.spawnSync`, or `node:child_process`, and logs a message to the console. This prevents zombie processes from lingering after timed-out tests.

Only the processes started by the test or hook callback that timed out are killed. A timeout does not kill processes started in `beforeAll` or `beforeEach`, by an earlier test, or at the top level of the file.

A cleanup hook that times out is the exception, because the processes it was going to stop have no other cleanup:

- `afterEach`: Bun also kills the processes that the test and its `beforeEach` hooks started, in every attempt of a test with `retry`.
- `afterAll`: Bun also kills the processes that the tests and hooks of its `describe` block started. At the top level of a file, those are all tests and hooks of the file.

A test that times out while it waits in `expect(promise).resolves` or `.rejects` can keep Bun from running anything else until the promise settles. Bun then kills the processes of all tests and hooks, because only that can end the wait. When such a wait still lasts at the end of a file, Bun kills the processes that earlier timeouts left running.

`bun test --bail` kills the processes of all tests and hooks before it exits. `test.concurrent` tests that run together do not get this cleanup. Bun cannot tell their processes apart, so an `afterAll` timeout also kills the processes of tests that ran together with the first test of its block.

## Test Modifiers

### test.skip
Expand Down
94 changes: 74 additions & 20 deletions src/jsc/ProcessAutoKiller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,13 @@ bun_core::declare_scope!(AutoKiller, hidden);
pub struct ProcessAutoKiller {
/// Keys are intrusively-refcounted `*Process` (ref()'d on insert, deref()'d
/// on remove/drop). Stored as raw ptr for identity-hash semantics.
pub(crate) processes: ArrayHashMap<*mut Process, ()>,
pub(crate) processes: ArrayHashMap<*mut Process, u32>,
pub enabled: bool,
pub(crate) ever_enabled: bool,
/// Map value of each tracked process; the test runner begins a new one per test or hook callback.
scope: u32,
/// The scope of the last [`Self::kill_since`], which left the processes of earlier scopes alive.
spared_until: Option<u32>,
}

impl ProcessAutoKiller {
Expand All @@ -24,34 +28,84 @@ impl ProcessAutoKiller {
self.enabled = false;
}

pub fn begin_scope(&mut self) {
self.scope = self.scope.wrapping_add(1);
}

/// The current scope, as a mark for [`Self::kill_since`].
pub fn scope(&self) -> u32 {
self.scope
}

/// Every tracked process: for the `--isolate` swap between files, and where no cleanup hook is left to run.
pub fn kill(&mut self) -> Result {
Result {
processes: self.kill_processes(),
self.spared_until = None;
let mut count: u32 = 0;
while let Some(entry) = self.processes.pop() {
count += Self::kill_and_release(entry.key);
}
Result { processes: count }
}

/// The processes of the current scope.
pub fn kill_scope(&mut self) -> Result {
self.kill_since(self.scope.wrapping_sub(1))
}

fn kill_processes(&mut self) -> u32 {
/// The processes of every scope begun after `mark` was read from [`Self::scope`]. Earlier scopes stay tracked for [`Self::kill_spared`] and [`Self::kill`].
pub fn kill_since(&mut self, mark: u32) -> Result {
self.spared_until = Some(self.scope);
self.kill_where(mark, false)
}

/// The processes that [`Self::kill_since`] left alive: those of the scopes up to its last call.
pub fn kill_spared(&mut self) -> Result {
match self.spared_until.take() {
Some(mark) => self.kill_where(mark, true),
None => Result { processes: 0 },
}
}

/// The scopes begun after `mark`, or with `up_to` the scopes up to `mark`.
fn kill_where(&mut self, mark: u32, up_to: bool) -> Result {
let span = self.scope.wrapping_sub(mark);
let mut count: u32 = 0;
while let Some(entry) = self.processes.pop() {
{
// SAFETY: every key in `processes` was ref()'d on insert and is
// live until the matching deref() below; popped entry is
// exclusively owned for this scope so `&mut Process` is unaliased.
let p: &mut Process = unsafe { &mut *entry.key };
if !p.has_exited() {
bun_core::scoped_log!(AutoKiller, "process.kill {}", p.pid);
count += p.kill(SignalCode::DEFAULT.0).is_ok() as u32;
}
let mut index = self.processes.len();
while index > 0 {
index -= 1;
if (self.scope.wrapping_sub(self.processes.values()[index]) >= span) != up_to {
continue;
}
// SAFETY: key live until this releases the ref taken on insert.
unsafe { Process::deref(entry.key) };
// Walking backwards, so the entry swapped into `index` was already visited.
let (process, _) = self.processes.swap_remove_at(index);
count += Self::kill_and_release(process);
}
count
Result { processes: count }
}

/// `process` must already be removed from `processes`; this releases its ref.
fn kill_and_release(process: *mut Process) -> u32 {
let killed = {
// SAFETY: every key in `processes` was ref()'d on insert and is
// live until the matching deref() below; the entry was removed
// from the map by the caller, so `&mut Process` is unaliased.
let p: &mut Process = unsafe { &mut *process };
if p.has_exited() {
false
} else {
bun_core::scoped_log!(AutoKiller, "process.kill {}", p.pid);
p.kill(SignalCode::DEFAULT.0).is_ok()
}
};
// SAFETY: key live until this releases the ref taken on insert.
unsafe { Process::deref(process) };
killed as u32
}

pub fn clear(&mut self) {
self.spared_until = None;
for process in self.processes.keys() {
// SAFETY: see kill_processes — key is live until deref().
// SAFETY: see kill_and_release: key is live until deref().
unsafe { Process::deref(*process) };
}

Expand All @@ -69,7 +123,7 @@ impl ProcessAutoKiller {
if self.enabled {
// Alloc failure means we never took
// a ref, so just bail. `put` here is fallible only on OOM.
if self.processes.put(process.as_ptr(), ()).is_err() {
if self.processes.put(process.as_ptr(), self.scope).is_err() {
return;
}
// SAFETY: caller passes a live Process; we take a ref to extend its
Expand Down Expand Up @@ -99,7 +153,7 @@ pub struct Result {
impl Drop for ProcessAutoKiller {
fn drop(&mut self) {
for process in self.processes.keys() {
// SAFETY: see kill_processes — key is live until deref().
// SAFETY: see kill_and_release: key is live until deref().
unsafe { Process::deref(*process) };
}
// `self.processes` storage freed by its own Drop.
Expand Down
11 changes: 9 additions & 2 deletions src/jsc/event_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ pub struct EventLoop {
pub uws_loop: Option<NonNull<uws::Loop>>,

pub entered_event_loop_count: isize,
/// [`Self::wait_for_promise`] calls on the stack. Each is a frame that turns this loop itself and returns only when its promise settles.
pub waits_in_place: u32,
/// How many times `tasks` was drained. One uws loop iteration drains it several times, so a
/// `fetch` can start and finish within one iteration. Atomic because a Worker may read it.
task_drains: AtomicU64,
Expand Down Expand Up @@ -135,6 +137,7 @@ impl Default for EventLoop {
deferred_tasks: DeferredTaskQueue::DeferredTaskQueue::default(),
uws_loop: None,
entered_event_loop_count: 0,
waits_in_place: 0,
task_drains: AtomicU64::new(0),
concurrent_ref: AtomicI32::new(0),
imminent_gc_timer: AtomicPtr::new(core::ptr::null_mut()),
Expand Down Expand Up @@ -1138,19 +1141,23 @@ impl EventLoop {
if promise.status() != PromiseStatus::Pending {
return Ok(());
}
self.waits_in_place += 1;
let mut waited = Ok(());
while promise.status() == PromiseStatus::Pending {
if jsc_vm.execution_forbidden()
|| !self.vm_ref().script_allowed()
|| self.global_ref().has_pending_termination_exception()
{
return Err(jsc::Stopped);
waited = Err(jsc::Stopped);
break;
}
self.tick();
if promise.status() == PromiseStatus::Pending {
self.auto_tick();
}
}
Ok(())
self.waits_in_place -= 1;
waited
}

pub fn wakeup(&self) {
Expand Down
8 changes: 2 additions & 6 deletions src/runtime/api/bun/js_bun_spawn_bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,19 +280,15 @@ pub(crate) fn spawn_sync(
let mut bun_test_deadline: Option<Timespec> = None;
let result = spawn_maybe_sync(true, cx, args, secondary_args_value, &mut bun_test_deadline);
// A bun:test deadline that passed while the isolated loop was blocking is reported only now: the loop is torn down and the child reaped, so the runner's callback re-enters nothing that is mid-flight. With an exception pending (spawn failure, termination) the file timer is left armed and reports it from the main loop instead.
if let Some(deadline) = bun_test_deadline
if bun_test_deadline.is_some()
&& result.is_ok()
&& !cx.global().has_exception()
&& let Some(runner) = crate::test_runner::jest::Jest::runner()
&& let Some(active_file) = runner.bun_test_root.active_file.clone()
{
let vm = cx.vm().as_mut();
runner.remove_active_timeout(vm);
crate::test_runner::bun_test::BunTest::bun_test_timeout_callback(
&active_file,
&deadline,
vm,
);
crate::test_runner::bun_test::BunTest::bun_test_timeout_in_spawn_sync(&active_file, vm);
Comment thread
robobun marked this conversation as resolved.
Resolved
if cx.global().has_exception() {
return Ok(JSValue::ZERO);
}
Expand Down
2 changes: 2 additions & 0 deletions src/runtime/cli/test_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1434,6 +1434,8 @@ impl CommandLineReporter {
Output::flush();
self.write_junit_report_if_needed();
self.write_timings_if_needed();
// `--bail` exits without running the hooks that are left, so nothing else stops what the tests spawned.
bun_test::Execution::report_killed_processes(vm.auto_killer.kill().processes);

vm.exit_handler.exit_code = 1;
vm.exit_handler.skip_exit_listeners = skip_exit_listeners(self);
Expand Down
Loading
Loading