From 924986576db471fe4328c105ca4f1fc0c7ec9c51 Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Mon, 15 Jan 2024 22:08:21 +0000 Subject: [PATCH] Do not block signals for child processes (#11402) # Description / User-Facing Changes Signals are no longer blocked for child processes launched from both interactive and non-interactive mode. The only exception is that `SIGTSTP`, `SIGTTIN`, and `SIGTTOU` remain blocked for child processes launched only from **interactive** mode. This is to help prevent nushell from getting into an unrecoverable state, since we don't support background jobs. Anyways, this fully fixes #9026. # Other Notes - Needs Rust version `>= 1.66` for a fix in `std::process::Command::spawn`, but it looks our current Rust version is way above this. - Uses `sigaction` instead of `signal`, since the behavior of `signal` can apparently differ across systems. Also, the `sigaction` man page says: > The sigaction() function supersedes the signal() function, and should be used in preference. Additionally, using both `sigaction` and `signal` is not recommended. Since we were already using `sigaction` in some places (and possibly some of our dependencies as well), this PR replaces all usages of `signal`. # Tests Might want to wait for #11178 for testing. --- Cargo.lock | 1 - Cargo.toml | 1 - crates/nu-command/src/system/run_external.rs | 148 ++++++------ crates/nu-system/src/foreground.rs | 225 +++++++++---------- crates/nu-system/src/lib.rs | 2 +- src/main.rs | 25 ++- src/terminal.rs | 102 ++++----- 7 files changed, 231 insertions(+), 273 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5be86af1f2..6d8367201b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2637,7 +2637,6 @@ dependencies = [ "rstest", "serde_json", "serial_test", - "signal-hook", "simplelog", "tempfile", "time", diff --git a/Cargo.toml b/Cargo.toml index f9252c009f..363d6f0e61 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -90,7 +90,6 @@ time = "0.3" [target.'cfg(not(target_os = "windows"))'.dependencies] # Our dependencies don't use OpenSSL on Windows openssl = { version = "0.10", features = ["vendored"], optional = true } -signal-hook = { version = "0.3", default-features = false } [target.'cfg(windows)'.build-dependencies] winresource = "0.1" diff --git a/crates/nu-command/src/system/run_external.rs b/crates/nu-command/src/system/run_external.rs index 7f56efc487..5be3d09374 100644 --- a/crates/nu-command/src/system/run_external.rs +++ b/crates/nu-command/src/system/run_external.rs @@ -9,7 +9,7 @@ use nu_protocol::{ Category, Example, ListStream, PipelineData, RawStream, ShellError, Signature, Span, Spanned, SyntaxShape, Type, Value, }; -use nu_system::ForegroundProcess; +use nu_system::ForegroundChild; use nu_utils::IgnoreCaseExt; use os_pipe::PipeReader; use pathdiff::diff_paths; @@ -216,82 +216,69 @@ impl ExternalCommand { #[allow(unused_mut)] let (cmd, mut reader) = self.create_process(&input, false, head)?; - let mut fg_process = - ForegroundProcess::new(cmd, engine_state.pipeline_externals_state.clone()); - // mut is used in the windows branch only, suppress warning on other platforms - #[allow(unused_mut)] - let mut child; + + #[cfg(all(not(unix), not(windows)))] // are there any systems like this? + let child = ForegroundChild::spawn(cmd); #[cfg(windows)] - { - // Running external commands on Windows has 2 points of complication: - // 1. Some common Windows commands are actually built in to cmd.exe, not executables in their own right. - // 2. We need to let users run batch scripts etc. (.bat, .cmd) without typing their extension + let child = match ForegroundChild::spawn(cmd) { + Ok(child) => Ok(child), + Err(err) => { + // Running external commands on Windows has 2 points of complication: + // 1. Some common Windows commands are actually built in to cmd.exe, not executables in their own right. + // 2. We need to let users run batch scripts etc. (.bat, .cmd) without typing their extension - // To support these situations, we have a fallback path that gets run if a command - // fails to be run as a normal executable: - // 1. "shell out" to cmd.exe if the command is a known cmd.exe internal command - // 2. Otherwise, use `which-rs` to look for batch files etc. then run those in cmd.exe - match fg_process.spawn(engine_state.is_interactive) { - Err(err) => { - // set the default value, maybe we'll override it later - child = Err(err); + // To support these situations, we have a fallback path that gets run if a command + // fails to be run as a normal executable: + // 1. "shell out" to cmd.exe if the command is a known cmd.exe internal command + // 2. Otherwise, use `which-rs` to look for batch files etc. then run those in cmd.exe - // This has the full list of cmd.exe "internal" commands: https://ss64.com/nt/syntax-internal.html - // I (Reilly) went through the full list and whittled it down to ones that are potentially useful: - const CMD_INTERNAL_COMMANDS: [&str; 9] = [ - "ASSOC", "CLS", "ECHO", "FTYPE", "MKLINK", "PAUSE", "START", "VER", "VOL", - ]; - let command_name = &self.name.item; - let looks_like_cmd_internal = CMD_INTERNAL_COMMANDS - .iter() - .any(|&cmd| command_name.eq_ignore_ascii_case(cmd)); + // set the default value, maybe we'll override it later + let mut child = Err(err); - if looks_like_cmd_internal { - let (cmd, new_reader) = self.create_process(&input, true, head)?; - reader = new_reader; - let mut cmd_process = ForegroundProcess::new( - cmd, - engine_state.pipeline_externals_state.clone(), - ); - child = cmd_process.spawn(engine_state.is_interactive); - } else { - #[cfg(feature = "which-support")] + // This has the full list of cmd.exe "internal" commands: https://ss64.com/nt/syntax-internal.html + // I (Reilly) went through the full list and whittled it down to ones that are potentially useful: + const CMD_INTERNAL_COMMANDS: [&str; 9] = [ + "ASSOC", "CLS", "ECHO", "FTYPE", "MKLINK", "PAUSE", "START", "VER", "VOL", + ]; + let command_name = &self.name.item; + let looks_like_cmd_internal = CMD_INTERNAL_COMMANDS + .iter() + .any(|&cmd| command_name.eq_ignore_ascii_case(cmd)); + + if looks_like_cmd_internal { + let (cmd, new_reader) = self.create_process(&input, true, head)?; + reader = new_reader; + child = ForegroundChild::spawn(cmd); + } else { + #[cfg(feature = "which-support")] + { + // maybe it's a batch file (foo.cmd) and the user typed `foo`. Try to find it with `which-rs` + // TODO: clean this up with an if-let chain once those are stable + if let Ok(path) = + nu_engine::env::path_str(engine_state, stack, self.name.span) { - // maybe it's a batch file (foo.cmd) and the user typed `foo`. Try to find it with `which-rs` - // TODO: clean this up with an if-let chain once those are stable - if let Ok(path) = - nu_engine::env::path_str(engine_state, stack, self.name.span) - { - if let Some(cwd) = self.env_vars.get("PWD") { - // append cwd to PATH so `which-rs` looks in the cwd too. - // this approximates what cmd.exe does. - let path_with_cwd = format!("{};{}", cwd, path); - if let Ok(which_path) = - which::which_in(&self.name.item, Some(path_with_cwd), cwd) - { - if let Some(file_name) = which_path.file_name() { - if !file_name - .to_string_lossy() - .eq_ignore_case(command_name) - { - // which-rs found an executable file with a slightly different name - // than the one the user tried. Let's try running it - let mut new_command = self.clone(); - new_command.name = Spanned { - item: file_name.to_string_lossy().to_string(), - span: self.name.span, - }; - let (cmd, new_reader) = new_command - .create_process(&input, true, head)?; - reader = new_reader; - let mut cmd_process = ForegroundProcess::new( - cmd, - engine_state.pipeline_externals_state.clone(), - ); - child = - cmd_process.spawn(engine_state.is_interactive); - } + if let Some(cwd) = self.env_vars.get("PWD") { + // append cwd to PATH so `which-rs` looks in the cwd too. + // this approximates what cmd.exe does. + let path_with_cwd = format!("{};{}", cwd, path); + if let Ok(which_path) = + which::which_in(&self.name.item, Some(path_with_cwd), cwd) + { + if let Some(file_name) = which_path.file_name() { + if !file_name.to_string_lossy().eq_ignore_case(command_name) + { + // which-rs found an executable file with a slightly different name + // than the one the user tried. Let's try running it + let mut new_command = self.clone(); + new_command.name = Spanned { + item: file_name.to_string_lossy().to_string(), + span: self.name.span, + }; + let (cmd, new_reader) = + new_command.create_process(&input, true, head)?; + reader = new_reader; + child = ForegroundChild::spawn(cmd); } } } @@ -299,16 +286,17 @@ impl ExternalCommand { } } } - Ok(process) => { - child = Ok(process); - } - } - } - #[cfg(not(windows))] - { - child = fg_process.spawn(engine_state.is_interactive) - } + child + } + }; + + #[cfg(unix)] + let child = ForegroundChild::spawn( + cmd, + engine_state.is_interactive, + &engine_state.pipeline_externals_state, + ); match child { Err(err) => { diff --git a/crates/nu-system/src/foreground.rs b/crates/nu-system/src/foreground.rs index b1df11fa44..f2aa28b25a 100644 --- a/crates/nu-system/src/foreground.rs +++ b/crates/nu-system/src/foreground.rs @@ -1,66 +1,78 @@ use std::{ + io, process::{Child, Command}, +}; + +#[cfg(unix)] +use std::{ + io::IsTerminal, sync::{ atomic::{AtomicU32, Ordering}, Arc, }, }; -/// A simple wrapper for `std::process::Command` +/// A simple wrapper for [`std::process::Child`] /// -/// ## Spawn behavior -/// ### Unix +/// It can only be created by [`ForegroundChild::spawn`]. /// -/// The spawned child process will get its own process group id, and it's going to foreground (by making stdin belong's to child's process group). +/// # Spawn behavior +/// ## Unix /// +/// For interactive shells, the spawned child process will get its own process group id, +/// and it will be put in the foreground (by making stdin belong to the child's process group). /// On drop, the calling process's group will become the foreground process group once again. /// -/// ### Windows -/// It does nothing special on windows system, `spawn` is the same as [std::process::Command::spawn](std::process::Command::spawn) -pub struct ForegroundProcess { - inner: Command, - pipeline_state: Arc<(AtomicU32, AtomicU32)>, -} - -/// A simple wrapper for `std::process::Child` +/// For non-interactive mode, processes are spawned normally without any foreground process handling. /// -/// It can only be created by `ForegroundProcess::spawn`. +/// ## Other systems +/// +/// It does nothing special on non-unix systems, so `spawn` is the same as [`std::process::Command::spawn`]. pub struct ForegroundChild { inner: Child, - pipeline_state: Arc<(AtomicU32, AtomicU32)>, - interactive: bool, + #[cfg(unix)] + pipeline_state: Option>, } -impl ForegroundProcess { - pub fn new(cmd: Command, pipeline_state: Arc<(AtomicU32, AtomicU32)>) -> Self { - Self { - inner: cmd, - pipeline_state, - } +impl ForegroundChild { + #[cfg(not(unix))] + pub fn spawn(mut command: Command) -> io::Result { + command.spawn().map(|child| Self { inner: child }) } - pub fn spawn(&mut self, interactive: bool) -> std::io::Result { - let (ref pgrp, ref pcnt) = *self.pipeline_state; - let existing_pgrp = pgrp.load(Ordering::SeqCst); - fg_process_setup::prepare_to_foreground(&mut self.inner, existing_pgrp, interactive); - self.inner - .spawn() - .map(|child| { - fg_process_setup::set_foreground(&child, existing_pgrp, interactive); - let _ = pcnt.fetch_add(1, Ordering::SeqCst); - if existing_pgrp == 0 { - pgrp.store(child.id(), Ordering::SeqCst); - } - ForegroundChild { - inner: child, - pipeline_state: self.pipeline_state.clone(), - interactive, - } - }) - .map_err(|e| { - fg_process_setup::reset_foreground_id(interactive); - e + #[cfg(unix)] + pub fn spawn( + mut command: Command, + interactive: bool, + pipeline_state: &Arc<(AtomicU32, AtomicU32)>, + ) -> io::Result { + if interactive && io::stdin().is_terminal() { + let (pgrp, pcnt) = pipeline_state.as_ref(); + let existing_pgrp = pgrp.load(Ordering::SeqCst); + foreground_pgroup::prepare_command(&mut command, existing_pgrp); + command + .spawn() + .map(|child| { + foreground_pgroup::set(&child, existing_pgrp); + let _ = pcnt.fetch_add(1, Ordering::SeqCst); + if existing_pgrp == 0 { + pgrp.store(child.id(), Ordering::SeqCst); + } + Self { + inner: child, + pipeline_state: Some(pipeline_state.clone()), + } + }) + .map_err(|e| { + foreground_pgroup::reset(); + e + }) + } else { + command.spawn().map(|child| Self { + inner: child, + pipeline_state: None, }) + } } } @@ -70,122 +82,85 @@ impl AsMut for ForegroundChild { } } +#[cfg(unix)] impl Drop for ForegroundChild { fn drop(&mut self) { - let (ref pgrp, ref pcnt) = *self.pipeline_state; - if pcnt.fetch_sub(1, Ordering::SeqCst) == 1 { - pgrp.store(0, Ordering::SeqCst); - fg_process_setup::reset_foreground_id(self.interactive) + if let Some((pgrp, pcnt)) = self.pipeline_state.as_deref() { + if pcnt.fetch_sub(1, Ordering::SeqCst) == 1 { + pgrp.store(0, Ordering::SeqCst); + foreground_pgroup::reset() + } } } } // It's a simpler version of fish shell's external process handling. #[cfg(unix)] -mod fg_process_setup { +mod foreground_pgroup { use nix::{ - sys::signal, + libc, + sys::signal::{sigaction, SaFlags, SigAction, SigHandler, SigSet, Signal}, unistd::{self, Pid}, }; - use std::io::IsTerminal; - use std::os::unix::prelude::{CommandExt, RawFd}; + use std::{ + os::unix::prelude::CommandExt, + process::{Child, Command}, + }; - // TODO: when raising MSRV past 1.63.0, switch to OwnedFd - struct TtyHandle(RawFd); - - impl Drop for TtyHandle { - fn drop(&mut self) { - let _ = unistd::close(self.0); - } - } - - pub(super) fn prepare_to_foreground( - external_command: &mut std::process::Command, - existing_pgrp: u32, - interactive: bool, - ) { - let tty = TtyHandle(unistd::dup(nix::libc::STDIN_FILENO).expect("dup")); - let interactive = interactive && std::io::stdin().is_terminal(); + pub fn prepare_command(external_command: &mut Command, existing_pgrp: u32) { unsafe { // Safety: // POSIX only allows async-signal-safe functions to be called. - // `sigprocmask`, `setpgid` and `tcsetpgrp` are async-signal-safe according to: + // `sigaction` and `getpid` are async-signal-safe according to: // https://manpages.ubuntu.com/manpages/bionic/man7/signal-safety.7.html + // Also, `set_foreground_pid` is async-signal-safe. external_command.pre_exec(move || { - // When this callback is run, std::process has already done: - // - pthread_sigmask(SIG_SETMASK) with an empty sigset - // - signal(SIGPIPE, SIG_DFL) - // However, we do need TTOU/TTIN blocked again during this setup. - let mut sigset = signal::SigSet::empty(); - sigset.add(signal::Signal::SIGTSTP); - sigset.add(signal::Signal::SIGTTOU); - sigset.add(signal::Signal::SIGTTIN); - sigset.add(signal::Signal::SIGCHLD); - signal::sigprocmask(signal::SigmaskHow::SIG_BLOCK, Some(&sigset), None) - .expect("signal mask"); + // When this callback is run, std::process has already: + // - reset SIGPIPE to SIG_DFL // According to glibc's job control manual: // https://www.gnu.org/software/libc/manual/html_node/Launching-Jobs.html // This has to be done *both* in the parent and here in the child due to race conditions. - if interactive { - set_foreground_pid(unistd::getpid(), existing_pgrp, tty.0); - } + set_foreground_pid(Pid::this(), existing_pgrp); - // Now let the child process have all the signals by resetting with SIG_SETMASK. - let mut sigset = signal::SigSet::empty(); - sigset.add(signal::Signal::SIGTSTP); // for now not really all: we don't support background jobs, so keep this one blocked - signal::sigprocmask(signal::SigmaskHow::SIG_SETMASK, Some(&sigset), None) - .expect("signal mask"); + // Reset signal handlers for child, sync with `terminal.rs` + let default = SigAction::new(SigHandler::SigDfl, SaFlags::empty(), SigSet::empty()); + // SIGINT has special handling + let _ = sigaction(Signal::SIGQUIT, &default); + // We don't support background jobs, so keep some signals blocked for now + // let _ = sigaction(Signal::SIGTSTP, &default); + // let _ = sigaction(Signal::SIGTTIN, &default); + // let _ = sigaction(Signal::SIGTTOU, &default); + let _ = sigaction(Signal::SIGTERM, &default); Ok(()) }); } } - pub(super) fn set_foreground( - process: &std::process::Child, - existing_pgrp: u32, - interactive: bool, - ) { - // called from the parent shell process - do the stdin tty check here - if interactive && std::io::stdin().is_terminal() { - set_foreground_pid( - Pid::from_raw(process.id() as i32), - existing_pgrp, - nix::libc::STDIN_FILENO, - ); - } + pub fn set(process: &Child, existing_pgrp: u32) { + set_foreground_pid(Pid::from_raw(process.id() as i32), existing_pgrp); } - // existing_pgrp is 0 when we don't have an existing foreground process in the pipeline. - // Conveniently, 0 means "current pid" to setpgid. But not to tcsetpgrp. - fn set_foreground_pid(pid: Pid, existing_pgrp: u32, tty: RawFd) { - let _ = unistd::setpgid(pid, Pid::from_raw(existing_pgrp as i32)); - let _ = unistd::tcsetpgrp( - tty, - if existing_pgrp == 0 { - pid - } else { - Pid::from_raw(existing_pgrp as i32) - }, - ); + fn set_foreground_pid(pid: Pid, existing_pgrp: u32) { + // Safety: needs to be async-signal-safe. + // `setpgid` and `tcsetpgrp` are async-signal-safe. + + // `existing_pgrp` is 0 when we don't have an existing foreground process in the pipeline. + // A pgrp of 0 means the calling process's pid for `setpgid`. But not for `tcsetpgrp`. + let pgrp = if existing_pgrp == 0 { + pid + } else { + Pid::from_raw(existing_pgrp as i32) + }; + let _ = unistd::setpgid(pid, pgrp); + let _ = unistd::tcsetpgrp(libc::STDIN_FILENO, pgrp); } /// Reset the foreground process group to the shell - pub(super) fn reset_foreground_id(interactive: bool) { - if interactive && std::io::stdin().is_terminal() { - if let Err(e) = nix::unistd::tcsetpgrp(nix::libc::STDIN_FILENO, unistd::getpgrp()) { - println!("ERROR: reset foreground id failed, tcsetpgrp result: {e:?}"); - } + pub fn reset() { + if let Err(e) = unistd::tcsetpgrp(libc::STDIN_FILENO, unistd::getpgrp()) { + eprintln!("ERROR: reset foreground id failed, tcsetpgrp result: {e:?}"); } } } - -#[cfg(not(unix))] -mod fg_process_setup { - pub(super) fn prepare_to_foreground(_: &mut std::process::Command, _: u32, _: bool) {} - - pub(super) fn set_foreground(_: &std::process::Child, _: u32, _: bool) {} - - pub(super) fn reset_foreground_id(_: bool) {} -} diff --git a/crates/nu-system/src/lib.rs b/crates/nu-system/src/lib.rs index 7f04a6d270..99e4365e40 100644 --- a/crates/nu-system/src/lib.rs +++ b/crates/nu-system/src/lib.rs @@ -7,7 +7,7 @@ pub mod os_info; #[cfg(target_os = "windows")] mod windows; -pub use self::foreground::{ForegroundChild, ForegroundProcess}; +pub use self::foreground::ForegroundChild; #[cfg(any(target_os = "android", target_os = "linux"))] pub use self::linux::*; #[cfg(target_os = "macos")] diff --git a/src/main.rs b/src/main.rs index 27f13ecb1e..eca31ed5ac 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4,6 +4,7 @@ mod ide; mod logger; mod run; mod signals; +#[cfg(unix)] mod terminal; mod test_bins; #[cfg(test)] @@ -17,7 +18,6 @@ use crate::{ command::parse_commandline_args, config_files::set_config_path, logger::{configure, logger}, - terminal::acquire_terminal, }; use command::gather_commandline_args; use log::Level; @@ -185,16 +185,19 @@ fn main() -> Result<()> { use_color, ); - start_time = std::time::Instant::now(); - acquire_terminal(engine_state.is_interactive); - perf( - "acquire_terminal", - start_time, - file!(), - line!(), - column!(), - use_color, - ); + #[cfg(unix)] + { + start_time = std::time::Instant::now(); + terminal::acquire(engine_state.is_interactive); + perf( + "acquire_terminal", + start_time, + file!(), + line!(), + column!(), + use_color, + ); + } if let Some(include_path) = &parsed_nu_cli_args.include_path { let span = include_path.span; diff --git a/src/terminal.rs b/src/terminal.rs index 1a8b2d465f..6387a0ae50 100644 --- a/src/terminal.rs +++ b/src/terminal.rs @@ -1,63 +1,46 @@ -#[cfg(unix)] use std::{ io::IsTerminal, sync::atomic::{AtomicI32, Ordering}, }; -#[cfg(unix)] use nix::{ errno::Errno, libc, - sys::signal::{self, raise, signal, SaFlags, SigAction, SigHandler, SigSet, Signal}, + sys::signal::{killpg, raise, sigaction, SaFlags, SigAction, SigHandler, SigSet, Signal}, unistd::{self, Pid}, }; -#[cfg(unix)] static INITIAL_PGID: AtomicI32 = AtomicI32::new(-1); -#[cfg(unix)] -pub(crate) fn acquire_terminal(interactive: bool) { +pub(crate) fn acquire(interactive: bool) { if interactive && std::io::stdin().is_terminal() { // see also: https://www.gnu.org/software/libc/manual/html_node/Initializing-the-Shell.html + if unsafe { libc::atexit(restore_terminal) } != 0 { + eprintln!("ERROR: failed to set exit function"); + std::process::exit(1); + }; + let initial_pgid = take_control(); INITIAL_PGID.store(initial_pgid.into(), Ordering::Relaxed); unsafe { - if libc::atexit(restore_terminal) != 0 { - eprintln!("ERROR: failed to set exit function"); - std::process::exit(1); - }; - // SIGINT has special handling - signal(Signal::SIGQUIT, SigHandler::SigIgn).expect("signal ignore"); - signal(Signal::SIGTSTP, SigHandler::SigIgn).expect("signal ignore"); - signal(Signal::SIGTTIN, SigHandler::SigIgn).expect("signal ignore"); - signal(Signal::SIGTTOU, SigHandler::SigIgn).expect("signal ignore"); - - // TODO: determine if this is necessary or not, since this breaks `rm` on macOS - // signal(Signal::SIGCHLD, SigHandler::SigIgn).expect("signal ignore"); - - signal_hook::low_level::register(signal_hook::consts::SIGTERM, || { - // Safety: can only call async-signal-safe functions here - // restore_terminal, signal, and raise are all async-signal-safe - - restore_terminal(); - - if signal(Signal::SIGTERM, SigHandler::SigDfl).is_err() { - // Failed to set signal handler to default. - // This should not be possible, but if it does happen, - // then this could result in an infitite loop due to the raise below. - // So, we'll just exit immediately if this happens. - std::process::exit(1); - }; - - if raise(Signal::SIGTERM).is_err() { - std::process::exit(1); - }; - }) - .expect("signal hook"); + let ignore = SigAction::new(SigHandler::SigIgn, SaFlags::empty(), SigSet::empty()); + sigaction(Signal::SIGQUIT, &ignore).expect("signal ignore"); + sigaction(Signal::SIGTSTP, &ignore).expect("signal ignore"); + sigaction(Signal::SIGTTIN, &ignore).expect("signal ignore"); + sigaction(Signal::SIGTTOU, &ignore).expect("signal ignore"); + sigaction( + Signal::SIGTERM, + &SigAction::new( + SigHandler::Handler(sigterm_handler), + SaFlags::empty(), + SigSet::empty(), + ), + ) + .expect("signal action"); } // Put ourselves in our own process group, if not already @@ -78,12 +61,8 @@ pub(crate) fn acquire_terminal(interactive: bool) { } } -#[cfg(not(unix))] -pub(crate) fn acquire_terminal(_: bool) {} - // Inspired by fish's acquire_tty_or_exit // Returns our original pgid -#[cfg(unix)] fn take_control() -> Pid { let shell_pgid = unistd::getpgrp(); @@ -101,16 +80,12 @@ fn take_control() -> Pid { } // Reset all signal handlers to default + let default = SigAction::new(SigHandler::SigDfl, SaFlags::empty(), SigSet::empty()); for sig in Signal::iterator() { - unsafe { - if let Ok(old_act) = signal::sigaction( - sig, - &SigAction::new(SigHandler::SigDfl, SaFlags::empty(), SigSet::empty()), - ) { - // fish preserves ignored SIGHUP, presumably for nohup support, so let's do the same - if sig == Signal::SIGHUP && old_act.handler() == SigHandler::SigIgn { - let _ = signal::sigaction(sig, &old_act); - } + if let Ok(old_act) = unsafe { sigaction(sig, &default) } { + // fish preserves ignored SIGHUP, presumably for nohup support, so let's do the same + if sig == Signal::SIGHUP && old_act.handler() == SigHandler::SigIgn { + let _ = unsafe { sigaction(sig, &old_act) }; } } } @@ -131,7 +106,7 @@ fn take_control() -> Pid { } _ => { // fish also has other heuristics than "too many attempts" for the orphan check, but they're optional - if signal::killpg(shell_pgid, Signal::SIGTTIN).is_err() { + if killpg(shell_pgid, Signal::SIGTTIN).is_err() { eprintln!("ERROR: failed to SIGTTIN ourselves"); std::process::exit(1); } @@ -143,12 +118,31 @@ fn take_control() -> Pid { std::process::exit(1); } -#[cfg(unix)] extern "C" fn restore_terminal() { // Safety: can only call async-signal-safe functions here - // tcsetpgrp and getpgrp are async-signal-safe + // `tcsetpgrp` and `getpgrp` are async-signal-safe let initial_pgid = Pid::from_raw(INITIAL_PGID.load(Ordering::Relaxed)); if initial_pgid.as_raw() > 0 && initial_pgid != unistd::getpgrp() { let _ = unistd::tcsetpgrp(libc::STDIN_FILENO, initial_pgid); } } + +extern "C" fn sigterm_handler(_signum: libc::c_int) { + // Safety: can only call async-signal-safe functions here + // `restore_terminal`, `sigaction`, `raise`, and `_exit` are all async-signal-safe + + restore_terminal(); + + let default = SigAction::new(SigHandler::SigDfl, SaFlags::empty(), SigSet::empty()); + if unsafe { sigaction(Signal::SIGTERM, &default) }.is_err() { + // Failed to set signal handler to default. + // This should not be possible, but if it does happen, + // then this could result in an infinite loop due to the raise below. + // So, we'll just exit immediately if this happens. + unsafe { libc::_exit(1) }; + }; + + if raise(Signal::SIGTERM).is_err() { + unsafe { libc::_exit(1) }; + }; +}