From 1aab23241f463ed4d43e0cfca8b91c6cb29ba73c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matheus=20Bregu=C3=AAz?= Date: Sun, 27 Sep 2026 05:58:38 -0300 Subject: [PATCH] fix(until): terminate surviving descendants after readiness --- src/timer/mod.rs | 51 ++++++++++++++++++++++++++++++++++++++ tests/until_tests.rs | 58 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+) diff --git a/src/timer/mod.rs b/src/timer/mod.rs index 3bc00ad..ae4bdec 100644 --- a/src/timer/mod.rs +++ b/src/timer/mod.rs @@ -151,6 +151,56 @@ impl TerminateWatcher { } } +/// Ensure all processes in the process group are terminated safely. +/// +/// After SIGTERM is delivered and the process group leader exits, descendants that +/// ignore SIGTERM may still be running. This gives well-behaved processes a brief +/// grace period to exit cleanly before forcefully killing any surviving descendants +/// with SIGKILL. +#[cfg(not(windows))] +fn terminate_process_group(pid: u32) { + if pid <= 1 { + return; + } + let pgid = -(pid as libc::pid_t); + + let is_group_alive = || -> bool { + // SAFETY: kill with signal 0 checks for process existence without delivering a signal. + let ret = unsafe { libc::kill(pgid, 0) }; + if ret == 0 { + true + } else { + std::io::Error::last_os_error().raw_os_error() != Some(libc::ESRCH) + } + }; + + if !is_group_alive() { + return; + } + + // Give remaining processes that received SIGTERM a short chance to exit cleanly. + for _ in 0..10 { + std::thread::sleep(std::time::Duration::from_millis(5)); + if !is_group_alive() { + return; + } + } + + // Forcefully kill any remaining processes in the group (e.g. descendants ignoring SIGTERM). + // SAFETY: negative pid targets the process group created via command.process_group(0). + unsafe { + libc::kill(pgid, libc::SIGKILL); + } + + // Wait until all processes in the group have exited. + for _ in 0..50 { + if !is_group_alive() { + return; + } + std::thread::sleep(std::time::Duration::from_millis(10)); + } +} + /// The last bytes of a run's stdout and stderr (`--show-output-on-failure`) #[derive(Debug, Clone, Default, PartialEq, Eq)] pub struct CapturedOutput { @@ -374,6 +424,7 @@ pub fn execute_and_measure( let fallback = TerminateWatcher::arm(pid, std::time::Duration::from_secs(2)); let (raw_status, usage) = self::unix_timer::wait_with_rusage(&child)?; fallback.disarm(); + terminate_process_group(pid); let _ = raw_status; time_user = usage.user; time_system = usage.system; diff --git a/tests/until_tests.rs b/tests/until_tests.rs index d5c12af..5f0a13d 100644 --- a/tests/until_tests.rs +++ b/tests/until_tests.rs @@ -152,4 +152,62 @@ mod unix { start.elapsed() ); } + + #[test] + fn until_kills_orphan_descendant_ignoring_sigterm() { + use std::io::Write; + + let dir = tempfile::tempdir().unwrap(); + let pid_file = dir.path().join("child.pid"); + let ready_file = dir.path().join("child.ready"); + let survived_file = dir.path().join("child.survived"); + let script_file = dir.path().join("test_orphan.sh"); + + let mut f = std::fs::File::create(&script_file).unwrap(); + writeln!( + f, + "#!/bin/sh\n(trap '' TERM; echo ready > \"$2\"; sleep 1; echo survived > \"$3\"; sleep 30) &\necho $! > \"$1\"\nwhile [ ! -f \"$2\" ]; do sleep 0.01; done\necho READY\nexit 0" + ) + .unwrap(); + drop(f); + + let script_cmd = format!( + "sh {} {} {} {}", + script_file.to_str().unwrap(), + pid_file.to_str().unwrap(), + ready_file.to_str().unwrap(), + survived_file.to_str().unwrap() + ); + + let start = std::time::Instant::now(); + hyperfine() + .args(["--until", "READY", "-N", "-r", "1", &script_cmd]) + .assert() + .success(); + + assert!( + start.elapsed() < std::time::Duration::from_secs(3), + "Benchmark took too long: {:?}", + start.elapsed() + ); + + let child_pid_str = + std::fs::read_to_string(&pid_file).expect("child pid file should have been written"); + let child_pid: libc::pid_t = child_pid_str.trim().parse().expect("valid child pid"); + + std::thread::sleep(std::time::Duration::from_millis(1500)); + + let res = unsafe { libc::kill(child_pid, 0) }; + let is_alive = + res == 0 || std::io::Error::last_os_error().raw_os_error() != Some(libc::ESRCH); + if is_alive { + unsafe { + libc::kill(child_pid, libc::SIGKILL); + } + } + assert!( + !survived_file.exists(), + "Descendant process {child_pid} continued running after --until returned" + ); + } }