tests/timeout: add tests for signal handling and update verbose output

Author: ChrisDryden

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

@@ -208,8 +208,14 @@ fn install_signal_handlers(term_signal: usize) {
     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,
+        Signal::SIGALRM,
+        Signal::SIGINT,
+        Signal::SIGQUIT,
+        Signal::SIGHUP,
+        Signal::SIGTERM,
+        Signal::SIGPIPE,
+        Signal::SIGUSR1,
+        Signal::SIGUSR2,
     ] {
         if sig != Signal::SIGPIPE || !sigpipe_ignored {
             let _ = unsafe { nix::sys::signal::signal(sig, handler) };
@@ -224,8 +230,18 @@ fn install_signal_handlers(term_signal: usize) {
 /// 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 {
-        let s = if signal == 0 { "0".to_string() } else { signal_name_by_value(signal).unwrap().to_string() };
-        let _ = writeln!(std::io::stderr(), "timeout: sending signal {} to command {}", s, cmd.quote());
+        let s = if signal == 0 {
+            "0".to_string()
+        } else {
+            signal_name_by_value(signal).unwrap().to_string()
+        };
+        let mut stderr = std::io::stderr();
+        let _ = writeln!(
+            stderr,
+            "timeout: {}",
+            translate!("timeout-verbose-sending-signal", "signal" => s, "command" => cmd.quote())
+        );
+        let _ = stderr.flush();
     }
 }
 
@@ -361,6 +377,9 @@ fn timeout(
         }
     }
 
+    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(),
@@ -372,8 +391,6 @@ fn timeout(
             translate!("timeout-error-failed-to-execute-process", "error" => err),
         )
     })?;
-    install_sigchld();
-    install_signal_handlers(signal);
 
     match process.wait_or_timeout(duration, Some(&SIGNALED)) {
         Ok(Some(status)) => Err(status
@@ -383,15 +400,29 @@ fn timeout(
         Ok(None) => {
             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 };
+            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) {
+                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())),
+                    Err(e) => Err(USimpleError::new(
+                        ExitStatus::TimeoutFailed.into(),
+                        e.to_string(),
+                    )),
                 };
             }
 
@@ -401,8 +432,13 @@ fn timeout(
             } 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()))
+                Err(status
+                    .code()
+                    .or_else(|| {
+                        status
+                            .signal()
+                            .map(|s| ExitStatus::SignalSent(s as usize).into())
+                    })
                     .unwrap_or(ExitStatus::CommandTimedOut.into())
                     .into())
             } else {

src/uu/yes/src/yes.rs

@@ -11,16 +11,22 @@ use std::ffi::OsString;
 use std::io::{self, Write};
 use uucore::error::{UResult, USimpleError};
 use uucore::format_usage;
-#[cfg(unix)]
-use uucore::signals::enable_pipe_errors;
 use uucore::translate;
 
 // it's possible that using a smaller or larger buffer might provide better performance on some
 // systems, but honestly this is good enough
 const BUF_SIZE: usize = 16 * 1024;
 
+#[cfg(unix)]
+uucore::init_sigpipe_capture!();
+
 #[uucore::main]
 pub fn uumain(args: impl uucore::Args) -> UResult<()> {
+    #[cfg(unix)]
+    if !uucore::signals::sigpipe_was_ignored() {
+        let _ = uucore::signals::enable_pipe_errors();
+    }
+
     let matches = uucore::clap_localization::handle_clap_result(uu_app(), args)?;
 
     let mut buffer = Vec::with_capacity(BUF_SIZE);
@@ -29,7 +35,16 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
 
     match exec(&buffer) {
         Ok(()) => Ok(()),
-        Err(err) if err.kind() == io::ErrorKind::BrokenPipe => Ok(()),
+        Err(err) if err.kind() == io::ErrorKind::BrokenPipe => {
+            #[cfg(unix)]
+            if uucore::signals::sigpipe_was_ignored() {
+                uucore::show_error!(
+                    "{}",
+                    translate!("yes-error-standard-output", "error" => err)
+                );
+            }
+            Ok(())
+        }
         Err(err) => Err(USimpleError::new(
             1,
             translate!("yes-error-standard-output", "error" => err),
@@ -113,8 +128,6 @@ fn prepare_buffer(buf: &mut Vec<u8>) {
 pub fn exec(bytes: &[u8]) -> io::Result<()> {
     let stdout = io::stdout();
     let mut stdout = stdout.lock();
-    #[cfg(unix)]
-    enable_pipe_errors()?;
 
     loop {
         stdout.write_all(bytes)?;

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

@@ -107,11 +107,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())

tests/by-util/test_timeout.rs

@@ -58,7 +58,7 @@ fn test_verbose() {
         new_ucmd!()
             .args(&[verbose_flag, "-s0", "-k.1", ".1", "sleep", "1"])
             .fails()
-            .stderr_only("timeout: sending signal EXIT to command 'sleep'\ntimeout: sending signal KILL to command 'sleep'\n");
+            .stderr_only("timeout: sending signal 0 to command 'sleep'\ntimeout: sending signal KILL to command 'sleep'\n");
     }
 }
 
@@ -235,3 +235,46 @@ fn test_command_cannot_invoke() {
     // Try to execute a directory (should give permission denied or similar)
     new_ucmd!().args(&["1", "/"]).fails_with_code(126);
 }
+
+#[test]
+#[cfg(unix)]
+fn test_sigchld_ignored_by_parent() {
+    new_ucmd!()
+        .args(&["10", "sh", "-c", "trap '' CHLD; exec timeout 1 true"])
+        .succeeds();
+}
+
+#[test]
+#[cfg(unix)]
+fn test_with_background_child() {
+    new_ucmd!()
+        .args(&[".5", "sh", "-c", "sleep .1 & sleep 2"])
+        .fails_with_code(124)
+        .no_stdout();
+}
+
+#[test]
+#[cfg(unix)]
+fn test_forward_sigint_to_child() {
+    let mut cmd = new_ucmd!()
+        .args(&[
+            "10",
+            "sh",
+            "-c",
+            "trap 'echo got_int; exit 42' INT; sleep 5",
+        ])
+        .run_no_wait();
+    cmd.delay(100);
+    cmd.kill_with_custom_signal(nix::sys::signal::Signal::SIGINT);
+    cmd.make_assertion()
+        .is_not_alive()
+        .with_current_output()
+        .stdout_contains("got_int");
+}
+
+#[test]
+fn test_foreground_signal0_kill_after() {
+    new_ucmd!()
+        .args(&["--foreground", "-s0", "-k.1", ".1", "sleep", "10"])
+        .fails_with_code(137);
+}