From ec5f6feaa2a6fb049f83e4bd7df569b23ebdcf62 Mon Sep 17 00:00:00 2001 From: Luv-Ray Date: Mon, 16 Jun 2025 12:02:39 +0800 Subject: [PATCH 1/2] timeout: catch TERM signal --- src/uu/timeout/src/status.rs | 4 ++++ src/uu/timeout/src/timeout.rs | 28 +++++++++++++++++++++++--- src/uucore/src/lib/features/process.rs | 20 ++++++++++++++---- 3 files changed, 45 insertions(+), 7 deletions(-) diff --git a/src/uu/timeout/src/status.rs b/src/uu/timeout/src/status.rs index 7a94c7f94..422a13ea8 100644 --- a/src/uu/timeout/src/status.rs +++ b/src/uu/timeout/src/status.rs @@ -30,6 +30,9 @@ pub(crate) enum ExitStatus { /// When there is a failure while waiting for the child process to terminate. WaitingFailed, + + /// When `SIGTERM` signal received. + Terminated, } impl From for i32 { @@ -39,6 +42,7 @@ impl From for i32 { ExitStatus::TimeoutFailed => 125, ExitStatus::SignalSent(s) => 128 + s as Self, ExitStatus::WaitingFailed => 124, + ExitStatus::Terminated => 143, } } } diff --git a/src/uu/timeout/src/timeout.rs b/src/uu/timeout/src/timeout.rs index 50d5afcfb..3c45acc2f 100644 --- a/src/uu/timeout/src/timeout.rs +++ b/src/uu/timeout/src/timeout.rs @@ -12,6 +12,7 @@ use std::collections::HashMap; use std::io::ErrorKind; use std::os::unix::process::ExitStatusExt; use std::process::{self, Child, Stdio}; +use std::sync::atomic::{self, AtomicBool}; use std::time::Duration; use uucore::display::Quotable; use uucore::error::{UClapError, UResult, USimpleError, UUsageError}; @@ -185,6 +186,23 @@ fn unblock_sigchld() { } } +/// We should terminate child process when receiving TERM signal. +static SIGNALED: AtomicBool = AtomicBool::new(false); + +fn catch_sigterm() { + use nix::sys::signal; + + extern "C" fn handle_sigterm(signal: libc::c_int) { + let signal = signal::Signal::try_from(signal).unwrap(); + if signal == signal::Signal::SIGTERM { + SIGNALED.store(true, atomic::Ordering::Relaxed); + } + } + + let handler = signal::SigHandler::Handler(handle_sigterm); + unsafe { signal::signal(signal::Signal::SIGTERM, handler) }.unwrap(); +} + /// Report that a signal is being sent if the verbose flag is set. fn report_if_verbose(signal: usize, cmd: &str, verbose: bool) { if verbose { @@ -246,7 +264,8 @@ fn wait_or_kill_process( foreground: bool, verbose: bool, ) -> std::io::Result { - match process.wait_or_timeout(duration) { + // ignore `SIGTERM` here + match process.wait_or_timeout(duration, None) { Ok(Some(status)) => { if preserve_status { Ok(status.code().unwrap_or_else(|| status.signal().unwrap())) @@ -330,6 +349,7 @@ fn timeout( ) })?; unblock_sigchld(); + catch_sigterm(); // Wait for the child process for the specified time period. // // If the process exits within the specified time period (the @@ -341,7 +361,7 @@ fn timeout( // TODO The structure of this block is extremely similar to the // structure of `wait_or_kill_process()`. They can probably be // refactored into some common function. - match process.wait_or_timeout(duration) { + match process.wait_or_timeout(duration, Some(&SIGNALED)) { Ok(Some(status)) => Err(status .code() .unwrap_or_else(|| preserve_signal_info(status.signal().unwrap())) @@ -352,7 +372,9 @@ fn timeout( match kill_after { None => { let status = process.wait()?; - if preserve_status { + if SIGNALED.load(atomic::Ordering::Relaxed) { + Err(ExitStatus::Terminated.into()) + } else if preserve_status { if let Some(ec) = status.code() { Err(ec.into()) } else if let Some(sc) = status.signal() { diff --git a/src/uucore/src/lib/features/process.rs b/src/uucore/src/lib/features/process.rs index 4656e7c13..55e8c3648 100644 --- a/src/uucore/src/lib/features/process.rs +++ b/src/uucore/src/lib/features/process.rs @@ -13,6 +13,8 @@ use nix::errno::Errno; use std::io; use std::process::Child; use std::process::ExitStatus; +use std::sync::atomic; +use std::sync::atomic::AtomicBool; use std::thread; use std::time::{Duration, Instant}; @@ -88,7 +90,11 @@ pub trait ChildExt { /// Wait for a process to finish or return after the specified duration. /// A `timeout` of zero disables the timeout. - fn wait_or_timeout(&mut self, timeout: Duration) -> io::Result>; + fn wait_or_timeout( + &mut self, + timeout: Duration, + signaled: Option<&AtomicBool>, + ) -> io::Result>; } impl ChildExt for Child { @@ -102,7 +108,7 @@ impl ChildExt for Child { fn send_signal_group(&mut self, signal: usize) -> io::Result<()> { // Ignore the signal, so we don't go into a signal loop. - if unsafe { libc::signal(signal as i32, libc::SIG_IGN) } != 0 { + if unsafe { libc::signal(signal as i32, libc::SIG_IGN) } == usize::MAX { return Err(io::Error::last_os_error()); } if unsafe { libc::kill(0, signal as i32) } == 0 { @@ -112,7 +118,11 @@ impl ChildExt for Child { } } - fn wait_or_timeout(&mut self, timeout: Duration) -> io::Result> { + fn wait_or_timeout( + &mut self, + timeout: Duration, + signaled: Option<&AtomicBool>, + ) -> io::Result> { if timeout == Duration::from_micros(0) { return self.wait().map(Some); } @@ -125,7 +135,9 @@ impl ChildExt for Child { return Ok(Some(status)); } - if start.elapsed() >= timeout { + if start.elapsed() >= timeout + || signaled.is_some_and(|signaled| signaled.load(atomic::Ordering::Relaxed)) + { break; } From baab9aa895dc02891fc6871a5f29cb579250eb61 Mon Sep 17 00:00:00 2001 From: Martin Kunkel Date: Sun, 22 Jun 2025 12:01:45 +0200 Subject: [PATCH 2/2] timeout: Add test for SIGTERM processing This is the test for issue #8040. --- tests/by-util/test_timeout.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/by-util/test_timeout.rs b/tests/by-util/test_timeout.rs index ee8e9d18f..38dcaa23f 100644 --- a/tests/by-util/test_timeout.rs +++ b/tests/by-util/test_timeout.rs @@ -206,3 +206,23 @@ fn test_hex_timeout_ending_with_d() { .fails_with_code(124) .no_output(); } + +#[test] +fn test_terminate_child_on_receiving_terminate() { + let mut timeout_cmd = new_ucmd!() + .args(&[ + "10", + "sh", + "-c", + "trap 'echo child received TERM' TERM; sleep 5", + ]) + .run_no_wait(); + timeout_cmd.delay(100); + timeout_cmd.kill_with_custom_signal(nix::sys::signal::Signal::SIGTERM); + timeout_cmd + .make_assertion() + .is_not_alive() + .with_current_output() + .code_is(143) + .stdout_contains("child received TERM"); +}