diff --git a/src/config.rs b/src/config.rs index cd7b58b124..c0ca480016 100644 --- a/src/config.rs +++ b/src/config.rs @@ -674,7 +674,7 @@ impl Config { pub(crate) fn run(self, loader: &Loader) -> Result<(), Error> { if let Err(error) = SignalHandler::install(self.verbosity) { - warn!("Failed to set CTRL-C handler: {error}"); + warn!("Failed to set signal handler: {error}"); } self.subcommand.execute(&self, loader) diff --git a/src/platform.rs b/src/platform.rs index e9a431dbfd..96044fc341 100644 --- a/src/platform.rs +++ b/src/platform.rs @@ -38,6 +38,7 @@ impl PlatformInterface for Platform { exit_status.signal() } + /// See [Exit Codes With Special Meanings](https://tldp.org/LDP/abs/html/exitcodes.html) fn exit_code_from_signal(signal: i32) -> i32 { signal + 128 } @@ -101,6 +102,10 @@ impl PlatformInterface for Platform { None } + fn exit_code_from_signal(signal: i32) -> i32 { + signal + } + fn convert_native_path(working_directory: &Path, path: &Path) -> Result { // Translate path from windows style to unix style let mut cygpath = Command::new("cygpath"); diff --git a/src/platform_interface.rs b/src/platform_interface.rs index fa59cd1410..d84770328b 100644 --- a/src/platform_interface.rs +++ b/src/platform_interface.rs @@ -16,9 +16,7 @@ pub(crate) trait PlatformInterface { /// signal fn signal_from_exit_status(exit_status: ExitStatus) -> Option; - /// Convert a signal into an exit code, for the case where a process was - /// terminated by a signal - /// See https://tldp.org/LDP/abs/html/exitcodes.html + /// Convert a signal number into an exit code fn exit_code_from_signal(signal: i32) -> i32; /// Translate a path from a "native" path to a path the interpreter expects diff --git a/src/run.rs b/src/run.rs index e5b5e12699..e47f98a41c 100644 --- a/src/run.rs +++ b/src/run.rs @@ -44,8 +44,7 @@ pub fn run() -> Result<(), i32> { Error::CommandStatus { status, .. } => match status.code() { Some(code) => code, None => Platform::signal_from_exit_status(status) - .map(Platform::exit_code_from_signal) - .unwrap_or(EXIT_FAILURE), + .map_or(EXIT_FAILURE, Platform::exit_code_from_signal), }, _ => EXIT_FAILURE, }, diff --git a/src/signal_handler.rs b/src/signal_handler.rs index 9741f7d8c4..9d5dd1932a 100644 --- a/src/signal_handler.rs +++ b/src/signal_handler.rs @@ -1,55 +1,39 @@ -use super::*; - -use command_group::{CommandGroup, GroupChild}; -use signal_hook::consts::signal::*; -use signal_hook::consts::TERM_SIGNALS; -use signal_hook::flag; -use signal_hook::iterator::exfiltrator::WithOrigin; -use signal_hook::iterator::SignalsInfo; -use std::io::Read; -use std::process::Command; -use std::sync::{atomic::AtomicBool, Arc}; +use { + super::*, + command_group::{CommandGroup, GroupChild}, + signal_hook::{ + consts::{signal::*, TERM_SIGNALS}, + iterator::{exfiltrator::WithOrigin, SignalsInfo}, + }, + std::{io::Read, process::Command}, +}; pub(crate) struct SignalHandler { verbosity: Verbosity, - signals: Option>, + signals_info: Option>, } impl SignalHandler { pub(crate) fn install(verbosity: Verbosity) -> Result<(), std::io::Error> { + const MISC_SIGNALS: &[i32] = &[SIGHUP]; + let mut instance = Self::instance(); instance.verbosity = verbosity; - let signal_flag = Arc::new(AtomicBool::new(false)); - - const MISC_SIGNALS: &[i32] = &[ - #[cfg(not(windows))] - SIGHUP, - // SIGABRT, - // SIGPIPE, - // SIGALRM, - ]; - - let mut sigs = MISC_SIGNALS.to_vec(); - sigs.extend(TERM_SIGNALS); + let mut signals = MISC_SIGNALS.to_vec(); + signals.extend(TERM_SIGNALS); - let signals = SignalsInfo::::new(&sigs)?; - instance.signals = Some(signals); + let signals_info = SignalsInfo::::new(&signals)?; + instance.signals_info = Some(signals_info); - for sig in TERM_SIGNALS { - // NOTE This could be set before the register handler, in order to terminate in a double ctrl-c - // flag::register_conditional_shutdown(*sig, 1, Arc::clone(&signal_flag))?; - flag::register(*sig, Arc::clone(&signal_flag))?; - } - - for sig in MISC_SIGNALS { - flag::register(*sig, Arc::clone(&signal_flag))?; - } Ok(()) } - pub(crate) fn instance() -> MutexGuard<'static, Self> { - static INSTANCE: Mutex = Mutex::new(SignalHandler::new()); + fn instance() -> MutexGuard<'static, Self> { + static INSTANCE: Mutex = Mutex::new(SignalHandler { + verbosity: Verbosity::default(), + signals_info: None, + }); match INSTANCE.lock() { Ok(guard) => guard, @@ -57,7 +41,7 @@ impl SignalHandler { eprintln!( "{}", Error::Internal { - message: format!("interrupt handler mutex poisoned: {poison_error}"), + message: format!("signal handler mutex poisoned: {poison_error}"), } .color_display(Color::auto().stderr()) ); @@ -66,27 +50,32 @@ impl SignalHandler { } } - const fn new() -> Self { - Self { - verbosity: Verbosity::default(), - signals: None, - } - } - fn wait_child(mut child: GroupChild) -> std::io::Result<(GroupChild, ExitStatus)> { let mut instance = Self::instance(); loop { - if let Some(signals) = instance.signals.as_mut() { - for signal_info in signals.pending() { - unsafe { - // Send signal to the child process group, thus the negative pid - libc::kill(-(child.id() as i32), signal_info.signal); + if let Some(signals_info) = instance.signals_info.as_mut() { + for info in signals_info.pending() { + let child_pid = child.id(); + if let Ok(pid) = i32::try_from(child_pid) { + unsafe { + // Forward signal to the process group using negative pid + libc::kill(-pid, info.signal); + } + } else { + eprintln!( + "{}", + Error::Internal { + message: format!("Could not convert child pid to i32: {child_pid}"), + } + .color_display(Color::auto().stderr()) + ); + process::exit(EXIT_FAILURE); } } } match child.try_wait() { Ok(None) => { - std::thread::sleep(std::time::Duration::from_millis(10)); + std::thread::sleep(std::time::Duration::from_millis(50)); // If the child process is still running, continue polling continue; } @@ -105,7 +94,7 @@ impl SignalHandler { let child = command .stdout(std::process::Stdio::piped()) .group_spawn() - .unwrap(); + .map_err(OutputError::Io)?; match Self::wait_child(child) { Ok((mut child, status)) => { if let Some(code) = status.code() { @@ -122,7 +111,7 @@ impl SignalHandler { match child.inner().stdout.as_mut() { Some(stdout) => { let mut buffer = Vec::new(); - stdout.read_to_end(&mut buffer).unwrap(); + stdout.read_to_end(&mut buffer).map_err(OutputError::Io)?; match str::from_utf8(buffer.as_slice()) { Err(error) => Err(OutputError::Utf8(error)), Ok(utf8) => Ok( @@ -145,7 +134,7 @@ impl SignalHandler { } pub(crate) fn guard(mut command: Command) -> Result { - let child = command.group_spawn().unwrap(); + let child = command.group_spawn()?; match Self::wait_child(child) { Ok((_, status)) => Ok(status), Err(error) => Err(error),