From 21ee4ffbf65df93bbc65a08a725755c081c6a0da Mon Sep 17 00:00:00 2001 From: WindSoilder Date: Tue, 2 Aug 2022 18:24:06 +0800 Subject: [PATCH] no need fg_process_setup module --- crates/nu-system/src/foreground.rs | 98 +++++++++++++++--------------- 1 file changed, 49 insertions(+), 49 deletions(-) diff --git a/crates/nu-system/src/foreground.rs b/crates/nu-system/src/foreground.rs index a474b6aaea..953b7c9818 100644 --- a/crates/nu-system/src/foreground.rs +++ b/crates/nu-system/src/foreground.rs @@ -28,9 +28,9 @@ impl ForegroundProcess { } pub fn spawn(&mut self) -> std::io::Result { - fg_process_setup::prepare_to_foreground(&mut self.inner); + prepare_to_foreground(&mut self.inner); self.inner.spawn().map(|child| { - fg_process_setup::set_foreground(&child); + set_foreground(&child); ForegroundChild { inner: child } }) } @@ -45,67 +45,67 @@ impl AsMut for ForegroundChild { 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() } + unsafe { reset_foreground_id() } } } +#[cfg(target_family = "unix")] +use std::os::unix::prelude::CommandExt; + // 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 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); +// +// The function does nothing on windows. +fn prepare_to_foreground(external_command: &mut std::process::Command) { + #[cfg(target_family = "unix")] + 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(()) - }); - } + // 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 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); - } +// On unix, if `prepare_to_foreground` function is not called, +// the function will fail with silence and do nothing. +// +// The function does nothing on windows. +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. + #[cfg(target_family = "unix")] + 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 unsafe fn reset_foreground_id() { +/// Reset foreground to current process, and reset back `SIGTTOU`, `SIGTTIN` single handler. +/// +/// ## Safety +/// On unix, it can only be called when you have called `set_foreground`, or results in undefined behavior. +/// +/// The function does nothing on windows. +unsafe fn reset_foreground_id() { + #[cfg(target_family = "unix")] + { 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 fn prepare_to_foreground(_external_command: &mut std::process::Command) {} - - pub fn set_foreground(_process: &std::process::Child) {} - - pub unsafe fn reset_foreground_id() {} -}