diff --git a/.github/workflows/test-timeout-macos.yml b/.github/workflows/test-timeout-macos.yml new file mode 100644 index 00000000000..0c52d60c124 --- /dev/null +++ b/.github/workflows/test-timeout-macos.yml @@ -0,0 +1,27 @@ +name: Test Timeout on macOS + +on: + push: + branches: + - fix-timeout-performance + workflow_dispatch: + +jobs: + test-macos: + runs-on: macos-latest + steps: + - uses: actions/checkout@v4 + + - name: Install Rust + uses: dtolnay/rust-toolchain@stable # spell-checker:ignore dtolnay + + - name: Build timeout + run: cargo build --release -p uu_timeout + + - name: Run timeout tests + run: cargo test --test tests test_timeout -- --nocapture # spell-checker:ignore nocapture + + - name: Test overflow case directly + run: | + ./target/release/timeout 9223372036854775808d sleep 0 + echo "Exit code: $?" diff --git a/src/uu/timeout/Cargo.toml b/src/uu/timeout/Cargo.toml index c6b795628f7..68c208a85b9 100644 --- a/src/uu/timeout/Cargo.toml +++ b/src/uu/timeout/Cargo.toml @@ -20,7 +20,7 @@ path = "src/timeout.rs" [dependencies] clap = { workspace = true } libc = { workspace = true } -nix = { workspace = true, features = ["signal"] } +nix = { workspace = true, features = ["signal", "event"] } uucore = { workspace = true, features = ["parser", "process", "signals"] } fluent = { workspace = true } diff --git a/src/uu/timeout/src/timeout.rs b/src/uu/timeout/src/timeout.rs index 94d469c7eb3..8117a5b8355 100644 --- a/src/uu/timeout/src/timeout.rs +++ b/src/uu/timeout/src/timeout.rs @@ -3,16 +3,23 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore (ToDO) tstr sigstr cmdname setpgid sigchld getpid +// spell-checker:ignore (ToDO) tstr sigstr cmdname setpgid sigchld getpid kqueue kevent +// spell-checker:ignore (signals) sigset sigprocmask Sigmask sigtimedwait SETMASK EAGAIN +// spell-checker:ignore (kqueue) EVFILT ONESHOT eventlist mod status; use crate::status::ExitStatus; use clap::{Arg, ArgAction, Command}; -use std::io::ErrorKind; -use std::os::unix::process::ExitStatusExt; +use nix::errno::Errno; +use nix::sys::signal::{SigSet, SigmaskHow, Signal, sigprocmask}; +use std::io::{self, ErrorKind}; + +#[cfg(any(target_os = "macos", target_os = "freebsd"))] +use nix::sys::event::{EvFlags, EventFilter, FilterFlag, KEvent, Kqueue}; +use std::os::unix::process::{CommandExt, ExitStatusExt}; use std::process::{self, Child, Stdio}; use std::sync::atomic::{self, AtomicBool}; -use std::time::Duration; +use std::time::{Duration, Instant}; use uucore::display::Quotable; use uucore::error::{UResult, USimpleError, UUsageError}; use uucore::parser::parse_time; @@ -50,6 +57,50 @@ struct Config { command: Vec, } +/// Parse a duration string with overflow protection +/// Caps extremely large values at a safe maximum that works on all platforms +fn parse_duration_with_overflow_protection(duration_str: &str) -> UResult { + // Pre-check for extremely large values that would overflow + // Only intercept if it's a simple integer + unit suffix pattern + let numeric_end = duration_str + .find(|c: char| !c.is_ascii_digit()) + .unwrap_or(duration_str.len()); + + // Only apply overflow protection if we have a simple pattern: all digits followed by optional unit + if numeric_end > 0 + && (numeric_end == duration_str.len() + || matches!( + duration_str.chars().nth(numeric_end), + Some('s') | Some('m') | Some('h') | Some('d') + )) + { + let numeric_part = &duration_str[..numeric_end]; + let unit_suffix = &duration_str[numeric_end..]; + + if let Ok(num) = numeric_part.parse::() { + // Check if this would cause overflow + let (_multiplier, max_safe) = match unit_suffix { + "" | "s" => (1u64, u64::MAX), + "m" => (60, u64::MAX / 60), + "h" => (3600, u64::MAX / 3600), + "d" => (86400, u64::MAX / 86400), + _ => (1u64, u64::MAX), // Shouldn't reach here + }; + + if num > max_safe as u128 { + // Cap at a safe maximum (~34 years) that works on all platforms + // This matches the cap in process.rs for kqueue/sigtimedwait + const MAX_SAFE_TIMEOUT_SECS: u64 = (i32::MAX / 2) as u64; + return Ok(Duration::from_secs(MAX_SAFE_TIMEOUT_SECS)); + } + } + } + + // For all other cases (including normal values), use the standard parser + parse_time::from_str(duration_str, true) + .map_err(|err| UUsageError::new(ExitStatus::TimeoutFailed.into(), err)) +} + impl Config { fn from(options: &clap::ArgMatches) -> UResult { let signal = match options.get_one::(options::SIGNAL) { @@ -70,15 +121,11 @@ impl Config { let kill_after = match options.get_one::(options::KILL_AFTER) { None => None, - Some(kill_after) => match parse_time::from_str(kill_after, true) { - Ok(k) => Some(k), - Err(err) => return Err(UUsageError::new(ExitStatus::TimeoutFailed.into(), err)), - }, + Some(kill_after_str) => Some(parse_duration_with_overflow_protection(kill_after_str)?), }; - let duration = - parse_time::from_str(options.get_one::(options::DURATION).unwrap(), true) - .map_err(|err| UUsageError::new(ExitStatus::TimeoutFailed.into(), err))?; + let duration_str = options.get_one::(options::DURATION).unwrap(); + let duration = parse_duration_with_overflow_protection(duration_str)?; let preserve_status: bool = options.get_flag(options::PRESERVE_STATUS); let foreground = options.get_flag(options::FOREGROUND); @@ -176,34 +223,10 @@ 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(); - } -} - /// We should terminate child process when receiving TERM signal. +/// This is now handled by sigtimedwait() in wait_or_timeout(). 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 { @@ -231,6 +254,145 @@ fn send_signal(process: &mut Child, signal: usize, foreground: bool) { } } +/// Wait for one of the specified signals to be delivered, with optional timeout. +/// +/// This function uses platform-specific mechanisms for efficient signal waiting: +/// - Linux/FreeBSD: `sigtimedwait()` for direct signal waiting +/// - macOS: `kqueue` with EVFILT_SIGNAL for event-driven signal monitoring +/// +/// Both approaches avoid polling and provide sub-millisecond precision. +/// +/// # Arguments +/// * `signals` - Signals to wait for (typically SIGCHLD and SIGTERM) +/// * `until` - Optional deadline (absolute time) +/// +/// # Returns +/// * `Ok(Some(signal))` - A signal was received +/// * `Ok(None)` - Timeout expired +/// * `Err(e)` - An error occurred +#[cfg(not(any(target_os = "macos", target_os = "freebsd")))] +fn wait_for_signal(signals: &[Signal], until: Option) -> io::Result> { + // Linux: Use sigtimedwait() for efficient signal waiting + // Create signal set from the provided signals + let mut sigset = SigSet::empty(); + for &sig in signals { + sigset.add(sig); + } + + // Retry on EINTR, recalculating timeout each iteration + loop { + // Calculate remaining timeout + let timeout = if let Some(deadline) = until { + deadline.saturating_duration_since(Instant::now()) + } else { + Duration::MAX + }; + + // Convert to timespec, handling overflow + let timeout_spec = if timeout.as_secs() > libc::time_t::MAX as u64 { + libc::timespec { + tv_sec: libc::time_t::MAX, + tv_nsec: 0, + } + } else { + // Cap timeout to avoid overflow in timespec conversion + let timeout_secs = timeout.as_secs().min(libc::time_t::MAX as u64); + libc::timespec { + tv_sec: timeout_secs as libc::time_t, + tv_nsec: timeout.subsec_nanos() as libc::c_long, + } + }; + + let result = unsafe { + libc::sigtimedwait( + std::ptr::from_ref(sigset.as_ref()), + std::ptr::null_mut(), // We don't need siginfo + std::ptr::from_ref(&timeout_spec), + ) + }; + + if result < 0 { + match Errno::last() { + // Timeout elapsed with no signal received + Errno::EAGAIN => return Ok(None), + // The wait was interrupted by a signal not in our set - retry with recalculated timeout + Errno::EINTR => continue, + // Some other error + err => return Err(io::Error::from(err)), + } + } + // Signal received - convert signal number to Signal enum + return Signal::try_from(result).map(Some).map_err(io::Error::other); + } +} + +/// macOS implementation using kqueue with EVFILT_SIGNAL +/// +/// kqueue is the native BSD/macOS mechanism for event notification, providing +/// efficient signal monitoring without polling. This is equivalent in performance +/// to sigtimedwait() on Linux. +#[cfg(any(target_os = "macos", target_os = "freebsd"))] +fn wait_for_signal(signals: &[Signal], until: Option) -> io::Result> { + // Create a kqueue for signal monitoring + let kq = Kqueue::new().map_err(|e| io::Error::other(e.to_string()))?; + + // Create events for each signal we want to monitor + let mut changelist = Vec::with_capacity(signals.len()); + for &sig in signals { + let event = KEvent::new( + sig as usize, + EventFilter::EVFILT_SIGNAL, + EvFlags::EV_ADD | EvFlags::EV_ONESHOT, + FilterFlag::empty(), + 0, + 0, + ); + changelist.push(event); + } + + // Calculate timeout + let timeout = if let Some(deadline) = until { + let remaining = deadline.saturating_duration_since(Instant::now()); + // Cap timeout to avoid overflow in timespec conversion + let timeout_secs = remaining.as_secs().min(libc::time_t::MAX as u64); + Some(libc::timespec { + tv_sec: timeout_secs as libc::time_t, + tv_nsec: remaining.subsec_nanos() as libc::c_long, + }) + } else { + None + }; + + // Wait for signal events + let mut eventlist = vec![KEvent::new( + 0, + EventFilter::EVFILT_SIGNAL, + EvFlags::empty(), + FilterFlag::empty(), + 0, + 0, + )]; + + match kq.kevent(&changelist, &mut eventlist, timeout) { + Ok(n) if n > 0 => { + // Signal received - extract signal number from event + let sig_num = eventlist[0].ident() as i32; + Signal::try_from(sig_num) + .map(Some) + .map_err(|e| io::Error::other(e.to_string())) + } + Ok(_) => { + // Timeout expired with no events + Ok(None) + } + Err(Errno::EINTR) => { + // Interrupted - retry + wait_for_signal(signals, until) + } + Err(e) => Err(io::Error::other(e.to_string())), + } +} + /// Wait for a child process and send a kill signal if it does not terminate. /// /// This function waits for the child `process` for the time period @@ -321,61 +483,154 @@ fn timeout( #[cfg(unix)] enable_pipe_errors()?; - let process = &mut process::Command::new(&cmd[0]) - .args(&cmd[1..]) - .stdin(Stdio::inherit()) - .stdout(Stdio::inherit()) - .stderr(Stdio::inherit()) - .spawn() - .map_err(|err| { - let status_code = if err.kind() == ErrorKind::NotFound { - // FIXME: not sure which to use - 127 - } else { - // FIXME: this may not be 100% correct... - 126 - }; - USimpleError::new( - status_code, - 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 - // `Ok(Some(_))` arm), then return the appropriate status code. - // - // If the process does not exit within that time (the `Ok(None)` - // arm) and `kill_after` is specified, then try sending `SIGKILL`. + // Block signals before spawning child - will be handled by sigtimedwait() + let mut sigset = SigSet::empty(); + sigset.add(Signal::SIGCHLD); + sigset.add(Signal::SIGTERM); + let mut old_sigset = SigSet::empty(); + sigprocmask(SigmaskHow::SIG_BLOCK, Some(&sigset), Some(&mut old_sigset)) + .map_err(|e| USimpleError::new(ExitStatus::TimeoutFailed.into(), e.to_string()))?; + + let process = &mut unsafe { + process::Command::new(&cmd[0]) + .args(&cmd[1..]) + .stdin(Stdio::inherit()) + .stdout(Stdio::inherit()) + .stderr(Stdio::inherit()) + .pre_exec(|| { + // Unblock signals that were blocked in parent for sigtimedwait + // Child needs to receive these signals normally + let mut unblock_set = SigSet::empty(); + unblock_set.add(Signal::SIGTERM); + unblock_set.add(Signal::SIGCHLD); + sigprocmask(SigmaskHow::SIG_UNBLOCK, Some(&unblock_set), None) + .map_err(std::io::Error::other)?; + Ok(()) + }) + .spawn() + } + .map_err(|err| { + // Restore signal mask before returning error + let _ = sigprocmask(SigmaskHow::SIG_SETMASK, Some(&old_sigset), None); + + let status_code = if err.kind() == ErrorKind::NotFound { + // FIXME: not sure which to use + 127 + } else { + // FIXME: this may not be 100% correct... + 126 + }; + USimpleError::new( + status_code, + translate!("timeout-error-failed-to-execute-process", "error" => err), + ) + })?; + + // Wait for the child process for the specified time period using sigtimedwait. + // This approach eliminates the 100ms polling delay and provides precise, efficient waiting. // - // 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, Some(&SIGNALED)) { - Ok(Some(status)) => Err(status + // The loop combines try_wait() with wait_for_signal() to handle race conditions: + // - try_wait() checks if the child has already exited + // - wait_for_signal() suspends until SIGCHLD, SIGTERM, or timeout + // - On SIGCHLD, we loop back to try_wait() to reap the child + // - On SIGTERM, we mark SIGNALED and break out (treat as timeout) + // - On timeout, we break out and send the termination signal + + // .try_wait() doesn't drop stdin, so we do it manually + drop(process.stdin.take()); + + // Handle zero timeout - run command without any timeout + if duration == Duration::ZERO { + let exit_status = process + .wait() + .map_err(|e| USimpleError::new(ExitStatus::TimeoutFailed.into(), e.to_string()))?; + + // Restore signal mask + sigprocmask(SigmaskHow::SIG_SETMASK, Some(&old_sigset), None) + .map_err(|e| USimpleError::new(ExitStatus::TimeoutFailed.into(), e.to_string()))?; + + return match exit_status.code() { + Some(0) => Ok(()), + Some(code) => Err(code.into()), + None => Err(ExitStatus::Terminated.into()), + }; + } + + let deadline = Instant::now() + .checked_add(duration) + .unwrap_or_else(|| Instant::now() + Duration::from_secs(86400 * 365 * 100)); + + // Wait for signals with timeout + // If child has already exited, SIGCHLD will be delivered immediately + let signal_result = wait_for_signal(&[Signal::SIGCHLD, Signal::SIGTERM], Some(deadline)); + let wait_result: Option = match signal_result { + Ok(Some(Signal::SIGCHLD)) => { + // Child state changed, reap it + match process.wait() { + Ok(status) => Some(status), + Err(e) => { + // Restore mask before returning error + let _ = sigprocmask(SigmaskHow::SIG_SETMASK, Some(&old_sigset), None); + return Err(e.into()); + } + } + } + Ok(Some(Signal::SIGTERM)) => { + // External termination request + SIGNALED.store(true, atomic::Ordering::Relaxed); + None // Treat as timeout + } + Ok(None) => { + // Timeout expired + None + } + Ok(Some(sig)) => { + // Unexpected signal (shouldn't happen since we only wait for SIGCHLD/SIGTERM) + let _ = sigprocmask(SigmaskHow::SIG_SETMASK, Some(&old_sigset), None); + return Err(USimpleError::new( + ExitStatus::TimeoutFailed.into(), + format!("Unexpected signal received: {sig:?}"), + )); + } + Err(e) => { + // wait_for_signal failed + let _ = sigprocmask(SigmaskHow::SIG_SETMASK, Some(&old_sigset), None); + return Err(e.into()); + } + }; + + let result = match wait_result { + Some(status) => Err(status .code() .unwrap_or_else(|| preserve_signal_info(status.signal().unwrap())) .into()), - Ok(None) => { + 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()) + match process.wait() { + Ok(status) => { + if SIGNALED.load(atomic::Ordering::Relaxed) { + Err(ExitStatus::Terminated.into()) + } else if preserve_status { + // When preserve_status is true and timeout occurred: + // Special case: SIGCONT doesn't kill, so if it was sent and process + // completed successfully, return 0. + // All other signals: return 128+signal we sent (not child's status) + if signal == libc::SIGCONT.try_into().unwrap() && status.success() { + Ok(()) + } else { + Err(ExitStatus::SignalSent(signal).into()) + } + } else { + Err(ExitStatus::CommandTimedOut.into()) + } + } + Err(e) => { + let _ = sigprocmask(SigmaskHow::SIG_SETMASK, Some(&old_sigset), None); + return Err(e.into()); } - } else { - Err(ExitStatus::CommandTimedOut.into()) } } Some(kill_after) => { @@ -396,11 +651,11 @@ fn timeout( } } } - Err(_) => { - // We're going to return ERR_EXIT_STATUS regardless of - // whether `send_signal()` succeeds or fails - send_signal(process, signal, foreground); - Err(ExitStatus::TimeoutFailed.into()) - } - } + }; + + // Restore the original signal mask before returning + // This is CRITICAL - without this, signals stay blocked across invocations! + let _ = sigprocmask(SigmaskHow::SIG_SETMASK, Some(&old_sigset), None); + + result } diff --git a/src/uucore/Cargo.toml b/src/uucore/Cargo.toml index e9afb554283..ffc73273684 100644 --- a/src/uucore/Cargo.toml +++ b/src/uucore/Cargo.toml @@ -95,6 +95,7 @@ nix = { workspace = true, features = [ "signal", "dir", "user", + "event", ] } xattr = { workspace = true, optional = true } diff --git a/src/uucore/src/lib/features/process.rs b/src/uucore/src/lib/features/process.rs index 55e8c36482b..344568ce24e 100644 --- a/src/uucore/src/lib/features/process.rs +++ b/src/uucore/src/lib/features/process.rs @@ -4,19 +4,28 @@ // file that was distributed with this source code. // spell-checker:ignore (vars) cvar exitstatus cmdline kworker getsid getpid -// spell-checker:ignore (sys/unix) WIFSIGNALED ESRCH +// spell-checker:ignore (sys/unix) WIFSIGNALED ESRCH sigtimedwait timespec kqueue kevent EVFILT +// spell-checker:ignore (signals) setpgid PGID sigset EAGAIN ETIMEDOUT ONESHOT eventlist // spell-checker:ignore pgrep pwait snice getpgrp use libc::{gid_t, pid_t, uid_t}; #[cfg(not(target_os = "redox"))] use nix::errno::Errno; +#[cfg(not(any(target_os = "macos", target_os = "freebsd")))] +use nix::sys::signal::SigSet; +use nix::sys::signal::Signal; 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}; +use std::time::Duration; + +#[cfg(any(target_os = "macos", target_os = "freebsd"))] +use std::time::Instant; + +#[cfg(any(target_os = "macos", target_os = "freebsd"))] +use nix::sys::event::{EvFlags, EventFilter, FilterFlag, KEvent, Kqueue}; // SAFETY: These functions always succeed and return simple integers. @@ -111,13 +120,16 @@ impl ChildExt for Child { 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 { + // Send to our own process group (which the child inherits) + // After calling setpgid(0, 0), our PGID equals our PID + if unsafe { libc::kill(-libc::getpid(), signal as i32) } == 0 { Ok(()) } else { Err(io::Error::last_os_error()) } } + #[cfg(not(any(target_os = "macos", target_os = "freebsd")))] fn wait_or_timeout( &mut self, timeout: Duration, @@ -126,28 +138,192 @@ impl ChildExt for Child { if timeout == Duration::from_micros(0) { return self.wait().map(Some); } + // .try_wait() doesn't drop stdin, so we do it manually drop(self.stdin.take()); - let start = Instant::now(); - loop { - if let Some(status) = self.try_wait()? { - return Ok(Some(status)); - } + // Use sigtimedwait for efficient, precise waiting + // This suspends the process until either: + // - SIGCHLD is received (child exited/stopped/continued) + // - The timeout expires + // - SIGTERM is received (if signaled parameter is provided) + // + // NOTE: Signals must be blocked by the caller BEFORE spawning the child + // to avoid race conditions. We assume SIGCHLD/SIGTERM are already blocked. + + // Create signal set for signals we want to wait for + let mut sigset = SigSet::empty(); + sigset.add(Signal::SIGCHLD); + if signaled.is_some() { + sigset.add(Signal::SIGTERM); + } + + // Convert Duration to timespec for sigtimedwait + // Cap at safe value to avoid platform-specific time_t limits + const MAX_SAFE_TIMEOUT_SECS: u64 = (i32::MAX / 2) as u64; + let timeout_spec = libc::timespec { + tv_sec: timeout.as_secs().min(MAX_SAFE_TIMEOUT_SECS) as libc::time_t, + tv_nsec: if timeout.as_secs() >= MAX_SAFE_TIMEOUT_SECS { + 0 + } else { + timeout.subsec_nanos() as libc::c_long + }, + }; + + // Wait for signals with timeout + // We don't need the siginfo details, so pass NULL for the second parameter + let ret = unsafe { + libc::sigtimedwait( + std::ptr::from_ref(sigset.as_ref()), + std::ptr::null_mut(), + std::ptr::from_ref(&timeout_spec), + ) + }; + + match ret { + // Signal received + sig if sig > 0 => { + let signal = Signal::try_from(sig).ok(); + + // Check if SIGTERM was received (external termination request) + if signal == Some(Signal::SIGTERM) { + if let Some(flag) = signaled { + flag.store(true, atomic::Ordering::Relaxed); + } + return Ok(None); // Indicate timeout/termination + } - if start.elapsed() >= timeout - || signaled.is_some_and(|signaled| signaled.load(atomic::Ordering::Relaxed)) - { - break; + // SIGCHLD received - child has changed state (exited, stopped, or continued) + // Use blocking wait() since we know the child has changed state + // This ensures we properly reap the child after receiving SIGCHLD + self.wait().map(Some) } + // Timeout expired (EAGAIN or ETIMEDOUT) + -1 => { + let err = Errno::last(); + if err == Errno::EAGAIN || err == Errno::ETIMEDOUT { + // Timeout reached, child still running + Ok(None) + } else { + // Some other error + Err(io::Error::last_os_error()) + } + } + // Shouldn't happen + _ => Err(io::Error::other("unexpected sigtimedwait return value")), + } + } - // 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)); + #[cfg(any(target_os = "macos", target_os = "freebsd"))] + fn wait_or_timeout( + &mut self, + timeout: Duration, + signaled: Option<&AtomicBool>, + ) -> io::Result> { + // macOS implementation using kqueue for efficient signal monitoring + if timeout == Duration::from_micros(0) { + return self.wait().map(Some); } - Ok(None) + // .try_wait() doesn't drop stdin, so we do it manually + drop(self.stdin.take()); + + // Check if child has already exited before setting up kqueue + // This avoids missing SIGCHLD if child exits very quickly + if let Some(status) = self.try_wait()? { + return Ok(Some(status)); + } + + // Create kqueue for signal monitoring + let kq = Kqueue::new().map_err(|e| io::Error::other(e.to_string()))?; + + // Create events for signals we want to monitor + let mut changelist = Vec::with_capacity(2); + + // Always monitor SIGCHLD + changelist.push(KEvent::new( + Signal::SIGCHLD as usize, + EventFilter::EVFILT_SIGNAL, + EvFlags::EV_ADD | EvFlags::EV_ONESHOT, + FilterFlag::empty(), + 0, + 0, + )); + + // Optionally monitor SIGTERM + if signaled.is_some() { + changelist.push(KEvent::new( + Signal::SIGTERM as usize, + EventFilter::EVFILT_SIGNAL, + EvFlags::EV_ADD | EvFlags::EV_ONESHOT, + FilterFlag::empty(), + 0, + 0, + )); + } + + // Calculate deadline for retry logic + let deadline = Instant::now().checked_add(timeout); + // Use None (wait indefinitely) for very large timeouts to avoid platform limits + // ~34 years is effectively infinite and safe on all platforms + const MAX_SAFE_TIMEOUT_SECS: u64 = (i32::MAX / 2) as u64; + let timeout_spec = if timeout.as_secs() >= MAX_SAFE_TIMEOUT_SECS { + None + } else { + Some(libc::timespec { + tv_sec: timeout.as_secs() as libc::time_t, + tv_nsec: timeout.subsec_nanos() as libc::c_long, + }) + }; + + // Wait for signal events + let mut eventlist = vec![KEvent::new( + 0, + EventFilter::EVFILT_SIGNAL, + EvFlags::empty(), + FilterFlag::empty(), + 0, + 0, + )]; + + match kq.kevent(&changelist, &mut eventlist, timeout_spec) { + Ok(n) if n > 0 => { + // Signal received - check which one + let sig_num = eventlist[0].ident() as i32; + let signal = Signal::try_from(sig_num).ok(); + + // Check if SIGTERM was received + if signal == Some(Signal::SIGTERM) { + if let Some(flag) = signaled { + flag.store(true, atomic::Ordering::Relaxed); + } + return Ok(None); + } + + // SIGCHLD received - reap the child + self.wait().map(Some) + } + Ok(_) => { + // Timeout expired + Ok(None) + } + Err(Errno::EINTR) => { + // Interrupted - check if we still have time and retry + if let Some(deadline) = deadline { + let remaining = deadline.saturating_duration_since(Instant::now()); + if remaining > Duration::ZERO { + // Recursively retry with remaining time + self.wait_or_timeout(remaining, signaled) + } else { + Ok(None) + } + } else { + // No deadline (timeout overflowed), just retry with original timeout + self.wait_or_timeout(timeout, signaled) + } + } + Err(e) => Err(io::Error::other(e.to_string())), + } } }