timeout: add all signal handlers, pass on signals to child, add ignored signal handling (uutils#10254)

There was quite a bunch of different features missing in timeout, to start off, now that we have a mechanism to read the SIGPIPE handlers before they are overwritten by the rust runtime, it means that we can not propagate this signal down to the child processes if the signal is set to ignore.

This also includes all of the latest changes since 9.9 where the specific signal sent to timeout will be propagated instead of just defaulting to a TERM signal.

Author: ChrisDryden

.vscode/cspell.dictionaries/jargon.wordlist.txt

@@ -117,8 +117,10 @@ noxfer
 ofile
 oflag
 oflags
+pdeathsig
 peekable
 performant
+prctl
 precompiled
 precompute
 preload
@@ -143,8 +145,17 @@ SETFL
 setlocale
 shortcode
 shortcodes
+setpgid
 sigaction
+CHLD
+chld
+SIGCHLD
+sigchld
 siginfo
+SIGTTIN
+sigttin
+SIGTTOU
+sigttou
 sigusr
 strcasecmp
 subcommand

src/uu/timeout/src/status.rs

@@ -33,9 +33,6 @@ pub(crate) enum ExitStatus {
 
     /// When a signal is sent to the child process or `timeout` itself.
     SignalSent(usize),
-
-    /// When `SIGTERM` signal received.
-    Terminated,
 }
 
 impl From<ExitStatus> for i32 {
@@ -46,7 +43,6 @@ impl From<ExitStatus> for i32 {
             ExitStatus::CannotInvoke => 126,
             ExitStatus::CommandNotFound => 127,
             ExitStatus::SignalSent(s) => 128 + s as Self,
-            ExitStatus::Terminated => 143,
         }
     }
 }

src/uu/timeout/src/timeout.rs

@@ -9,8 +9,8 @@ mod status;
 
 use crate::status::ExitStatus;
 use clap::{Arg, ArgAction, Command};
-use std::io::ErrorKind;
-use std::os::unix::process::{CommandExt, ExitStatusExt};
+use std::io::{ErrorKind, Write};
+use std::os::unix::process::ExitStatusExt;
 use std::process::{self, Child, Stdio};
 use std::sync::atomic::{self, AtomicBool};
 use std::time::Duration;
@@ -21,12 +21,14 @@ use uucore::process::ChildExt;
 use uucore::translate;
 
 use uucore::{
-    format_usage, show_error,
+    format_usage,
     signals::{signal_by_name_or_value, signal_name_by_value},
 };
 
-use nix::sys::signal::{Signal, kill};
+use nix::sys::signal::{SigHandler, Signal, kill};
 use nix::unistd::{Pid, getpid, setpgid};
