From 236cde79929c80016f65bce2475f55c832bf6ab6 Mon Sep 17 00:00:00 2001 From: Yuri Khrustalev Date: Tue, 15 Sep 2026 21:59:51 -0400 Subject: [PATCH 1/6] fix: don't wait forever for a child that ignores SIGTERM On a timeout, terminate_child() sent one SIGTERM and then blocked in wait() with no bound. A cargo process that ignores the signal, or that has been stopped and so never receives it, hung the whole run at that point. Wait only for a short grace period, then SIGKILL the process group. This needs a second signal, so the errno handling moves into a signal_group() helper rather than being duplicated. --- NEWS.md | 2 ++ book/src/timeouts.md | 5 +++++ src/process.rs | 38 ++++++++++++++++++++++++++++++++++---- src/process/unix.rs | 27 ++++++++++++++++++++++----- src/process/windows.rs | 6 ++++++ 5 files changed, 69 insertions(+), 9 deletions(-) diff --git a/NEWS.md b/NEWS.md index ed5bd2bb..f7619c39 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,8 @@ ## Unreleased +- Fixed: After a timeout, cargo-mutants waits only a short grace period for the child to exit after `SIGTERM` before sending `SIGKILL`. Previously it waited indefinitely, so a process that ignored `SIGTERM`, or that had been stopped, hung the whole run. + - New: `#[mutants::exclude_re("pattern")]` attribute to exclude specific mutations by regex, without disabling all mutations on the function. The attribute can be placed on functions, `impl` blocks, `trait` blocks, modules, files, and on expressions that can carry an attribute (such as `match`, struct literals, call expressions, method calls, and unary expressions). Multiple patterns can be applied. Also supported within `cfg_attr`. Requires the [mutants](https://crates.io/crates/mutants) crate version `0.0.5` or later. - Fixed: `#[mutants::skip]` (and `#[cfg_attr(..., mutants::skip)]`) is now honoured when placed on `const` and `static` items, including associated constants in `impl` and `trait` blocks. Previously the attribute was silently ignored on these items and operator mutants inside the initializer expression were still generated ([#508](https://github.com/sourcefrog/cargo-mutants/issues/508)). diff --git a/book/src/timeouts.md b/book/src/timeouts.md index bff23255..3de52892 100644 --- a/book/src/timeouts.md +++ b/book/src/timeouts.md @@ -33,6 +33,11 @@ You can set an explicit timeouts with the `--timeout` option, also measured in s You can also set the test timeout as a multiple of the duration of the baseline test, with the `--timeout-multiplier` option and the `timeout_multiplier` configuration key. The multiplier only has an effect if the baseline is not skipped and if `--timeout` is not specified. +When a timeout expires, the process is sent `SIGTERM` so that it can clean up, and then +`SIGKILL` if it has not exited within a short grace period. A test process that ignores +`SIGTERM`, or that has been stopped and so never receives it, therefore cannot stall the +whole run. + ## Build timeouts `const` expressions may be evaluated at compile time. In the same way that mutations can cause tests to hang, mutations to const code may potentially cause the compiler to enter an infinite loop. diff --git a/src/process.rs b/src/process.rs index 0763e617..b8268611 100644 --- a/src/process.rs +++ b/src/process.rs @@ -26,15 +26,21 @@ use crate::output::ScenarioOutput; /// How frequently to check if a subprocess finished. const WAIT_POLL_INTERVAL: Duration = Duration::from_millis(50); +/// How long to let a process wind up after `SIGTERM` before sending `SIGKILL`. +const TERM_GRACE: Duration = Duration::from_millis(500); + +/// How often to check whether a signalled process has gone away. +const TERM_POLL_INTERVAL: Duration = Duration::from_millis(20); + #[cfg(windows)] mod windows; #[cfg(windows)] -use windows::{configure_command, terminate_child}; +use windows::{configure_command, kill_child, terminate_child}; #[cfg(unix)] mod unix; #[cfg(unix)] -use unix::{configure_command, terminate_child}; +use unix::{configure_command, kill_child, terminate_child}; pub struct Process { child: Child, @@ -124,6 +130,10 @@ impl Process { /// /// Blocks until the subprocess is terminated and then returns the exit status. /// + /// `SIGTERM` first, so it gets a chance to clean up, but only for a bounded grace + /// period: a process that ignores it, or that is stopped and so never receives it, + /// would otherwise hang the run here forever. + /// /// The status might not be `Timeout` if this raced with a normal exit. #[mutants::skip] // would leak processes from tests if skipped fn terminate(&mut self) -> Result<()> { @@ -131,9 +141,29 @@ impl Process { debug!("terminating child process"); terminate_child(&mut self.child)?; trace!("wait for child after termination"); + let deadline = Instant::now() + TERM_GRACE; + loop { + match self.child.try_wait() { + Ok(Some(exit)) => { + debug!("terminated child exit status {exit:?}"); + return Ok(()); + } + Ok(None) => {} + Err(err) => { + debug!(?err, "Failed to wait for child after termination"); + return Ok(()); + } + } + if Instant::now() >= deadline { + debug!("child did not exit after SIGTERM; killing it"); + kill_child(&mut self.child)?; + break; + } + sleep(TERM_POLL_INTERVAL); + } match self.child.wait() { - Err(err) => debug!(?err, "Failed to wait for child after termination"), - Ok(exit) => debug!("terminated child exit status {exit:?}"), + Err(err) => debug!(?err, "Failed to wait for child after kill"), + Ok(exit) => debug!("killed child exit status {exit:?}"), } Ok(()) } diff --git a/src/process/unix.rs b/src/process/unix.rs index 1eb1866f..6d9eefed 100644 --- a/src/process/unix.rs +++ b/src/process/unix.rs @@ -11,11 +11,10 @@ use crate::Result; use super::Exit; -#[allow(unknown_lints, clippy::needless_pass_by_ref_mut)] // To match Windows +/// Send a signal to a process group, treating "nothing there" as success. #[mutants::skip] // hard to exercise the ESRCH edge case -pub(super) fn terminate_child(child: &mut Child) -> Result<()> { - let pid = Pid::from_raw(child.id().try_into().unwrap()); - match killpg(pid, Signal::SIGTERM) { +fn signal_group(pgid: Pid, signal: Signal) -> Result<()> { + match killpg(pgid, signal) { Ok(()) => Ok(()), Err(Errno::ESRCH) => { Ok(()) // Probably already gone @@ -25,13 +24,31 @@ pub(super) fn terminate_child(child: &mut Child) -> Result<()> { } Err(errno) => { // TODO: Maybe strerror? - let message = format!("failed to terminate child: error {errno}"); + let message = format!("failed to signal process group {pgid}: error {errno}"); warn!("{}", message); bail!(message); } } } +/// The process group id of a child, which (because we start it with `process_group(0)`) +/// is the same as its pid. +fn child_pgid(child: &Child) -> Pid { + Pid::from_raw(child.id().try_into().expect("child pid fits in pid_t")) +} + +#[allow(unknown_lints, clippy::needless_pass_by_ref_mut)] // To match Windows +#[mutants::skip] // hard to exercise the ESRCH edge case +pub(super) fn terminate_child(child: &mut Child) -> Result<()> { + signal_group(child_pgid(child), Signal::SIGTERM) +} + +#[allow(unknown_lints, clippy::needless_pass_by_ref_mut)] // To match Windows +#[mutants::skip] // would leak processes from tests if skipped +pub(super) fn kill_child(child: &mut Child) -> Result<()> { + signal_group(child_pgid(child), Signal::SIGKILL) +} + #[mutants::skip] pub(super) fn configure_command(command: &mut Command) { command.process_group(0); diff --git a/src/process/windows.rs b/src/process/windows.rs index c37950fd..cca4981a 100644 --- a/src/process/windows.rs +++ b/src/process/windows.rs @@ -11,6 +11,12 @@ pub(super) fn terminate_child(child: &mut Child) -> Result<()> { child.kill().context("Kill child") } +/// Windows has no `SIGTERM`, so `terminate_child` already killed it outright. +#[mutants::skip] // would leak processes from tests if skipped +pub(super) fn kill_child(child: &mut Child) -> Result<()> { + child.kill().context("Kill child") +} + #[mutants::skip] pub(super) fn configure_command(_command: &mut Command) {} From 1ce1e5550d3f3ccbd94c372a67e1168ac310549b Mon Sep 17 00:00:00 2001 From: Yuri Khrustalev Date: Tue, 15 Sep 2026 22:01:21 -0400 Subject: [PATCH 2/6] fix: sweep the child's process group after every phase Tests can leave processes running after they exit: a helper a test spawned and forgot, or a test binary that was never reaped. Until now cargo-mutants only signalled the child's process group on a timeout, so on a normal exit those processes survived and kept allocating while later mutants were tested, in a window where no cargo phase is running at all. Sweep the group in the process layer, so every phase gets it: once the direct child exits with any status, SIGTERM the group, wait a bounded grace period, then SIGKILL whatever is left. What was reaped goes into the scenario log, and the pids into the debug log, enumerated from /proc on Linux. Windows has no process groups and the job object equivalent is out of scope, so the sweep is a no-op there. --- NEWS.md | 2 + book/src/timeouts.md | 17 +++ src/cargo.rs | 2 +- src/process.rs | 86 ++++++++++++-- src/process/unix.rs | 111 +++++++++++++++--- src/process/windows.rs | 9 +- .../spawns_background_child/Cargo_test.toml | 13 ++ testdata/spawns_background_child/src/lib.rs | 42 +++++++ tests/main.rs | 67 +++++++++++ 9 files changed, 321 insertions(+), 28 deletions(-) create mode 100644 testdata/spawns_background_child/Cargo_test.toml create mode 100644 testdata/spawns_background_child/src/lib.rs diff --git a/NEWS.md b/NEWS.md index f7619c39..b8315cf0 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,8 @@ ## Unreleased +- Fixed: Each cargo invocation now runs as the leader of its own process group, and that group is swept after every phase, not only after a timeout. Processes left running by a scenario's tests are `SIGTERM`ed, given a short grace period, and then `SIGKILL`ed, so they can't keep consuming memory or CPU while later mutants are tested. Unix only. + - Fixed: After a timeout, cargo-mutants waits only a short grace period for the child to exit after `SIGTERM` before sending `SIGKILL`. Previously it waited indefinitely, so a process that ignored `SIGTERM`, or that had been stopped, hung the whole run. - New: `#[mutants::exclude_re("pattern")]` attribute to exclude specific mutations by regex, without disabling all mutations on the function. The attribute can be placed on functions, `impl` blocks, `trait` blocks, modules, files, and on expressions that can carry an attribute (such as `match`, struct literals, call expressions, method calls, and unary expressions). Multiple patterns can be applied. Also supported within `cfg_attr`. Requires the [mutants](https://crates.io/crates/mutants) crate version `0.0.5` or later. diff --git a/book/src/timeouts.md b/book/src/timeouts.md index 3de52892..660cdd59 100644 --- a/book/src/timeouts.md +++ b/book/src/timeouts.md @@ -50,6 +50,23 @@ In this case you can use the `--build-timeout` or `--build-timeout-multiplier` o You might also choose to skip mutants that can cause long-running const evaluation. +## Leftover processes + +A test can leave processes running after it returns: a daemon it started, a helper it +forgot to wait for, or a test binary that was not reaped. Those processes keep running +— and keep allocating — while cargo-mutants moves on to the next mutant, so they can +exhaust the machine's memory in a window where no cargo phase is running at all. + +To prevent this, cargo-mutants starts each cargo invocation as the leader of its own +process group, and sweeps that group after *every* phase, not only after a timeout. +Once the cargo process itself exits, anything left in the group is sent `SIGTERM`, given +a short grace period, and then `SIGKILL`ed. What was reaped is recorded in the +scenario's log, and the pids are shown at `--level=debug`. + +This has no effect on how a mutant is classified; it only stops work from one scenario +leaking into the next. Windows has no process groups, and cargo-mutants does not yet use +job objects, so this sweep is Unix-only. + ## Exceptions The multiplier timeout options cannot be used when the baseline is skipped diff --git a/src/cargo.rs b/src/cargo.rs index 909dd881..0309fd27 100644 --- a/src/cargo.rs +++ b/src/cargo.rs @@ -55,7 +55,7 @@ pub fn run_cargo( debug!(?encoded_rustflags); env.push(("CARGO_ENCODED_RUSTFLAGS".to_owned(), encoded_rustflags)); } - let process_status = Process::run( + let (process_status, _sweep) = Process::run( &argv, &env, build_dir.path(), diff --git a/src/process.rs b/src/process.rs index b8268611..1903bc9a 100644 --- a/src/process.rs +++ b/src/process.rs @@ -15,6 +15,7 @@ use std::time::{Duration, Instant}; use anyhow::Context; use camino::Utf8Path; +use itertools::Itertools; use serde::Serialize; use tracing::{Level, debug, span, trace}; @@ -26,21 +27,57 @@ use crate::output::ScenarioOutput; /// How frequently to check if a subprocess finished. const WAIT_POLL_INTERVAL: Duration = Duration::from_millis(50); -/// How long to let a process wind up after `SIGTERM` before sending `SIGKILL`. +/// How long to let a process, or a process group, wind up after `SIGTERM` before +/// sending `SIGKILL`. const TERM_GRACE: Duration = Duration::from_millis(500); -/// How often to check whether a signalled process has gone away. +/// How often to check whether a signalled process or group has gone away. const TERM_POLL_INTERVAL: Duration = Duration::from_millis(20); #[cfg(windows)] mod windows; #[cfg(windows)] -use windows::{configure_command, kill_child, terminate_child}; +use windows::{configure_command, kill_child, sweep_process_group, terminate_child}; #[cfg(unix)] mod unix; #[cfg(unix)] -use unix::{configure_command, kill_child, terminate_child}; +use unix::{configure_command, kill_child, sweep_process_group, terminate_child}; + +/// What sweeping a finished child's process group found and did. +/// +/// A scenario's tests can leave processes running: a test binary that was not waited +/// for, or anything a test spawned and forgot. Those processes stay in the child's +/// process group, and would otherwise keep running (and keep allocating) while +/// cargo-mutants moves on to later scenarios. +#[derive(Debug, Default, Clone, PartialEq, Eq, Serialize)] +pub struct Sweep { + /// The pids that were in the group, where the platform can enumerate them. + pub pids: Option>, + /// Whether any process was still in the group after the direct child exited. + pub strays: bool, + /// Whether `SIGTERM` was not enough, and `SIGKILL` had to be sent. + pub killed: bool, +} + +impl Sweep { + /// Describe what was reaped, or None if nothing was left over. + pub fn describe(&self) -> Option { + if !self.strays { + return None; + } + let how = if self.killed { "SIGKILLed" } else { "reaped" }; + Some(match &self.pids { + Some(pids) if !pids.is_empty() => format!( + "left {n} stray process{es} behind ({how}: {list})", + n = pids.len(), + es = if pids.len() == 1 { "" } else { "es" }, + list = pids.iter().join(", ") + ), + _ => format!("left stray processes behind ({how})"), + }) + } +} pub struct Process { child: Child, @@ -51,6 +88,9 @@ pub struct Process { impl Process { /// Run a subprocess to completion, watching for interrupts, with a timeout, while /// ticking the progress bar. + /// + /// Whatever the outcome, the child's process group is swept before returning, so + /// that nothing it left running survives into the next scenario. pub fn run( argv: &[String], env: &[(String, String)], @@ -59,17 +99,42 @@ impl Process { jobserver: Option<&jobserver::Client>, scenario_output: &mut ScenarioOutput, console: &Console, - ) -> Result { + ) -> Result<(Exit, Sweep)> { let mut child = Process::start(argv, env, cwd, timeout, jobserver, scenario_output)?; - let process_status = loop { - if let Some(exit_status) = child.poll()? { - break exit_status; + let result = loop { + match child.poll() { + Ok(Some(exit_status)) => break Ok(exit_status), + Ok(None) => {} + Err(err) => break Err(err), } console.tick(); sleep(WAIT_POLL_INTERVAL); }; + let sweep = child.sweep()?; + let process_status = result?; scenario_output.message(&format!("result: {process_status:?}"))?; - Ok(process_status) + if let Some(description) = sweep.describe() { + scenario_output.message(&description)?; + } + Ok((process_status, sweep)) + } + + /// Kill anything the child left running in its process group. + /// + /// This runs after every phase, not only after a timeout: a scenario that exited + /// cleanly can still have left a test binary or a process spawned by a test behind. + fn sweep(&mut self) -> Result { + let sweep = sweep_process_group(&self.child)?; + if sweep.strays { + debug!( + pids = ?sweep.pids, + killed = sweep.killed, + "swept processes left over in the child's process group" + ); + } else { + trace!("no processes left in the child's process group"); + } + Ok(sweep) } /// Launch a process, and return an object representing the child. @@ -132,7 +197,8 @@ impl Process { /// /// `SIGTERM` first, so it gets a chance to clean up, but only for a bounded grace /// period: a process that ignores it, or that is stopped and so never receives it, - /// would otherwise hang the run here forever. + /// would otherwise hang the run here forever. Whatever else is left in the process + /// group is dealt with by the sweep in [`Process::run`]. /// /// The status might not be `Timeout` if this raced with a normal exit. #[mutants::skip] // would leak processes from tests if skipped diff --git a/src/process/unix.rs b/src/process/unix.rs index 6d9eefed..61060255 100644 --- a/src/process/unix.rs +++ b/src/process/unix.rs @@ -1,5 +1,7 @@ use std::os::unix::process::{CommandExt, ExitStatusExt}; use std::process::{Child, Command, ExitStatus}; +use std::thread::sleep; +use std::time::Instant; use anyhow::bail; use nix::errno::Errno; @@ -9,18 +11,18 @@ use tracing::warn; use crate::Result; -use super::Exit; +use super::{Exit, Sweep, TERM_GRACE, TERM_POLL_INTERVAL}; -/// Send a signal to a process group, treating "nothing there" as success. +/// Send a signal to a process group, returning whether the group still had any member. +/// +/// A signal of `None` sends nothing and just probes for members. #[mutants::skip] // hard to exercise the ESRCH edge case -fn signal_group(pgid: Pid, signal: Signal) -> Result<()> { +fn signal_group(pgid: Pid, signal: Option) -> Result { match killpg(pgid, signal) { - Ok(()) => Ok(()), - Err(Errno::ESRCH) => { - Ok(()) // Probably already gone - } + Ok(()) => Ok(true), + Err(Errno::ESRCH) => Ok(false), // the group is empty Err(Errno::EPERM) if cfg!(target_os = "macos") => { - Ok(()) // If the process no longer exists then macos can return EPERM (maybe?) + Ok(false) // If the process no longer exists then macos can return EPERM (maybe?) } Err(errno) => { // TODO: Maybe strerror? @@ -31,22 +33,99 @@ fn signal_group(pgid: Pid, signal: Signal) -> Result<()> { } } +#[allow(unknown_lints, clippy::needless_pass_by_ref_mut)] // To match Windows +#[mutants::skip] // would leak processes from tests if skipped +pub(super) fn terminate_child(child: &mut Child) -> Result<()> { + signal_group(child_pgid(child), Some(Signal::SIGTERM))?; + Ok(()) +} + +#[allow(unknown_lints, clippy::needless_pass_by_ref_mut)] // To match Windows +#[mutants::skip] // would leak processes from tests if skipped +pub(super) fn kill_child(child: &mut Child) -> Result<()> { + signal_group(child_pgid(child), Some(Signal::SIGKILL))?; + Ok(()) +} + +/// Kill anything left in a child's process group once the child itself has exited. +/// +/// The child was started as the leader of its own process group, so anything it or its +/// descendants spawned and left running is still in that group, even after being +/// reparented. Send `SIGTERM`, give the group a moment to wind up, then `SIGKILL` +/// whatever is left. +#[mutants::skip] // would leak processes from tests if skipped +pub(super) fn sweep_process_group(child: &Child) -> Result { + let pgid = child_pgid(child); + // Probe before enumerating: almost every phase leaves nothing behind, and listing + // the group means reading every /proc//stat on the machine. + if !signal_group(pgid, None)? { + return Ok(Sweep::default()); + } + let pids = group_members(pgid); + signal_group(pgid, Some(Signal::SIGTERM))?; + let deadline = Instant::now() + TERM_GRACE; + loop { + if !signal_group(pgid, None)? { + return Ok(Sweep { + pids, + strays: true, + killed: false, + }); + } else if Instant::now() >= deadline { + break; + } + sleep(TERM_POLL_INTERVAL); + } + signal_group(pgid, Some(Signal::SIGKILL))?; + Ok(Sweep { + pids, + strays: true, + killed: true, + }) +} + /// The process group id of a child, which (because we start it with `process_group(0)`) /// is the same as its pid. +/// +/// Callers signal this after the child has been reaped, by which point the group may be +/// empty and the kernel free to hand the same number to something else. Hitting that +/// would take a full wrap of the pid space inside the microseconds between reaping and +/// signalling, so we live with it. fn child_pgid(child: &Child) -> Pid { Pid::from_raw(child.id().try_into().expect("child pid fits in pid_t")) } -#[allow(unknown_lints, clippy::needless_pass_by_ref_mut)] // To match Windows -#[mutants::skip] // hard to exercise the ESRCH edge case -pub(super) fn terminate_child(child: &mut Child) -> Result<()> { - signal_group(child_pgid(child), Signal::SIGTERM) +/// List the pids currently in a process group. +/// +/// Returns `None` on platforms where we can't enumerate processes: the sweep still +/// works there, it just can't say what it killed. +#[cfg(target_os = "linux")] +#[mutants::skip] // only affects what we can say in the debug log +fn group_members(pgid: Pid) -> Option> { + let members = std::fs::read_dir("/proc") + .ok()? + .flatten() + .filter_map(|dir_entry| dir_entry.file_name().to_string_lossy().parse::().ok()) + .filter(|pid| pgid_of(*pid) == Some(pgid.as_raw())) + .collect(); + Some(members) } -#[allow(unknown_lints, clippy::needless_pass_by_ref_mut)] // To match Windows -#[mutants::skip] // would leak processes from tests if skipped -pub(super) fn kill_child(child: &mut Child) -> Result<()> { - signal_group(child_pgid(child), Signal::SIGKILL) +/// Read a process's group id out of `/proc//stat`. +#[cfg(target_os = "linux")] +#[mutants::skip] // only affects what we can say in the debug log +fn pgid_of(pid: i32) -> Option { + let stat = std::fs::read_to_string(format!("/proc/{pid}/stat")).ok()?; + // The second field is the command name in parentheses, and may itself contain spaces + // and parentheses, so only split the fields after its closing paren. Counting from + // there, the fields are: state, ppid, pgrp. + let (_, after_comm) = stat.rsplit_once(')')?; + after_comm.split_whitespace().nth(2)?.parse().ok() +} + +#[cfg(not(target_os = "linux"))] +fn group_members(_pgid: Pid) -> Option> { + None } #[mutants::skip] diff --git a/src/process/windows.rs b/src/process/windows.rs index cca4981a..b23708ee 100644 --- a/src/process/windows.rs +++ b/src/process/windows.rs @@ -4,7 +4,7 @@ use anyhow::Context; use crate::Result; -use super::Exit; +use super::{Exit, Sweep}; #[mutants::skip] // hard to exercise the ESRCH edge case pub(super) fn terminate_child(child: &mut Child) -> Result<()> { @@ -17,6 +17,13 @@ pub(super) fn kill_child(child: &mut Child) -> Result<()> { child.kill().context("Kill child") } +/// Windows has no process groups; the equivalent would be a job object, which we don't +/// use yet, so there is nothing to sweep. +#[allow(clippy::unnecessary_wraps)] // To match Unix +pub(super) fn sweep_process_group(_child: &Child) -> Result { + Ok(Sweep::default()) +} + #[mutants::skip] pub(super) fn configure_command(_command: &mut Command) {} diff --git a/testdata/spawns_background_child/Cargo_test.toml b/testdata/spawns_background_child/Cargo_test.toml new file mode 100644 index 00000000..5857de59 --- /dev/null +++ b/testdata/spawns_background_child/Cargo_test.toml @@ -0,0 +1,13 @@ +# A tree whose test leaves a background process running after the test binary exits. +# +# cargo-mutants should sweep the cargo process group after every phase, so that no +# `sleep` process survives the scenario that spawned it. + +[package] +name = "cargo-mutants-testdata-spawns-background-child" +version = "0.1.0" +edition = "2018" +publish = false + +[lib] +doctest = false diff --git a/testdata/spawns_background_child/src/lib.rs b/testdata/spawns_background_child/src/lib.rs new file mode 100644 index 00000000..78ed86c0 --- /dev/null +++ b/testdata/spawns_background_child/src/lib.rs @@ -0,0 +1,42 @@ +//! A tree whose test spawns a background process and then exits successfully. +//! +//! The test records the pid of the background process in the file named by +//! `$BACKGROUND_CHILD_PID_FILE`, so that the cargo-mutants test suite can check that +//! the process group sweep reaps it. + +pub fn triple(x: i32) -> i32 { + x * 3 +} + +#[cfg(test)] +mod test { + use std::fs::OpenOptions; + use std::io::Write; + use std::process::{Command, Stdio}; + + /// Spawn a process that outlives this test, and record its pid. + fn leave_a_process_running() { + let child = Command::new("sleep") + .arg("300") + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .expect("spawn sleep"); + if let Ok(path) = std::env::var("BACKGROUND_CHILD_PID_FILE") { + let mut file = OpenOptions::new() + .create(true) + .append(true) + .open(&path) + .expect("open pid file"); + writeln!(file, "{}", child.id()).expect("write pid"); + } + } + + #[test] + fn triple_triples() { + leave_a_process_running(); + // 3 is chosen so that every mutant of `x * 3` gives a different answer. + assert_eq!(super::triple(3), 9); + } +} diff --git a/tests/main.rs b/tests/main.rs index 0fb8973d..627b41cf 100644 --- a/tests/main.rs +++ b/tests/main.rs @@ -3846,3 +3846,70 @@ fn in_diff_with_nonexistent_file_returns_exit_code_6() { .code(6) .stderr(contains("Failed to read diff file").or(contains("Failed to open diff file"))); } + +/// A test can leave a background process running after it exits. cargo-mutants puts +/// each cargo invocation in its own process group and sweeps that group after every +/// phase, so nothing spawned by a scenario outlives it. +/// +/// The `spawns_background_child` tree records the pids it leaves behind, so we can +/// probe them once cargo-mutants has finished. +#[cfg(unix)] +#[test] +fn processes_spawned_by_tests_are_swept_after_each_scenario() +-> Result<(), Box> { + use nix::sys::signal::{SIGKILL, kill}; + use nix::unistd::Pid; + + let tmp_src_dir = copy_of_testdata("spawns_background_child"); + let pid_dir = tempdir()?; + let pid_file = pid_dir.path().join("pids.txt"); + let assert = run() + .arg("mutants") + .args(["--timeout=60", "--build-timeout=120", "-L", "debug"]) + .env("BACKGROUND_CHILD_PID_FILE", &pid_file) + .current_dir(tmp_src_dir.path()) + .timeout(OUTER_TIMEOUT) + .assert(); + let stdout = String::from_utf8_lossy(&assert.get_output().stdout).into_owned(); + println!("stdout:\n{stdout}"); + assert.success(); + + let pids: Vec = read_to_string(&pid_file)? + .lines() + .map(str::trim) + .map(str::parse) + .collect::>()?; + let survivors: Vec = pids + .iter() + .copied() + .filter(|pid| kill(Pid::from_raw(*pid), None).is_ok()) + .collect(); + // Before any assertion, so that a failure doesn't also leave these running. + // (clippy::needless_for_each rules out the iterator form here.) + for pid in &survivors { + let _ = kill(Pid::from_raw(*pid), SIGKILL); + } + assert!( + !pids.is_empty(), + "the tree's test should have spawned background processes" + ); + assert_eq!( + survivors, + Vec::::new(), + "processes spawned by the tests were still running after cargo-mutants finished" + ); + + // Sweeping the process group must not change any verdict. + assert_eq!( + outcome_json_counts(&tmp_src_dir), + json!({ + "total_mutants": 5, + "caught": 5, + "missed": 0, + "timeout": 0, + "unviable": 0, + "success": 0, + }) + ); + Ok(()) +} From fc1ede60ca315c89f1f4523bbb2562a8b9dc640c Mon Sep 17 00:00:00 2001 From: Yuri Khrustalev Date: Tue, 15 Sep 2026 22:03:38 -0400 Subject: [PATCH 3/6] feat: add --max-memory to bound each scenario's memory use A mutant can turn a bounded loop into an unbounded allocator, and a test process growing at hundreds of MB/s exhausts the machine well before a 5x-baseline test timeout arrives. On a CI runner the VM is then torn down with no log, and the shard's mutants are never recorded. --max-memory SIZE, and the max_memory config key, put a ceiling on each scenario's cargo process tree instead. It is enforced with setrlimit (RLIMIT_AS), which limits address space rather than resident memory and so has to be set generously; which mechanism is in use is logged at startup. macOS accepts RLIMIT_AS and ignores it, so there the option warns and does nothing, as documented. Where it cannot be applied at all, giving the option is an error raised before any mutant runs, rather than a long run that silently had no limit. Zero is rejected too: unlike -t 0 it would mean 'stop everything', not 'no limit'. --- Cargo.toml | 2 +- NEWS.md | 2 + book/src/timeouts.md | 47 +++++++++ examples/custom_config.toml | 5 + src/cargo.rs | 3 + src/config.rs | 2 + src/lab.rs | 12 ++- src/main.rs | 7 ++ src/options.rs | 123 +++++++++++++++++++++- src/process.rs | 20 +++- src/process/memory.rs | 201 ++++++++++++++++++++++++++++++++++++ tests/main.rs | 27 +++++ 12 files changed, 446 insertions(+), 5 deletions(-) create mode 100644 src/process/memory.rs diff --git a/Cargo.toml b/Cargo.toml index e520cffa..5464616b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -78,7 +78,7 @@ version = "2.0.104" features = ["full", "extra-traits", "visit"] [target.'cfg(unix)'.dependencies] -nix = { version = "0.31", features = ["process", "signal"] } +nix = { version = "0.31", features = ["process", "resource", "signal"] } # reflink is disabled on musl for the time being due to # and diff --git a/NEWS.md b/NEWS.md index ed5bd2bb..27d46fdb 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,8 @@ ## Unreleased +- New: `--max-memory SIZE` (and the `max_memory` config key) bounds how much memory each scenario's cargo process tree may use, so that a mutant that turns a loop into an unbounded allocator is stopped by the kernel rather than taking the machine down with it. It is enforced with `setrlimit(RLIMIT_AS)`, which limits address space rather than resident memory, so set it generously. macOS does not enforce `RLIMIT_AS`, so the option is a no-op there. If the option is given and it cannot be applied, cargo-mutants fails before testing any mutant rather than running with no limit. + - New: `#[mutants::exclude_re("pattern")]` attribute to exclude specific mutations by regex, without disabling all mutations on the function. The attribute can be placed on functions, `impl` blocks, `trait` blocks, modules, files, and on expressions that can carry an attribute (such as `match`, struct literals, call expressions, method calls, and unary expressions). Multiple patterns can be applied. Also supported within `cfg_attr`. Requires the [mutants](https://crates.io/crates/mutants) crate version `0.0.5` or later. - Fixed: `#[mutants::skip]` (and `#[cfg_attr(..., mutants::skip)]`) is now honoured when placed on `const` and `static` items, including associated constants in `impl` and `trait` blocks. Previously the attribute was silently ignored on these items and operator mutants inside the initializer expression were still generated ([#508](https://github.com/sourcefrog/cargo-mutants/issues/508)). diff --git a/book/src/timeouts.md b/book/src/timeouts.md index bff23255..c775b85d 100644 --- a/book/src/timeouts.md +++ b/book/src/timeouts.md @@ -45,6 +45,53 @@ In this case you can use the `--build-timeout` or `--build-timeout-multiplier` o You might also choose to skip mutants that can cause long-running const evaluation. +## Memory limits + +A timeout is not always enough. A mutant can turn a bounded loop into an unbounded +allocator — flipping `+=` to `-=` on a parser's cursor, say — and a test that grows at +hundreds of megabytes per second can exhaust the machine long before the test timeout +arrives. On a CI runner the usual result is that the whole VM is torn down, with no log +and no record of which mutants had been tested. + +`--max-memory SIZE`, or the `max_memory` key in the configuration file, puts a ceiling on +each scenario's cargo process tree instead, so that the kernel stops the scenario rather +than the machine. Sizes may be plain byte counts, or carry a `K`, `M`, `G`, or `T` +suffix, which are binary multiples: `1K` is 1024 bytes. The smallest accepted value is +1M: unlike `--timeout=0`, `--max-memory=0` is not a way to turn the limit off, so it is +rejected rather than silently stopping every scenario. + +```shell +cargo mutants --max-memory 8G +``` + +```toml +# .cargo/mutants.toml +max_memory = "8G" +``` + +The limit is off by default, and applies to every phase of every scenario, builds +included, so leave room for the compiler as well as for the tests. + +It is enforced with `setrlimit(RLIMIT_AS)` on the cargo process, inherited by everything +it spawns. That limits *address space*, which is a much cruder proxy than resident +memory: allocators and rustc reserve far more address space than they ever make +resident, so a limit that would be comfortable as a resident-memory ceiling can fail +builds outright when applied this way. **Set it generously.** + +Which mechanism is in use is reported at startup: + +``` +INFO Limiting each scenario to 8589934592 bytes of memory using setrlimit(RLIMIT_AS) +``` + +On macOS, `RLIMIT_AS` is accepted by the kernel and then ignored, so `--max-memory` has +no effect there; cargo-mutants warns and carries on. On any platform where it cannot be +applied at all, giving `--max-memory` is an error, reported before any mutant is tested, +rather than a run that quietly had no limit. + +This option does not change how mutants are classified. A mutant whose tests are stopped +by the limit fails its tests and so is caught, in just the same way as one that panics. + ## Exceptions The multiplier timeout options cannot be used when the baseline is skipped diff --git a/examples/custom_config.toml b/examples/custom_config.toml index 7ae4322b..96aa73f1 100644 --- a/examples/custom_config.toml +++ b/examples/custom_config.toml @@ -30,6 +30,11 @@ timeout_multiplier = 2.0 # Minimum test timeout in seconds minimum_test_timeout = 60.0 +# Maximum memory for each scenario, so that a mutant that allocates without bound is +# stopped by the kernel rather than by the machine running out. Suffixes are binary +# multiples: "1K" is 1024 bytes. +max_memory = "8G" + # Copy VCS directories (.git, etc.) to build directories copy_vcs = true diff --git a/src/cargo.rs b/src/cargo.rs index 909dd881..b367577e 100644 --- a/src/cargo.rs +++ b/src/cargo.rs @@ -20,6 +20,7 @@ use crate::options::{Options, TestTool}; use crate::outcome::{Phase, PhaseResult}; use crate::output::ScenarioOutput; use crate::package::PackageSelection; +use crate::process::memory::MemoryLimit; use crate::process::{Exit, Process}; // Allowed nextest codes (those will be considered a mutation caught / ignored without a warning) @@ -37,6 +38,7 @@ pub fn run_cargo( packages: &PackageSelection, phase: Phase, timeout: Option, + memory_limit: Option<&MemoryLimit>, scenario_output: &mut ScenarioOutput, options: &Options, console: &Console, @@ -61,6 +63,7 @@ pub fn run_cargo( build_dir.path(), timeout, jobserver, + memory_limit, scenario_output, console, )?; diff --git a/src/config.rs b/src/config.rs index 7271192b..0bcee76b 100644 --- a/src/config.rs +++ b/src/config.rs @@ -64,6 +64,8 @@ pub struct Config { /// Space or comma separated list of features to activate. pub features: Vec, + /// Maximum memory for each scenario, e.g. "4G"; suffixes are binary multiples. + pub max_memory: Option, /// Minimum test timeout, in seconds, as a floor on the autoset value. pub minimum_test_timeout: Option, /// Do not activate the `default` feature. diff --git a/src/lab.rs b/src/lab.rs index 784f7d1a..8e6d830f 100644 --- a/src/lab.rs +++ b/src/lab.rs @@ -18,8 +18,8 @@ use tracing::{debug, debug_span, error, trace, warn}; use crate::{ BaselineStrategy, BuildDir, Console, Context, Mutant, Options, Phase, Result, Scenario, ScenarioOutcome, cargo::run_cargo, options::TestPackages, outcome::LabOutcome, - output::OutputDir, package::Package, package::PackageSelection, timeouts::Timeouts, - workspace::Workspace, + output::OutputDir, package::Package, package::PackageSelection, process::memory::MemoryLimit, + timeouts::Timeouts, workspace::Workspace, }; /// Run all possible mutation experiments. @@ -38,6 +38,9 @@ pub fn test_mutants( ) -> Result { let start_time = Instant::now(); console.set_debug_log(output_dir.open_debug_log()?); + // Fail here, before the tree is copied, rather than after a long run that silently + // had no limit. + let memory_limit = options.max_memory.map(MemoryLimit::new).transpose()?; if options.shuffle { fastrand::shuffle(&mut mutants); } @@ -62,6 +65,7 @@ pub fn test_mutants( let lab = Lab { output_mutex, jobserver, + memory_limit, tests_for_mutant, options, console, @@ -164,6 +168,7 @@ fn join_threads(threads: Vec>>) -> Resul struct Lab<'a> { output_mutex: Mutex, jobserver: Option, + memory_limit: Option, tests_for_mutant: TestsForMutant, options: &'a Options, console: &'a Console, @@ -207,6 +212,7 @@ impl Lab<'_> { build_dir, output_mutex: &self.output_mutex, jobserver: self.jobserver.as_ref(), + memory_limit: self.memory_limit.as_ref(), tests_for_mutant: &self.tests_for_mutant, options: self.options, console: self.console, @@ -222,6 +228,7 @@ struct Worker<'a> { build_dir: &'a BuildDir, output_mutex: &'a Mutex, jobserver: Option<&'a jobserver::Client>, + memory_limit: Option<&'a MemoryLimit>, tests_for_mutant: &'a TestsForMutant, options: &'a Options, console: &'a Console, @@ -289,6 +296,7 @@ impl Worker<'_> { test_packages, phase, timeout, + self.memory_limit, &mut scenario_output, self.options, self.console, diff --git a/src/main.rs b/src/main.rs index 3e771564..6cfdb4b9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -291,6 +291,13 @@ pub struct Args { #[arg(long, help_heading = "Execution")] list: bool, + /// Maximum memory for each scenario, e.g. 4G: a mutant that exceeds it is stopped + /// + /// Sizes may be given in bytes, or with a `K`, `M`, `G`, or `T` suffix, which are + /// binary multiples: `1K` is 1024 bytes. Enforced on Linux only; see the manual. + #[arg(long, help_heading = "Execution", value_name = "SIZE")] + max_memory: Option, + /// List source files, don't run anything. #[arg(long, help_heading = "Execution")] list_files: bool, diff --git a/src/options.rs b/src/options.rs index b983fb98..58e3a75b 100644 --- a/src/options.rs +++ b/src/options.rs @@ -13,7 +13,7 @@ use std::env; use std::ffi::OsString; use std::time::Duration; -use anyhow::Context; +use anyhow::{Context, bail}; use camino::{Utf8Path, Utf8PathBuf}; use clap::ArgAction; use globset::GlobSet; @@ -96,6 +96,9 @@ pub struct Options { /// The minimum test timeout, as a floor on the autoset value. pub minimum_test_timeout: Duration, + /// The maximum memory for each scenario's process tree, in bytes, if set. + pub max_memory: Option, + pub print_caught: bool, pub print_unviable: bool, @@ -237,6 +240,47 @@ fn join_slices(a: &[String], b: &[String]) -> Vec { a.iter().chain(b).cloned().collect() } +/// The smallest `--max-memory` worth accepting. +/// +/// Anything near zero stops every scenario before it can do anything, which is never +/// what someone means. In particular `--max-memory=0` is not "no limit", unlike `-t 0`. +const MIN_MAX_MEMORY: u64 = 1 << 20; + +/// Parse a memory size like `256M`, `2GiB`, or a plain count of bytes. +/// +/// Suffixes are binary multiples, as they conventionally are for memory: `1K` is 1024 +/// bytes, not 1000. +fn parse_size(s: &str) -> Result { + let s = s.trim(); + let digits_end = s.find(|c: char| !c.is_ascii_digit()).unwrap_or(s.len()); + let (digits, suffix) = s.split_at(digits_end); + let number: u64 = digits + .parse() + .with_context(|| format!("{s:?} does not start with a number of bytes"))?; + let multiple: u64 = match suffix.trim().to_ascii_lowercase().as_str() { + "" | "b" => 1, + "k" | "kb" | "kib" => 1 << 10, + "m" | "mb" | "mib" => 1 << 20, + "g" | "gb" | "gib" => 1 << 30, + "t" | "tb" | "tib" => 1 << 40, + _ => bail!("unrecognized size suffix {suffix:?} in {s:?}"), + }; + number + .checked_mul(multiple) + .with_context(|| format!("size {s:?} is too large")) +} + +/// Parse and sanity-check a `--max-memory` value. +fn parse_max_memory(s: &str) -> Result { + let bytes = parse_size(s)?; + if bytes < MIN_MAX_MEMORY { + bail!( + "{s:?} is too small to run anything in: --max-memory must be at least {MIN_MAX_MEMORY} bytes" + ); + } + Ok(bytes) +} + /// Should ANSI colors be drawn? #[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Display, Deserialize, ValueEnum)] #[strum(serialize_all = "snake_case")] @@ -366,6 +410,13 @@ impl Options { jobserver: args.jobserver, jobserver_tasks: args.jobserver_tasks, leak_dirs: args.leak_dirs, + max_memory: args + .max_memory + .as_deref() + .or(config.max_memory.as_deref()) + .map(parse_max_memory) + .transpose() + .context("Failed to parse --max-memory")?, minimum_test_timeout, no_default_features: args.no_default_features || config.no_default_features.unwrap_or(false), @@ -570,6 +621,76 @@ mod test { assert_eq!(options.build_timeout_multiplier, Some(3.5)); } + #[test] + fn parse_size_understands_binary_suffixes_and_rejects_nonsense() { + // Input -> bytes, where None means it should not parse at all. + let cases = [ + ("0", Some(0)), + ("1024", Some(1024)), + ("1024B", Some(1024)), + ("256M", Some(256 * 1024 * 1024)), + ("256MiB", Some(256 * 1024 * 1024)), + (" 4g ", Some(4 * 1024 * 1024 * 1024)), + ("2T", Some(2 * (1u64 << 40))), + ("", None), + ("M", None), + ("-1", None), + ("1.5G", None), + ("1 zettabyte", None), + ("18446744073709551615K", None), + ]; + for (input, expected) in cases { + assert_eq!(parse_size(input).ok(), expected, "input: {input:?}"); + } + } + + /// Zero is a footgun rather than a way to turn the limit off, unlike `-t 0`. + #[test] + fn parse_max_memory_rejects_uselessly_small_limits() { + for tiny in ["0", "1", "1K", "1023K"] { + assert!( + parse_max_memory(tiny).is_err(), + "{tiny:?} should be rejected as too small" + ); + } + assert_eq!(parse_max_memory("1M").ok(), Some(1 << 20)); + } + + #[test] + fn options_from_max_memory_arg() -> Result<(), Box> { + let args = Args::parse_from(["mutants", "--max-memory=256M"]); + let options = Options::new(&args, &Config::default())?; + assert_eq!(options.max_memory, Some(256 * 1024 * 1024)); + + let args = Args::parse_from(["mutants"]); + let options = Options::new(&args, &Config::default())?; + assert_eq!(options.max_memory, None); + Ok(()) + } + + #[test] + fn cli_max_memory_overrides_config() -> Result<(), Box> { + let config: Config = "max_memory = \"8G\"".parse()?; + let options = Options::new(&Args::parse_from(["mutants"]), &config)?; + assert_eq!(options.max_memory, Some(8 * 1024 * 1024 * 1024)); + + let args = Args::parse_from(["mutants", "--max-memory=1G"]); + let options = Options::new(&args, &config)?; + assert_eq!(options.max_memory, Some(1024 * 1024 * 1024)); + Ok(()) + } + + #[test] + fn unparseable_max_memory_is_an_error() { + let args = Args::parse_from(["mutants", "--max-memory=lots"]); + let err = Options::new(&args, &Config::default()) + .expect_err("--max-memory=lots should not be accepted"); + assert!( + format!("{err:#}").contains("--max-memory"), + "unhelpful error message: {err:#}" + ); + } + #[test] fn cli_timeout_multiplier_overrides_config() { let config = indoc! { r" diff --git a/src/process.rs b/src/process.rs index 0763e617..656406ca 100644 --- a/src/process.rs +++ b/src/process.rs @@ -36,6 +36,9 @@ mod unix; #[cfg(unix)] use unix::{configure_command, terminate_child}; +pub mod memory; +use memory::MemoryLimit; + pub struct Process { child: Child, start: Instant, @@ -45,16 +48,26 @@ pub struct Process { impl Process { /// Run a subprocess to completion, watching for interrupts, with a timeout, while /// ticking the progress bar. + #[allow(clippy::too_many_arguments)] // parallel to run_cargo pub fn run( argv: &[String], env: &[(String, String)], cwd: &Utf8Path, timeout: Option, jobserver: Option<&jobserver::Client>, + memory_limit: Option<&MemoryLimit>, scenario_output: &mut ScenarioOutput, console: &Console, ) -> Result { - let mut child = Process::start(argv, env, cwd, timeout, jobserver, scenario_output)?; + let mut child = Process::start( + argv, + env, + cwd, + timeout, + jobserver, + memory_limit, + scenario_output, + )?; let process_status = loop { if let Some(exit_status) = child.poll()? { break exit_status; @@ -67,12 +80,14 @@ impl Process { } /// Launch a process, and return an object representing the child. + #[allow(clippy::too_many_arguments)] // parallel to run_cargo pub fn start( argv: &[String], env: &[(String, String)], cwd: &Utf8Path, timeout: Option, jobserver: Option<&jobserver::Client>, + memory_limit: Option<&MemoryLimit>, scenario_output: &mut ScenarioOutput, ) -> Result { let start = Instant::now(); @@ -92,6 +107,9 @@ impl Process { js.configure(&mut command); } configure_command(&mut command); + if let Some(memory_limit) = memory_limit { + memory_limit.configure_command(&mut command); + } let child = command .spawn() .with_context(|| format!("failed to spawn {}", argv.join(" ")))?; diff --git a/src/process/memory.rs b/src/process/memory.rs new file mode 100644 index 00000000..1ca05dd2 --- /dev/null +++ b/src/process/memory.rs @@ -0,0 +1,201 @@ +// Copyright 2026 Martin Pool + +//! Bound how much memory one scenario's process tree can use. +//! +//! A mutant can turn a bounded loop into an unbounded allocator, and a test process that +//! grows at hundreds of MB/s can exhaust the machine before the test timeout arrives. +//! `--max-memory` puts a ceiling on each scenario instead, so the kernel stops the +//! scenario rather than the machine. +//! +//! The mechanism here is `setrlimit(RLIMIT_AS)`, which limits the *address space* of each +//! process in the tree. That is a crude proxy for memory use -- allocators reserve far +//! more address space than they ever make resident -- so the limit has to be set +//! generously, and it is only enforced on Linux. +//! +//! `any(target_os = "linux", target_os = "android", target_os = "macos")` recurs below: +//! it is where `nix` exposes `RLIMIT_AS` and cargo-mutants is supported. Everywhere else +//! the `RlimitAs` variant does not exist at all, which is what makes it unconstructible +//! rather than merely unreachable. + +use std::process::Command; + +use anyhow::bail; +use tracing::{info, warn}; + +use crate::Result; + +/// How a `--max-memory` limit is applied to a scenario's process tree. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum MemoryMechanism { + /// `setrlimit(RLIMIT_AS)` on the cargo process, inherited by everything it spawns. + RlimitAs, + /// Nothing on this platform enforces a memory limit, so the option does nothing. + Unenforced, +} + +impl MemoryMechanism { + fn describe(self) -> &'static str { + match self { + MemoryMechanism::RlimitAs => "setrlimit(RLIMIT_AS)", + MemoryMechanism::Unenforced => "no enforced mechanism", + } + } +} + +/// Choose how to apply `--max-memory`, from what this platform and process can offer. +/// +/// `settable` says whether `RLIMIT_AS` can be set to the requested limit at all; +/// `enforced` says whether the kernel would then act on it, which macOS does not. +/// +/// Returns an error, rather than quietly running unlimited, when the user asked for a +/// limit and nothing can enforce it. +pub fn choose_mechanism(settable: bool, enforced: bool) -> Result { + if settable && enforced { + Ok(MemoryMechanism::RlimitAs) + } else if settable { + Ok(MemoryMechanism::Unenforced) + } else { + bail!( + "--max-memory was requested but no mechanism on this platform can enforce it: \ + cargo-mutants can use setrlimit(RLIMIT_AS), and it is not available" + ) + } +} + +/// A per-scenario memory limit, set up once and used for every phase of every scenario. +/// +/// Each variant owns exactly the state its mechanism needs. +#[derive(Debug)] +pub enum MemoryLimit { + #[cfg(any(target_os = "linux", target_os = "android", target_os = "macos"))] + RlimitAs { + bytes: u64, + }, + Unenforced, +} + +impl MemoryLimit { + /// Set up a limit of `bytes` per scenario, or fail if nothing here can enforce one. + /// + /// This is done once, before any mutant is tested, so that an unenforceable limit is + /// an error the user sees immediately rather than a run that silently had no limit. + pub fn new(bytes: u64) -> Result { + let limit = match choose_mechanism(rlimit::settable(bytes), rlimit::ENFORCED)? { + #[cfg(any(target_os = "linux", target_os = "android", target_os = "macos"))] + MemoryMechanism::RlimitAs => MemoryLimit::RlimitAs { bytes }, + _ => MemoryLimit::Unenforced, + }; + if limit.mechanism() == MemoryMechanism::Unenforced { + warn!( + "--max-memory has no effect on this platform: RLIMIT_AS is accepted but not enforced here" + ); + } else { + info!( + "Limiting each scenario to {bytes} bytes of memory using {}", + limit.mechanism().describe() + ); + } + Ok(limit) + } + + fn mechanism(&self) -> MemoryMechanism { + match self { + #[cfg(any(target_os = "linux", target_os = "android", target_os = "macos"))] + MemoryLimit::RlimitAs { .. } => MemoryMechanism::RlimitAs, + MemoryLimit::Unenforced => MemoryMechanism::Unenforced, + } + } + + /// Arrange for the command, once forked, to be subject to the limit, so that it + /// applies from the very first allocation the child makes. + pub fn configure_command(&self, command: &mut Command) { + match self { + #[cfg(any(target_os = "linux", target_os = "android", target_os = "macos"))] + MemoryLimit::RlimitAs { bytes } => rlimit::apply_to_child(command, *bytes), + MemoryLimit::Unenforced => {} + } + } +} + +#[cfg(any(target_os = "linux", target_os = "android", target_os = "macos"))] +mod rlimit { + use std::io; + use std::os::unix::process::CommandExt; + use std::process::Command; + + use nix::sys::resource::{Resource, getrlimit, setrlimit}; + use tracing::debug; + + /// Whether the kernel acts on `RLIMIT_AS`, as opposed to merely accepting it. + /// + /// macOS accepts the call and ignores it, so a limit set there would be a lie. + pub const ENFORCED: bool = cfg!(any(target_os = "linux", target_os = "android")); + + /// Whether `RLIMIT_AS` can be set to `bytes`: the inherited hard limit is the ceiling + /// on what an unprivileged process may ask for. + pub fn settable(bytes: u64) -> bool { + match getrlimit(Resource::RLIMIT_AS) { + Ok((_soft, hard)) => hard >= bytes, + Err(errno) => { + debug!(?errno, "failed to read RLIMIT_AS"); + false + } + } + } + + /// Limit the address space of the child, and so of everything it goes on to spawn. + pub fn apply_to_child(command: &mut Command, bytes: u64) { + // SAFETY: setrlimit is a bare syscall that does not allocate or take locks, so it + // is safe to call between fork and exec. + unsafe { + command.pre_exec(move || { + setrlimit(Resource::RLIMIT_AS, bytes, bytes).map_err(io::Error::from) + }); + } + } +} + +#[cfg(not(any(target_os = "linux", target_os = "android", target_os = "macos")))] +mod rlimit { + pub const ENFORCED: bool = false; + + pub fn settable(_bytes: u64) -> bool { + false + } +} + +#[cfg(test)] +mod test { + use super::{MemoryMechanism, choose_mechanism}; + + /// The mechanism is picked from what the platform can do. + #[test] + fn choose_mechanism_picks_by_availability() { + // (RLIMIT_AS settable, RLIMIT_AS enforced) -> mechanism, where None means + // --max-memory should be rejected outright. + let cases = [ + ((true, true), Some(MemoryMechanism::RlimitAs)), + // macOS: the call is accepted and then ignored. + ((true, false), Some(MemoryMechanism::Unenforced)), + ((false, false), None), + ((false, true), None), + ]; + for ((settable, enforced), expected) in cases { + assert_eq!( + choose_mechanism(settable, enforced).ok(), + expected, + "settable={settable} enforced={enforced}" + ); + } + } + + #[test] + fn choose_mechanism_with_no_usable_mechanism_names_the_option() { + let err = choose_mechanism(false, false) + .expect_err("--max-memory with no mechanism should be an error"); + assert!( + err.to_string().contains("--max-memory"), + "unhelpful error message: {err}" + ); + } +} diff --git a/tests/main.rs b/tests/main.rs index 0fb8973d..906b6f43 100644 --- a/tests/main.rs +++ b/tests/main.rs @@ -3846,3 +3846,30 @@ fn in_diff_with_nonexistent_file_returns_exit_code_6() { .code(6) .stderr(contains("Failed to read diff file").or(contains("Failed to open diff file"))); } + +/// `--max-memory=0` reads like "no limit" by analogy with `--timeout=0`, but would mean +/// "stop every scenario immediately", so it's rejected rather than obeyed. +#[test] +fn max_memory_too_small_is_rejected() { + let tmp_src_dir = copy_of_testdata("small_well_tested"); + run() + .args(["mutants", "--max-memory=0", "-d"]) + .arg(tmp_src_dir.path()) + .assert() + .failure() + .stderr(contains("--max-memory must be at least")); +} + +/// A limit generous enough for the compiler doesn't disturb an ordinary run. +#[cfg(unix)] +#[test] +fn max_memory_does_not_disturb_a_normal_run() { + let tmp_src_dir = copy_of_testdata("small_well_tested"); + run() + .args(["mutants", "--max-memory=8G", "-d"]) + .arg(tmp_src_dir.path()) + .timeout(OUTER_TIMEOUT) + .assert() + .success() + .stdout(contains("4 mutants tested")); +} From bd0372ced758628568759e6df0410b79188949c8 Mon Sep 17 00:00:00 2001 From: Yuri Khrustalev Date: Tue, 15 Sep 2026 22:06:52 -0400 Subject: [PATCH 4/6] feat: say why a scenario's processes died A mutant caught because its tests were killed is indistinguishable, in the counts, from one caught by a failing assertion -- which is exactly what you want to know when diagnosing a run. Carry the process group sweep's findings on each phase result, and render them, with the name of any signal that killed the phase, in parentheses on the outcome line, in the scenario log, and in outcomes.json. Classification is deliberately untouched: this only makes the reason visible. --- NEWS.md | 2 + book/src/timeouts.md | 18 ++++++++ src/cargo.rs | 3 +- src/console.rs | 4 ++ src/outcome.rs | 98 ++++++++++++++++++++++++++++++++++++++++++-- src/process.rs | 2 + src/process/unix.rs | 8 ++++ tests/main.rs | 9 +++- 8 files changed, 139 insertions(+), 5 deletions(-) diff --git a/NEWS.md b/NEWS.md index b8315cf0..3622110f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,8 @@ ## Unreleased +- New: Outcome lines, scenario logs, and `outcomes.json` now say when a phase's process was killed by a signal, and when processes left running by the tests had to be reaped. The caught / missed / unviable / timeout classification is unchanged. + - Fixed: Each cargo invocation now runs as the leader of its own process group, and that group is swept after every phase, not only after a timeout. Processes left running by a scenario's tests are `SIGTERM`ed, given a short grace period, and then `SIGKILL`ed, so they can't keep consuming memory or CPU while later mutants are tested. Unix only. - Fixed: After a timeout, cargo-mutants waits only a short grace period for the child to exit after `SIGTERM` before sending `SIGKILL`. Previously it waited indefinitely, so a process that ignored `SIGTERM`, or that had been stopped, hung the whole run. diff --git a/book/src/timeouts.md b/book/src/timeouts.md index 660cdd59..48a98566 100644 --- a/book/src/timeouts.md +++ b/book/src/timeouts.md @@ -67,6 +67,24 @@ This has no effect on how a mutant is classified; it only stops work from one sc leaking into the next. Windows has no process groups, and cargo-mutants does not yet use job objects, so this sweep is Unix-only. +## Why a scenario died + +A mutant caught because the kernel killed its tests looks, in the summary counts, exactly +like a mutant caught by a failing assertion. When there is more to say, cargo-mutants says +it in parentheses on the outcome line: + +``` +caught src/server.rs:88:5: replace listen -> bool with false (test killed by SIGABRT; test left 1 stray process behind (SIGKILLed: 30411)) in 2s build + 9s test +``` + +Two things get reported this way: the signal that killed a phase's cargo process, if it +died by one, and anything the process group sweep had to clean up. The same information +is written to the scenario's log and, in `mutants.out/outcomes.json`, to a `sweep` field +on each phase result. + +This does not change the caught / missed / unviable / timeout classification. It only +makes the reason visible. + ## Exceptions The multiplier timeout options cannot be used when the baseline is skipped diff --git a/src/cargo.rs b/src/cargo.rs index 0309fd27..1b094002 100644 --- a/src/cargo.rs +++ b/src/cargo.rs @@ -55,7 +55,7 @@ pub fn run_cargo( debug!(?encoded_rustflags); env.push(("CARGO_ENCODED_RUSTFLAGS".to_owned(), encoded_rustflags)); } - let (process_status, _sweep) = Process::run( + let (process_status, sweep) = Process::run( &argv, &env, build_dir.path(), @@ -79,6 +79,7 @@ pub fn run_cargo( duration: start.elapsed(), process_status, argv, + sweep, }) } diff --git a/src/console.rs b/src/console.rs index 3c2c0cc2..970b4516 100644 --- a/src/console.rs +++ b/src/console.rs @@ -90,6 +90,10 @@ impl Console { style_outcome(outcome), style_scenario(scenario, true), ); + let death_reasons = outcome.death_reasons(); + if !death_reasons.is_empty() { + write!(s, " ({})", death_reasons.join("; ")).expect("format death reasons"); + } if options.show_times { let prs: Vec = outcome .phase_results() diff --git a/src/outcome.rs b/src/outcome.rs index ec5b7861..ed3a1700 100644 --- a/src/outcome.rs +++ b/src/outcome.rs @@ -17,7 +17,9 @@ use tracing::warn; use crate::console::{format_duration, plural}; use crate::exit_code::ExitCode; -use crate::process::Exit; +#[cfg(unix)] +use crate::process::signal_name; +use crate::process::{Exit, Sweep}; use crate::{Options, Result, Scenario, output}; /// What phase of running a scenario. @@ -245,6 +247,19 @@ impl ScenarioOutcome { .any(|pr| pr.phase != Phase::Test && pr.process_status.is_failure()) } + /// Say, for each phase that has something unusual to report, how its process tree + /// ended: killed by a signal, or leaving stray processes behind. + /// + /// This has no bearing on how the mutant is classified. It exists so that a mutant + /// caught because the kernel killed its tests can be told apart from one caught by a + /// failing assertion. + pub fn death_reasons(&self) -> Vec { + self.phase_results + .iter() + .flat_map(PhaseResult::death_reasons) + .collect() + } + /// True if this outcome is a caught mutant: it's a mutant and the tests failed. pub fn mutant_caught(&self) -> bool { self.scenario.is_mutant() @@ -303,12 +318,27 @@ pub struct PhaseResult { pub process_status: Exit, /// What command was run, as an argv list. pub argv: Vec, + /// What the sweep of the child's process group found and did. + pub sweep: Sweep, } impl PhaseResult { pub fn is_success(&self) -> bool { self.process_status.is_success() } + + fn death_reasons(&self) -> Vec { + let phase = self.phase.name(); + let mut reasons = Vec::new(); + #[cfg(unix)] + if let Exit::Signalled(signal) = self.process_status { + reasons.push(format!("{phase} killed by {}", signal_name(signal))); + } + if let Some(sweep) = self.sweep.describe() { + reasons.push(format!("{phase} {sweep}")); + } + reasons + } } impl Serialize for PhaseResult { @@ -316,11 +346,12 @@ impl Serialize for PhaseResult { where S: Serializer, { - let mut ss = serializer.serialize_struct("PhaseResult", 4)?; + let mut ss = serializer.serialize_struct("PhaseResult", 5)?; ss.serialize_field("phase", &self.phase)?; ss.serialize_field("duration", &self.duration.as_secs_f64())?; ss.serialize_field("process_status", &self.process_status)?; ss.serialize_field("argv", &self.argv)?; + ss.serialize_field("sweep", &self.sweep)?; ss.end() } } @@ -341,10 +372,68 @@ pub enum SummaryOutcome { mod test { use std::time::Duration; - use crate::process::Exit; + use crate::process::{Exit, Sweep}; use super::{Phase, PhaseResult, Scenario, ScenarioOutcome}; + fn phase_result(phase: Phase, process_status: Exit, sweep: Sweep) -> PhaseResult { + PhaseResult { + phase, + duration: Duration::from_secs(1), + process_status, + argv: vec!["cargo".into(), "test".into()], + sweep, + } + } + + fn outcome_of(phase_results: Vec) -> ScenarioOutcome { + ScenarioOutcome { + output_dir: "output".into(), + log_path: "log".into(), + diff_path: None, + scenario: Scenario::Baseline, + phase_results, + } + } + + #[test] + fn no_death_reasons_for_an_ordinary_test_failure() { + let outcome = outcome_of(vec![phase_result( + Phase::Test, + Exit::Failure(101), + Sweep::default(), + )]); + assert_eq!(outcome.death_reasons(), Vec::::new()); + } + + #[cfg(unix)] + #[test] + fn death_reasons_name_the_signal_that_killed_the_phase() { + let outcome = outcome_of(vec![phase_result( + Phase::Test, + Exit::Signalled(9), + Sweep::default(), + )]); + assert_eq!(outcome.death_reasons(), ["test killed by SIGKILL"]); + } + + #[test] + fn death_reasons_name_processes_left_behind_by_the_tests() { + let outcome = outcome_of(vec![phase_result( + Phase::Test, + Exit::Success, + Sweep { + pids: Some(vec![101, 102]), + strays: true, + killed: true, + }, + )]); + assert_eq!( + outcome.death_reasons(), + ["test left 2 stray processes behind (SIGKILLed: 101, 102)"] + ); + } + #[test] fn find_phase_result() { let outcome = ScenarioOutcome { @@ -358,12 +447,14 @@ mod test { duration: Duration::from_secs(2), process_status: Exit::Success, argv: vec!["cargo".into(), "build".into()], + sweep: Sweep::default(), }, PhaseResult { phase: Phase::Test, duration: Duration::from_secs(3), process_status: Exit::Success, argv: vec!["cargo".into(), "test".into()], + sweep: Sweep::default(), }, ], }; @@ -374,6 +465,7 @@ mod test { duration: Duration::from_secs(2), process_status: Exit::Success, argv: vec!["cargo".into(), "build".into()], + sweep: Sweep::default(), }) ); assert_eq!( diff --git a/src/process.rs b/src/process.rs index 1903bc9a..9db74ec1 100644 --- a/src/process.rs +++ b/src/process.rs @@ -42,6 +42,8 @@ use windows::{configure_command, kill_child, sweep_process_group, terminate_chil #[cfg(unix)] mod unix; #[cfg(unix)] +pub use unix::signal_name; +#[cfg(unix)] use unix::{configure_command, kill_child, sweep_process_group, terminate_child}; /// What sweeping a finished child's process group found and did. diff --git a/src/process/unix.rs b/src/process/unix.rs index 61060255..2098dae3 100644 --- a/src/process/unix.rs +++ b/src/process/unix.rs @@ -84,6 +84,14 @@ pub(super) fn sweep_process_group(child: &Child) -> Result { }) } +/// The name of a signal, like `SIGKILL`, or its number if we don't recognize it. +pub fn signal_name(signal: i32) -> String { + Signal::try_from(signal).map_or_else( + |_| format!("signal {signal}"), + |signal| signal.as_str().to_owned(), + ) +} + /// The process group id of a child, which (because we start it with `process_group(0)`) /// is the same as its pid. /// diff --git a/tests/main.rs b/tests/main.rs index 627b41cf..8241caa3 100644 --- a/tests/main.rs +++ b/tests/main.rs @@ -3865,7 +3865,7 @@ fn processes_spawned_by_tests_are_swept_after_each_scenario() let pid_file = pid_dir.path().join("pids.txt"); let assert = run() .arg("mutants") - .args(["--timeout=60", "--build-timeout=120", "-L", "debug"]) + .args(["--timeout=60", "--build-timeout=120", "-L", "debug", "-v"]) .env("BACKGROUND_CHILD_PID_FILE", &pid_file) .current_dir(tmp_src_dir.path()) .timeout(OUTER_TIMEOUT) @@ -3899,6 +3899,13 @@ fn processes_spawned_by_tests_are_swept_after_each_scenario() "processes spawned by the tests were still running after cargo-mutants finished" ); + // The outcome line should say what was left behind, so that a scenario that leaks + // processes is visible rather than silent. + assert!( + stdout.contains("stray process"), + "no mention of the stray processes in:\n{stdout}" + ); + // Sweeping the process group must not change any verdict. assert_eq!( outcome_json_counts(&tmp_src_dir), From d3f54810f942d45b9a91b838146ef933580c364b Mon Sep 17 00:00:00 2001 From: Yuri Khrustalev Date: Tue, 15 Sep 2026 22:05:26 -0400 Subject: [PATCH 5/6] feat: prefer cgroup v2 memory.max for --max-memory RLIMIT_AS limits address space, not resident memory, which is a poor proxy: rustc reserves far more than it makes resident, so a limit tight enough to stop a runaway test can fail the build instead. A cgroup v2 memory.max per scenario limits what we actually care about. Finding somewhere to create those cgroups is the fiddly part, because the kernel won't let a cgroup that holds processes delegate the memory controller to its children. We try our own cgroup, then our parent -- which already delegates memory whenever something has fenced us in with a memory.max, the case this is meant for -- and only as a last resort move ourselves into a leaf. Using the parent puts scenario cgroups outside that outer fence, so that path warns. The cgroup is removed when the scenario's limit is dropped, so no failure part-way through a phase can leak it. --- NEWS.md | 2 +- book/src/timeouts.md | 43 ++- src/process.rs | 12 +- src/process/memory.rs | 150 ++++++++-- src/process/memory/cgroup.rs | 265 ++++++++++++++++++ testdata/unbounded_allocation/Cargo_test.toml | 11 + testdata/unbounded_allocation/src/lib.rs | 29 ++ tests/main.rs | 71 +++++ 8 files changed, 541 insertions(+), 42 deletions(-) create mode 100644 src/process/memory/cgroup.rs create mode 100644 testdata/unbounded_allocation/Cargo_test.toml create mode 100644 testdata/unbounded_allocation/src/lib.rs diff --git a/NEWS.md b/NEWS.md index 27d46fdb..79aa1c0e 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,7 +2,7 @@ ## Unreleased -- New: `--max-memory SIZE` (and the `max_memory` config key) bounds how much memory each scenario's cargo process tree may use, so that a mutant that turns a loop into an unbounded allocator is stopped by the kernel rather than taking the machine down with it. It is enforced with `setrlimit(RLIMIT_AS)`, which limits address space rather than resident memory, so set it generously. macOS does not enforce `RLIMIT_AS`, so the option is a no-op there. If the option is given and it cannot be applied, cargo-mutants fails before testing any mutant rather than running with no limit. +- New: `--max-memory SIZE` (and the `max_memory` config key) bounds how much memory each scenario's cargo process tree may use, so that a mutant that turns a loop into an unbounded allocator is stopped by the kernel rather than taking the machine down with it. On Linux it uses a cgroup v2 `memory.max` where a writable cgroup is available, and otherwise `setrlimit(RLIMIT_AS)`; the mechanism in use is logged. macOS does not enforce `RLIMIT_AS`, so the option is a no-op there. If the option is given and it cannot be applied, cargo-mutants fails before testing any mutant rather than running with no limit. - New: `#[mutants::exclude_re("pattern")]` attribute to exclude specific mutations by regex, without disabling all mutations on the function. The attribute can be placed on functions, `impl` blocks, `trait` blocks, modules, files, and on expressions that can carry an attribute (such as `match`, struct literals, call expressions, method calls, and unary expressions). Multiple patterns can be applied. Also supported within `cfg_attr`. Requires the [mutants](https://crates.io/crates/mutants) crate version `0.0.5` or later. diff --git a/book/src/timeouts.md b/book/src/timeouts.md index c775b85d..cf5afd04 100644 --- a/book/src/timeouts.md +++ b/book/src/timeouts.md @@ -72,22 +72,43 @@ max_memory = "8G" The limit is off by default, and applies to every phase of every scenario, builds included, so leave room for the compiler as well as for the tests. -It is enforced with `setrlimit(RLIMIT_AS)` on the cargo process, inherited by everything -it spawns. That limits *address space*, which is a much cruder proxy than resident -memory: allocators and rustc reserve far more address space than they ever make -resident, so a limit that would be comfortable as a resident-memory ceiling can fail -builds outright when applied this way. **Set it generously.** +Two mechanisms can enforce it, and they are not equivalent: -Which mechanism is in use is reported at startup: +* **cgroup v2** `memory.max`, on a cgroup created for each scenario. This limits + *resident* memory for the whole process tree, which is what you actually care about. + It is preferred whenever a writable cgroup is available. Swap is also capped, where the + kernel accounts for it; on kernels that do not, the limit covers resident memory only. + +* **`setrlimit(RLIMIT_AS)`** on the cargo process, inherited by everything it spawns. + This limits *address space*, a much cruder proxy: allocators and rustc reserve far more + address space than they ever make resident, so a limit that is comfortable as a + resident-memory ceiling can fail builds outright when applied this way. If cargo-mutants + falls back to this mechanism, set the limit generously. + +Which one is in use is reported at startup, for example: ``` -INFO Limiting each scenario to 8589934592 bytes of memory using setrlimit(RLIMIT_AS) +INFO Limiting each scenario to 8589934592 bytes of memory using cgroup v2 memory.max ``` -On macOS, `RLIMIT_AS` is accepted by the kernel and then ignored, so `--max-memory` has -no effect there; cargo-mutants warns and carries on. On any platform where it cannot be -applied at all, giving `--max-memory` is an error, reported before any mutant is tested, -rather than a run that quietly had no limit. +For the cgroup mechanism, cargo-mutants needs somewhere it may create child cgroups with +`memory.max`. It looks at its own cgroup first, writing `+memory` to that cgroup's +`cgroup.subtree_control` if it isn't set already. Failing that it looks at the parent, +which works when something has already put a `memory.max` fence around cargo-mutants — a +CI shard running under a memory-limited systemd scope or container, for instance. As a +last resort it moves itself into a `cargo-mutants-supervisor` cgroup of its own so that +its original cgroup can delegate the memory controller. + +> **The parent fallback escapes an enclosing limit.** Scenario cgroups made under the +> parent are *siblings* of cargo-mutants' own cgroup, so a `memory.max` set on +> cargo-mutants does not contain them: with `--jobs N` the run can use up to N × +> `--max-memory` in total, whatever that outer fence says. cargo-mutants warns when it +> takes this path. + +On macOS, `RLIMIT_AS` is accepted by the kernel and then ignored, and cgroups do not +exist, so `--max-memory` has no effect there; cargo-mutants warns and carries on. On any +platform where *neither* mechanism can be applied, giving `--max-memory` is an error, +reported before any mutant is tested, rather than a run that quietly had no limit. This option does not change how mutants are classified. A mutant whose tests are stopped by the limit fails its tests and so is caught, in just the same way as one that panics. diff --git a/src/process.rs b/src/process.rs index 656406ca..2d3b2678 100644 --- a/src/process.rs +++ b/src/process.rs @@ -37,12 +37,16 @@ mod unix; use unix::{configure_command, terminate_child}; pub mod memory; -use memory::MemoryLimit; +use memory::{MemoryLimit, ScenarioMemoryLimit}; pub struct Process { child: Child, start: Instant, timeout: Option, + /// The memory limit in force for this process tree, if any. Held so that any cgroup + /// outlives the child and is removed once it has exited. + #[allow(dead_code)] // its Drop is the point + memory: Option, } impl Process { @@ -107,8 +111,9 @@ impl Process { js.configure(&mut command); } configure_command(&mut command); - if let Some(memory_limit) = memory_limit { - memory_limit.configure_command(&mut command); + let memory = memory_limit.map(MemoryLimit::start).transpose()?; + if let Some(memory) = &memory { + memory.configure_command(&mut command)?; } let child = command .spawn() @@ -117,6 +122,7 @@ impl Process { child, start, timeout, + memory, }) } diff --git a/src/process/memory.rs b/src/process/memory.rs index 1ca05dd2..5b0ef63e 100644 --- a/src/process/memory.rs +++ b/src/process/memory.rs @@ -7,19 +7,29 @@ //! `--max-memory` puts a ceiling on each scenario instead, so the kernel stops the //! scenario rather than the machine. //! -//! The mechanism here is `setrlimit(RLIMIT_AS)`, which limits the *address space* of each -//! process in the tree. That is a crude proxy for memory use -- allocators reserve far -//! more address space than they ever make resident -- so the limit has to be set -//! generously, and it is only enforced on Linux. +//! Two mechanisms can do this, and they are not equivalent: +//! +//! * cgroup v2 `memory.max`, which limits *resident* memory for the whole process tree. +//! This is what we want, when we can get it. +//! * `setrlimit(RLIMIT_AS)`, which limits the *address space* of each process. It is a +//! cruder proxy -- allocators reserve far more address space than they use -- so the +//! limit has to be set generously, and it is only enforced on Linux. //! //! `any(target_os = "linux", target_os = "android", target_os = "macos")` recurs below: //! it is where `nix` exposes `RLIMIT_AS` and cargo-mutants is supported. Everywhere else //! the `RlimitAs` variant does not exist at all, which is what makes it unconstructible //! rather than merely unreachable. +#[cfg(target_os = "linux")] +mod cgroup; + use std::process::Command; +#[cfg(target_os = "linux")] +use anyhow::Context; use anyhow::bail; +#[cfg(target_os = "linux")] +use tracing::debug; use tracing::{info, warn}; use crate::Result; @@ -27,6 +37,8 @@ use crate::Result; /// How a `--max-memory` limit is applied to a scenario's process tree. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum MemoryMechanism { + /// A cgroup v2 `memory.max` on a cgroup created for each scenario. + CgroupV2, /// `setrlimit(RLIMIT_AS)` on the cargo process, inherited by everything it spawns. RlimitAs, /// Nothing on this platform enforces a memory limit, so the option does nothing. @@ -36,6 +48,7 @@ pub enum MemoryMechanism { impl MemoryMechanism { fn describe(self) -> &'static str { match self { + MemoryMechanism::CgroupV2 => "cgroup v2 memory.max", MemoryMechanism::RlimitAs => "setrlimit(RLIMIT_AS)", MemoryMechanism::Unenforced => "no enforced mechanism", } @@ -49,15 +62,21 @@ impl MemoryMechanism { /// /// Returns an error, rather than quietly running unlimited, when the user asked for a /// limit and nothing can enforce it. -pub fn choose_mechanism(settable: bool, enforced: bool) -> Result { - if settable && enforced { +pub fn choose_mechanism( + cgroup_available: bool, + settable: bool, + enforced: bool, +) -> Result { + if cgroup_available { + Ok(MemoryMechanism::CgroupV2) + } else if settable && enforced { Ok(MemoryMechanism::RlimitAs) } else if settable { Ok(MemoryMechanism::Unenforced) } else { bail!( "--max-memory was requested but no mechanism on this platform can enforce it: \ - cargo-mutants can use setrlimit(RLIMIT_AS), and it is not available" + cargo-mutants can use cgroup v2 or setrlimit(RLIMIT_AS), and neither is available" ) } } @@ -67,6 +86,11 @@ pub fn choose_mechanism(settable: bool, enforced: bool) -> Result Result { - let limit = match choose_mechanism(rlimit::settable(bytes), rlimit::ENFORCED)? { - #[cfg(any(target_os = "linux", target_os = "android", target_os = "macos"))] - MemoryMechanism::RlimitAs => MemoryLimit::RlimitAs { bytes }, - _ => MemoryLimit::Unenforced, - }; + let limit = MemoryLimit::detect(bytes)?; if limit.mechanism() == MemoryMechanism::Unenforced { warn!( - "--max-memory has no effect on this platform: RLIMIT_AS is accepted but not enforced here" + "--max-memory has no effect on this platform: RLIMIT_AS is accepted but not enforced here, and cgroups are not available" ); } else { info!( @@ -98,22 +118,96 @@ impl MemoryLimit { Ok(limit) } + #[cfg(target_os = "linux")] + fn detect(bytes: u64) -> Result { + match cgroup::CgroupTree::probe(bytes) { + Ok(tree) => Ok(MemoryLimit::CgroupV2 { bytes, tree }), + Err(err) => { + debug!(?err, "cgroup v2 memory limits are not available"); + MemoryLimit::without_cgroups(bytes) + } + } + } + + #[cfg(not(target_os = "linux"))] + fn detect(bytes: u64) -> Result { + MemoryLimit::without_cgroups(bytes) + } + + /// The limit to fall back to when no cgroup can be used. + fn without_cgroups(bytes: u64) -> Result { + match choose_mechanism(false, rlimit::settable(bytes), rlimit::ENFORCED)? { + #[cfg(any(target_os = "linux", target_os = "android", target_os = "macos"))] + MemoryMechanism::RlimitAs => Ok(MemoryLimit::RlimitAs { bytes }), + // We passed `false` for cgroups, so `CgroupV2` cannot come back; folding it in + // here keeps that a dead branch rather than a panic. + _ => Ok(MemoryLimit::Unenforced), + } + } + fn mechanism(&self) -> MemoryMechanism { match self { + #[cfg(target_os = "linux")] + MemoryLimit::CgroupV2 { .. } => MemoryMechanism::CgroupV2, #[cfg(any(target_os = "linux", target_os = "android", target_os = "macos"))] MemoryLimit::RlimitAs { .. } => MemoryMechanism::RlimitAs, MemoryLimit::Unenforced => MemoryMechanism::Unenforced, } } + /// Set up the limit for one scenario phase, before its command is spawned. + #[cfg_attr(not(target_os = "linux"), allow(clippy::unnecessary_wraps))] + pub fn start(&self) -> Result { + Ok(match self { + #[cfg(target_os = "linux")] + MemoryLimit::CgroupV2 { bytes, tree } => ScenarioMemoryLimit::CgroupV2( + tree.create_scenario(*bytes) + .context("create a cgroup for this scenario")?, + ), + #[cfg(any(target_os = "linux", target_os = "android", target_os = "macos"))] + MemoryLimit::RlimitAs { bytes } => ScenarioMemoryLimit::RlimitAs(*bytes), + MemoryLimit::Unenforced => ScenarioMemoryLimit::Unenforced, + }) + } +} + +/// The memory limit in force for one scenario phase. +/// +/// A cgroup is per-scenario state that has to outlive the spawn, which is why this is +/// separate from [`MemoryLimit`]; the cgroup is removed when this is dropped. +#[derive(Debug)] +pub enum ScenarioMemoryLimit { + #[cfg(target_os = "linux")] + CgroupV2(cgroup::ScenarioCgroup), + #[cfg(any(target_os = "linux", target_os = "android", target_os = "macos"))] + RlimitAs(u64), + Unenforced, +} + +impl ScenarioMemoryLimit { /// Arrange for the command, once forked, to be subject to the limit, so that it /// applies from the very first allocation the child makes. - pub fn configure_command(&self, command: &mut Command) { + #[cfg_attr(not(target_os = "linux"), allow(clippy::unnecessary_wraps))] + pub fn configure_command(&self, command: &mut Command) -> Result<()> { match self { + #[cfg(target_os = "linux")] + ScenarioMemoryLimit::CgroupV2(cgroup) => { + use std::io::Write; + use std::os::unix::process::CommandExt; + + let procs = cgroup.open_procs()?; + // SAFETY: opened before the fork, so the closure only writes to an + // already-open descriptor -- no allocation, no locks -- which is safe + // between fork and exec. "0" means "the process doing the writing". + unsafe { + command.pre_exec(move || (&procs).write_all(b"0\n")); + } + } #[cfg(any(target_os = "linux", target_os = "android", target_os = "macos"))] - MemoryLimit::RlimitAs { bytes } => rlimit::apply_to_child(command, *bytes), - MemoryLimit::Unenforced => {} + ScenarioMemoryLimit::RlimitAs(bytes) => rlimit::apply_to_child(command, *bytes), + ScenarioMemoryLimit::Unenforced => {} } + Ok(()) } } @@ -168,30 +262,32 @@ mod rlimit { mod test { use super::{MemoryMechanism, choose_mechanism}; - /// The mechanism is picked from what the platform can do. + /// The mechanism is picked from what the platform can do, in order of preference. #[test] fn choose_mechanism_picks_by_availability() { - // (RLIMIT_AS settable, RLIMIT_AS enforced) -> mechanism, where None means - // --max-memory should be rejected outright. + // (cgroups available, RLIMIT_AS settable, RLIMIT_AS enforced) -> mechanism, where + // None means --max-memory should be rejected outright. let cases = [ - ((true, true), Some(MemoryMechanism::RlimitAs)), + ((true, true, true), Some(MemoryMechanism::CgroupV2)), + ((true, false, false), Some(MemoryMechanism::CgroupV2)), + ((false, true, true), Some(MemoryMechanism::RlimitAs)), // macOS: the call is accepted and then ignored. - ((true, false), Some(MemoryMechanism::Unenforced)), - ((false, false), None), - ((false, true), None), + ((false, true, false), Some(MemoryMechanism::Unenforced)), + ((false, false, false), None), + ((false, false, true), None), ]; - for ((settable, enforced), expected) in cases { + for ((cgroup, settable, enforced), expected) in cases { assert_eq!( - choose_mechanism(settable, enforced).ok(), + choose_mechanism(cgroup, settable, enforced).ok(), expected, - "settable={settable} enforced={enforced}" + "cgroup={cgroup} settable={settable} enforced={enforced}" ); } } #[test] fn choose_mechanism_with_no_usable_mechanism_names_the_option() { - let err = choose_mechanism(false, false) + let err = choose_mechanism(false, false, false) .expect_err("--max-memory with no mechanism should be an error"); assert!( err.to_string().contains("--max-memory"), diff --git a/src/process/memory/cgroup.rs b/src/process/memory/cgroup.rs new file mode 100644 index 00000000..aa9d2366 --- /dev/null +++ b/src/process/memory/cgroup.rs @@ -0,0 +1,265 @@ +// Copyright 2026 Martin Pool + +//! Per-scenario memory limits using cgroup v2. +//! +//! Each scenario's cargo process tree gets a cgroup of its own with `memory.max` set, so +//! the kernel stops it -- and tells us that it did, through `memory.events` -- rather +//! than letting it eat the machine. +//! +//! The awkward part is finding somewhere to put those cgroups. A cgroup's children only +//! have `memory.max` if the cgroup itself lists `memory` in `cgroup.subtree_control`, and +//! the kernel refuses to set that on a cgroup that contains processes. Since cargo-mutants +//! is itself a process in its own cgroup, we look at, in order of preference: +//! +//! 1. Our own cgroup, if the memory controller is or can be delegated from it: a scenario +//! cgroup there stays inside whatever limit the operator already put on us. Making it +//! delegable writes `+memory` to our own `cgroup.subtree_control`, which is a lasting +//! change to a cgroup we do not own, though an additive one. +//! 2. Our parent, if it already delegates the memory controller -- which is exactly the +//! case when something has already put a `memory.max` fence around us. Scenario +//! cgroups are then siblings of ours and so sit *outside* that fence; see the warning +//! in [`CgroupTree::probe`]. +//! 3. Our own cgroup again, after moving ourselves down into a leaf so that it no longer +//! holds any processes. This is a visible side effect, so it's the last resort. + +use std::fs::{File, OpenOptions, create_dir, read_to_string, remove_dir, write}; +use std::io::ErrorKind; +use std::path::{Path, PathBuf}; +use std::process; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::thread::sleep; +use std::time::Duration; + +use anyhow::{Context, anyhow, bail}; +use tracing::{debug, warn}; + +use crate::Result; + +/// Where the unified cgroup v2 hierarchy is mounted. +const CGROUP_ROOT: &str = "/sys/fs/cgroup"; + +/// How many times, and how far apart, to retry removing a scenario cgroup that the +/// kernel still considers occupied because a killed process has not been reaped yet. +const REMOVE_ATTEMPTS: u32 = 10; +const REMOVE_RETRY_INTERVAL: Duration = Duration::from_millis(20); + +/// How many names to try before giving up on finding a free one, in case a previous run +/// died without cleaning up and this process reused its pid. +const CREATE_ATTEMPTS: u32 = 100; + +/// A cgroup under which one memory-limited cgroup can be made per scenario. +#[derive(Debug)] +pub struct CgroupTree { + /// A cgroup with the memory controller delegated to its children. + parent: PathBuf, +} + +impl CgroupTree { + /// Find somewhere to make memory-limited cgroups, or explain why we can't. + /// + /// `bytes` is the limit we'll want later: it's applied to a throwaway cgroup here, so + /// that an unusable hierarchy is a complaint now rather than a failure mid-run. + pub fn probe(bytes: u64) -> Result { + let own = own_cgroup()?; + let mut problems: Vec = Vec::new(); + let try_using = + |dir: &Path, problems: &mut Vec| match CgroupTree::check(dir.to_owned(), bytes) + { + Ok(tree) => Some(tree), + Err(err) => { + problems.push(format!("{}: {err:#}", dir.display())); + None + } + }; + + if delegates_memory(&own).unwrap_or(false) || enable_memory_delegation(&own).is_ok() { + if let Some(tree) = try_using(&own, &mut problems) { + return Ok(tree); + } + } else { + problems.push(format!( + "{}: can't delegate the memory controller", + own.display() + )); + } + + if own != Path::new(CGROUP_ROOT) + && let Some(parent) = own.parent() + && delegates_memory(parent).unwrap_or(false) + && let Some(tree) = try_using(parent, &mut problems) + { + warn!( + "Scenario cgroups will be siblings of cargo-mutants' own cgroup, not inside \ + it, because its own cgroup cannot delegate the memory controller. \ + --max-memory still bounds each scenario, but their total is bounded only by \ + {}, not by any memory.max set on cargo-mutants itself", + parent.display() + ); + return Ok(tree); + } + + // Nothing else worked, so get out of our own cgroup and try it once more. + match move_self_into_leaf(&own).and_then(|()| enable_memory_delegation(&own)) { + Ok(()) => { + if let Some(tree) = try_using(&own, &mut problems) { + return Ok(tree); + } + } + Err(err) => problems.push(format!("{}: {err:#}", own.display())), + } + bail!("no usable cgroup v2 hierarchy: {}", problems.join("; ")) + } + + /// Prove that a scenario cgroup can really be made here before promising the user one. + fn check(parent: PathBuf, bytes: u64) -> Result { + let tree = CgroupTree { parent }; + // Dropped immediately, which removes it again. + tree.create_scenario(bytes) + .context("test-create a scenario cgroup")?; + debug!(?tree.parent, "using cgroup v2 for per-scenario memory limits"); + Ok(tree) + } + + /// Make a cgroup to hold one scenario's process tree, limited to `bytes` of memory. + pub fn create_scenario(&self, bytes: u64) -> Result { + let cgroup = ScenarioCgroup { + dir: self.claim_dir()?, + }; + cgroup.write("memory.max", &bytes.to_string())?; + // Swapping a runaway scenario out instead of stopping it would be slower than the + // failure we're trying to cause, but this knob only exists where the kernel + // accounts for swap, and memory.max alone still bounds resident memory. + if let Err(err) = cgroup.write("memory.swap.max", "0") { + debug!(?err, "no memory.swap.max: the limit will not cover swap"); + } + Ok(cgroup) + } + + /// Create a scenario cgroup directory with a name nothing else is using. + fn claim_dir(&self) -> Result { + /// Distinguishes concurrent scenarios from each other. + static SERIAL: AtomicU64 = AtomicU64::new(0); + (0..CREATE_ATTEMPTS) + .find_map(|_| { + let dir = self.parent.join(format!( + "cargo-mutants-{pid}-{serial}", + pid = process::id(), + serial = SERIAL.fetch_add(1, Ordering::Relaxed) + )); + match create_dir(&dir) { + Ok(()) => Some(Ok(dir)), + // A run that died without cleaning up leaves directories behind, and + // this process may have been given its pid; just take the next name. + Err(err) if err.kind() == ErrorKind::AlreadyExists => None, + Err(err) => { + Some(Err(err).with_context(|| format!("create cgroup {}", dir.display()))) + } + } + }) + .unwrap_or_else(|| { + Err(anyhow!( + "no free scenario cgroup name under {}", + self.parent.display() + )) + }) + } +} + +/// The cgroup holding one scenario phase's process tree. +#[derive(Debug)] +pub struct ScenarioCgroup { + dir: PathBuf, +} + +impl ScenarioCgroup { + /// Open this cgroup's `cgroup.procs`, so that a forked child can move itself in by + /// writing to an already-open file, without allocating. + pub fn open_procs(&self) -> Result { + let path = self.dir.join("cgroup.procs"); + OpenOptions::new() + .write(true) + .open(&path) + .with_context(|| format!("open {}", path.display())) + } + + fn write(&self, name: &str, value: &str) -> Result<()> { + let path = self.dir.join(name); + write(&path, value).with_context(|| format!("write {value:?} to {}", path.display())) + } +} + +/// Removing the cgroup on drop is what keeps a failure part-way through a scenario -- +/// or part-way through creating the cgroup itself -- from leaking it. +impl Drop for ScenarioCgroup { + fn drop(&mut self) { + // The kernel only removes a cgroup with no members left. The process group sweep + // has already killed everything in here, but a killed process still counts as a + // member until it is reaped, so give the kernel a moment to catch up. + let last_failure = (0..REMOVE_ATTEMPTS).find_map(|attempt| { + if attempt > 0 { + sleep(REMOVE_RETRY_INTERVAL); + } + match remove_dir(&self.dir) { + Ok(()) => Some(Ok(())), + Err(err) if attempt + 1 == REMOVE_ATTEMPTS => Some(Err(err)), + Err(_) => None, + } + }); + if let Some(Err(err)) = last_failure { + // Not worth failing a scenario over: an abandoned empty cgroup costs an inode. + debug!(?self.dir, ?err, "gave up removing scenario cgroup"); + } + } +} + +/// The directory of the cgroup v2 cgroup that this process is in. +fn own_cgroup() -> Result { + let root = Path::new(CGROUP_ROOT); + if !root.join("cgroup.controllers").exists() { + bail!("{CGROUP_ROOT} is not a cgroup v2 unified hierarchy"); + } + // The v2 entry in /proc/self/cgroup is the one with hierarchy id 0 and no named + // controllers; any others are v1 and of no use to us. + let own = read_to_string("/proc/self/cgroup").context("read /proc/self/cgroup")?; + let relative = own + .lines() + .find_map(|line| line.strip_prefix("0::")) + .context("no cgroup v2 entry in /proc/self/cgroup")?; + Ok(root.join(relative.trim().trim_start_matches('/'))) +} + +/// Whether children of this cgroup get `memory.max`. +fn delegates_memory(dir: &Path) -> Result { + let path = dir.join("cgroup.subtree_control"); + Ok(read_to_string(&path) + .with_context(|| format!("read {}", path.display()))? + .split_whitespace() + .any(|controller| controller == "memory")) +} + +/// Ask the kernel to give this cgroup's children `memory.max`. +/// +/// This fails while the cgroup contains processes, which is the usual case for our own +/// cgroup. +fn enable_memory_delegation(dir: &Path) -> Result<()> { + let path = dir.join("cgroup.subtree_control"); + write(&path, "+memory").with_context(|| { + format!( + "delegate the memory controller by writing to {}", + path.display() + ) + }) +} + +/// Move this process into a child of `dir`, so that `dir` itself holds no processes and +/// can therefore delegate controllers. +fn move_self_into_leaf(dir: &Path) -> Result<()> { + let leaf = dir.join("cargo-mutants-supervisor"); + if !leaf.exists() { + create_dir(&leaf).with_context(|| format!("create cgroup {}", leaf.display()))?; + } + let procs = leaf.join("cgroup.procs"); + write(&procs, "0\n").with_context(|| format!("move this process into {}", procs.display()))?; + debug!(?leaf, "moved cargo-mutants into a cgroup of its own"); + Ok(()) +} diff --git a/testdata/unbounded_allocation/Cargo_test.toml b/testdata/unbounded_allocation/Cargo_test.toml new file mode 100644 index 00000000..6760dbc7 --- /dev/null +++ b/testdata/unbounded_allocation/Cargo_test.toml @@ -0,0 +1,11 @@ +# A tree with a function that allocates without bound when one of its mutants is +# applied, so that `--max-memory` can be seen to stop it. + +[package] +name = "cargo-mutants-testdata-unbounded-allocation" +version = "0.1.0" +edition = "2018" +publish = false + +[lib] +doctest = false diff --git a/testdata/unbounded_allocation/src/lib.rs b/testdata/unbounded_allocation/src/lib.rs new file mode 100644 index 00000000..b14e6880 --- /dev/null +++ b/testdata/unbounded_allocation/src/lib.rs @@ -0,0 +1,29 @@ +//! A tree whose tests allocate without bound when a particular mutant is applied. +//! +//! `replace big_enough -> bool with false` turns the loop in `grow_buffer` into an +//! unbounded allocator, which grows fast enough to hit a memory limit long before any +//! reasonable test timeout. + +/// Is the buffer big enough to stop growing it? +fn big_enough(chunks: usize) -> bool { + chunks >= 8 +} + +/// Grow a buffer a mebibyte at a time until it's big enough, and return its size in +/// mebibytes. +pub fn grow_buffer() -> usize { + let mut buffer: Vec> = Vec::new(); + while !big_enough(buffer.len()) { + // Written, not just reserved, so that the memory is really resident. + buffer.push(vec![0xab; 1 << 20]); + } + buffer.len() +} + +#[cfg(test)] +mod test { + #[test] + fn grow_buffer_stops_when_big_enough() { + assert_eq!(super::grow_buffer(), 8); + } +} diff --git a/tests/main.rs b/tests/main.rs index 906b6f43..449a1645 100644 --- a/tests/main.rs +++ b/tests/main.rs @@ -30,6 +30,8 @@ use tempfile::{NamedTempFile, TempDir, tempdir}; mod integration_util; mod util; use integration_util::run; +#[cfg(target_os = "linux")] +use util::outcome_json; use util::{ CommandInstaExt, OUTER_TIMEOUT, assert_bytes_eq_json, copy_of_testdata, copy_testdata_to, outcome_json_counts, @@ -3873,3 +3875,72 @@ fn max_memory_does_not_disturb_a_normal_run() { .success() .stdout(contains("4 mutants tested")); } + +/// A mutant can turn a bounded loop into an unbounded allocator. `--max-memory` puts a +/// ceiling on each scenario, so the kernel stops the runaway mutant in a fraction of a +/// second, rather than the machine filling up until the test timeout arrives. +/// +/// Only the cgroup mechanism limits resident memory, so this is gated to Linux and skips +/// where no writable cgroup is available. +#[cfg(target_os = "linux")] +#[test] +fn max_memory_catches_a_mutant_that_allocates_without_bound() +-> Result<(), Box> { + let tmp_src_dir = copy_of_testdata("unbounded_allocation"); + let assert = run() + .arg("mutants") + .args([ + "--max-memory=256M", + "--regex=replace big_enough -> bool with false", + "--baseline=skip", + // Generous, so that "not a timeout" really means the memory limit stopped it. + "--timeout=60", + "--build-timeout=120", + "-L", + "debug", + "-v", + ]) + .current_dir(tmp_src_dir.path()) + .timeout(OUTER_TIMEOUT) + .assert(); + let stdout = String::from_utf8_lossy(&assert.get_output().stdout).into_owned(); + let stderr = String::from_utf8_lossy(&assert.get_output().stderr).into_owned(); + println!("stdout:\n{stdout}\nstderr:\n{stderr}"); + + // Under the RLIMIT_AS fallback a 256M address-space cap is too tight even to build, + // so there is nothing to assert. + if !stderr.contains("cgroup v2 memory.max") { + eprintln!("skipped: no writable cgroup v2 here, so --max-memory fell back to RLIMIT_AS"); + return Ok(()); + } + assert.success(); + + assert_eq!( + outcome_json_counts(&tmp_src_dir), + json!({ + "total_mutants": 1, + "caught": 1, + "missed": 0, + "timeout": 0, + "unviable": 0, + "success": 0, + }), + "the runaway mutant should be caught by the memory limit, not by the timeout" + ); + + // It should die on the memory limit long before the 60s test timeout. + let outcomes = outcome_json(&tmp_src_dir); + let test_secs = outcomes["outcomes"][0]["phase_results"] + .as_array() + .ok_or("no phase_results in outcomes.json")? + .iter() + .find(|pr| pr["phase"] == "Test") + .ok_or("the mutant never reached the test phase")?["duration"] + .as_f64() + .ok_or("no duration on the test phase")?; + assert!( + test_secs < 20.0, + "the memory-limited test took {test_secs}s, which is not well under the timeout" + ); + Ok(()) +} From f3ae4a446832a05f369516c4df81e5356e9296d9 Mon Sep 17 00:00:00 2001 From: Yuri Khrustalev Date: Tue, 15 Sep 2026 22:26:35 -0400 Subject: [PATCH 6/6] feat: report OOM kills from the scenario's memory cgroup An OOM-killed mutant is also a caught mutant, so without this it is indistinguishable from one caught by a failing assertion -- which is exactly what you want to know when a sweep keeps hitting the memory fence. Read memory.events after each phase, carry the oom_kill count alongside the sweep on the phase result, and name it on the outcome line and in outcomes.json. Count the scenarios it stopped in the run summary too, since caught mutants are not printed by default. Classification is unchanged. --- NEWS.md | 7 +-- book/src/timeouts.md | 105 ++++++++++++++++++---------------- src/cargo.rs | 4 +- src/outcome.rs | 108 ++++++++++++++++++++++++++++------- src/process.rs | 54 ++++++++++++------ src/process/memory.rs | 43 ++++++++------ src/process/memory/cgroup.rs | 15 ++++- tests/main.rs | 23 ++++++-- 8 files changed, 247 insertions(+), 112 deletions(-) diff --git a/NEWS.md b/NEWS.md index e860e99c..fa96921f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,12 +2,11 @@ ## Unreleased -- New: Outcome lines, scenario logs, and `outcomes.json` now say when a phase's process was killed by a signal, and when processes left running by the tests had to be reaped. The caught / missed / unviable / timeout classification is unchanged. +- New: Outcome lines, scenario logs, and `outcomes.json` now say when a phase's process was killed by a signal, when the kernel OOM-killed something in the scenario's memory cgroup, and when processes left running by the tests had to be reaped, so that (for example) an OOM-caught mutant can be told apart from one caught by a failing assertion. The caught / missed / unviable / timeout classification is unchanged. -- Fixed: Each cargo invocation now runs as the leader of its own process group, and that group is swept after every phase, not only after a timeout. Processes left running by a scenario's tests are `SIGTERM`ed, given a short grace period, and then `SIGKILL`ed, so they can't keep consuming memory or CPU while later mutants are tested. Unix only. +- New: `--max-memory SIZE` (and the `max_memory` config key) bounds how much memory each scenario's cargo process tree may use, so that a mutant that turns a loop into an unbounded allocator is stopped by the kernel rather than taking the machine down with it. On Linux this uses a cgroup v2 `memory.max` where a writable cgroup is available, and otherwise `setrlimit(RLIMIT_AS)`; the mechanism in use is logged. macOS does not enforce `RLIMIT_AS`, so the option is a no-op there. If the option is given and neither mechanism can be applied, cargo-mutants fails before testing any mutant rather than running with no limit. Scenarios stopped by the limit are counted in the run summary. -- Fixed: After a timeout, cargo-mutants waits only a short grace period for the child to exit after `SIGTERM` before sending `SIGKILL`. Previously it waited indefinitely, so a process that ignored `SIGTERM`, or that had been stopped, hung the whole run. -- New: `--max-memory SIZE` (and the `max_memory` config key) bounds how much memory each scenario's cargo process tree may use, so that a mutant that turns a loop into an unbounded allocator is stopped by the kernel rather than taking the machine down with it. On Linux it uses a cgroup v2 `memory.max` where a writable cgroup is available, and otherwise `setrlimit(RLIMIT_AS)`; the mechanism in use is logged. macOS does not enforce `RLIMIT_AS`, so the option is a no-op there. If the option is given and it cannot be applied, cargo-mutants fails before testing any mutant rather than running with no limit. +- Fixed: Each cargo invocation now runs as the leader of its own process group, and that group is swept after every phase, not only after a timeout. Processes left running by a scenario's tests are `SIGTERM`ed, given a short grace period, and then `SIGKILL`ed, so they can't keep consuming memory or CPU while later mutants are tested. Unix only. - New: `#[mutants::exclude_re("pattern")]` attribute to exclude specific mutations by regex, without disabling all mutations on the function. The attribute can be placed on functions, `impl` blocks, `trait` blocks, modules, files, and on expressions that can carry an attribute (such as `match`, struct literals, call expressions, method calls, and unary expressions). Multiple patterns can be applied. Also supported within `cfg_attr`. Requires the [mutants](https://crates.io/crates/mutants) crate version `0.0.5` or later. diff --git a/book/src/timeouts.md b/book/src/timeouts.md index 8712f59a..71969256 100644 --- a/book/src/timeouts.md +++ b/book/src/timeouts.md @@ -33,11 +33,6 @@ You can set an explicit timeouts with the `--timeout` option, also measured in s You can also set the test timeout as a multiple of the duration of the baseline test, with the `--timeout-multiplier` option and the `timeout_multiplier` configuration key. The multiplier only has an effect if the baseline is not skipped and if `--timeout` is not specified. -When a timeout expires, the process is sent `SIGTERM` so that it can clean up, and then -`SIGKILL` if it has not exited within a short grace period. A test process that ignores -`SIGTERM`, or that has been stopped and so never receives it, therefore cannot stall the -whole run. - ## Build timeouts `const` expressions may be evaluated at compile time. In the same way that mutations can cause tests to hang, mutations to const code may potentially cause the compiler to enter an infinite loop. @@ -50,40 +45,6 @@ In this case you can use the `--build-timeout` or `--build-timeout-multiplier` o You might also choose to skip mutants that can cause long-running const evaluation. -## Leftover processes - -A test can leave processes running after it returns: a daemon it started, a helper it -forgot to wait for, or a test binary that was not reaped. Those processes keep running -— and keep allocating — while cargo-mutants moves on to the next mutant, so they can -exhaust the machine's memory in a window where no cargo phase is running at all. - -To prevent this, cargo-mutants starts each cargo invocation as the leader of its own -process group, and sweeps that group after *every* phase, not only after a timeout. -Once the cargo process itself exits, anything left in the group is sent `SIGTERM`, given -a short grace period, and then `SIGKILL`ed. What was reaped is recorded in the -scenario's log, and the pids are shown at `--level=debug`. - -This has no effect on how a mutant is classified; it only stops work from one scenario -leaking into the next. Windows has no process groups, and cargo-mutants does not yet use -job objects, so this sweep is Unix-only. - -## Why a scenario died - -A mutant caught because the kernel killed its tests looks, in the summary counts, exactly -like a mutant caught by a failing assertion. When there is more to say, cargo-mutants says -it in parentheses on the outcome line: - -``` -caught src/server.rs:88:5: replace listen -> bool with false (test killed by SIGABRT; test left 1 stray process behind (SIGKILLed: 30411)) in 2s build + 9s test -``` - -Two things get reported this way: the signal that killed a phase's cargo process, if it -died by one, and anything the process group sweep had to clean up. The same information -is written to the scenario's log and, in `mutants.out/outcomes.json`, to a `sweep` field -on each phase result. - -This does not change the caught / missed / unviable / timeout classification. It only -makes the reason visible. ## Memory limits A timeout is not always enough. A mutant can turn a bounded loop into an unbounded @@ -114,15 +75,17 @@ included, so leave room for the compiler as well as for the tests. Two mechanisms can enforce it, and they are not equivalent: * **cgroup v2** `memory.max`, on a cgroup created for each scenario. This limits - *resident* memory for the whole process tree, which is what you actually care about. - It is preferred whenever a writable cgroup is available. Swap is also capped, where the - kernel accounts for it; on kernels that do not, the limit covers resident memory only. + *resident* memory for the whole process tree, and the kernel reports what it did through + `memory.events`, so an OOM-killed mutant can be told apart from one caught by a failing + assertion. This is preferred whenever a writable cgroup is available. Swap is also + capped, where the kernel accounts for it; on kernels that do not, the limit covers + resident memory only. * **`setrlimit(RLIMIT_AS)`** on the cargo process, inherited by everything it spawns. - This limits *address space*, a much cruder proxy: allocators and rustc reserve far more - address space than they ever make resident, so a limit that is comfortable as a - resident-memory ceiling can fail builds outright when applied this way. If cargo-mutants - falls back to this mechanism, set the limit generously. + This limits *address space*, which is a much cruder proxy: allocators and rustc reserve + far more address space than they ever make resident, so a limit that is comfortable as + a resident-memory ceiling can fail builds outright when applied this way. If + cargo-mutants falls back to this mechanism, set the limit generously. Which one is in use is reported at startup, for example: @@ -149,8 +112,54 @@ exist, so `--max-memory` has no effect there; cargo-mutants warns and carries on platform where *neither* mechanism can be applied, giving `--max-memory` is an error, reported before any mutant is tested, rather than a run that quietly had no limit. -This option does not change how mutants are classified. A mutant whose tests are stopped -by the limit fails its tests and so is caught, in just the same way as one that panics. +This option does not change how mutants are classified. A mutant whose tests are +OOM-killed fails its tests and so is caught, in just the same way as one that panics. So +that those are not invisible, the run summary counts them separately: + +``` +40 mutants tested in 3m 2s: 40 caught (3 stopped by the --max-memory limit) +``` + +## Leftover processes + +A test can leave processes running after it returns: a daemon it started, a helper it +forgot to wait for, or a test binary that was not reaped. Those processes keep running +— and keep allocating — while cargo-mutants moves on to the next mutant, so they can +exhaust the machine's memory in a window where no cargo phase is running at all. + +To prevent this, cargo-mutants starts each cargo invocation as the leader of its own +process group, and sweeps that group after *every* phase, not only after a timeout. +Once the cargo process itself exits, anything left in the group is sent `SIGTERM`, given +a short grace period, and then `SIGKILL`ed. What was reaped is recorded in the +scenario's log, and the pids are shown at `--level=debug`. + +The same escalation applies to the cargo process itself on a timeout: it is sent +`SIGTERM`, and `SIGKILL`ed if it has not exited by the end of the grace period, so a +child that ignores `SIGTERM` cannot stall the run. + +This has no effect on how a mutant is classified; it only stops work from one scenario +leaking into the next. Windows has no process groups, and cargo-mutants does not yet use +job objects, so this sweep is Unix-only. + +## Why a scenario died + +A mutant caught because the kernel killed its tests looks, in the summary counts, exactly +like a mutant caught by a failing assertion. When there is more to say, cargo-mutants says +it in parentheses on the outcome line: + +``` +caught src/parse.rs:41:9: replace += with -= in Cursor::advance (test OOM-killed by the kernel (1 process) for exceeding the memory limit) in 3s build + 1s test +caught src/server.rs:88:5: replace listen -> bool with false (test killed by SIGABRT; test left 1 stray process behind (SIGKILLed: 30411)) in 2s build + 9s test +``` + +Three things get reported this way: the signal that killed a phase's cargo process, if it +died by one; the kernel's `oom_kill` count from the scenario's cgroup, when the cgroup +memory limit is in use; and anything the process group sweep had to clean up. The same +information is written to the scenario's log and, in `mutants.out/outcomes.json`, to a +`report` field on each phase result. + +None of this changes the caught / missed / unviable / timeout classification. It only +makes the reason visible. ## Exceptions diff --git a/src/cargo.rs b/src/cargo.rs index 6ca957eb..d80771dd 100644 --- a/src/cargo.rs +++ b/src/cargo.rs @@ -57,7 +57,7 @@ pub fn run_cargo( debug!(?encoded_rustflags); env.push(("CARGO_ENCODED_RUSTFLAGS".to_owned(), encoded_rustflags)); } - let (process_status, sweep) = Process::run( + let (process_status, report) = Process::run( &argv, &env, build_dir.path(), @@ -82,7 +82,7 @@ pub fn run_cargo( duration: start.elapsed(), process_status, argv, - sweep, + report, }) } diff --git a/src/outcome.rs b/src/outcome.rs index ed3a1700..0c3e109c 100644 --- a/src/outcome.rs +++ b/src/outcome.rs @@ -19,7 +19,7 @@ use crate::console::{format_duration, plural}; use crate::exit_code::ExitCode; #[cfg(unix)] use crate::process::signal_name; -use crate::process::{Exit, Sweep}; +use crate::process::{Exit, ProcessReport}; use crate::{Options, Result, Scenario, output}; /// What phase of running a scenario. @@ -68,6 +68,13 @@ pub struct LabOutcome { pub timeout: usize, pub unviable: usize, pub success: usize, + /// How many scenarios had something OOM-killed in their memory cgroup. + /// + /// Not a category of its own -- an OOM-killed mutant is also a caught one -- so it is + /// reported alongside the counts rather than in them, and kept out of the JSON, where + /// each phase already carries its own report. + #[serde(skip)] + pub oom_killed: usize, pub start_time: Timestamp, pub end_time: Option, pub cargo_mutants_version: String, @@ -83,6 +90,7 @@ impl LabOutcome { timeout: 0, unviable: 0, success: 0, + oom_killed: 0, start_time, end_time: None, cargo_mutants_version: crate::VERSION.to_string(), @@ -91,6 +99,11 @@ impl LabOutcome { /// Record the event of one test. pub fn add(&mut self, outcome: ScenarioOutcome) { + // Counted for the baseline too: if the unmutated tree can't fit in the limit, + // that's the most important thing to say about the run. + if outcome.was_oom_killed() { + self.oom_killed += 1; + } if outcome.scenario.is_mutant() { self.total_mutants += 1; match outcome.summary() { @@ -151,6 +164,12 @@ impl LabOutcome { by_outcome.push(format!("{} succeeded", self.success)); } s.push(by_outcome.join(", ")); + if self.oom_killed > 0 { + s.push(format!( + " ({} stopped by the --max-memory limit)", + self.oom_killed + )); + } s.join("") } } @@ -248,11 +267,11 @@ impl ScenarioOutcome { } /// Say, for each phase that has something unusual to report, how its process tree - /// ended: killed by a signal, or leaving stray processes behind. + /// ended: killed by a signal, stopped by the memory limit, or leaving strays behind. /// /// This has no bearing on how the mutant is classified. It exists so that a mutant - /// caught because the kernel killed its tests can be told apart from one caught by a - /// failing assertion. + /// caught because the kernel OOM-killed its tests can be told apart from one caught + /// by a failing assertion. pub fn death_reasons(&self) -> Vec { self.phase_results .iter() @@ -260,6 +279,13 @@ impl ScenarioOutcome { .collect() } + /// True if the kernel OOM-killed anything in this scenario's memory cgroup. + pub fn was_oom_killed(&self) -> bool { + self.phase_results + .iter() + .any(|pr| pr.report.oom_kills.is_some_and(|n| n > 0)) + } + /// True if this outcome is a caught mutant: it's a mutant and the tests failed. pub fn mutant_caught(&self) -> bool { self.scenario.is_mutant() @@ -318,8 +344,8 @@ pub struct PhaseResult { pub process_status: Exit, /// What command was run, as an argv list. pub argv: Vec, - /// What the sweep of the child's process group found and did. - pub sweep: Sweep, + /// What became of the process tree, beyond the exit status. + pub report: ProcessReport, } impl PhaseResult { @@ -334,7 +360,13 @@ impl PhaseResult { if let Exit::Signalled(signal) = self.process_status { reasons.push(format!("{phase} killed by {}", signal_name(signal))); } - if let Some(sweep) = self.sweep.describe() { + if let Some(oom_kills) = self.report.oom_kills.filter(|n| *n > 0) { + reasons.push(format!( + "{phase} OOM-killed by the kernel ({oom_kills} process{es}) for exceeding the memory limit", + es = if oom_kills == 1 { "" } else { "es" } + )); + } + if let Some(sweep) = self.report.sweep.describe() { reasons.push(format!("{phase} {sweep}")); } reasons @@ -351,7 +383,7 @@ impl Serialize for PhaseResult { ss.serialize_field("duration", &self.duration.as_secs_f64())?; ss.serialize_field("process_status", &self.process_status)?; ss.serialize_field("argv", &self.argv)?; - ss.serialize_field("sweep", &self.sweep)?; + ss.serialize_field("report", &self.report)?; ss.end() } } @@ -372,17 +404,17 @@ pub enum SummaryOutcome { mod test { use std::time::Duration; - use crate::process::{Exit, Sweep}; + use crate::process::{Exit, ProcessReport, Sweep}; use super::{Phase, PhaseResult, Scenario, ScenarioOutcome}; - fn phase_result(phase: Phase, process_status: Exit, sweep: Sweep) -> PhaseResult { + fn phase_result(phase: Phase, process_status: Exit, report: ProcessReport) -> PhaseResult { PhaseResult { phase, duration: Duration::from_secs(1), process_status, argv: vec!["cargo".into(), "test".into()], - sweep, + report, } } @@ -401,7 +433,7 @@ mod test { let outcome = outcome_of(vec![phase_result( Phase::Test, Exit::Failure(101), - Sweep::default(), + ProcessReport::default(), )]); assert_eq!(outcome.death_reasons(), Vec::::new()); } @@ -412,20 +444,56 @@ mod test { let outcome = outcome_of(vec![phase_result( Phase::Test, Exit::Signalled(9), - Sweep::default(), + ProcessReport::default(), )]); assert_eq!(outcome.death_reasons(), ["test killed by SIGKILL"]); } + #[test] + fn death_reasons_name_an_oom_kill() { + let outcome = outcome_of(vec![ + phase_result(Phase::Build, Exit::Success, ProcessReport::default()), + phase_result( + Phase::Test, + Exit::Failure(101), + ProcessReport { + oom_kills: Some(1), + ..ProcessReport::default() + }, + ), + ]); + assert_eq!( + outcome.death_reasons(), + ["test OOM-killed by the kernel (1 process) for exceeding the memory limit"] + ); + } + + /// A cgroup that was watched but never hit its limit has nothing to say. + #[test] + fn no_death_reason_for_zero_oom_kills() { + let outcome = outcome_of(vec![phase_result( + Phase::Test, + Exit::Success, + ProcessReport { + oom_kills: Some(0), + ..ProcessReport::default() + }, + )]); + assert_eq!(outcome.death_reasons(), Vec::::new()); + } + #[test] fn death_reasons_name_processes_left_behind_by_the_tests() { let outcome = outcome_of(vec![phase_result( Phase::Test, Exit::Success, - Sweep { - pids: Some(vec![101, 102]), - strays: true, - killed: true, + ProcessReport { + sweep: Sweep { + pids: Some(vec![101, 102]), + strays: true, + killed: true, + }, + oom_kills: None, }, )]); assert_eq!( @@ -447,14 +515,14 @@ mod test { duration: Duration::from_secs(2), process_status: Exit::Success, argv: vec!["cargo".into(), "build".into()], - sweep: Sweep::default(), + report: ProcessReport::default(), }, PhaseResult { phase: Phase::Test, duration: Duration::from_secs(3), process_status: Exit::Success, argv: vec!["cargo".into(), "test".into()], - sweep: Sweep::default(), + report: ProcessReport::default(), }, ], }; @@ -465,7 +533,7 @@ mod test { duration: Duration::from_secs(2), process_status: Exit::Success, argv: vec!["cargo".into(), "build".into()], - sweep: Sweep::default(), + report: ProcessReport::default(), }) ); assert_eq!( diff --git a/src/process.rs b/src/process.rs index fc48528b..7c829466 100644 --- a/src/process.rs +++ b/src/process.rs @@ -46,6 +46,9 @@ pub use unix::signal_name; #[cfg(unix)] use unix::{configure_command, kill_child, sweep_process_group, terminate_child}; +pub mod memory; +use memory::{MemoryLimit, ScenarioMemoryLimit}; + /// What sweeping a finished child's process group found and did. /// /// A scenario's tests can leave processes running: a test binary that was not waited @@ -63,7 +66,7 @@ pub struct Sweep { } impl Sweep { - /// Describe what was reaped, or None if nothing was left over. + /// Describe what was reaped, for an outcome line, or None if nothing was left over. pub fn describe(&self) -> Option { if !self.strays { return None; @@ -81,16 +84,25 @@ impl Sweep { } } -pub mod memory; -use memory::{MemoryLimit, ScenarioMemoryLimit}; +/// What became of a phase's process tree, beyond the exit status of the direct child. +/// +/// This is only ever reported, never used to classify the mutant: its job is to make an +/// OOM-killed or signalled scenario distinguishable from one whose tests simply failed. +#[derive(Debug, Default, Clone, PartialEq, Eq, Serialize)] +pub struct ProcessReport { + /// What the process group sweep found and did. + pub sweep: Sweep, + /// How many times the kernel OOM-killed a process in this phase's cgroup, when the + /// cgroup memory limit mechanism is in use. + pub oom_kills: Option, +} pub struct Process { child: Child, start: Instant, timeout: Option, - /// The memory limit in force for this process tree, if any. Held so that any cgroup - /// outlives the child and is removed once it has exited. - #[allow(dead_code)] // its Drop is the point + /// The memory limit in force for this process tree, if any, held until the process + /// group has been swept so that its cgroup is empty before we remove it. memory: Option, } @@ -110,7 +122,7 @@ impl Process { memory_limit: Option<&MemoryLimit>, scenario_output: &mut ScenarioOutput, console: &Console, - ) -> Result<(Exit, Sweep)> { + ) -> Result<(Exit, ProcessReport)> { let mut child = Process::start( argv, env, @@ -130,12 +142,24 @@ impl Process { sleep(WAIT_POLL_INTERVAL); }; let sweep = child.sweep()?; + // Only safe once the sweep has emptied the cgroup: the kernel won't let us + // remove a cgroup that still has members. + let oom_kills = child.memory.take().and_then(ScenarioMemoryLimit::finish); + if let Some(oom_kills) = oom_kills { + debug!(oom_kills, "cgroup memory.events after phase"); + } + let report = ProcessReport { sweep, oom_kills }; let process_status = result?; scenario_output.message(&format!("result: {process_status:?}"))?; - if let Some(description) = sweep.describe() { + if let Some(description) = report.sweep.describe() { scenario_output.message(&description)?; } - Ok((process_status, sweep)) + if let Some(oom_kills) = report.oom_kills.filter(|n| *n > 0) { + scenario_output.message(&format!( + "the kernel OOM-killed {oom_kills} process(es) in this scenario's memory cgroup" + ))?; + } + Ok((process_status, report)) } /// Kill anything the child left running in its process group. @@ -217,16 +241,12 @@ impl Process { } } - /// Terminate the subprocess, initially gently and then harshly. - /// - /// Blocks until the subprocess is terminated and then returns the exit status. + /// Stop the subprocess, and block until it has gone. /// /// `SIGTERM` first, so it gets a chance to clean up, but only for a bounded grace - /// period: a process that ignores it, or that is stopped and so never receives it, - /// would otherwise hang the run here forever. Whatever else is left in the process - /// group is dealt with by the sweep in [`Process::run`]. - /// - /// The status might not be `Timeout` if this raced with a normal exit. + /// period: a child that ignores it would otherwise hang us here forever, and the + /// sweep in [`Process::run`] would never get to run. Whatever else is left in the + /// process group is dealt with by that sweep. #[mutants::skip] // would leak processes from tests if skipped fn terminate(&mut self) -> Result<()> { let _span = span!(Level::DEBUG, "terminate_child", pid = self.child.id()).entered(); diff --git a/src/process/memory.rs b/src/process/memory.rs index 5b0ef63e..dcfc7d84 100644 --- a/src/process/memory.rs +++ b/src/process/memory.rs @@ -9,15 +9,16 @@ //! //! Two mechanisms can do this, and they are not equivalent: //! -//! * cgroup v2 `memory.max`, which limits *resident* memory for the whole process tree. -//! This is what we want, when we can get it. +//! * cgroup v2 `memory.max`, which limits *resident* memory for the whole process tree +//! and reports OOM kills through `memory.events`. This is what we want, when we can +//! get it. //! * `setrlimit(RLIMIT_AS)`, which limits the *address space* of each process. It is a -//! cruder proxy -- allocators reserve far more address space than they use -- so the -//! limit has to be set generously, and it is only enforced on Linux. +//! cruder proxy -- allocators reserve far more address space than they use -- and it +//! is only enforced on Linux. //! //! `any(target_os = "linux", target_os = "android", target_os = "macos")` recurs below: //! it is where `nix` exposes `RLIMIT_AS` and cargo-mutants is supported. Everywhere else -//! the `RlimitAs` variant does not exist at all, which is what makes it unconstructible +//! the `RlimitAs` variants do not exist at all, which is what makes them unconstructible //! rather than merely unreachable. #[cfg(target_os = "linux")] @@ -57,21 +58,21 @@ impl MemoryMechanism { /// Choose how to apply `--max-memory`, from what this platform and process can offer. /// -/// `settable` says whether `RLIMIT_AS` can be set to the requested limit at all; -/// `enforced` says whether the kernel would then act on it, which macOS does not. +/// `rlimit_settable` says whether `RLIMIT_AS` can be set to the requested limit at all; +/// `rlimit_enforced` says whether the kernel would then act on it, which macOS does not. /// /// Returns an error, rather than quietly running unlimited, when the user asked for a -/// limit and nothing can enforce it. +/// limit and neither mechanism is available. pub fn choose_mechanism( cgroup_available: bool, - settable: bool, - enforced: bool, + rlimit_settable: bool, + rlimit_enforced: bool, ) -> Result { if cgroup_available { Ok(MemoryMechanism::CgroupV2) - } else if settable && enforced { + } else if rlimit_settable && rlimit_enforced { Ok(MemoryMechanism::RlimitAs) - } else if settable { + } else if rlimit_settable { Ok(MemoryMechanism::Unenforced) } else { bail!( @@ -83,7 +84,8 @@ pub fn choose_mechanism( /// A per-scenario memory limit, set up once and used for every phase of every scenario. /// -/// Each variant owns exactly the state its mechanism needs. +/// Each variant owns exactly the state its mechanism needs, so there is no way to be in +/// cgroup mode without a cgroup to put scenarios in. #[derive(Debug)] pub enum MemoryLimit { #[cfg(target_os = "linux")] @@ -172,9 +174,6 @@ impl MemoryLimit { } /// The memory limit in force for one scenario phase. -/// -/// A cgroup is per-scenario state that has to outlive the spawn, which is why this is -/// separate from [`MemoryLimit`]; the cgroup is removed when this is dropped. #[derive(Debug)] pub enum ScenarioMemoryLimit { #[cfg(target_os = "linux")] @@ -209,6 +208,18 @@ impl ScenarioMemoryLimit { } Ok(()) } + + /// Finish with the limit, returning how many times the kernel OOM-killed something + /// in this scenario, where the mechanism can tell us. + /// + /// Any cgroup is removed as this is dropped. + pub fn finish(self) -> Option { + match self { + #[cfg(target_os = "linux")] + ScenarioMemoryLimit::CgroupV2(cgroup) => cgroup.oom_kills(), + _ => None, + } + } } #[cfg(any(target_os = "linux", target_os = "android", target_os = "macos"))] diff --git a/src/process/memory/cgroup.rs b/src/process/memory/cgroup.rs index aa9d2366..dba64139 100644 --- a/src/process/memory/cgroup.rs +++ b/src/process/memory/cgroup.rs @@ -31,7 +31,7 @@ use std::thread::sleep; use std::time::Duration; use anyhow::{Context, anyhow, bail}; -use tracing::{debug, warn}; +use tracing::{debug, trace, warn}; use crate::Result; @@ -182,6 +182,19 @@ impl ScenarioCgroup { .with_context(|| format!("open {}", path.display())) } + /// How many times the kernel OOM-killed a process in this cgroup, from + /// `memory.events`. + pub fn oom_kills(&self) -> Option { + let events = read_to_string(self.dir.join("memory.events")) + .inspect_err(|err| debug!(?self.dir, ?err, "failed to read cgroup memory.events")) + .ok()?; + trace!(?self.dir, %events, "cgroup memory.events"); + events + .lines() + .find_map(|line| line.strip_prefix("oom_kill ")) + .and_then(|count| count.trim().parse().ok()) + } + fn write(&self, name: &str, value: &str) -> Result<()> { let path = self.dir.join(name); write(&path, value).with_context(|| format!("write {value:?} to {}", path.display())) diff --git a/tests/main.rs b/tests/main.rs index e6a72a49..05e41baa 100644 --- a/tests/main.rs +++ b/tests/main.rs @@ -3954,8 +3954,7 @@ fn max_memory_does_not_disturb_a_normal_run() { /// ceiling on each scenario, so the kernel stops the runaway mutant in a fraction of a /// second, rather than the machine filling up until the test timeout arrives. /// -/// Only the cgroup mechanism limits resident memory, so this is gated to Linux and skips -/// where no writable cgroup is available. +/// Only Linux enforces a per-scenario memory limit, so this is gated to Linux. #[cfg(target_os = "linux")] #[test] fn max_memory_catches_a_mutant_that_allocates_without_bound() @@ -3980,9 +3979,13 @@ fn max_memory_catches_a_mutant_that_allocates_without_bound() let stdout = String::from_utf8_lossy(&assert.get_output().stdout).into_owned(); let stderr = String::from_utf8_lossy(&assert.get_output().stderr).into_owned(); println!("stdout:\n{stdout}\nstderr:\n{stderr}"); + println!( + "debug log:\n{}", + read_to_string(tmp_src_dir.path().join("mutants.out/debug.log")).unwrap_or_default() + ); - // Under the RLIMIT_AS fallback a 256M address-space cap is too tight even to build, - // so there is nothing to assert. + // Only the cgroup mechanism limits resident memory. Under the RLIMIT_AS fallback a + // 256M address-space cap is too tight even to build, so there is nothing to assert. if !stderr.contains("cgroup v2 memory.max") { eprintln!("skipped: no writable cgroup v2 here, so --max-memory fell back to RLIMIT_AS"); return Ok(()); @@ -4002,6 +4005,18 @@ fn max_memory_catches_a_mutant_that_allocates_without_bound() "the runaway mutant should be caught by the memory limit, not by the timeout" ); + // An OOM-caught mutant should be distinguishable from one caught by a failing + // assertion, so the outcome line has to say the kernel did it... + assert!( + stdout.contains("OOM-killed"), + "no mention of the OOM kill in:\n{stdout}" + ); + // ...and the run summary has to say it without needing -v. + assert!( + stdout.contains("1 stopped by the --max-memory limit"), + "no mention of the memory limit in the summary:\n{stdout}" + ); + // It should die on the memory limit long before the 60s test timeout. let outcomes = outcome_json(&tmp_src_dir); let test_secs = outcomes["outcomes"][0]["phase_results"]