Implement per-pipeline process groups for background jobs

Currently in nushell, when running in an interactive session,
every pipeline gets its own process group, which is set
as the foreground process group. Meanwhile, in non-interactive sessions,
all processes use the shell's process group.

This commit makes so that background jobs also use one processs group
per pipeline, but without moving them to foreground.

This commit also fixes a bug in nushell's trunk, which occurs if
a process in a non-interactive session gets signaled with the TSTP
signal, which causes the shell to try to regain foreground status, and
read a confirmation message from stdin (which should not occur in an
interactive session).

The `foreground.rs`'s inner `foreground_pgrp` module has been renamed to
`child_pgrp` since it now manages process groups in general.

Author: cosineblast

crates/nu-command/src/env/config/config_.rs

@@ -106,6 +106,7 @@ pub(super) fn start_editor(
     let child = ForegroundChild::spawn(
         command,
         engine_state.is_interactive,
+        engine_state.current_thread_job.is_some(),
         &engine_state.pipeline_externals_state,
     );
 

crates/nu-command/src/experimental/job_spawn.rs

@@ -60,7 +60,7 @@ impl Command for JobSpawn {
         let job_signals = Signals::new(Arc::new(AtomicBool::new(false)));
         job_state.set_signals(job_signals.clone());
 
-        // the new job has a separate process group for its processes
+        // the new job has a separate process group state for its processes
         job_state.pipeline_externals_state = Arc::new((AtomicU32::new(0), AtomicU32::new(0)));
 
         job_state.exit_warning_given = Arc::new(AtomicBool::new(false));

crates/nu-command/src/system/run_external.rs

@@ -251,6 +251,7 @@ impl Command for External {
         let child = ForegroundChild::spawn(
             command,
             engine_state.is_interactive,
+            engine_state.current_thread_job.is_some(),
             &engine_state.pipeline_externals_state,
         );
 

crates/nu-system/src/foreground.rs

@@ -10,7 +10,7 @@ use crate::ExitStatus;
 use std::{io::IsTerminal, sync::atomic::Ordering};
 
 #[cfg(unix)]
-pub use foreground_pgroup::stdin_fd;
+pub use child_pgroup::stdin_fd;
 
 #[cfg(unix)]
 use nix::{sys::signal, sys::wait, unistd::Pid};
@@ -35,6 +35,10 @@ pub struct ForegroundChild {
     inner: Child,
     #[cfg(unix)]
     pipeline_state: Option<Arc<(AtomicU32, AtomicU32)>>,
+
+    // this is unix-only since we don't have to deal with process groups in windows
+    #[cfg(unix)]
+    interactive: bool,
 }
 
 impl ForegroundChild {
@@ -47,32 +51,42 @@ impl ForegroundChild {
     pub fn spawn(
         mut command: Command,
         interactive: bool,
+        background: bool,
         pipeline_state: &Arc<(AtomicU32, AtomicU32)>,
     ) -> io::Result<Self> {
-        if interactive && io::stdin().is_terminal() {
+        let interactive = interactive && io::stdin().is_terminal();
+
+        let uses_dedicated_process_group = interactive || background;
+
+        if uses_dedicated_process_group {
             let (pgrp, pcnt) = pipeline_state.as_ref();
             let existing_pgrp = pgrp.load(Ordering::SeqCst);
-            foreground_pgroup::prepare_command(&mut command, existing_pgrp);
+            child_pgroup::prepare_command(&mut command, existing_pgrp, background);
             command
                 .spawn()
                 .map(|child| {
-                    foreground_pgroup::set(&child, existing_pgrp);
+                    child_pgroup::set(&child, existing_pgrp, background);
+
                     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()),
+                        interactive,
                     }
                 })
                 .inspect_err(|_e| {
-                    foreground_pgroup::reset();
+                    if interactive {
+                        child_pgroup::reset();
+                    }
                 })
         } else {
             command.spawn().map(|child| Self {
                 inner: child,
                 pipeline_state: None,
+                interactive,
             })
         }
     }
@@ -103,10 +117,9 @@ impl ForegroundChild {
                         }));
                     }
                     Ok(wait::WaitStatus::Stopped(_, _)) => {
-                        // TODO: consider only bringing our
-                        // process group back to foreground, if we are in an
-                        // interactive session
-                        foreground_pgroup::reset();
+                        if self.interactive {
+                            child_pgroup::reset();
+                        }
 
                         let handle = UnfreezeHandle { child_pid };
 
@@ -212,7 +225,10 @@ impl Drop for ForegroundChild {
         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()
+
+                if self.interactive {
+                    child_pgroup::reset()
+                }
             }
         }
     }
