From 872eb2c3dfd8a674502f2f6c2e5be28612bcb90f Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Fri, 8 Sep 2023 16:19:01 +0000 Subject: [PATCH] Restore initial foreground process group on exit (#10021) # Description When launching nushell interactively from another shell, the parent shell usually gives us own our process group and handles restoring control to itself. However, other programs that do not support job control expect us to give control of the terminal back to them. This PR makes it so that we record the initial foreground process group and restore it when nushell exits. An "exit" can be from the `exit` command, a panic, or a `SIGTERM` signal. The changes in `terminal.rs` mostly follow [fish's example](https://github.com/fish-shell/fish-shell/blob/0874dd6a965a3baf160ef4c656d6635b92f87cdc/fish-rust/src/common.rs#L1634). # User-Facing Changes Fixes interactions between nushell and other interactive CLI commands (e.g., VIFM #10015). --- src/terminal.rs | 89 ++++++++++++++++++++++++++++++++++++------------- 1 file changed, 66 insertions(+), 23 deletions(-) diff --git a/src/terminal.rs b/src/terminal.rs index c87f714d6d..fe54c99ff0 100644 --- a/src/terminal.rs +++ b/src/terminal.rs @@ -1,22 +1,60 @@ +#[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}, + unistd::{self, Pid}, +}; + +#[cfg(unix)] +static INITIAL_PGID: AtomicI32 = AtomicI32::new(-1); + #[cfg(unix)] pub(crate) fn acquire_terminal(interactive: bool) { - use nix::{ - errno::Errno, - sys::signal::{signal, SigHandler, Signal}, - unistd, - }; - use std::io::IsTerminal; - if interactive && std::io::stdin().is_terminal() { // see also: https://www.gnu.org/software/libc/manual/html_node/Initializing-the-Shell.html - take_control(); + let initial_pgid = take_control(); + + INITIAL_PGID.store(initial_pgid.into(), Ordering::Relaxed); unsafe { - // SIGINT and SIGQUIT have special handling above + if libc::atexit(restore_terminal) != 0 { + eprintln!("ERROR: failed to set exit function"); + std::process::exit(1); + }; + + // SIGINT and SIGQUIT have special handling signal(Signal::SIGTSTP, SigHandler::SigIgn).expect("signal ignore"); signal(Signal::SIGTTIN, SigHandler::SigIgn).expect("signal ignore"); signal(Signal::SIGTTOU, SigHandler::SigIgn).expect("signal ignore"); + 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"); } // Put ourselves in our own process group, if not already @@ -33,7 +71,7 @@ pub(crate) fn acquire_terminal(interactive: bool) { } } // Set our possibly new pgid to be in control of terminal - let _ = unistd::tcsetpgrp(nix::libc::STDIN_FILENO, shell_pgid); + let _ = unistd::tcsetpgrp(libc::STDIN_FILENO, shell_pgid); } } @@ -41,25 +79,20 @@ pub(crate) fn acquire_terminal(interactive: bool) { pub(crate) fn acquire_terminal(_: bool) {} // Inspired by fish's acquire_tty_or_exit +// Returns our original pgid #[cfg(unix)] -fn take_control() { - use nix::{ - errno::Errno, - sys::signal::{self, SaFlags, SigAction, SigHandler, SigSet, Signal}, - unistd::{self, Pid}, - }; - +fn take_control() -> Pid { let shell_pgid = unistd::getpgrp(); match unistd::tcgetpgrp(nix::libc::STDIN_FILENO) { Ok(owner_pgid) if owner_pgid == shell_pgid => { // Common case, nothing to do - return; + return owner_pgid; } Ok(owner_pgid) if owner_pgid == unistd::getpid() => { // This can apparently happen with sudo: https://github.com/fish-shell/fish-shell/issues/7388 let _ = unistd::setpgid(owner_pgid, owner_pgid); - return; + return owner_pgid; } _ => (), } @@ -80,14 +113,14 @@ fn take_control() { } for _ in 0..4096 { - match unistd::tcgetpgrp(nix::libc::STDIN_FILENO) { + match unistd::tcgetpgrp(libc::STDIN_FILENO) { Ok(owner_pgid) if owner_pgid == shell_pgid => { // success - return; + return owner_pgid; } Ok(owner_pgid) if owner_pgid == Pid::from_raw(0) => { // Zero basically means something like "not owned" and we can just take it - let _ = unistd::tcsetpgrp(nix::libc::STDIN_FILENO, shell_pgid); + let _ = unistd::tcsetpgrp(libc::STDIN_FILENO, shell_pgid); } Err(Errno::ENOTTY) => { eprintln!("ERROR: no TTY for interactive shell"); @@ -103,6 +136,16 @@ fn take_control() { } } - eprintln!("ERROR: failed take control of the terminal, we might be orphaned"); + eprintln!("ERROR: failed to take control of the terminal, we might be orphaned"); 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 + 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); + } +}