fix: sweep the child's process group after every phase

NEWS.md

@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+- Fixed: Each cargo invocation now runs as the leader of its own process group, and that group is swept after every phase, not only after a timeout. Processes left running by a scenario's tests are `SIGTERM`ed, given a short grace period, and then `SIGKILL`ed, so they can't keep consuming memory or CPU while later mutants are tested. Unix only.
+
+- Fixed: After a timeout, cargo-mutants waits only a short grace period for the child to exit after `SIGTERM` before sending `SIGKILL`. Previously it waited indefinitely, so a process that ignored `SIGTERM`, or that had been stopped, hung the whole run.
+
 - New: `#[mutants::exclude_re("pattern")]` attribute to exclude specific mutations by regex, without disabling all mutations on the function. The attribute can be placed on functions, `impl` blocks, `trait` blocks, modules, files, and on expressions that can carry an attribute (such as `match`, struct literals, call expressions, method calls, and unary expressions). Multiple patterns can be applied. Also supported within `cfg_attr`. Requires the [mutants](https://crates.io/crates/mutants) crate version `0.0.5` or later.
 
 - 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)).

book/src/timeouts.md

@@ -33,6 +33,11 @@ You can set an explicit timeouts with the `--timeout` option, also measured in s
 You can also set the test timeout as a multiple of the duration of the baseline test, with the `--timeout-multiplier` option and the `timeout_multiplier` configuration key.
 The multiplier only has an effect if the baseline is not skipped and if `--timeout` is not specified.
 
+When a timeout expires, the process is sent `SIGTERM` so that it can clean up, and then
+`SIGKILL` if it has not exited within a short grace period. A test process that ignores
+`SIGTERM`, or that has been stopped and so never receives it, therefore cannot stall the
+whole run.
+
 ## Build timeouts
 
 `const` expressions may be evaluated at compile time. In the same way that mutations can cause tests to hang, mutations to const code may potentially cause the compiler to enter an infinite loop.
@@ -45,6 +50,23 @@ In this case you can use the `--build-timeout` or `--build-timeout-multiplier` o
 
 You might also choose to skip mutants that can cause long-running const evaluation.
 
+## Leftover processes
+
+A test can leave processes running after it returns: a daemon it started, a helper it
+forgot to wait for, or a test binary that was not reaped. Those processes keep running
+— and keep allocating — while cargo-mutants moves on to the next mutant, so they can
+exhaust the machine's memory in a window where no cargo phase is running at all.
+
+To prevent this, cargo-mutants starts each cargo invocation as the leader of its own
+process group, and sweeps that group after *every* phase, not only after a timeout.
+Once the cargo process itself exits, anything left in the group is sent `SIGTERM`, given
+a short grace period, and then `SIGKILL`ed. What was reaped is recorded in the
+scenario's log, and the pids are shown at `--level=debug`.
+
+This has no effect on how a mutant is classified; it only stops work from one scenario
+leaking into the next. Windows has no process groups, and cargo-mutants does not yet use
+job objects, so this sweep is Unix-only.
+
 ## Exceptions
 
 The multiplier timeout options cannot be used when the baseline is skipped

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(),

src/process.rs

@@ -15,6 +15,7 @@ use std::time::{Duration, Instant};
 
 use anyhow::Context;
 use camino::Utf8Path;
+use itertools::Itertools;
 use serde::Serialize;
 use tracing::{Level, debug, span, trace};
 
@@ -26,15 +27,57 @@ use crate::output::ScenarioOutput;
 /// How frequently to check if a subprocess finished.
 const WAIT_POLL_INTERVAL: Duration = Duration::from_millis(50);
 
+/// How long to let a process, 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, terminate_child};
+use windows::{configure_command, kill_child, sweep_process_group, terminate_child};
 
 #[cfg(unix)]
 mod unix;
 #[cfg(unix)]
