feat(timeout): add benchmarking support and optimize signal handling

.github/workflows/benchmarks.yml

@@ -41,6 +41,7 @@ jobs:
           - { package: uu_sort }
           - { package: uu_split }
           - { package: uu_tsort }
+          - { package: uu_timeout }
           - { package: uu_unexpand }
           - { package: uu_uniq }
           - { package: uu_wc }

src/uu/timeout/Cargo.toml

@@ -27,3 +27,11 @@ fluent = { workspace = true }
 [[bin]]
 name = "timeout"
 path = "src/main.rs"
+
+[dev-dependencies]
+divan = { workspace = true }
+uucore = { workspace = true, features = ["benchmark"] }
+
+[[bench]]
+name = "timeout_bench"
+harness = false

src/uu/timeout/benches/timeout_bench.rs

@@ -0,0 +1,115 @@
+// This file is part of the uutils coreutils package.
+//
+// For the full copyright and license information, please view the LICENSE
+// file that was distributed with this source code.
+
+use std::env;
+use std::process;
+use std::time::Duration;
+
+const CHILD_FLAG: &str = "--timeout-bench-child";
+
+fn maybe_run_child_mode() {
+    let mut args = env::args();
+    let _ = args.next(); // skip executable path
+
+    while let Some(arg) = args.next() {
+        if arg == CHILD_FLAG {
+            let mode = args
+                .next()
+                .unwrap_or_else(|| panic!("missing child mode after {CHILD_FLAG}"));
+            run_child(mode);
+        }
+    }
+}
+
+#[cfg(unix)]
+fn run_child(mode: String) -> ! {
+    match mode.as_str() {
+        "quick-exit" => process::exit(0),
+        "short-sleep" => {
+            std::thread::sleep(Duration::from_millis(5));
+            process::exit(0);
+        }
+        "long-sleep" => {
+            std::thread::sleep(Duration::from_millis(200));
+            process::exit(0);
+        }
+        "ignore-term" => {
+            use nix::sys::signal::{SigHandler, Signal, signal};
+
+            unsafe {
+                signal(Signal::SIGTERM, SigHandler::SigIgn)
+                    .expect("failed to ignore SIGTERM in bench child");
+            }
+
+            loop {
+                std::thread::sleep(Duration::from_millis(100));
+            }
+        }
+        other => {
+            eprintln!("unknown child mode: {other}");
+            process::exit(1);
+        }
+    }
+}
+
+#[cfg(not(unix))]
+fn run_child(_: String) -> ! {
+    // The timeout benchmarks are Unix-only, but ensure child invocations still terminate.
+    process::exit(0);
+}
+
+#[cfg(unix)]
+mod unix {
+    use super::*;
+    use divan::{Bencher, black_box};
+    use uu_timeout::uumain;
+    use uucore::benchmark::run_util_function;
+
+    fn bench_timeout_with_mode(bencher: Bencher, args: &[&str], child_mode: &str) {
+        let child_path = env::current_exe()
+            .expect("failed to locate timeout bench executable")
+            .into_os_string()
+            .into_string()
+            .expect("bench executable path must be valid UTF-8");
+
+        let mut owned_args: Vec<String> = args.iter().map(|s| (*s).to_string()).collect();
+        owned_args.push(child_path);
+        owned_args.push(CHILD_FLAG.into());
+        owned_args.push(child_mode.to_string());
+
+        let arg_refs: Vec<&str> = owned_args.iter().map(|s| s.as_str()).collect();
+
+        bencher.bench(|| {
+            black_box(run_util_function(uumain, &arg_refs));
+        });
+    }
+
+    /// Benchmark the fast path where the command exits immediately.
+    #[divan::bench]
+    fn timeout_quick_exit(bencher: Bencher) {
+        bench_timeout_with_mode(bencher, &["0.02"], "quick-exit");
+    }
+
+    /// Benchmark a command that runs longer than the threshold and receives the default signal.
+    #[divan::bench]
+    fn timeout_enforced(bencher: Bencher) {
+        bench_timeout_with_mode(bencher, &["0.02"], "long-sleep");
+    }
+
+    pub fn run() {
+        divan::main();
+    }
+}
+
+#[cfg(unix)]
+fn main() {
+    maybe_run_child_mode();
+    unix::run();
+}
+
+#[cfg(not(unix))]
+fn main() {
+    maybe_run_child_mode();
+}

src/uu/timeout/src/timeout.rs

@@ -11,7 +11,10 @@ use clap::{Arg, ArgAction, Command};
 use std::io::ErrorKind;
 use std::os::unix::process::ExitStatusExt;
 use std::process::{self, Child, Stdio};
-use std::sync::atomic::{self, AtomicBool};
+use std::sync::{
+    OnceLock,
+    atomic::{self, AtomicBool},
+};
 use std::time::Duration;
 use uucore::display::Quotable;
 use uucore::error::{UResult, USimpleError, UUsageError};