+#[cfg(unix)]
+use std::os::unix::process::CommandExt;
 
 pub mod options {
     pub static FOREGROUND: &str = "foreground";
@@ -177,32 +179,46 @@ pub fn uu_app() -> Command {
         .after_help(translate!("timeout-after-help"))
 }
 
-/// Remove pre-existing SIGCHLD handlers that would make waiting for the child's exit code fail.
-fn unblock_sigchld() {
-    unsafe {
-        nix::sys::signal::signal(
-            nix::sys::signal::Signal::SIGCHLD,
-            nix::sys::signal::SigHandler::SigDfl,
-        )
-        .unwrap();
-    }
+/// Install SIGCHLD handler to ensure waiting for child works even if parent ignored SIGCHLD.
+fn install_sigchld() {
+    extern "C" fn chld(_: libc::c_int) {}
+    let _ = unsafe { nix::sys::signal::signal(Signal::SIGCHLD, SigHandler::Handler(chld)) };
 }
 
-/// We should terminate child process when receiving TERM signal.
+/// We should terminate child process when receiving termination signals.
 static SIGNALED: AtomicBool = AtomicBool::new(false);
+/// Track which signal was received (0 = none/timeout expired naturally).
+static RECEIVED_SIGNAL: std::sync::atomic::AtomicI32 = std::sync::atomic::AtomicI32::new(0);
+
+/// Install signal handlers for termination signals.
+fn install_signal_handlers(term_signal: usize) {
+    extern "C" fn handle_signal(sig: libc::c_int) {
+        SIGNALED.store(true, atomic::Ordering::Relaxed);
+        RECEIVED_SIGNAL.store(sig, atomic::Ordering::Relaxed);
+    }
 
-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 = SigHandler::Handler(handle_signal);
+    let sigpipe_ignored = uucore::signals::sigpipe_was_ignored();
+
+    for sig in [
+        Signal::SIGALRM,
+        Signal::SIGINT,
+        Signal::SIGQUIT,
+        Signal::SIGHUP,
+        Signal::SIGTERM,
+        Signal::SIGPIPE,
+        Signal::SIGUSR1,
+        Signal::SIGUSR2,
+    ] {
+        if sig == Signal::SIGPIPE && sigpipe_ignored {
+            continue; // Skip SIGPIPE if it was ignored by parent
         }
+        let _ = unsafe { nix::sys::signal::signal(sig, handler) };
     }
 
-    let handler = signal::SigHandler::Handler(handle_sigterm);
-    unsafe { signal::signal(signal::Signal::SIGTERM, handler) }.unwrap();
+    if let Ok(sig) = Signal::try_from(term_signal as i32) {
+        let _ = unsafe { nix::sys::signal::signal(sig, handler) };
+    }
 }
 
 /// Report that a signal is being sent if the verbose flag is set.
@@ -213,26 +229,29 @@ fn report_if_verbose(signal: usize, cmd: &str, verbose: bool) {
         } else {
             signal_name_by_value(signal).unwrap().to_string()
         };
-        show_error!(
-            "{}",
+        let mut stderr = std::io::stderr();
+        let _ = writeln!(
+            stderr,
+            "timeout: {}",
             translate!("timeout-verbose-sending-signal", "signal" => s, "command" => cmd.quote())
         );
+        let _ = stderr.flush();
     }
 }
 
 fn send_signal(process: &mut Child, signal: usize, foreground: bool) {
     // NOTE: GNU timeout doesn't check for errors of signal.
     // The subprocess might have exited just after the timeout.
-    // Sending a signal now would return "No such process", but we should still try to kill the children.
-    if foreground {
-        let _ = process.send_signal(signal);
-    } else {
-        let _ = process.send_signal_group(signal);
-        let kill_signal = signal_by_name_or_value("KILL").unwrap();
-        let continued_signal = signal_by_name_or_value("CONT").unwrap();
-        if signal != kill_signal && signal != continued_signal {
-            _ = process.send_signal_group(continued_signal);
-        }
+    let _ = process.send_signal(signal);
+    if signal == 0 || foreground {
+        return;
+    }
+    let _ = process.send_signal_group(signal);
+    let kill_signal = signal_by_name_or_value("KILL").unwrap();
+    let continued_signal = signal_by_name_or_value("CONT").unwrap();
+    if signal != kill_signal && signal != continued_signal {
+        let _ = process.send_signal(continued_signal);
+        let _ = process.send_signal_group(continued_signal);
     }
 }
 
@@ -330,24 +349,46 @@ fn timeout(
         let _ = setpgid(Pid::from_raw(0), Pid::from_raw(0));
     }
 
-    let mut command = process::Command::new(&cmd[0]);
-    command
+    let mut cmd_builder = process::Command::new(&cmd[0]);
+    cmd_builder
         .args(&cmd[1..])
         .stdin(Stdio::inherit())
         .stdout(Stdio::inherit())
         .stderr(Stdio::inherit());
 
-    // If stdin was closed before Rust reopened it as /dev/null, close it in child
-    if uucore::signals::stdin_was_closed() {
+    #[cfg(unix)]
+    {
+        #[cfg(target_os = "linux")]
+        let death_sig = Signal::try_from(signal as i32).ok();
+        let sigpipe_was_ignored = uucore::signals::sigpipe_was_ignored();
+        let stdin_was_closed = uucore::signals::stdin_was_closed();
+
         unsafe {
-            command.pre_exec(|| {
-                libc::close(libc::STDIN_FILENO);
+            cmd_builder.pre_exec(move || {
+                // Reset terminal signals to default
+                let _ = nix::sys::signal::signal(Signal::SIGTTIN, SigHandler::SigDfl);
+                let _ = nix::sys::signal::signal(Signal::SIGTTOU, SigHandler::SigDfl);
+                // Preserve SIGPIPE ignore status if parent had it ignored
+                if sigpipe_was_ignored {
+                    let _ = nix::sys::signal::signal(Signal::SIGPIPE, SigHandler::SigIgn);
+                }
+                // If stdin was closed before Rust reopened it as /dev/null, close it in child
+                if stdin_was_closed {
+                    libc::close(libc::STDIN_FILENO);
+                }
+                #[cfg(target_os = "linux")]
+                if let Some(sig) = death_sig {
+                    let _ = nix::sys::prctl::set_pdeathsig(sig);
+                }
                 Ok(())
             });
         }
     }
 
-    let process = &mut command.spawn().map_err(|err| {
+    install_sigchld();
+    install_signal_handlers(signal);
+
+    let process = &mut cmd_builder.spawn().map_err(|err| {
         let status_code = match err.kind() {
             ErrorKind::NotFound => ExitStatus::CommandNotFound.into(),
             ErrorKind::PermissionDenied => ExitStatus::CannotInvoke.into(),
@@ -358,8 +399,7 @@ fn timeout(
             translate!("timeout-error-failed-to-execute-process", "error" => err),
         )
     })?;
-    unblock_sigchld();
-    catch_sigterm();
+
     // Wait for the child process for the specified time period.
     //
     // If the process exits within the specified time period (the
@@ -381,41 +421,51 @@ fn timeout(
             Err(exit_code.into())
         }
         Ok(None) => {
-            report_if_verbose(signal, &cmd[0], verbose);
-            send_signal(process, signal, foreground);
-            match kill_after {
-                None => {
-                    let status = process.wait()?;
-                    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() {
-                            Err(ExitStatus::SignalSent(sc.try_into().unwrap()).into())
-                        } else {
-                            Err(ExitStatus::CommandTimedOut.into())
-                        }
-                    } else {
-                        Err(ExitStatus::CommandTimedOut.into())
-                    }
-                }
-                Some(kill_after) => {
-                    match wait_or_kill_process(
-                        process,
-                        &cmd[0],
-                        kill_after,
-                        preserve_status,
-                        foreground,
-                        verbose,
-                    ) {
-                        Ok(status) => Err(status.into()),
-                        Err(e) => Err(USimpleError::new(
-                            ExitStatus::TimeoutFailed.into(),
-                            e.to_string(),
-                        )),
-                    }
-                }
+            let received_sig = RECEIVED_SIGNAL.load(atomic::Ordering::Relaxed);
+            let is_external_signal = received_sig > 0 && received_sig != libc::SIGALRM;
+            let signal_to_send = if is_external_signal {
+                received_sig as usize
+            } else {
+                signal
+            };
+
+            report_if_verbose(signal_to_send, &cmd[0], verbose);
+            send_signal(process, signal_to_send, foreground);
+
+            if let Some(kill_after) = kill_after {
+                return match wait_or_kill_process(
+                    process,
+                    &cmd[0],
+                    kill_after,
+                    preserve_status,
+                    foreground,
+                    verbose,
+                ) {
+                    Ok(status) => Err(status.into()),
+                    Err(e) => Err(USimpleError::new(
+                        ExitStatus::TimeoutFailed.into(),
+                        e.to_string(),
+                    )),
+                };
+            }
+
+            let status = process.wait()?;
+            if is_external_signal {
+                Err(ExitStatus::SignalSent(received_sig as usize).into())
+            } else if SIGNALED.load(atomic::Ordering::Relaxed) {
+                Err(ExitStatus::CommandTimedOut.into())
+            } else if preserve_status {
+                Err(status
+                    .code()
+                    .or_else(|| {
+                        status
+                            .signal()
+                            .map(|s| ExitStatus::SignalSent(s as usize).into())
+                    })
+                    .unwrap_or(ExitStatus::CommandTimedOut.into())
+                    .into())
+            } else {
+                Err(ExitStatus::CommandTimedOut.into())
             }
         }
         Err(_) => {

src/uucore/src/lib/features/process.rs

@@ -105,11 +105,29 @@ 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) } == usize::MAX {
-            return Err(io::Error::last_os_error());
+        // Send signal to our process group (group 0 = caller's group).
+        // This matches GNU coreutils behavior: if the child has remained in our
+        // process group, it will receive this signal along with all other processes
+        // in the group. If the child has created its own process group (via setpgid),
+        // it won't receive this group signal, but will have received the direct signal.
+
+        // Signal 0 is special - it just checks if process exists, doesn't send anything.
+        // No need to manipulate signal handlers for it.
+        if signal == 0 {
+            let result = unsafe { libc::kill(0, 0) };
+            return if result == 0 {
+                Ok(())
+            } else {
+                Err(io::Error::last_os_error())
+            };
         }
-        if unsafe { libc::kill(0, signal as i32) } == 0 {
+
+        // Ignore the signal temporarily so we don't receive it ourselves.
+        let old_handler = unsafe { libc::signal(signal as i32, libc::SIG_IGN) };
+        let result = unsafe { libc::kill(0, signal as i32) };
+        // Restore the old handler
+        unsafe { libc::signal(signal as i32, old_handler) };
+        if result == 0 {
             Ok(())
         } else {
             Err(io::Error::last_os_error())