-use unix::{configure_command, terminate_child};
+use unix::{configure_command, kill_child, sweep_process_group, terminate_child};
+
+/// What sweeping a finished child's process group found and did.
+///
+/// A scenario's tests can leave processes running: a test binary that was not waited
+/// for, or anything a test spawned and forgot. Those processes stay in the child's
+/// process group, and would otherwise keep running (and keep allocating) while
+/// cargo-mutants moves on to later scenarios.
+#[derive(Debug, Default, Clone, PartialEq, Eq, Serialize)]
+pub struct Sweep {
+    /// The pids that were in the group, where the platform can enumerate them.
+    pub pids: Option<Vec<i32>>,
+    /// Whether any process was still in the group after the direct child exited.
+    pub strays: bool,
+    /// Whether `SIGTERM` was not enough, and `SIGKILL` had to be sent.
+    pub killed: bool,
+}
+
+impl Sweep {
+    /// Describe what was reaped, or None if nothing was left over.
+    pub fn describe(&self) -> Option<String> {
+        if !self.strays {
+            return None;
+        }
+        let how = if self.killed { "SIGKILLed" } else { "reaped" };
+        Some(match &self.pids {
+            Some(pids) if !pids.is_empty() => format!(
+                "left {n} stray process{es} behind ({how}: {list})",
+                n = pids.len(),
+                es = if pids.len() == 1 { "" } else { "es" },
+                list = pids.iter().join(", ")
+            ),
+            _ => format!("left stray processes behind ({how})"),
+        })
+    }
+}
 
 pub struct Process {
     child: Child,
@@ -45,6 +88,9 @@ pub struct Process {
 impl Process {
     /// Run a subprocess to completion, watching for interrupts, with a timeout, while
     /// ticking the progress bar.
+    ///
+    /// Whatever the outcome, the child's process group is swept before returning, so
+    /// that nothing it left running survives into the next scenario.
     pub fn run(
         argv: &[String],
         env: &[(String, String)],
@@ -53,17 +99,42 @@ impl Process {
         jobserver: Option<&jobserver::Client>,
         scenario_output: &mut ScenarioOutput,
         console: &Console,
-    ) -> Result<Exit> {
+    ) -> 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<Sweep> {
+        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.
@@ -124,16 +195,41 @@ impl Process {
     ///
     /// Blocks until the subprocess is terminated and then returns the exit status.
     ///
+    /// `SIGTERM` first, so it gets a chance to clean up, but only for a bounded grace
+    /// period: a process that ignores it, or that is stopped and so never receives it,
+    /// would otherwise hang the run here forever. Whatever else is left in the process
+    /// group is dealt with by the sweep in [`Process::run`].
+    ///
     /// The status might not be `Timeout` if this raced with a normal exit.
     #[mutants::skip] // would leak processes from tests if skipped
     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(())
     }
@@ -197,7 +293,68 @@ fn quote_argv<S: AsRef<str>, I: IntoIterator<Item = S>>(argv: I) -> String {
 
 #[cfg(test)]
 mod test {
-    use super::quote_argv;
+    use super::{Sweep, quote_argv};
+
+    #[test]
+    fn sweep_describes_what_it_reaped() {
+        // (sweep, expected description)
+        let cases = [
+            (Sweep::default(), None),
+            (
+                Sweep {
+                    pids: None,
+                    strays: true,
+                    killed: false,
+                },
+                Some("left stray processes behind (reaped)"),
+            ),
+            (
+                Sweep {
+                    pids: None,
+                    strays: true,
+                    killed: true,
+                },
+                Some("left stray processes behind (SIGKILLed)"),
+            ),
+            (
+                // Enumeration ran but came up empty, so don't claim "0 processes".
+                Sweep {
+                    pids: Some(vec![]),
+                    strays: true,
+                    killed: false,
+                },
+                Some("left stray processes behind (reaped)"),
+            ),
+            (
+                Sweep {
+                    pids: Some(vec![101]),
+                    strays: true,
+                    killed: false,
+                },
+                Some("left 1 stray process behind (reaped: 101)"),
+            ),
+            (
+                Sweep {
+                    pids: Some(vec![101, 102]),
+                    strays: true,
+                    killed: true,
+                },
+                Some("left 2 stray processes behind (SIGKILLed: 101, 102)"),
+            ),
+            (
+                // Nothing was left behind, whatever else the sweep recorded.
+                Sweep {
+                    pids: Some(vec![101]),
+                    strays: false,
+                    killed: true,
+                },
+                None,
+            ),
+        ];
+        for (sweep, expected) in cases {
+            assert_eq!(sweep.describe().as_deref(), expected, "sweep: {sweep:?}");
+        }
+    }
 
     #[test]
     fn shell_quoting() {