diff --git a/crates/nu-command/src/system/run_external.rs b/crates/nu-command/src/system/run_external.rs index cfc47da660..06502f5e23 100644 --- a/crates/nu-command/src/system/run_external.rs +++ b/crates/nu-command/src/system/run_external.rs @@ -13,6 +13,7 @@ use nu_protocol::{Category, Example, ListStream, PipelineData, RawStream, Span, use itertools::Itertools; use nu_engine::CallExt; +use nu_system::ForegroundProcess; use pathdiff::diff_paths; use regex::Regex; @@ -122,15 +123,16 @@ impl ExternalCommand { let ctrlc = engine_state.ctrlc.clone(); - let mut process = self.create_process(&input, false, head)?; + let mut fg_process = ForegroundProcess::new(self.create_process(&input, false, head)?); let child; #[cfg(windows)] { - match process.spawn() { + match fg_process.spawn() { Err(_) => { - let mut process = self.create_process(&input, true, head)?; - child = process.spawn(); + let mut fg_process = + ForegroundProcess::new(self.create_process(&input, true, head)?); + child = fg_process.spawn(); } Ok(process) => { child = Ok(process); @@ -140,7 +142,7 @@ impl ExternalCommand { #[cfg(not(windows))] { - child = process.spawn() + child = fg_process.spawn() } match child { @@ -158,7 +160,7 @@ impl ExternalCommand { engine_state.config.use_ansi_coloring = false; // if there is a string or a stream, that is sent to the pipe std - if let Some(mut stdin_write) = child.stdin.take() { + if let Some(mut stdin_write) = child.as_mut().stdin.take() { std::thread::spawn(move || { let input = crate::Table::run( &crate::Table, @@ -199,7 +201,7 @@ impl ExternalCommand { // and we create a ListStream that can be consumed if redirect_stderr { - let stderr = child.stderr.take().ok_or_else(|| { + let stderr = child.as_mut().stderr.take().ok_or_else(|| { ShellError::ExternalCommand( "Error taking stderr from external".to_string(), "Redirects need access to stderr of an external command" @@ -238,7 +240,7 @@ impl ExternalCommand { } if redirect_stdout { - let stdout = child.stdout.take().ok_or_else(|| { + let stdout = child.as_mut().stdout.take().ok_or_else(|| { ShellError::ExternalCommand( "Error taking stdout from external".to_string(), "Redirects need access to stdout of an external command" @@ -276,7 +278,7 @@ impl ExternalCommand { } } - match child.wait() { + match child.as_mut().wait() { Err(err) => Err(ShellError::ExternalCommand( "External command exited with error".into(), err.to_string(), diff --git a/crates/nu-system/Cargo.toml b/crates/nu-system/Cargo.toml index d46e63e5de..8c473691b6 100644 --- a/crates/nu-system/Cargo.toml +++ b/crates/nu-system/Cargo.toml @@ -13,7 +13,7 @@ name = "ps" path = "src/main.rs" [dependencies] - +libc = "0.2" [target.'cfg(any(target_os = "linux", target_os = "android"))'.dependencies] procfs = "0.13.0" @@ -21,11 +21,9 @@ procfs = "0.13.0" [target.'cfg(target_os = "macos")'.dependencies] libproc = "0.12.0" errno = "0.2" -libc = "0.2" [target.'cfg(target_os = "windows")'.dependencies] winapi = { version = "0.3.9", features = ["tlhelp32", "fileapi", "handleapi", "ifdef", "ioapiset", "minwindef", "pdh", "psapi", "synchapi", "sysinfoapi", "winbase", "winerror", "winioctl", "winnt", "oleauto", "wbemcli", "rpcdce", "combaseapi", "objidl", "powerbase", "netioapi", "lmcons", "lmaccess", "lmapibuf", "memoryapi", "shellapi", "std", "securitybaseapi"] } chrono = "0.4" -libc = "0.2" ntapi = "0.3" once_cell = "1.0" diff --git a/crates/nu-system/src/foreground.rs b/crates/nu-system/src/foreground.rs new file mode 100644 index 0000000000..05da611f92 --- /dev/null +++ b/crates/nu-system/src/foreground.rs @@ -0,0 +1,111 @@ +use std::process::{Child, Command}; + +/// A simple wrapper for `std::process::Command` +/// +/// ## spawn behavior +/// ### Unix +/// When invoke `spawn`, current process will ignore `SIGTTOU`, `SIGTTIN` signal, spawned child process will get it's +/// own process group id, and it's going to foreground(by making stdin belong's to child's process group). +/// +/// When child is to over, `SIGTTOU`, `SIGTTIN` signal will be reset, foreground process is back to callers' process. +/// +/// ### 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, +} + +/// A simple wrapper for `std::process::Child` +/// +/// It can only be created by `ForegroundProcess::spawn`. +pub struct ForegroundChild { + inner: Child, +} + +impl ForegroundProcess { + pub fn new(cmd: Command) -> Self { + Self { inner: cmd } + } + + pub fn spawn(&mut self) -> std::io::Result { + fg_process_setup::prepare_to_foreground(&mut self.inner); + self.inner.spawn().map(|child| { + fg_process_setup::set_foreground(&child); + ForegroundChild { inner: child } + }) + } +} + +impl AsMut for ForegroundChild { + fn as_mut(&mut self) -> &mut Child { + &mut self.inner + } +} + +impl Drop for ForegroundChild { + fn drop(&mut self) { + // It's ok to use here because we have called `set_foreground` during creation. + unsafe { fg_process_setup::reset_foreground_id() } + } +} + +// It's a simpler version of fish shell's external process handling. +// +// For more information, please check `child_setup_process` function in fish shell. +// https://github.com/fish-shell/fish-shell/blob/3f90efca38079922b4b21707001d7bb9630107eb/src/postfork.cpp#L140 +#[cfg(target_family = "unix")] +mod fg_process_setup { + use std::os::unix::prelude::CommandExt; + pub(super) fn prepare_to_foreground(external_command: &mut std::process::Command) { + unsafe { + libc::signal(libc::SIGTTOU, libc::SIG_IGN); + libc::signal(libc::SIGTTIN, libc::SIG_IGN); + + // Safety: + // POSIX only allows async-signal-safe functions to be called. + // And `setpgid` is async-signal-safe function according to: + // https://manpages.ubuntu.com/manpages/bionic/man7/signal-safety.7.html + // So we're ok to invoke `libc::setpgid` inside `pre_exec`. + external_command.pre_exec(|| { + // make the command startup with new process group. + // The process group id must be the same as external commands' pid. + // Or else we'll failed to set it as foreground process. + // For more information, check `fork_child_for_process` function: + // https://github.com/fish-shell/fish-shell/blob/023042098396aa450d2c4ea1eb9341312de23126/src/exec.cpp#L398 + libc::setpgid(0, 0); + Ok(()) + }); + } + } + + // If `prepare_to_foreground` function is not called, the function will fail with silence and do nothing. + pub(super) fn set_foreground(process: &std::process::Child) { + // it's ok to use unsafe here + // the implementaion here is just the same as + // https://docs.rs/nix/latest/nix/unistd/fn.tcsetpgrp.html, which is a safe function. + unsafe { + libc::tcsetpgrp(libc::STDIN_FILENO, process.id() as i32); + } + } + + /// Reset foreground to current process, and reset back `SIGTTOU`, `SIGTTIN` single handler. + /// + /// ## Safety + /// It can only be called when you have called `set_foreground`, or results in undefined behavior. + pub(super) unsafe fn reset_foreground_id() { + libc::tcsetpgrp(libc::STDIN_FILENO, libc::getpgrp()); + libc::signal(libc::SIGTTOU, libc::SIG_DFL); + libc::signal(libc::SIGTTIN, libc::SIG_DFL); + } +} + +// TODO: investigate if we can set foreground process through windows system call. +#[cfg(target_family = "windows")] +mod fg_process_setup { + + pub(super) fn prepare_to_foreground(_external_command: &mut std::process::Command) {} + + pub(super) fn set_foreground(_process: &std::process::Child) {} + + pub(super) unsafe fn reset_foreground_id() {} +} diff --git a/crates/nu-system/src/lib.rs b/crates/nu-system/src/lib.rs index 1ad89d73d5..2283a94b78 100644 --- a/crates/nu-system/src/lib.rs +++ b/crates/nu-system/src/lib.rs @@ -1,3 +1,4 @@ +mod foreground; #[cfg(any(target_os = "android", target_os = "linux"))] mod linux; #[cfg(target_os = "macos")] @@ -5,6 +6,7 @@ mod macos; #[cfg(target_os = "windows")] mod windows; +pub use self::foreground::{ForegroundChild, ForegroundProcess}; #[cfg(any(target_os = "android", target_os = "linux"))] pub use self::linux::*; #[cfg(target_os = "macos")]