diff --git a/Cargo.lock b/Cargo.lock index 5bec3df72d..c9f874340e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -540,6 +540,7 @@ dependencies = [ "heck", "lexiclean", "libc", + "nix", "num_cpus", "once_cell", "percent-encoding", diff --git a/Cargo.toml b/Cargo.toml index cd922e35ad..16ca91805c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,6 +33,7 @@ edit-distance = "2.0.0" heck = "0.5.0" lexiclean = "0.0.1" libc = "0.2.0" +nix = { version = "0.29.0", features = ["user"] } num_cpus = "1.15.0" once_cell = "1.19.0" percent-encoding = "2.3.1" @@ -72,6 +73,7 @@ struct_excessive_bools = "allow" struct_field_names = "allow" too_many_arguments = "allow" too_many_lines = "allow" +undocumented_unsafe_blocks = "deny" unnecessary_wraps = "allow" wildcard_imports = "allow" diff --git a/README.md b/README.md index a8c1559817..2b6b667d92 100644 --- a/README.md +++ b/README.md @@ -3923,6 +3923,57 @@ the [`chrono` library docs](https://docs.rs/chrono/latest/chrono/format/strftime/index.html) for details. +### Signal Handling + +[Signals](https://en.wikipedia.org/wiki/Signal_(IPC)) are messsages sent to +running programs to trigger specific behavior. For example, the `SIGINT` signal +is sent to all processes in the forground process group when `ctrl-c` is typed +at the terminal, + +`just` tries to exit when requested and avoid leaving behind running child +proccesses, two goals which are somewhat in conflict. + +If `just` exits leaving behind child processes, the user will have no recourse +but to `ps aux | grep` for the children and manually `kill` them, a tedious +endevour. + +#### Fatal Signals + +`SIGHUP`, `SIGINT`, and `SIGQUIT` are generated when the user closes the +terminal, types `ctrl-c`, or types `ctrl-\`, respectively, and are sent to all +processes in the foreground process group. + +`SIGTERM` is the default signal sent by the `kill` command, and is delivered +only to its intended victim. + +When a child process is not running, `just` will exit immediately on receipt of +any of the above signals. + +When a child process *is* running, `just` will wait until it terminates, to +avoid leaving it behind. + +Additionally, on receipt of `SIGTERM`, `just` will forward `SIGTERM` to any +running childrenmaster, since unlike other fatal signals, `SIGTERM`, +was likely sent to `just` alone. + +Regardless of whether a child process terminates successfully after `just` +receives a fatal signal, `just` halts execution. + +#### `SIGINFO` + +`SIGINFO` is sent to all processes in the foreground process group when the +user types `ctrl-t` on +[BSD](https://en.wikipedia.org/wiki/Berkeley_Software_Distribution)-derived +operating systems, including MacOS, but not Linux. + +`just` responds by printing a list of all child process IDs and +commandsmaster. + +#### Windows + +On Windows, `just` behaves as if it had received `SIGINT` when the user types +`ctrl-c`. Other signals are unsupported. + Changelog --------- diff --git a/src/command_ext.rs b/src/command_ext.rs index 40d9da1600..0275227d24 100644 --- a/src/command_ext.rs +++ b/src/command_ext.rs @@ -7,9 +7,13 @@ pub(crate) trait CommandExt { dotenv: &BTreeMap, scope: &Scope, unexports: &HashSet, - ); + ) -> &mut Command; fn export_scope(&mut self, settings: &Settings, scope: &Scope, unexports: &HashSet); + + fn status_guard(self) -> (io::Result, Option); + + fn output_guard(self) -> (io::Result, Option); } impl CommandExt for Command { @@ -19,7 +23,7 @@ impl CommandExt for Command { dotenv: &BTreeMap, scope: &Scope, unexports: &HashSet, - ) { + ) -> &mut Command { for (name, value) in dotenv { self.env(name, value); } @@ -27,6 +31,8 @@ impl CommandExt for Command { if let Some(parent) = scope.parent() { self.export_scope(settings, parent, unexports); } + + self } fn export_scope(&mut self, settings: &Settings, scope: &Scope, unexports: &HashSet) { @@ -44,4 +50,12 @@ impl CommandExt for Command { } } } + + fn status_guard(self) -> (io::Result, Option) { + SignalHandler::spawn(self, |mut child| child.wait()) + } + + fn output_guard(self) -> (io::Result, Option) { + SignalHandler::spawn(self, process::Child::wait_with_output) + } } diff --git a/src/error.rs b/src/error.rs index a07c4efd62..c2e2263176 100644 --- a/src/error.rs +++ b/src/error.rs @@ -116,6 +116,9 @@ pub(crate) enum Error<'src> { Internal { message: String, }, + Interrupted { + signal: Signal, + }, Io { recipe: &'src str, io_error: io::Error, @@ -291,6 +294,7 @@ impl ColorDisplay for Error<'_> { OutputError::Code(code) => write!(f, "Backtick failed with exit code {code}")?, OutputError::Signal(signal) => write!(f, "Backtick was terminated by signal {signal}")?, OutputError::Unknown => write!(f, "Backtick failed for an unknown reason")?, + OutputError::Interrupted(signal) => write!(f, "Backtick succeeded but `just` was interrupted by signal {signal}")?, OutputError::Io(io_error) => match io_error.kind() { io::ErrorKind::NotFound => write!(f, "Backtick could not be run because just could not find the shell:\n{io_error}"), io::ErrorKind::PermissionDenied => write!(f, "Backtick could not be run because just could not run the shell:\n{io_error}"), @@ -340,6 +344,7 @@ impl ColorDisplay for Error<'_> { OutputError::Code(code) => write!(f, "Cygpath failed with exit code {code} while translating recipe `{recipe}` shebang interpreter path")?, OutputError::Signal(signal) => write!(f, "Cygpath terminated by signal {signal} while translating recipe `{recipe}` shebang interpreter path")?, OutputError::Unknown => write!(f, "Cygpath experienced an unknown failure while translating recipe `{recipe}` shebang interpreter path")?, + OutputError::Interrupted(signal) => write!(f, "Cygpath succeeded but `just` was interrupted by {signal}")?, OutputError::Io(io_error) => { match io_error.kind() { io::ErrorKind::NotFound => write!(f, "Could not find `cygpath` executable to translate recipe `{recipe}` shebang interpreter path:\n{io_error}"), @@ -402,6 +407,9 @@ impl ColorDisplay for Error<'_> { write!(f, "Internal runtime error, this may indicate a bug in just: {message} \ consider filing an issue: https://github.com/casey/just/issues/new")?; } + Interrupted { signal } => { + write!(f, "Interrupted by {signal}")?; + } Io { recipe, io_error } => { match io_error.kind() { io::ErrorKind::NotFound => write!(f, "Recipe `{recipe}` could not be run because just could not find the shell: {io_error}"), diff --git a/src/evaluator.rs b/src/evaluator.rs index 46955980f4..66e284fb92 100644 --- a/src/evaluator.rs +++ b/src/evaluator.rs @@ -267,22 +267,44 @@ impl<'src, 'run> Evaluator<'src, 'run> { .module .settings .shell_command(self.context.config); - cmd.arg(command); - cmd.args(args); - cmd.current_dir(self.context.working_directory()); - cmd.export( - &self.context.module.settings, - self.context.dotenv, - &self.scope, - &self.context.module.unexports, - ); - cmd.stdin(Stdio::inherit()); - cmd.stderr(if self.context.config.verbosity.quiet() { - Stdio::null() - } else { - Stdio::inherit() - }); - InterruptHandler::guard(|| output(cmd)) + + cmd + .arg(command) + .args(args) + .current_dir(self.context.working_directory()) + .export( + &self.context.module.settings, + self.context.dotenv, + &self.scope, + &self.context.module.unexports, + ) + .stdin(Stdio::inherit()) + .stderr(if self.context.config.verbosity.quiet() { + Stdio::null() + } else { + Stdio::inherit() + }) + .stdout(Stdio::piped()); + + let (result, caught) = cmd.output_guard(); + + let output = result.map_err(OutputError::Io)?; + + OutputError::result_from_exit_status(output.status)?; + + let output = str::from_utf8(&output.stdout).map_err(OutputError::Utf8)?; + + if let Some(signal) = caught { + return Err(OutputError::Interrupted(signal)); + } + + Ok( + output + .strip_suffix("\r\n") + .or_else(|| output.strip_suffix("\n")) + .unwrap_or(output) + .into(), + ) } pub(crate) fn evaluate_line( diff --git a/src/interrupt_guard.rs b/src/interrupt_guard.rs deleted file mode 100644 index 23afa1dd87..0000000000 --- a/src/interrupt_guard.rs +++ /dev/null @@ -1,16 +0,0 @@ -use super::*; - -pub(crate) struct InterruptGuard; - -impl InterruptGuard { - pub(crate) fn new() -> Self { - InterruptHandler::instance().block(); - Self - } -} - -impl Drop for InterruptGuard { - fn drop(&mut self) { - InterruptHandler::instance().unblock(); - } -} diff --git a/src/interrupt_handler.rs b/src/interrupt_handler.rs deleted file mode 100644 index 1105954bfe..0000000000 --- a/src/interrupt_handler.rs +++ /dev/null @@ -1,86 +0,0 @@ -use super::*; - -pub(crate) struct InterruptHandler { - blocks: u32, - interrupted: bool, - verbosity: Verbosity, -} - -impl InterruptHandler { - pub(crate) fn install(verbosity: Verbosity) -> Result<(), ctrlc::Error> { - let mut instance = Self::instance(); - instance.verbosity = verbosity; - ctrlc::set_handler(|| Self::instance().interrupt()) - } - - pub(crate) fn instance() -> MutexGuard<'static, Self> { - static INSTANCE: Mutex = Mutex::new(InterruptHandler::new()); - - match INSTANCE.lock() { - Ok(guard) => guard, - Err(poison_error) => { - eprintln!( - "{}", - Error::Internal { - message: format!("interrupt handler mutex poisoned: {poison_error}"), - } - .color_display(Color::auto().stderr()) - ); - process::exit(EXIT_FAILURE); - } - } - } - - const fn new() -> Self { - Self { - blocks: 0, - interrupted: false, - verbosity: Verbosity::default(), - } - } - - fn interrupt(&mut self) { - self.interrupted = true; - - if self.blocks > 0 { - return; - } - - Self::exit(); - } - - fn exit() { - process::exit(130); - } - - pub(crate) fn block(&mut self) { - self.blocks += 1; - } - - pub(crate) fn unblock(&mut self) { - if self.blocks == 0 { - if self.verbosity.loud() { - eprintln!( - "{}", - Error::Internal { - message: "attempted to unblock interrupt handler, but handler was not blocked" - .to_owned(), - } - .color_display(Color::auto().stderr()) - ); - } - process::exit(EXIT_FAILURE); - } - - self.blocks -= 1; - - if self.interrupted { - Self::exit(); - } - } - - pub(crate) fn guard T>(function: F) -> T { - let _guard = InterruptGuard::new(); - function() - } -} diff --git a/src/justfile.rs b/src/justfile.rs index 2ce96ece40..1b0f2398c4 100644 --- a/src/justfile.rs +++ b/src/justfile.rs @@ -109,20 +109,20 @@ impl<'src> Justfile<'src> { Command::new(binary) }; - command.args(arguments); - - command.current_dir(&search.working_directory); + command + .args(arguments) + .current_dir(&search.working_directory); let scope = scope.child(); command.export(&self.settings, &dotenv, &scope, &self.unexports); - let status = InterruptHandler::guard(|| command.status()).map_err(|io_error| { - Error::CommandInvoke { - binary: binary.clone(), - arguments: arguments.clone(), - io_error, - } + let (result, caught) = command.status_guard(); + + let status = result.map_err(|io_error| Error::CommandInvoke { + binary: binary.clone(), + arguments: arguments.clone(), + io_error, })?; if !status.success() { @@ -133,6 +133,10 @@ impl<'src> Justfile<'src> { }); }; + if let Some(signal) = caught { + return Err(Error::Interrupted { signal }); + } + return Ok(()); } Subcommand::Evaluate { variable, .. } => { diff --git a/src/lib.rs b/src/lib.rs index 30777e513d..7d1d6db231 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -42,8 +42,6 @@ pub(crate) use { fragment::Fragment, function::Function, interpreter::Interpreter, - interrupt_guard::InterruptGuard, - interrupt_handler::InterruptHandler, item::Item, justfile::Justfile, keyed::Keyed, @@ -57,7 +55,6 @@ pub(crate) use { name::Name, namepath::Namepath, ordinal::Ordinal, - output::output, output_error::OutputError, parameter::Parameter, parameter_kind::ParameterKind, @@ -80,6 +77,9 @@ pub(crate) use { settings::Settings, shebang::Shebang, show_whitespace::ShowWhitespace, + signal::Signal, + signal_handler::SignalHandler, + signals::Signals, source::Source, string_delimiter::StringDelimiter, string_kind::StringKind, @@ -118,18 +118,22 @@ pub(crate) use { env, ffi::OsString, fmt::{self, Debug, Display, Formatter}, - fs, + fs::{self, File}, io::{self, Read, Seek, Write}, iter::{self, FromIterator}, mem, ops::Deref, ops::{Index, Range, RangeInclusive}, + os::fd::{BorrowedFd, IntoRawFd}, path::{self, Path, PathBuf}, process::{self, Command, ExitStatus, Stdio}, rc::Rc, str::{self, Chars}, - sync::{Mutex, MutexGuard, OnceLock}, - vec, + sync::{ + atomic::{self, AtomicI32}, + Mutex, MutexGuard, OnceLock, + }, + thread, vec, }, strum::{Display, EnumDiscriminants, EnumString, IntoStaticStr}, tempfile::tempfile, @@ -216,8 +220,6 @@ mod expression; mod fragment; mod function; mod interpreter; -mod interrupt_guard; -mod interrupt_handler; mod item; mod justfile; mod keyed; @@ -231,7 +233,6 @@ mod module_path; mod name; mod namepath; mod ordinal; -mod output; mod output_error; mod parameter; mod parameter_kind; @@ -255,6 +256,9 @@ mod setting; mod settings; mod shebang; mod show_whitespace; +mod signal; +mod signal_handler; +mod signals; mod source; mod string_delimiter; mod string_kind; diff --git a/src/output.rs b/src/output.rs deleted file mode 100644 index f9f4ab024c..0000000000 --- a/src/output.rs +++ /dev/null @@ -1,34 +0,0 @@ -use super::*; - -/// Run a command and return the data it wrote to stdout as a string -pub(crate) fn output(mut command: Command) -> Result { - match command.output() { - Ok(output) => { - if let Some(code) = output.status.code() { - if code != 0 { - return Err(OutputError::Code(code)); - } - } else { - let signal = Platform::signal_from_exit_status(output.status); - return Err(match signal { - Some(signal) => OutputError::Signal(signal), - None => OutputError::Unknown, - }); - } - match str::from_utf8(&output.stdout) { - Err(error) => Err(OutputError::Utf8(error)), - Ok(output) => Ok( - if output.ends_with("\r\n") { - &output[0..output.len() - 2] - } else if output.ends_with('\n') { - &output[0..output.len() - 1] - } else { - output - } - .to_owned(), - ), - } - } - Err(io_error) => Err(OutputError::Io(io_error)), - } -} diff --git a/src/output_error.rs b/src/output_error.rs index b1a165583b..fcd06e3167 100644 --- a/src/output_error.rs +++ b/src/output_error.rs @@ -6,6 +6,8 @@ pub(crate) enum OutputError { Code(i32), /// IO error Io(io::Error), + /// Interrupted by signal + Interrupted(Signal), /// Terminated by signal Signal(i32), /// Unknown failure @@ -14,11 +16,28 @@ pub(crate) enum OutputError { Utf8(str::Utf8Error), } +impl OutputError { + pub(crate) fn result_from_exit_status(exit_status: ExitStatus) -> Result<(), OutputError> { + match exit_status.code() { + Some(0) => Ok(()), + Some(code) => Err(Self::Code(code)), + None => match Platform::signal_from_exit_status(exit_status) { + Some(signal) => Err(Self::Signal(signal)), + None => Err(Self::Unknown), + }, + } + } +} + impl Display for OutputError { fn fmt(&self, f: &mut Formatter) -> fmt::Result { match *self { Self::Code(code) => write!(f, "Process exited with status code {code}"), Self::Io(ref io_error) => write!(f, "Error executing process: {io_error}"), + Self::Interrupted(signal) => write!( + f, + "Process succeded but `just` was interrupted by signal {signal}" + ), Self::Signal(signal) => write!(f, "Process terminated by signal {signal}"), Self::Unknown => write!(f, "Process experienced an unknown failure"), Self::Utf8(ref err) => write!(f, "Could not convert process stdout to UTF-8: {err}"), diff --git a/src/platform.rs b/src/platform.rs index ece80fe133..d7a677d0a7 100644 --- a/src/platform.rs +++ b/src/platform.rs @@ -44,6 +44,17 @@ impl PlatformInterface for Platform { .map(str::to_string) .ok_or_else(|| String::from("Error getting current directory: unicode decode error")) } + + fn install_signal_handler(handler: T) -> RunResult<'static> { + // todo: name thread + thread::spawn(move || { + // todo: handle errors? + for signal in Signals::new().unwrap() { + handler(signal.unwrap()); + } + }); + Ok(()) + } } #[cfg(windows)] @@ -112,4 +123,11 @@ impl PlatformInterface for Platform { .ok_or_else(|| String::from("Error getting current directory: unicode decode error")), } } + + fn install_signal_handler( + handler: T, + ) -> Result<(), Box> { + ctrlc::set_handler(|| handler(Signal::Interrupt))?; + Ok(()) + } } diff --git a/src/platform_interface.rs b/src/platform_interface.rs index 463f6650a6..9020a5ab54 100644 --- a/src/platform_interface.rs +++ b/src/platform_interface.rs @@ -1,21 +1,23 @@ use super::*; pub(crate) trait PlatformInterface { - /// Construct a command equivalent to running the script at `path` with the - /// shebang line `shebang` + /// translate path from "native" path to a path the interpreter expects + fn convert_native_path(working_directory: &Path, path: &Path) -> FunctionResult; + + /// install handler, may only be called once + fn install_signal_handler(handler: T) -> RunResult<'static>; + + /// construct command equivalent to running script at `path` with shebang + /// line `shebang` fn make_shebang_command( path: &Path, working_directory: Option<&Path>, shebang: Shebang, ) -> Result; - /// Set the execute permission on the file pointed to by `path` + /// set the execute permission on file pointed to by `path` fn set_execute_permission(path: &Path) -> io::Result<()>; - /// Extract the signal from a process exit status, if it was terminated by a - /// signal + /// extract signal from process exit status fn signal_from_exit_status(exit_status: ExitStatus) -> Option; - - /// Translate a path from a "native" path to a path the interpreter expects - fn convert_native_path(working_directory: &Path, path: &Path) -> FunctionResult; } diff --git a/src/recipe.rs b/src/recipe.rs index d53a44bb40..e275ec3dac 100644 --- a/src/recipe.rs +++ b/src/recipe.rs @@ -296,7 +296,9 @@ impl<'src, D> Recipe<'src, D> { &context.module.unexports, ); - match InterruptHandler::guard(|| cmd.status()) { + let (result, caught) = cmd.status_guard(); + + match result { Ok(exit_status) => { if let Some(code) = exit_status.code() { if code != 0 && !infallible_line { @@ -308,11 +310,13 @@ impl<'src, D> Recipe<'src, D> { }); } } else { - return Err(error_from_signal( - self.name(), - Some(line_number), - exit_status, - )); + if !infallible_line { + return Err(error_from_signal( + self.name(), + Some(line_number), + exit_status, + )); + } } } Err(io_error) => { @@ -322,6 +326,12 @@ impl<'src, D> Recipe<'src, D> { }); } }; + + if !infallible_line { + if let Some(signal) = caught { + return Err(Error::Interrupted { signal }); + } + } } } @@ -438,7 +448,9 @@ impl<'src, D> Recipe<'src, D> { ); // run it! - match InterruptHandler::guard(|| command.status()) { + let (result, caught) = command.status_guard(); + + match result { Ok(exit_status) => exit_status.code().map_or_else( || Err(error_from_signal(self.name(), None, exit_status)), |code| { @@ -453,9 +465,15 @@ impl<'src, D> Recipe<'src, D> { }) } }, - ), - Err(io_error) => Err(executor.error(io_error, self.name())), + )?, + Err(io_error) => return Err(executor.error(io_error, self.name())), + } + + if let Some(signal) = caught { + return Err(Error::Interrupted { signal }); } + + Ok(()) } pub(crate) fn groups(&self) -> BTreeSet { diff --git a/src/run.rs b/src/run.rs index a07b73bbc9..53e37fb52b 100644 --- a/src/run.rs +++ b/src/run.rs @@ -24,7 +24,7 @@ pub fn run(args: impl Iterator + Clone>) -> Result<() config .and_then(|config| { - InterruptHandler::install(config.verbosity).ok(); + SignalHandler::install(config.verbosity)?; config.subcommand.execute(&config, &loader) }) .map_err(|error| { diff --git a/src/signal.rs b/src/signal.rs new file mode 100644 index 0000000000..7e916b0653 --- /dev/null +++ b/src/signal.rs @@ -0,0 +1,123 @@ +use super::*; + +#[derive(Clone, Copy, Debug, PartialEq)] +#[repr(i32)] +pub(crate) enum Signal { + Hangup = libc::SIGHUP, + #[cfg(any( + target_os = "dragonfly", + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "netbsd", + target_os = "openbsd", + ))] + Info = libc::SIGINFO, + Interrupt = libc::SIGINT, + Quit = libc::SIGQUIT, + Terminate = libc::SIGTERM, +} + +impl Signal { + pub(crate) const ALL: &[Signal] = &[ + Signal::Hangup, + #[cfg(any( + target_os = "dragonfly", + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "netbsd", + target_os = "openbsd", + ))] + Signal::Info, + Signal::Interrupt, + Signal::Quit, + Signal::Terminate, + ]; + + pub(crate) fn number(self) -> libc::c_int { + self as libc::c_int + } +} + +impl Display for Signal { + fn fmt(&self, f: &mut Formatter) -> fmt::Result { + write!( + f, + "{}", + match self { + Signal::Hangup => "SIGHUP", + #[cfg(any( + target_os = "dragonfly", + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "netbsd", + target_os = "openbsd", + ))] + Signal::Info => "SIGINFO", + Signal::Interrupt => "SIGINT", + Signal::Quit => "SIGQUIT", + Signal::Terminate => "SIGTERM", + } + ) + } +} + +impl From for nix::sys::signal::Signal { + fn from(signal: Signal) -> Self { + match signal { + Signal::Hangup => Self::SIGHUP, + #[cfg(any( + target_os = "dragonfly", + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "netbsd", + target_os = "openbsd", + ))] + Signal::Info => Self::SIGINFO, + Signal::Interrupt => Self::SIGINT, + Signal::Quit => Self::SIGQUIT, + Signal::Terminate => Self::SIGTERM, + } + } +} + +impl TryFrom for Signal { + type Error = io::Error; + + fn try_from(n: u8) -> Result { + match n.into() { + libc::SIGHUP => Ok(Signal::Hangup), + #[cfg(any( + target_os = "dragonfly", + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "netbsd", + target_os = "openbsd", + ))] + libc::SIGINFO => Ok(Signal::Info), + libc::SIGINT => Ok(Signal::Interrupt), + libc::SIGQUIT => Ok(Signal::Quit), + libc::SIGTERM => Ok(Signal::Terminate), + _ => Err(io::Error::new( + io::ErrorKind::Other, + format!("unexpected signal: {n}"), + )), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn signals_fit_in_u8() { + for signal in Signal::ALL { + assert!(signal.number() <= i32::from(u8::MAX)); + } + } +} diff --git a/src/signal_handler.rs b/src/signal_handler.rs new file mode 100644 index 0000000000..688bb74ad6 --- /dev/null +++ b/src/signal_handler.rs @@ -0,0 +1,143 @@ +use super::*; + +use nix::unistd::Pid; + +pub(crate) struct SignalHandler { + caught: Option, + children: BTreeMap, + verbosity: Verbosity, +} + +impl SignalHandler { + pub(crate) fn install(verbosity: Verbosity) -> RunResult<'static> { + let mut instance = Self::instance(); + instance.verbosity = verbosity; + Platform::install_signal_handler(|signal| Self::instance().interrupt(signal)) + } + + pub(crate) fn instance() -> MutexGuard<'static, Self> { + static INSTANCE: Mutex = Mutex::new(SignalHandler::new()); + + match INSTANCE.lock() { + Ok(guard) => guard, + Err(poison_error) => { + eprintln!( + "{}", + Error::Internal { + message: format!("signal handler mutex poisoned: {poison_error}"), + } + .color_display(Color::auto().stderr()) + ); + process::exit(EXIT_FAILURE); + } + } + } + + const fn new() -> Self { + Self { + caught: None, + children: BTreeMap::new(), + verbosity: Verbosity::default(), + } + } + + fn interrupt(&mut self, signal: Signal) { + if self.children.is_empty() { + process::exit(128 + signal.number()); + } + + #[cfg(any( + target_os = "dragonfly", + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "netbsd", + target_os = "openbsd", + ))] + if signal != Signal::Info && self.caught.is_none() { + self.caught = Some(signal); + } + + match signal { + // SIGHUP, SIGINT, and SIGQUIT are normally sent on terminal close, + // ctrl-c, and ctrl-\, respectively, and are sent to all processes in the + // foreground process group. this includes child processes, so we ignore + // the signal and wait for them to exit + Signal::Hangup | Signal::Interrupt | Signal::Quit => {} + #[cfg(any( + target_os = "dragonfly", + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "netbsd", + target_os = "openbsd", + ))] + Signal::Info => { + let id = process::id(); + if self.children.is_empty() { + eprintln!("just {id}: no child processes"); + } else { + let n = self.children.len(); + + let mut message = format!( + "just {id}: {n} child {}:\n", + if n == 1 { "process" } else { "processes" } + ); + + for (&child, command) in &self.children { + message.push_str(&format!("{child}: {command:?}\n")); + } + + eprint!("{message}"); + } + } + // SIGTERM is the default signal sent by kill. forward it to child + // processes and wait for them to exit + Signal::Terminate => { + for &child in self.children.keys() { + if self.verbosity.loquacious() { + eprintln!("just: sending SIGTERM to child process {child}"); + } + nix::sys::signal::kill(child, Some(Signal::Terminate.into())).ok(); + } + } + } + } + + pub(crate) fn spawn( + mut command: Command, + f: impl Fn(process::Child) -> io::Result, + ) -> (io::Result, Option) { + let mut instance = Self::instance(); + + let child = match command.spawn() { + Err(err) => return (Err(err), None), + Ok(child) => child, + }; + + let pid = match child.id().try_into() { + Err(err) => { + return ( + Err(io::Error::new( + io::ErrorKind::Other, + format!("invalid child PID: {err}"), + )), + None, + ) + } + Ok(pid) => Pid::from_raw(pid), + }; + + instance.children.insert(pid, command); + + drop(instance); + + let result = f(child); + + let mut instance = Self::instance(); + + instance.children.remove(&pid); + + (result, instance.caught) + } +} diff --git a/src/signals.rs b/src/signals.rs new file mode 100644 index 0000000000..e6302a6c2e --- /dev/null +++ b/src/signals.rs @@ -0,0 +1,110 @@ +use { + super::*, + nix::{ + errno::Errno, + sys::signal::{SaFlags, SigAction, SigHandler, SigSet}, + }, +}; + +const INVALID_FILENO: i32 = -1; + +static WRITE: AtomicI32 = AtomicI32::new(INVALID_FILENO); + +fn die(message: &str) -> ! { + // SAFETY: + // + // Standard error is open for the duration of the program. + const STDERR: BorrowedFd = unsafe { BorrowedFd::borrow_raw(libc::STDERR_FILENO) }; + + let mut i = 0; + let mut buffer = [0; 512]; + + let mut append = |s: &str| { + let remaining = buffer.len() - i; + let n = s.len().min(remaining); + let end = i + n; + buffer[i..end].copy_from_slice(&s.as_bytes()[0..n]); + i = end; + }; + + append("error: "); + append(message); + append("\n"); + + nix::unistd::write(STDERR, &buffer[0..i]).ok(); + + process::abort(); +} + +extern "C" fn handler(signal: libc::c_int) { + let errno = Errno::last(); + + let Ok(signal) = u8::try_from(signal) else { + die("unexpected signal"); + }; + + // SAFETY: + // + // `WRITE` is initialized before the signal handler can run and remains open + // for the duration of the program. + let fd = unsafe { BorrowedFd::borrow_raw(WRITE.load(atomic::Ordering::Relaxed)) }; + + if let Err(err) = nix::unistd::write(fd, &[signal]) { + die(err.desc()); + } + + errno.set(); +} + +pub(crate) struct Signals(File); + +impl Signals { + pub(crate) fn new() -> io::Result { + let (read, write) = nix::unistd::pipe()?; + + if WRITE + .compare_exchange( + INVALID_FILENO, + write.into_raw_fd(), + atomic::Ordering::Relaxed, + atomic::Ordering::Relaxed, + ) + .is_err() + { + panic!("signal iterator cannot be initialized twice"); + } + + let sa = SigAction::new( + SigHandler::Handler(handler), + SaFlags::SA_RESTART, + SigSet::empty(), + ); + + for &signal in Signal::ALL { + // SAFETY: + // + // This is the only place we modify signal handlers, and + // `nix::sys::signal::sigaction` is unsafe only if an invalid signal + // handler has already been installed. + unsafe { + nix::sys::signal::sigaction(signal.into(), &sa)?; + } + } + + Ok(Self(File::from(read))) + } +} + +impl Iterator for Signals { + type Item = io::Result; + + fn next(&mut self) -> Option { + let mut signal = [0]; + Some( + self + .0 + .read_exact(&mut signal) + .and_then(|()| Signal::try_from(signal[0])), + ) + } +} diff --git a/tests/format.rs b/tests/format.rs index 04d7d87eef..3db1aedfac 100644 --- a/tests/format.rs +++ b/tests/format.rs @@ -103,7 +103,7 @@ fn write_error() { // skip this test if running as root, since root can write files even if // permissions would otherwise forbid it #[cfg(not(windows))] - if unsafe { libc::getuid() } == 0 { + if nix::unistd::getuid() == nix::unistd::ROOT { return; } diff --git a/tests/interrupts.rs b/tests/interrupts.rs index bd0077c237..f0b9ff59a7 100644 --- a/tests/interrupts.rs +++ b/tests/interrupts.rs @@ -1,12 +1,11 @@ -use { - super::*, - std::time::{Duration, Instant}, -}; +use super::*; fn kill(process_id: u32) { - unsafe { - libc::kill(process_id.try_into().unwrap(), libc::SIGINT); - } + nix::sys::signal::kill( + nix::unistd::Pid::from_raw(process_id.try_into().unwrap()), + nix::sys::signal::Signal::SIGINT, + ) + .unwrap(); } fn interrupt_test(arguments: &[&str], justfile: &str) { diff --git a/tests/lib.rs b/tests/lib.rs index a86a3da769..aaacf93f01 100644 --- a/tests/lib.rs +++ b/tests/lib.rs @@ -23,6 +23,7 @@ pub(crate) use { path::{Path, PathBuf, MAIN_SEPARATOR, MAIN_SEPARATOR_STR}, process::{Command, Stdio}, str, + time::{Duration, Instant}, }, tempfile::TempDir, temptree::{temptree, tree, Tree}, @@ -110,6 +111,7 @@ mod shebang; mod shell; mod shell_expansion; mod show; +mod signals; mod slash_operator; mod string; mod subsequents; diff --git a/tests/signals.rs b/tests/signals.rs new file mode 100644 index 0000000000..8b12636dba --- /dev/null +++ b/tests/signals.rs @@ -0,0 +1,10 @@ +// todo: +// - create issue for sighup handling: can we detect when we receive sighup and are the only recipient? +// - fix and test on windows +// - tests: +// - just terminates immediately on receipt of fatal signal if no child process is running +// - just reports correct error if child process is terminated with fatal signal and indicates failure +// - just still terminates if child process is terminated with fatal signal and reports success +// - just prints info receipt of SIGINFO +// - contexts: recipe line, recipe script, --command, backtick (any others?) +// - get rid of old interrupt tests