@@ -339,7 +355,7 @@ impl ForegroundGuard {
             if pcnt.fetch_sub(1, Ordering::SeqCst) == 1 {
                 // Clean up if we are the last one around
                 pgrp.store(0, Ordering::SeqCst);
-                foreground_pgroup::reset()
+                child_pgroup::reset()
             }
         }
     }
@@ -353,7 +369,7 @@ impl Drop for ForegroundGuard {
 
 // It's a simpler version of fish shell's external process handling.
 #[cfg(unix)]
-mod foreground_pgroup {
+mod child_pgroup {
     use nix::{
         sys::signal::{sigaction, SaFlags, SigAction, SigHandler, SigSet, Signal},
         unistd::{self, Pid},
@@ -377,7 +393,7 @@ mod foreground_pgroup {
         unsafe { BorrowedFd::borrow_raw(nix::libc::STDIN_FILENO) }
     }
 
-    pub fn prepare_command(external_command: &mut Command, existing_pgrp: u32) {
+    pub fn prepare_command(external_command: &mut Command, existing_pgrp: u32, background: bool) {
         unsafe {
             // Safety:
             // POSIX only allows async-signal-safe functions to be called.
@@ -391,19 +407,13 @@ mod foreground_pgroup {
                 // 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.
-                set_foreground_pid(Pid::this(), existing_pgrp);
+                set_foreground_pid(Pid::this(), existing_pgrp, background);
 
-                // Reset signal handlers for child, sync with `terminal.rs`
+                // `terminal.rs` makes the shell process ignore some signals,
+                //  so we set them to their default behavior for our child
                 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::SIGTTIN, &default);
-                // let _ = sigaction(Signal::SIGTTOU, &default);
-                // We do need to reset SIGTSTP though, since some TUI
-                // applications implement their own Ctrl-Z handling, and
-                // ForegroundChild::wait() needs to be able to react to the
-                // child being stopped.
                 let _ = sigaction(Signal::SIGTSTP, &default);
                 let _ = sigaction(Signal::SIGTERM, &default);
 
@@ -412,11 +422,15 @@ mod foreground_pgroup {
         }
     }
 
-    pub fn set(process: &Child, existing_pgrp: u32) {
-        set_foreground_pid(Pid::from_raw(process.id() as i32), existing_pgrp);
+    pub fn set(process: &Child, existing_pgrp: u32, background: bool) {
+        set_foreground_pid(
+            Pid::from_raw(process.id() as i32),
+            existing_pgrp,
+            background,
+        );
     }
 
-    pub fn set_foreground_pid(pid: Pid, existing_pgrp: u32) {
+    fn set_foreground_pid(pid: Pid, existing_pgrp: u32, background: bool) {
         // Safety: needs to be async-signal-safe.
         // `setpgid` and `tcsetpgrp` are async-signal-safe.
 
@@ -428,7 +442,10 @@ mod foreground_pgroup {
             Pid::from_raw(existing_pgrp as i32)
         };
         let _ = unistd::setpgid(pid, pgrp);
-        let _ = unistd::tcsetpgrp(unsafe { stdin_fd() }, pgrp);
+
+        if !background {
+            let _ = unistd::tcsetpgrp(unsafe { stdin_fd() }, pgrp);
+        }
     }
 
     /// Reset the foreground process group to the shell