@@ -189,6 +192,22 @@ fn unblock_sigchld() {
 
 /// We should terminate child process when receiving TERM signal.
 static SIGNALED: AtomicBool = AtomicBool::new(false);
+static SIGNAL_KILL: OnceLock<usize> = OnceLock::new();
+static SIGNAL_CONT: OnceLock<usize> = OnceLock::new();
+
+#[inline]
+fn signal_kill() -> usize {
+    *SIGNAL_KILL.get_or_init(|| {
+        signal_by_name_or_value("KILL").expect("KILL signal must be available on this platform")
+    })
+}
+
+#[inline]
+fn signal_cont() -> usize {
+    *SIGNAL_CONT.get_or_init(|| {
+        signal_by_name_or_value("CONT").expect("CONT signal must be available on this platform")
+    })
+}
 
 fn catch_sigterm() {
     use nix::sys::signal;
@@ -215,6 +234,16 @@ fn report_if_verbose(signal: usize, cmd: &str, verbose: bool) {
     }
 }
 
+fn preserved_child_exit(status: std::process::ExitStatus) -> i32 {
+    if let Some(code) = status.code() {
+        code
+    } else if let Some(signal) = status.signal() {
+        ExitStatus::SignalSent(signal.try_into().unwrap()).into()
+    } else {
+        ExitStatus::CommandTimedOut.into()
+    }
+}
+
 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.
@@ -223,10 +252,10 @@ fn send_signal(process: &mut Child, signal: usize, foreground: bool) {
         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();
+        let kill_signal = signal_kill();
+        let continued_signal = signal_cont();
         if signal != kill_signal && signal != continued_signal {
-            _ = process.send_signal_group(continued_signal);
+            let _ = process.send_signal_group(continued_signal);
         }
     }
 }
@@ -242,10 +271,11 @@ fn send_signal(process: &mut Child, signal: usize, foreground: bool) {
 /// If the child process terminates within the given time period and
 /// `preserve_status` is `true`, then the status code of the child
 /// process is returned. If the child process terminates within the
-/// given time period and `preserve_status` is `false`, then 124 is
+/// given time period and `preserve_status` is `false`, then 125 is
 /// returned. If the child does not terminate within the time period,
-/// then 137 is returned. Finally, if there is an error while waiting
-/// for the child process to terminate, then 124 is returned.
+/// then 137 is returned (or 143 if `timeout` itself receives
+/// `SIGTERM`). Finally, if there is an error while waiting for the
+/// child process to terminate, then 124 is returned.
 ///
 /// # Errors
 ///
@@ -258,22 +288,27 @@ fn wait_or_kill_process(
     preserve_status: bool,
     foreground: bool,
     verbose: bool,
+    signaled: &AtomicBool,
 ) -> std::io::Result<i32> {
     // ignore `SIGTERM` here
-    match process.wait_or_timeout(duration, None) {
+    match process.wait_or_timeout(duration, Some(signaled)) {
         Ok(Some(status)) => {
             if preserve_status {
-                Ok(status.code().unwrap_or_else(|| status.signal().unwrap()))
+                Ok(preserved_child_exit(status))
             } else {
                 Ok(ExitStatus::TimeoutFailed.into())
             }
         }
         Ok(None) => {
-            let signal = signal_by_name_or_value("KILL").unwrap();
+            let signal = signal_kill();
             report_if_verbose(signal, cmd, verbose);
             send_signal(process, signal, foreground);
             process.wait()?;
-            Ok(ExitStatus::SignalSent(signal).into())
+            if signaled.load(atomic::Ordering::Relaxed) {
+                Ok(ExitStatus::Terminated.into())
+            } else {
+                Ok(ExitStatus::SignalSent(signal).into())
+            }
         }
         Err(_) => Ok(ExitStatus::WaitingFailed.into()),
     }
@@ -364,19 +399,14 @@ fn timeout(
             match kill_after {
                 None => {
                     let status = process.wait()?;
-                    if SIGNALED.load(atomic::Ordering::Relaxed) {
-                        Err(ExitStatus::Terminated.into())
+                    let exit_code = if SIGNALED.load(atomic::Ordering::Relaxed) {
+                        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())
-                        }
+                        preserved_child_exit(status)
                     } else {
-                        Err(ExitStatus::CommandTimedOut.into())
-                    }
+                        ExitStatus::CommandTimedOut.into()
+                    };
+                    Err(exit_code.into())
                 }
                 Some(kill_after) => {
                     match wait_or_kill_process(
@@ -386,6 +416,7 @@ fn timeout(
                         preserve_status,
                         foreground,
                         verbose,
+                        &SIGNALED,
                     ) {
                         Ok(status) => Err(status.into()),
                         Err(e) => Err(USimpleError::new(

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

@@ -129,22 +129,33 @@ impl ChildExt for Child {
         // .try_wait() doesn't drop stdin, so we do it manually
         drop(self.stdin.take());
 
+        const MAX_SLEEP: Duration = Duration::from_millis(50);
+        const MIN_SLEEP: Duration = Duration::from_micros(500);
+
         let start = Instant::now();
         loop {
             if let Some(status) = self.try_wait()? {
                 return Ok(Some(status));
             }
 
-            if start.elapsed() >= timeout
-                || signaled.is_some_and(|signaled| signaled.load(atomic::Ordering::Relaxed))
-            {
+            if signaled.is_some_and(|signaled| signaled.load(atomic::Ordering::Relaxed)) {
+                break;
+            }
+
+            let Some(remaining) = timeout.checked_sub(start.elapsed()) else {
                 break;
+            };
+            if remaining.is_zero() {
+                break;
+            }
+
+            // For tiny remaining durations, yield so we do not oversleep.
+            if remaining < MIN_SLEEP {
+                std::thread::yield_now();
+                continue;
             }
 
-            // XXX: this is kinda gross, but it's cleaner than starting a thread just to wait
-            //      (which was the previous solution).  We might want to use a different duration
-            //      here as well
-            thread::sleep(Duration::from_millis(100));
+            thread::sleep(remaining.min(MAX_SLEEP));
         }
 
         Ok(None)