From 542c9b343a1f82e8b3523b290560d3b32d5815e6 Mon Sep 17 00:00:00 2001 From: Yuri Khrustalev Date: Tue, 15 Sep 2026 20:45:46 -0400 Subject: [PATCH 1/7] fix: sweep the child's process group after every phase Processes left running by a scenario's tests survived a normal exit and kept allocating while later mutants were tested. Sweep the group after every phase, not only on timeout: SIGTERM, a bounded grace period, then SIGKILL. The timeout path now shares this sweep instead of its own kill. --- NEWS.md | 2 + book/src/timeouts.md | 17 +++ src/cargo.rs | 2 +- src/process.rs | 86 +++++++++++-- src/process/unix.rs | 114 ++++++++++++++++-- src/process/windows.rs | 9 +- .../spawns_background_child/Cargo_test.toml | 13 ++ testdata/spawns_background_child/src/lib.rs | 42 +++++++ tests/main.rs | 66 ++++++++++ 9 files changed, 327 insertions(+), 24 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 ed5bd2bb..bfa438bf 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. + - 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..75ff974e 100644 --- a/book/src/timeouts.md +++ b/book/src/timeouts.md @@ -45,6 +45,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 0763e617..7ea2cecd 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}; @@ -29,12 +30,47 @@ const WAIT_POLL_INTERVAL: Duration = Duration::from_millis(50); #[cfg(windows)] mod windows; #[cfg(windows)] -use windows::{configure_command, terminate_child}; +use windows::{configure_command, sweep_process_group, terminate_child}; #[cfg(unix)] mod unix; #[cfg(unix)] -use unix::{configure_command, terminate_child}; +use unix::{configure_command, 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)] +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, for an outcome line, or None if nothing was left over. + pub fn describe(&self) -> Option { + if !self.strays { + return None; + } + let how = if self.killed { "killed" } else { "reaped" }; + Some(match &self.pids { + Some(pids) => format!( + "{how} {n} stray process{es} left over by the tests: {list}", + n = pids.len(), + es = if pids.len() == 1 { "" } else { "es" }, + list = pids.iter().join(", ") + ), + None => format!("{how} stray processes left over by the tests"), + }) + } +} pub struct Process { child: Child, @@ -45,6 +81,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)], @@ -53,17 +92,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. @@ -120,11 +184,11 @@ impl Process { } } - /// Terminate the subprocess, initially gently and then harshly. - /// - /// Blocks until the subprocess is terminated and then returns the exit status. + /// Ask the subprocess to stop, and block until it has. /// - /// The status might not be `Timeout` if this raced with a normal exit. + /// This only gets the direct child out of the way so that we can stop waiting on + /// it; anything else in its process group, including anything that ignored the + /// `SIGTERM`, is dealt with by the sweep in [`Process::run`]. #[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/unix.rs b/src/process/unix.rs index 1eb1866f..6a9234e6 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::{Duration, Instant}; use anyhow::bail; use nix::errno::Errno; @@ -9,29 +11,119 @@ use tracing::warn; use crate::Result; -use super::Exit; +use super::{Exit, Sweep}; -#[allow(unknown_lints, clippy::needless_pass_by_ref_mut)] // To match Windows +/// How long to let a process group wind up after `SIGTERM` before sending `SIGKILL`. +const SWEEP_GRACE: Duration = Duration::from_millis(500); + +/// How often to check whether a signalled process group has emptied out. +const SWEEP_POLL_INTERVAL: Duration = Duration::from_millis(20); + +/// 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 -pub(super) fn terminate_child(child: &mut Child) -> Result<()> { - let pid = Pid::from_raw(child.id().try_into().unwrap()); - match killpg(pid, Signal::SIGTERM) { - Ok(()) => Ok(()), - Err(Errno::ESRCH) => { - Ok(()) // Probably already gone - } +fn signal_group(pgid: Pid, signal: Option) -> Result { + match killpg(pgid, signal) { + 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? - let message = format!("failed to terminate child: error {errno}"); + let message = format!("failed to signal process group {pgid}: error {errno}"); warn!("{}", message); bail!(message); } } } +#[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(()) +} + +/// 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); + let pids = group_members(pgid); + if !signal_group(pgid, Some(Signal::SIGTERM))? { + return Ok(Sweep::default()); + } + let deadline = Instant::now() + SWEEP_GRACE; + loop { + if !signal_group(pgid, None)? { + return Ok(Sweep { + pids, + strays: true, + killed: false, + }); + } else if Instant::now() >= deadline { + break; + } + sleep(SWEEP_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. +fn child_pgid(child: &Child) -> Pid { + Pid::from_raw(child.id().try_into().expect("child pid fits in pid_t")) +} + +/// 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 mut pids = Vec::new(); + for dir_entry in std::fs::read_dir("/proc").ok()?.flatten() { + let Ok(pid) = dir_entry.file_name().to_string_lossy().parse::() else { + continue; // not a process directory + }; + let Ok(stat) = std::fs::read_to_string(format!("/proc/{pid}/stat")) else { + continue; // it exited while we were looking + }; + // 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 Some(after_comm) = stat.rsplit_once(')').map(|(_, rest)| rest) else { + continue; + }; + if after_comm + .split_whitespace() + .nth(2) + .and_then(|field| field.parse::().ok()) + == Some(pgid.as_raw()) + { + pids.push(pid); + } + } + Some(pids) +} + +#[cfg(not(target_os = "linux"))] +fn group_members(_pgid: Pid) -> Option> { + None +} + #[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..58d5b60a 100644 --- a/src/process/windows.rs +++ b/src/process/windows.rs @@ -4,13 +4,20 @@ 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<()> { 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..c99cf59b 100644 --- a/tests/main.rs +++ b/tests/main.rs @@ -3846,3 +3846,69 @@ 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() { + use nix::sys::signal::{SIGKILL, kill}; + use nix::unistd::Pid; + + let tmp_src_dir = copy_of_testdata("spawns_background_child"); + let pid_dir = tempdir().unwrap(); + 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(); + println!( + "stdout:\n{}", + String::from_utf8_lossy(&assert.get_output().stdout) + ); + assert.success(); + + let pids: Vec = read_to_string(&pid_file) + .expect("read background child pid file") + .lines() + .map(|line| line.trim().parse().expect("parse pid")) + .collect(); + assert!( + !pids.is_empty(), + "the tree's test should have spawned background processes" + ); + let survivors: Vec = pids + .iter() + .copied() + .filter(|pid| kill(Pid::from_raw(*pid), None).is_ok()) + .collect(); + // Don't leave them running even if the assertion below fails. + for pid in &survivors { + let _ = kill(Pid::from_raw(*pid), SIGKILL); + } + 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, + }) + ); +} From 532101bfcf01b707b8ff149ce0d5127235aeb5e4 Mon Sep 17 00:00:00 2001 From: Yuri Khrustalev Date: Tue, 15 Sep 2026 21:20:50 -0400 Subject: [PATCH 2/7] feat: add --max-memory to bound each scenario's memory use A mutant can turn a bounded loop into an unbounded allocator that exhausts the machine well before the test timeout fires. Prefer a cgroup v2 memory.max per scenario, falling back to setrlimit(RLIMIT_AS), log which is in use, and fail before any mutant runs if neither can apply. --- Cargo.toml | 2 +- NEWS.md | 2 + book/src/timeouts.md | 59 ++++ examples/custom_config.toml | 5 + src/cargo.rs | 3 + src/config.rs | 2 + src/lab.rs | 13 +- src/main.rs | 12 + src/options.rs | 97 +++++- src/process.rs | 30 +- src/process/memory.rs | 305 ++++++++++++++++++ src/process/memory/cgroup.rs | 227 +++++++++++++ src/process/unix.rs | 6 +- testdata/unbounded_allocation/Cargo_test.toml | 11 + testdata/unbounded_allocation/src/lib.rs | 29 ++ tests/main.rs | 63 ++++ 16 files changed, 858 insertions(+), 8 deletions(-) create mode 100644 src/process/memory.rs 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/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 bfa438bf..911beaf2 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. 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. + - 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 75ff974e..484b4b88 100644 --- a/book/src/timeouts.md +++ b/book/src/timeouts.md @@ -45,6 +45,65 @@ 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. + +```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. + +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, 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. + +* **`setrlimit(RLIMIT_AS)`** on the cargo process, inherited by everything it spawns. + 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: + +``` +INFO Limiting each scenario to 8589934592 bytes of memory using cgroup v2 memory.max +``` + +For the cgroup mechanism, cargo-mutants needs somewhere it may create child cgroups with +`memory.max`. It looks at its own cgroup first, and then at its 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. + +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 +OOM-killed fails its tests and so is caught, in just the same way as one that panics. + ## Leftover processes A test can leave processes running after it returns: a daemon it started, a helper it 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 0309fd27..947aa18a 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..2e6bf25f 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,10 @@ pub fn test_mutants( ) -> Result { let start_time = Instant::now(); console.set_debug_log(output_dir.open_debug_log()?); + // Before copying the tree or running anything: if the user asked for a memory limit + // that can't be enforced here, they should hear about it now, not 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 +66,7 @@ pub fn test_mutants( let lab = Lab { output_mutex, jobserver, + memory_limit, tests_for_mutant, options, console, @@ -164,6 +169,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 +213,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 +229,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 +297,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..581c427f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -291,6 +291,18 @@ 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. + /// + /// On Linux this uses a cgroup v2 `memory.max` if one can be created, and otherwise + /// `setrlimit(RLIMIT_AS)`, which limits address space rather than resident memory. + /// macOS accepts `RLIMIT_AS` but does not act on it, so this option has no effect + /// there. + #[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..da4d5010 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,30 @@ fn join_slices(a: &[String], b: &[String]) -> Vec { a.iter().chain(b).cloned().collect() } +/// 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")) +} + /// Should ANSI colors be drawn? #[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Display, Deserialize, ValueEnum)] #[strum(serialize_all = "snake_case")] @@ -366,6 +393,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_size) + .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 +604,67 @@ mod test { assert_eq!(options.build_timeout_multiplier, Some(3.5)); } + #[test] + fn parse_size_understands_binary_suffixes() { + assert_eq!(parse_size("0").unwrap(), 0); + assert_eq!(parse_size("1024").unwrap(), 1024); + assert_eq!(parse_size("1024B").unwrap(), 1024); + assert_eq!(parse_size("256M").unwrap(), 256 * 1024 * 1024); + assert_eq!(parse_size("256MiB").unwrap(), 256 * 1024 * 1024); + assert_eq!(parse_size(" 4g ").unwrap(), 4 * 1024 * 1024 * 1024); + assert_eq!(parse_size("2T").unwrap(), 2 * (1u64 << 40)); + } + + #[test] + fn parse_size_rejects_nonsense() { + for bad in [ + "", + "M", + "-1", + "1.5G", + "1 zettabyte", + "18446744073709551615K", + ] { + assert!( + parse_size(bad).is_err(), + "{bad:?} should not parse as a size" + ); + } + } + + #[test] + fn options_from_max_memory_arg() { + let args = Args::parse_from(["mutants", "--max-memory=256M"]); + let options = Options::new(&args, &Config::default()).unwrap(); + assert_eq!(options.max_memory, Some(256 * 1024 * 1024)); + + let args = Args::parse_from(["mutants"]); + let options = Options::new(&args, &Config::default()).unwrap(); + assert_eq!(options.max_memory, None); + } + + #[test] + fn cli_max_memory_overrides_config() { + let config: Config = "max_memory = \"8G\"".parse().unwrap(); + let options = Options::new(&Args::parse_from(["mutants"]), &config).unwrap(); + 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).unwrap(); + assert_eq!(options.max_memory, Some(1024 * 1024 * 1024)); + } + + #[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 7ea2cecd..ee65ca84 100644 --- a/src/process.rs +++ b/src/process.rs @@ -37,6 +37,9 @@ mod unix; #[cfg(unix)] use unix::{configure_command, 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 @@ -76,6 +79,9 @@ pub struct Process { child: Child, start: Instant, timeout: Option, + /// 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, } impl Process { @@ -84,16 +90,26 @@ impl Process { /// /// Whatever the outcome, the child's process group is swept before returning, so /// that nothing it left running survives into the next scenario. + #[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<(Exit, Sweep)> { - 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 result = loop { match child.poll() { Ok(Some(exit_status)) => break Ok(exit_status), @@ -104,6 +120,11 @@ 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. + if let Some(oom_kills) = child.memory.take().and_then(ScenarioMemoryLimit::finish) { + debug!(oom_kills, "cgroup memory.events after phase"); + } let process_status = result?; scenario_output.message(&format!("result: {process_status:?}"))?; if let Some(description) = sweep.describe() { @@ -131,12 +152,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(); @@ -156,6 +179,10 @@ impl Process { js.configure(&mut command); } 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() .with_context(|| format!("failed to spawn {}", argv.join(" ")))?; @@ -163,6 +190,7 @@ impl Process { child, start, timeout, + memory, }) } diff --git a/src/process/memory.rs b/src/process/memory.rs new file mode 100644 index 00000000..08808d1f --- /dev/null +++ b/src/process/memory.rs @@ -0,0 +1,305 @@ +// 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. +//! +//! Two mechanisms can do this, and they are not equivalent: +//! +//! * 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 -- and it +//! is only enforced on Linux. + +#[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; + +/// 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. + Unenforced, +} + +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", + } + } +} + +/// Choose how to apply `--max-memory`, from what this platform and process can offer. +/// +/// `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 neither mechanism is available. +pub fn choose_mechanism( + cgroup_available: bool, + rlimit_settable: bool, + rlimit_enforced: bool, +) -> Result { + if cgroup_available { + Ok(MemoryMechanism::CgroupV2) + } else if rlimit_settable && rlimit_enforced { + Ok(MemoryMechanism::RlimitAs) + } else if rlimit_settable { + Ok(MemoryMechanism::Unenforced) + } else { + bail!( + "--max-memory was requested but no mechanism on this platform can enforce it: \ + cargo-mutants can use cgroup v2 or setrlimit(RLIMIT_AS), and neither is available" + ) + } +} + +/// A per-scenario memory limit, set up once and used for every phase of every scenario. +#[derive(Debug)] +pub struct MemoryLimit { + bytes: u64, + mechanism: MemoryMechanism, + #[cfg(target_os = "linux")] + cgroups: Option, +} + +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 { + #[cfg(target_os = "linux")] + let cgroups = match cgroup::CgroupTree::probe(bytes) { + Ok(tree) => Some(tree), + Err(err) => { + debug!(?err, "cgroup v2 memory limits are not available"); + None + } + }; + #[cfg(target_os = "linux")] + let cgroup_available = cgroups.is_some(); + #[cfg(not(target_os = "linux"))] + let cgroup_available = false; + + let mechanism = + choose_mechanism(cgroup_available, rlimit::settable(bytes), rlimit::ENFORCED)?; + if mechanism == MemoryMechanism::Unenforced { + warn!( + "--max-memory has no effect on this platform: RLIMIT_AS is accepted but not enforced here, and cgroups are not available" + ); + } else { + info!( + "Limiting each scenario to {bytes} bytes of memory using {}", + mechanism.describe() + ); + } + Ok(MemoryLimit { + bytes, + mechanism, + #[cfg(target_os = "linux")] + cgroups, + }) + } + + /// Set up the limit for one scenario phase, before its command is spawned. + #[allow(clippy::unnecessary_wraps)] // fallible only where cgroups exist + pub fn start(&self) -> Result { + #[cfg(target_os = "linux")] + let cgroup = self + .cgroups + .as_ref() + .map(|tree| tree.create_scenario(self.bytes)) + .transpose() + .context("create a cgroup for this scenario")?; + Ok(ScenarioMemoryLimit { + bytes: self.bytes, + mechanism: self.mechanism, + #[cfg(target_os = "linux")] + cgroup, + }) + } +} + +/// The memory limit in force for one scenario phase. +#[derive(Debug)] +pub struct ScenarioMemoryLimit { + bytes: u64, + mechanism: MemoryMechanism, + #[cfg(target_os = "linux")] + cgroup: Option, +} + +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) -> Result<()> { + match self.mechanism { + MemoryMechanism::CgroupV2 => self.move_into_cgroup(command), + MemoryMechanism::RlimitAs => { + rlimit::apply_to_child(command, self.bytes); + Ok(()) + } + MemoryMechanism::Unenforced => Ok(()), + } + } + + /// Finish with the limit, returning how many times the kernel OOM-killed something + /// in this scenario, where the mechanism can tell us. + #[allow(clippy::unused_self)] // only cgroups have anything to report + pub fn finish(self) -> Option { + #[cfg(target_os = "linux")] + if let Some(cgroup) = self.cgroup { + let oom_kills = cgroup.oom_kills(); + cgroup.remove(); + return oom_kills; + } + None + } + + #[cfg(target_os = "linux")] + fn move_into_cgroup(&self, command: &mut Command) -> Result<()> { + use std::io::Write; + use std::os::unix::process::CommandExt; + + let cgroup = self + .cgroup + .as_ref() + .expect("a cgroup was created when the cgroup mechanism was chosen"); + // Open before forking: the child can then move itself in with a single write to + // an already-open descriptor, which is safe to do between fork and exec. + let procs = cgroup.open_procs()?; + // SAFETY: the closure only writes to an already-open file descriptor, and does + // not allocate or take locks, so it is safe to run between fork and exec. + unsafe { + command.pre_exec(move || { + // "0" means "the process doing the writing". + (&procs).write_all(b"0\n") + }); + } + Ok(()) + } + + #[cfg(not(target_os = "linux"))] + #[allow(clippy::unused_self, clippy::unnecessary_wraps)] + fn move_into_cgroup(&self, _command: &mut Command) -> Result<()> { + unreachable!("the cgroup mechanism is only ever chosen on Linux") + } +} + +/// `setrlimit(RLIMIT_AS)`, on the platforms where `nix` exposes it and cargo-mutants is +/// supported. Elsewhere there is no rlimit fallback at all, and `--max-memory` is an +/// error unless a cgroup can be used. +#[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 { + use std::process::Command; + + pub const ENFORCED: bool = false; + + pub fn settable(_bytes: u64) -> bool { + false + } + + pub fn apply_to_child(_command: &mut Command, _bytes: u64) { + unreachable!("the RLIMIT_AS mechanism is only ever chosen where it can be set") + } +} + +#[cfg(test)] +mod test { + use super::{MemoryMechanism, choose_mechanism}; + + #[test] + fn choose_mechanism_prefers_cgroups_over_rlimit() { + assert_eq!( + choose_mechanism(true, true, true).unwrap(), + MemoryMechanism::CgroupV2 + ); + assert_eq!( + choose_mechanism(false, true, true).unwrap(), + MemoryMechanism::RlimitAs + ); + } + + /// On macOS `RLIMIT_AS` can be set but is ignored, so say so rather than pretending. + #[test] + fn choose_mechanism_is_unenforced_when_rlimit_is_accepted_but_ignored() { + assert_eq!( + choose_mechanism(false, true, false).unwrap(), + MemoryMechanism::Unenforced + ); + } + + #[test] + fn choose_mechanism_with_no_usable_mechanism_is_an_error() { + 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"), + "unhelpful error message: {err}" + ); + // Also an error if RLIMIT_AS would be enforced but can't be set at all. + assert!(choose_mechanism(false, false, true).is_err()); + } +} diff --git a/src/process/memory/cgroup.rs b/src/process/memory/cgroup.rs new file mode 100644 index 00000000..15416dff --- /dev/null +++ b/src/process/memory/cgroup.rs @@ -0,0 +1,227 @@ +// 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. +//! 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. +//! 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::path::{Path, PathBuf}; +use std::process; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::thread::sleep; +use std::time::Duration; + +use anyhow::{Context, bail}; +use tracing::{debug, trace}; + +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); + +/// 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) + { + 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 }; + tree.create_scenario(bytes) + .context("test-create a scenario cgroup")? + .remove(); + 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 { + /// Distinguishes concurrent scenarios from each other. + static SERIAL: AtomicU64 = AtomicU64::new(0); + let dir = self.parent.join(format!( + "cargo-mutants-{pid}-{serial}", + pid = process::id(), + serial = SERIAL.fetch_add(1, Ordering::Relaxed) + )); + create_dir(&dir).with_context(|| format!("create cgroup {}", dir.display()))?; + let cgroup = ScenarioCgroup { dir }; + cgroup.write("memory.max", &bytes.to_string())?; + // Without this the kernel would swap a runaway scenario out instead of stopping + // it, which is slower than the failure we're trying to cause. + cgroup.write("memory.swap.max", "0")?; + Ok(cgroup) + } +} + +/// 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())) + } + + /// 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")).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()) + } + + /// Remove the cgroup, which the kernel only allows once it has no members left. + pub fn remove(self) { + // The process group sweep has already killed everything in here, but a process + // that has been killed still counts as a member until it is reaped, so give the + // kernel a moment to catch up. + for _ in 0..REMOVE_ATTEMPTS { + match remove_dir(&self.dir) { + Ok(()) => return, + Err(_) => sleep(REMOVE_RETRY_INTERVAL), + } + } + if let Err(err) = remove_dir(&self.dir) { + // Not worth failing a scenario over: an abandoned empty cgroup costs an inode. + debug!(?self.dir, ?err, "failed to remove scenario cgroup"); + } + } + + 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())) + } +} + +/// 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/src/process/unix.rs b/src/process/unix.rs index 6a9234e6..88e6282d 100644 --- a/src/process/unix.rs +++ b/src/process/unix.rs @@ -95,10 +95,10 @@ fn child_pgid(child: &Child) -> Pid { fn group_members(pgid: Pid) -> Option> { let mut pids = Vec::new(); for dir_entry in std::fs::read_dir("/proc").ok()?.flatten() { - let Ok(pid) = dir_entry.file_name().to_string_lossy().parse::() else { + let Ok(member) = dir_entry.file_name().to_string_lossy().parse::() else { continue; // not a process directory }; - let Ok(stat) = std::fs::read_to_string(format!("/proc/{pid}/stat")) else { + let Ok(stat) = std::fs::read_to_string(format!("/proc/{member}/stat")) else { continue; // it exited while we were looking }; // The second field is the command name in parentheses, and may itself contain @@ -113,7 +113,7 @@ fn group_members(pgid: Pid) -> Option> { .and_then(|field| field.parse::().ok()) == Some(pgid.as_raw()) { - pids.push(pid); + pids.push(member); } } Some(pids) 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 c99cf59b..d08cf5dd 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, @@ -3912,3 +3914,64 @@ fn processes_spawned_by_tests_are_swept_after_each_scenario() { }) ); } + +/// 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 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() { + 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(); + println!("stdout:\n{stdout}"); + println!( + "debug log:\n{}", + read_to_string(tmp_src_dir.path().join("mutants.out/debug.log")).unwrap_or_default() + ); + 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_phase = outcomes["outcomes"][0]["phase_results"] + .as_array() + .expect("phase_results") + .iter() + .find(|pr| pr["phase"] == "Test") + .expect("the mutant reached the test phase"); + let test_secs = test_phase["duration"].as_f64().expect("duration"); + assert!( + test_secs < 20.0, + "the memory-limited test took {test_secs}s, which is not well under the timeout" + ); +} From 776aba9a4c985e9b425c5e890217191717f273d2 Mon Sep 17 00:00:00 2001 From: Yuri Khrustalev Date: Tue, 15 Sep 2026 21:20:50 -0400 Subject: [PATCH 3/7] feat: report why a scenario's processes died Surface the killing signal, the cgroup oom_kill count, and anything the process group sweep reaped on the outcome line, in the scenario log, and in outcomes.json, so an OOM-caught mutant is distinguishable from one caught by a failing assertion. Classification rules are unchanged. --- NEWS.md | 2 + book/src/timeouts.md | 20 ++++++ src/cargo.rs | 3 +- src/console.rs | 4 ++ src/outcome.rs | 142 ++++++++++++++++++++++++++++++++++++++++++- src/process.rs | 40 +++++++++--- src/process/unix.rs | 8 +++ tests/main.rs | 22 +++++-- 8 files changed, 223 insertions(+), 18 deletions(-) diff --git a/NEWS.md b/NEWS.md index 911beaf2..9ef5209b 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, 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. + - 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. - 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. diff --git a/book/src/timeouts.md b/book/src/timeouts.md index 484b4b88..1355df59 100644 --- a/book/src/timeouts.md +++ b/book/src/timeouts.md @@ -121,6 +121,26 @@ 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/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 The multiplier timeout options cannot be used when the baseline is skipped diff --git a/src/cargo.rs b/src/cargo.rs index 947aa18a..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,6 +82,7 @@ pub fn run_cargo( duration: start.elapsed(), process_status, argv, + report, }) } 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..a2af9731 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, ProcessReport}; 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, 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 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() + .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,35 @@ pub struct PhaseResult { pub process_status: Exit, /// What command was run, as an argv list. pub argv: Vec, + /// What became of the process tree, beyond the exit status. + pub report: ProcessReport, } impl PhaseResult { pub fn is_success(&self) -> bool { self.process_status.is_success() } + + /// Anything worth saying about how this phase's process tree ended, beyond its exit + /// status. + 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(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 + } } impl Serialize for PhaseResult { @@ -316,11 +354,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("report", &self.report)?; ss.end() } } @@ -341,10 +380,104 @@ pub enum SummaryOutcome { mod test { use std::time::Duration; - use crate::process::Exit; + use crate::process::{Exit, ProcessReport, Sweep}; use super::{Phase, PhaseResult, Scenario, ScenarioOutcome}; + 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()], + report, + } + } + + 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), + ProcessReport::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), + 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, + ProcessReport { + sweep: Sweep { + pids: Some(vec![101, 102]), + strays: true, + killed: true, + }, + oom_kills: None, + }, + )]); + assert_eq!( + outcome.death_reasons(), + ["test left 2 stray processes behind (SIGKILLed: 101, 102)"] + ); + } + #[test] fn find_phase_result() { let outcome = ScenarioOutcome { @@ -358,12 +491,14 @@ mod test { duration: Duration::from_secs(2), process_status: Exit::Success, argv: vec!["cargo".into(), "build".into()], + report: ProcessReport::default(), }, PhaseResult { phase: Phase::Test, duration: Duration::from_secs(3), process_status: Exit::Success, argv: vec!["cargo".into(), "test".into()], + report: ProcessReport::default(), }, ], }; @@ -374,6 +509,7 @@ mod test { duration: Duration::from_secs(2), process_status: Exit::Success, argv: vec!["cargo".into(), "build".into()], + report: ProcessReport::default(), }) ); assert_eq!( diff --git a/src/process.rs b/src/process.rs index ee65ca84..c3eb1767 100644 --- a/src/process.rs +++ b/src/process.rs @@ -35,6 +35,8 @@ use windows::{configure_command, sweep_process_group, terminate_child}; #[cfg(unix)] mod unix; #[cfg(unix)] +pub use unix::signal_name; +#[cfg(unix)] use unix::{configure_command, sweep_process_group, terminate_child}; pub mod memory; @@ -46,7 +48,7 @@ use memory::{MemoryLimit, ScenarioMemoryLimit}; /// 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)] +#[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>, @@ -62,19 +64,32 @@ impl Sweep { if !self.strays { return None; } - let how = if self.killed { "killed" } else { "reaped" }; + let how = if self.killed { "SIGKILLed" } else { "reaped" }; Some(match &self.pids { - Some(pids) => format!( - "{how} {n} stray process{es} left over by the tests: {list}", + 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(", ") ), - None => format!("{how} stray processes left over by the tests"), + _ => format!("left stray processes behind ({how})"), }) } } +/// 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, @@ -100,7 +115,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, @@ -122,15 +137,22 @@ impl Process { 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. - if let Some(oom_kills) = child.memory.take().and_then(ScenarioMemoryLimit::finish) { + 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. diff --git a/src/process/unix.rs b/src/process/unix.rs index 88e6282d..beac7cc8 100644 --- a/src/process/unix.rs +++ b/src/process/unix.rs @@ -80,6 +80,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. fn child_pgid(child: &Child) -> Pid { diff --git a/tests/main.rs b/tests/main.rs index d08cf5dd..835e7f53 100644 --- a/tests/main.rs +++ b/tests/main.rs @@ -3866,15 +3866,13 @@ 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) .assert(); - println!( - "stdout:\n{}", - String::from_utf8_lossy(&assert.get_output().stdout) - ); + 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) @@ -3901,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), @@ -3961,6 +3966,13 @@ 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}" + ); + // It should die on the memory limit long before the 60s test timeout. let outcomes = outcome_json(&tmp_src_dir); let test_phase = outcomes["outcomes"][0]["phase_results"] From b17d491a72080244c6a44a00a63b0c3732cc2d74 Mon Sep 17 00:00:00 2001 From: Yuri Khrustalev Date: Tue, 15 Sep 2026 21:48:17 -0400 Subject: [PATCH 4/7] fix: remove scenario cgroups on drop, not only on the happy path Creation was not atomic: a failure writing memory.max left the directory behind, as did any early return between creating the cgroup and finishing with it. Move removal into Drop so every path is covered, and stop treating a missing memory.swap.max as fatal -- it only exists where the kernel accounts for swap, and memory.max alone still bounds resident memory. Also tolerate a name left over by a run that died with this pid, and say so in the log when memory.events can't be read. --- src/process/memory/cgroup.rs | 121 +++++++++++++++++++++++++---------- 1 file changed, 86 insertions(+), 35 deletions(-) diff --git a/src/process/memory/cgroup.rs b/src/process/memory/cgroup.rs index 15416dff..dba64139 100644 --- a/src/process/memory/cgroup.rs +++ b/src/process/memory/cgroup.rs @@ -12,21 +12,26 @@ //! 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. +//! 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. +//! 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, bail}; -use tracing::{debug, trace}; +use anyhow::{Context, anyhow, bail}; +use tracing::{debug, trace, warn}; use crate::Result; @@ -38,6 +43,10 @@ const CGROUP_ROOT: &str = "/sys/fs/cgroup"; 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 { @@ -79,6 +88,13 @@ impl CgroupTree { && 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); } @@ -97,30 +113,56 @@ impl CgroupTree { /// 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")? - .remove(); + .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 { - /// Distinguishes concurrent scenarios from each other. - static SERIAL: AtomicU64 = AtomicU64::new(0); - let dir = self.parent.join(format!( - "cargo-mutants-{pid}-{serial}", - pid = process::id(), - serial = SERIAL.fetch_add(1, Ordering::Relaxed) - )); - create_dir(&dir).with_context(|| format!("create cgroup {}", dir.display()))?; - let cgroup = ScenarioCgroup { dir }; + let cgroup = ScenarioCgroup { + dir: self.claim_dir()?, + }; cgroup.write("memory.max", &bytes.to_string())?; - // Without this the kernel would swap a runaway scenario out instead of stopping - // it, which is slower than the failure we're trying to cause. - cgroup.write("memory.swap.max", "0")?; + // 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. @@ -143,7 +185,9 @@ impl ScenarioCgroup { /// 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")).ok()?; + 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() @@ -151,27 +195,34 @@ impl ScenarioCgroup { .and_then(|count| count.trim().parse().ok()) } - /// Remove the cgroup, which the kernel only allows once it has no members left. - pub fn remove(self) { - // The process group sweep has already killed everything in here, but a process - // that has been killed still counts as a member until it is reaped, so give the - // kernel a moment to catch up. - for _ in 0..REMOVE_ATTEMPTS { + 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(()) => return, - Err(_) => sleep(REMOVE_RETRY_INTERVAL), + Ok(()) => Some(Ok(())), + Err(err) if attempt + 1 == REMOVE_ATTEMPTS => Some(Err(err)), + Err(_) => None, } - } - if let Err(err) = remove_dir(&self.dir) { + }); + if let Some(Err(err)) = last_failure { // Not worth failing a scenario over: an abandoned empty cgroup costs an inode. - debug!(?self.dir, ?err, "failed to remove scenario cgroup"); + debug!(?self.dir, ?err, "gave up removing scenario cgroup"); } } - - 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())) - } } /// The directory of the cgroup v2 cgroup that this process is in. From a1ceb451dac677cba1f47f8d25c620a071427927 Mon Sep 17 00:00:00 2001 From: Yuri Khrustalev Date: Tue, 15 Sep 2026 21:48:17 -0400 Subject: [PATCH 5/7] refactor: make the memory limit's mechanism and state one enum Carrying a MemoryMechanism beside an Option let the two disagree, and the code paid for it with an expect(), two unreachable!()s and five clippy allows. Give each variant the state its mechanism needs, and cfg out the RlimitAs variants where nix has no RLIMIT_AS, so the impossible combinations cannot be written rather than merely never happening. --- src/process/memory.rs | 231 +++++++++++++++++++++--------------------- 1 file changed, 117 insertions(+), 114 deletions(-) diff --git a/src/process/memory.rs b/src/process/memory.rs index 08808d1f..dcfc7d84 100644 --- a/src/process/memory.rs +++ b/src/process/memory.rs @@ -15,6 +15,11 @@ //! * `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 -- 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` variants do not exist at all, which is what makes them unconstructible +//! rather than merely unreachable. #[cfg(target_os = "linux")] mod cgroup; @@ -78,12 +83,21 @@ 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, so there is no way to be in +/// cgroup mode without a cgroup to put scenarios in. #[derive(Debug)] -pub struct MemoryLimit { - bytes: u64, - mechanism: MemoryMechanism, +pub enum MemoryLimit { #[cfg(target_os = "linux")] - cgroups: Option, + CgroupV2 { + bytes: u64, + tree: cgroup::CgroupTree, + }, + #[cfg(any(target_os = "linux", target_os = "android", target_os = "macos"))] + RlimitAs { + bytes: u64, + }, + Unenforced, } impl MemoryLimit { @@ -92,127 +106,122 @@ impl MemoryLimit { /// 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 { - #[cfg(target_os = "linux")] - let cgroups = match cgroup::CgroupTree::probe(bytes) { - Ok(tree) => Some(tree), - Err(err) => { - debug!(?err, "cgroup v2 memory limits are not available"); - None - } - }; - #[cfg(target_os = "linux")] - let cgroup_available = cgroups.is_some(); - #[cfg(not(target_os = "linux"))] - let cgroup_available = false; - - let mechanism = - choose_mechanism(cgroup_available, rlimit::settable(bytes), rlimit::ENFORCED)?; - if mechanism == MemoryMechanism::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, and cgroups are not available" ); } else { info!( "Limiting each scenario to {bytes} bytes of memory using {}", - mechanism.describe() + limit.mechanism().describe() ); } - Ok(MemoryLimit { - bytes, - mechanism, + 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")] - cgroups, - }) + 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. - #[allow(clippy::unnecessary_wraps)] // fallible only where cgroups exist + #[cfg_attr(not(target_os = "linux"), allow(clippy::unnecessary_wraps))] pub fn start(&self) -> Result { - #[cfg(target_os = "linux")] - let cgroup = self - .cgroups - .as_ref() - .map(|tree| tree.create_scenario(self.bytes)) - .transpose() - .context("create a cgroup for this scenario")?; - Ok(ScenarioMemoryLimit { - bytes: self.bytes, - mechanism: self.mechanism, + Ok(match self { #[cfg(target_os = "linux")] - cgroup, + 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. #[derive(Debug)] -pub struct ScenarioMemoryLimit { - bytes: u64, - mechanism: MemoryMechanism, +pub enum ScenarioMemoryLimit { #[cfg(target_os = "linux")] - cgroup: Option, + 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. + #[cfg_attr(not(target_os = "linux"), allow(clippy::unnecessary_wraps))] pub fn configure_command(&self, command: &mut Command) -> Result<()> { - match self.mechanism { - MemoryMechanism::CgroupV2 => self.move_into_cgroup(command), - MemoryMechanism::RlimitAs => { - rlimit::apply_to_child(command, self.bytes); - Ok(()) + 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")); + } } - MemoryMechanism::Unenforced => Ok(()), + #[cfg(any(target_os = "linux", target_os = "android", target_os = "macos"))] + ScenarioMemoryLimit::RlimitAs(bytes) => rlimit::apply_to_child(command, *bytes), + ScenarioMemoryLimit::Unenforced => {} } + Ok(()) } /// Finish with the limit, returning how many times the kernel OOM-killed something /// in this scenario, where the mechanism can tell us. - #[allow(clippy::unused_self)] // only cgroups have anything to report + /// + /// Any cgroup is removed as this is dropped. pub fn finish(self) -> Option { - #[cfg(target_os = "linux")] - if let Some(cgroup) = self.cgroup { - let oom_kills = cgroup.oom_kills(); - cgroup.remove(); - return oom_kills; - } - None - } - - #[cfg(target_os = "linux")] - fn move_into_cgroup(&self, command: &mut Command) -> Result<()> { - use std::io::Write; - use std::os::unix::process::CommandExt; - - let cgroup = self - .cgroup - .as_ref() - .expect("a cgroup was created when the cgroup mechanism was chosen"); - // Open before forking: the child can then move itself in with a single write to - // an already-open descriptor, which is safe to do between fork and exec. - let procs = cgroup.open_procs()?; - // SAFETY: the closure only writes to an already-open file descriptor, and does - // not allocate or take locks, so it is safe to run between fork and exec. - unsafe { - command.pre_exec(move || { - // "0" means "the process doing the writing". - (&procs).write_all(b"0\n") - }); + match self { + #[cfg(target_os = "linux")] + ScenarioMemoryLimit::CgroupV2(cgroup) => cgroup.oom_kills(), + _ => None, } - Ok(()) - } - - #[cfg(not(target_os = "linux"))] - #[allow(clippy::unused_self, clippy::unnecessary_wraps)] - fn move_into_cgroup(&self, _command: &mut Command) -> Result<()> { - unreachable!("the cgroup mechanism is only ever chosen on Linux") } } -/// `setrlimit(RLIMIT_AS)`, on the platforms where `nix` exposes it and cargo-mutants is -/// supported. Elsewhere there is no rlimit fallback at all, and `--max-memory` is an -/// error unless a cgroup can be used. #[cfg(any(target_os = "linux", target_os = "android", target_os = "macos"))] mod rlimit { use std::io; @@ -253,53 +262,47 @@ mod rlimit { #[cfg(not(any(target_os = "linux", target_os = "android", target_os = "macos")))] mod rlimit { - use std::process::Command; - pub const ENFORCED: bool = false; pub fn settable(_bytes: u64) -> bool { false } - - pub fn apply_to_child(_command: &mut Command, _bytes: u64) { - unreachable!("the RLIMIT_AS mechanism is only ever chosen where it can be set") - } } #[cfg(test)] mod test { use super::{MemoryMechanism, choose_mechanism}; + /// The mechanism is picked from what the platform can do, in order of preference. #[test] - fn choose_mechanism_prefers_cgroups_over_rlimit() { - assert_eq!( - choose_mechanism(true, true, true).unwrap(), - MemoryMechanism::CgroupV2 - ); - assert_eq!( - choose_mechanism(false, true, true).unwrap(), - MemoryMechanism::RlimitAs - ); - } - - /// On macOS `RLIMIT_AS` can be set but is ignored, so say so rather than pretending. - #[test] - fn choose_mechanism_is_unenforced_when_rlimit_is_accepted_but_ignored() { - assert_eq!( - choose_mechanism(false, true, false).unwrap(), - MemoryMechanism::Unenforced - ); + fn choose_mechanism_picks_by_availability() { + // (cgroups available, RLIMIT_AS settable, RLIMIT_AS enforced) -> mechanism, where + // None means --max-memory should be rejected outright. + let cases = [ + ((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. + ((false, true, false), Some(MemoryMechanism::Unenforced)), + ((false, false, false), None), + ((false, false, true), None), + ]; + for ((cgroup, settable, enforced), expected) in cases { + assert_eq!( + choose_mechanism(cgroup, settable, enforced).ok(), + expected, + "cgroup={cgroup} settable={settable} enforced={enforced}" + ); + } } #[test] - fn choose_mechanism_with_no_usable_mechanism_is_an_error() { + fn choose_mechanism_with_no_usable_mechanism_names_the_option() { 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"), "unhelpful error message: {err}" ); - // Also an error if RLIMIT_AS would be enforced but can't be set at all. - assert!(choose_mechanism(false, false, true).is_err()); } } From 80e4d14b7b0e597dfd3fbbf794f710f7d4939166 Mon Sep 17 00:00:00 2001 From: Yuri Khrustalev Date: Tue, 15 Sep 2026 21:48:18 -0400 Subject: [PATCH 6/7] fix: escalate to SIGKILL when a child ignores SIGTERM on timeout terminate() sent one SIGTERM and then waited indefinitely, so a cargo process that ignored or could not receive it -- stopped, say -- hung the whole run, and the process group sweep that would have killed it never ran. Wait only for the shared grace period, then kill the group. Also probe the process group before listing it: nearly every phase leaves nothing behind, and listing meant reading every /proc//stat on the machine, once per phase. --- src/process.rs | 44 +++++++++++++++++++----- src/process/unix.rs | 76 ++++++++++++++++++++++-------------------- src/process/windows.rs | 6 ++++ 3 files changed, 82 insertions(+), 44 deletions(-) diff --git a/src/process.rs b/src/process.rs index c3eb1767..7c829466 100644 --- a/src/process.rs +++ b/src/process.rs @@ -27,17 +27,24 @@ 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, 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 or group has gone away. +const TERM_POLL_INTERVAL: Duration = Duration::from_millis(20); + #[cfg(windows)] mod windows; #[cfg(windows)] -use windows::{configure_command, sweep_process_group, terminate_child}; +use windows::{configure_command, kill_child, sweep_process_group, terminate_child}; #[cfg(unix)] mod unix; #[cfg(unix)] pub use unix::signal_name; #[cfg(unix)] -use unix::{configure_command, sweep_process_group, terminate_child}; +use unix::{configure_command, kill_child, sweep_process_group, terminate_child}; pub mod memory; use memory::{MemoryLimit, ScenarioMemoryLimit}; @@ -234,20 +241,41 @@ impl Process { } } - /// Ask the subprocess to stop, and block until it has. + /// Stop the subprocess, and block until it has gone. /// - /// This only gets the direct child out of the way so that we can stop waiting on - /// it; anything else in its process group, including anything that ignored the - /// `SIGTERM`, is dealt with by the sweep in [`Process::run`]. + /// `SIGTERM` first, so it gets a chance to clean up, but only for a bounded grace + /// 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(); 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 beac7cc8..2098dae3 100644 --- a/src/process/unix.rs +++ b/src/process/unix.rs @@ -1,7 +1,7 @@ use std::os::unix::process::{CommandExt, ExitStatusExt}; use std::process::{Child, Command, ExitStatus}; use std::thread::sleep; -use std::time::{Duration, Instant}; +use std::time::Instant; use anyhow::bail; use nix::errno::Errno; @@ -11,13 +11,7 @@ use tracing::warn; use crate::Result; -use super::{Exit, Sweep}; - -/// How long to let a process group wind up after `SIGTERM` before sending `SIGKILL`. -const SWEEP_GRACE: Duration = Duration::from_millis(500); - -/// How often to check whether a signalled process group has emptied out. -const SWEEP_POLL_INTERVAL: Duration = Duration::from_millis(20); +use super::{Exit, Sweep, TERM_GRACE, TERM_POLL_INTERVAL}; /// Send a signal to a process group, returning whether the group still had any member. /// @@ -46,6 +40,13 @@ pub(super) fn terminate_child(child: &mut Child) -> Result<()> { 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 @@ -55,11 +56,14 @@ pub(super) fn terminate_child(child: &mut Child) -> Result<()> { #[mutants::skip] // would leak processes from tests if skipped pub(super) fn sweep_process_group(child: &Child) -> Result { let pgid = child_pgid(child); - let pids = group_members(pgid); - if !signal_group(pgid, Some(Signal::SIGTERM))? { + // 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 deadline = Instant::now() + SWEEP_GRACE; + 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 { @@ -70,7 +74,7 @@ pub(super) fn sweep_process_group(child: &Child) -> Result { } else if Instant::now() >= deadline { break; } - sleep(SWEEP_POLL_INTERVAL); + sleep(TERM_POLL_INTERVAL); } signal_group(pgid, Some(Signal::SIGKILL))?; Ok(Sweep { @@ -90,6 +94,11 @@ pub fn signal_name(signal: i32) -> String { /// 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")) } @@ -101,30 +110,25 @@ fn child_pgid(child: &Child) -> Pid { #[cfg(target_os = "linux")] #[mutants::skip] // only affects what we can say in the debug log fn group_members(pgid: Pid) -> Option> { - let mut pids = Vec::new(); - for dir_entry in std::fs::read_dir("/proc").ok()?.flatten() { - let Ok(member) = dir_entry.file_name().to_string_lossy().parse::() else { - continue; // not a process directory - }; - let Ok(stat) = std::fs::read_to_string(format!("/proc/{member}/stat")) else { - continue; // it exited while we were looking - }; - // 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 Some(after_comm) = stat.rsplit_once(')').map(|(_, rest)| rest) else { - continue; - }; - if after_comm - .split_whitespace() - .nth(2) - .and_then(|field| field.parse::().ok()) - == Some(pgid.as_raw()) - { - pids.push(member); - } - } - Some(pids) + 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) +} + +/// 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"))] diff --git a/src/process/windows.rs b/src/process/windows.rs index 58d5b60a..b23708ee 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") +} + /// 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 From 520dafed22da6232f335e41d5eb2a56e404a7a69 Mon Sep 17 00:00:00 2001 From: Yuri Khrustalev Date: Tue, 15 Sep 2026 21:48:18 -0400 Subject: [PATCH 7/7] fix: reject unusable --max-memory values, and report limit stops --max-memory=0 was accepted and meant 'stop every scenario immediately', which is the opposite of what -t 0 means elsewhere in this tool; require at least 1M. An OOM-killed mutant is also a caught mutant, so it was invisible without -v: count those separately in the run summary. The Linux test needed a writable cgroup and failed rather than skipped without one, which would be red on most CI runners; it now detects the mechanism and skips. --- NEWS.md | 2 +- book/src/timeouts.md | 36 +++++++++++++++----- src/lab.rs | 5 ++- src/main.rs | 7 +--- src/options.rs | 80 +++++++++++++++++++++++++++++--------------- src/outcome.rs | 28 ++++++++++++++-- tests/main.rs | 55 ++++++++++++++++++++---------- 7 files changed, 148 insertions(+), 65 deletions(-) diff --git a/NEWS.md b/NEWS.md index 9ef5209b..fa96921f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -4,7 +4,7 @@ - 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. -- 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. +- 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: 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. diff --git a/book/src/timeouts.md b/book/src/timeouts.md index 1355df59..71969256 100644 --- a/book/src/timeouts.md +++ b/book/src/timeouts.md @@ -56,7 +56,9 @@ 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. +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 @@ -75,7 +77,9 @@ 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, 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. + 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*, which is a much cruder proxy: allocators and rustc reserve @@ -90,11 +94,18 @@ INFO Limiting each scenario to 8589934592 bytes of memory using cgroup v2 memory ``` For the cgroup mechanism, cargo-mutants needs somewhere it may create child cgroups with -`memory.max`. It looks at its own cgroup first, and then at its 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. +`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 @@ -102,7 +113,12 @@ platform where *neither* mechanism can be applied, giving `--max-memory` is an e 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 -OOM-killed fails its tests and so is caught, in just the same way as one that panics. +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 @@ -117,6 +133,10 @@ Once the cargo process itself exits, anything left in the group is sent `SIGTERM 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. diff --git a/src/lab.rs b/src/lab.rs index 2e6bf25f..8e6d830f 100644 --- a/src/lab.rs +++ b/src/lab.rs @@ -38,9 +38,8 @@ pub fn test_mutants( ) -> Result { let start_time = Instant::now(); console.set_debug_log(output_dir.open_debug_log()?); - // Before copying the tree or running anything: if the user asked for a memory limit - // that can't be enforced here, they should hear about it now, not after a long run - // that silently had no limit. + // 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); diff --git a/src/main.rs b/src/main.rs index 581c427f..6cfdb4b9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -294,12 +294,7 @@ pub struct Args { /// 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. - /// - /// On Linux this uses a cgroup v2 `memory.max` if one can be created, and otherwise - /// `setrlimit(RLIMIT_AS)`, which limits address space rather than resident memory. - /// macOS accepts `RLIMIT_AS` but does not act on it, so this option has no effect - /// there. + /// binary multiples: `1K` is 1024 bytes. Enforced on Linux only; see the manual. #[arg(long, help_heading = "Execution", value_name = "SIZE")] max_memory: Option, diff --git a/src/options.rs b/src/options.rs index da4d5010..58e3a75b 100644 --- a/src/options.rs +++ b/src/options.rs @@ -240,6 +240,12 @@ 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 @@ -264,6 +270,17 @@ fn parse_size(s: &str) -> Result { .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")] @@ -397,7 +414,7 @@ impl Options { .max_memory .as_deref() .or(config.max_memory.as_deref()) - .map(parse_size) + .map(parse_max_memory) .transpose() .context("Failed to parse --max-memory")?, minimum_test_timeout, @@ -605,53 +622,62 @@ mod test { } #[test] - fn parse_size_understands_binary_suffixes() { - assert_eq!(parse_size("0").unwrap(), 0); - assert_eq!(parse_size("1024").unwrap(), 1024); - assert_eq!(parse_size("1024B").unwrap(), 1024); - assert_eq!(parse_size("256M").unwrap(), 256 * 1024 * 1024); - assert_eq!(parse_size("256MiB").unwrap(), 256 * 1024 * 1024); - assert_eq!(parse_size(" 4g ").unwrap(), 4 * 1024 * 1024 * 1024); - assert_eq!(parse_size("2T").unwrap(), 2 * (1u64 << 40)); + 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_size_rejects_nonsense() { - for bad in [ - "", - "M", - "-1", - "1.5G", - "1 zettabyte", - "18446744073709551615K", - ] { + fn parse_max_memory_rejects_uselessly_small_limits() { + for tiny in ["0", "1", "1K", "1023K"] { assert!( - parse_size(bad).is_err(), - "{bad:?} should not parse as a size" + 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() { + fn options_from_max_memory_arg() -> Result<(), Box> { let args = Args::parse_from(["mutants", "--max-memory=256M"]); - let options = Options::new(&args, &Config::default()).unwrap(); + 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()).unwrap(); + let options = Options::new(&args, &Config::default())?; assert_eq!(options.max_memory, None); + Ok(()) } #[test] - fn cli_max_memory_overrides_config() { - let config: Config = "max_memory = \"8G\"".parse().unwrap(); - let options = Options::new(&Args::parse_from(["mutants"]), &config).unwrap(); + 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).unwrap(); + let options = Options::new(&args, &config)?; assert_eq!(options.max_memory, Some(1024 * 1024 * 1024)); + Ok(()) } #[test] diff --git a/src/outcome.rs b/src/outcome.rs index a2af9731..0c3e109c 100644 --- a/src/outcome.rs +++ b/src/outcome.rs @@ -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("") } } @@ -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() @@ -327,8 +353,6 @@ impl PhaseResult { self.process_status.is_success() } - /// Anything worth saying about how this phase's process tree ended, beyond its exit - /// status. fn death_reasons(&self) -> Vec { let phase = self.phase.name(); let mut reasons = Vec::new(); diff --git a/tests/main.rs b/tests/main.rs index 835e7f53..6c1db73b 100644 --- a/tests/main.rs +++ b/tests/main.rs @@ -3857,12 +3857,13 @@ fn in_diff_with_nonexistent_file_returns_exit_code_6() { /// probe them once cargo-mutants has finished. #[cfg(unix)] #[test] -fn processes_spawned_by_tests_are_swept_after_each_scenario() { +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().unwrap(); + let pid_dir = tempdir()?; let pid_file = pid_dir.path().join("pids.txt"); let assert = run() .arg("mutants") @@ -3875,24 +3876,25 @@ fn processes_spawned_by_tests_are_swept_after_each_scenario() { println!("stdout:\n{stdout}"); assert.success(); - let pids: Vec = read_to_string(&pid_file) - .expect("read background child pid file") + let pids: Vec = read_to_string(&pid_file)? .lines() - .map(|line| line.trim().parse().expect("parse pid")) - .collect(); - assert!( - !pids.is_empty(), - "the tree's test should have spawned background processes" - ); + .map(str::trim) + .map(str::parse) + .collect::>()?; let survivors: Vec = pids .iter() .copied() .filter(|pid| kill(Pid::from_raw(*pid), None).is_ok()) .collect(); - // Don't leave them running even if the assertion below fails. + // 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(), @@ -3918,6 +3920,7 @@ fn processes_spawned_by_tests_are_swept_after_each_scenario() { "success": 0, }) ); + Ok(()) } /// A mutant can turn a bounded loop into an unbounded allocator. `--max-memory` puts a @@ -3927,7 +3930,8 @@ fn processes_spawned_by_tests_are_swept_after_each_scenario() { /// 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() { +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") @@ -3946,11 +3950,19 @@ fn max_memory_catches_a_mutant_that_allocates_without_bound() { .timeout(OUTER_TIMEOUT) .assert(); let stdout = String::from_utf8_lossy(&assert.get_output().stdout).into_owned(); - println!("stdout:\n{stdout}"); + 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() ); + + // 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(()); + } assert.success(); assert_eq!( @@ -3967,23 +3979,30 @@ fn max_memory_catches_a_mutant_that_allocates_without_bound() { ); // 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. + // 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_phase = outcomes["outcomes"][0]["phase_results"] + let test_secs = outcomes["outcomes"][0]["phase_results"] .as_array() - .expect("phase_results") + .ok_or("no phase_results in outcomes.json")? .iter() .find(|pr| pr["phase"] == "Test") - .expect("the mutant reached the test phase"); - let test_secs = test_phase["duration"].as_f64().expect("duration"); + .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(()) }