Ensure the shell is in the foreground when job control is enabled

This commit implements part of the initialization of the job-control
shell that is specified in POSIX.1-2024. We do it slightly differently
from the standard due to the reason described in the comment for the
Env::ensure_foreground method.

This commit also ensures that the shell is in the foreground when job
control is enabled after the shell starts.

Fixes #481
Fixes #421

This commit also updates the test harness to use `tcgetsid` instead of
`tcgetpgrp` to check the session ID of the terminal. It is unclear
whether the current process becomes the foreground process group when it
obtains the controlling terminal, so we should not rely on the initial
foreground process group being the same as the shell's process group,
just in case.

Author: magicant

yash-builtin/CHANGELOG.md

@@ -23,6 +23,9 @@ specifying a delimiter character to terminate the input.
 The `read` built-in now returns a more specific exit status depending on the
 cause of the error. It also rejects an input containing a null byte.
 
+The `set` bulit-in now suspends itself when the `-m` option is enabled in the
+background.
+
 The `trap` built-in now implements the POSIX.1-2024 behavior of showing signal
 dispositions that are not explicitly set by the user. It also supports the `-p`
 (`--print`) option.
@@ -93,6 +96,8 @@ The `wait` built-in no longer treats suspended jobs as terminated jobs.
 - The `read::main` function now returns a more specific exit status depending on
   the cause of the error. It now returns `EXIT_STATUS_READ_ERROR` when finding a
   null byte in the input.
+- The `set::main` function now internally calls `yash_env::Env::ensure_foreground`
+  when the `-m` option is enabled.
 - The `trap::syntax::interpret` function now supports the `-p` option.
 - The output of the `trap` built-in now includes not only user-defined traps but
   also signal dispositions that are not explicitly set by the user.

yash-builtin/src/set.rs

@@ -166,13 +166,9 @@ pub mod syntax;
 // TODO pub mod semantics;
 
 /// Enables or disables the internal dispositions for the "stopper" signals
-/// depending on the `Interactive` and `Monitor` option states. The dispositions
-/// are disabled in subshells.
+/// depending on the `Interactive` and `Monitor` option states.
 fn update_internal_dispositions_for_stoppers(env: &mut Env) {
-    if env.options.get(Interactive) == State::On
-        && env.options.get(Monitor) == State::On
-        && !env.stack.contains(&Subshell)
-    {
+    if env.options.get(Interactive) == State::On && env.options.get(Monitor) == State::On {
         env.traps
             .enable_internal_dispositions_for_stoppers(&mut env.system)
     } else {
@@ -182,17 +178,34 @@ fn update_internal_dispositions_for_stoppers(env: &mut Env) {
     .ok();
 }
 
+/// Ensures that the shell is in the foreground process group if the `Monitor`
+/// option is enabled.
+fn ensure_foreground(env: &mut Env) {
+    if env.options.get(Monitor) == State::On {
+        env.ensure_foreground().ok();
+    }
+}
+
 /// Modifies shell options and positional parameters.
 fn modify(
     env: &mut Env,
     options: Vec<(yash_env::option::Option, State)>,
     positional_params: Option<Vec<Field>>,
 ) {
     // Modify options
+    let mut monitor_changed = false;
     for (option, state) in options {
         env.options.set(option, state);
+        monitor_changed |= option == Monitor;
+    }
+
+    // Reinitialize job control
+    if monitor_changed && !env.stack.contains(&Subshell) {
+        // We ignore errors in theses functions because they are not essential
+        // for updating the options.
+        update_internal_dispositions_for_stoppers(env);
+        ensure_foreground(env);
     }
-    update_internal_dispositions_for_stoppers(env);
 
     // Modify positional parameters
     if let Some(fields) = positional_params {
@@ -472,4 +485,8 @@ xtrace           off
         let disposition = state.processes[&env.main_pid].disposition(SIGTSTP);
         assert_eq!(disposition, Disposition::Default);
     }
+
+    // TODO Test the case when the -m option is enabled while the shell is not
+    // in the foreground. This requires the correct implementation of the
+    // `VirtualSystem::tcsetpgrp` method.
 }

yash-cli/CHANGELOG.md

@@ -29,6 +29,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   the next command line. This behavior basically conforms to POSIX.1-2024, but
   differs in that the shell does not resume with the remaining commands
   following the next asynchronous and-or list.
+- When the shell starts job control, if it is in the background, the shell now
+  suspends itself until it is resumed in the foreground. Previously, the shell
+  would continue running in the background, interfering with the foreground
+  process group.
 - If job control is enabled and the shell does not have a controlling terminal,
   the shell now proceeds without managing foreground-ness of process groups.
   Jobs are still assigned to their own process groups. Previously, the shell

yash-cli/src/startup.rs

@@ -86,6 +86,12 @@ pub fn configure_environment(env: &mut Env, run: Run) -> Work {
         }
     }
 
+    // Make sure the shell is in the foreground if job control is enabled
+    if env.options.get(Monitor) == On {
+        // Ignore failures as we can still proceed even if we can't get into the foreground
+        env.ensure_foreground().ok();
+    }
+
     // Prepare built-ins
     env.builtins.extend(BUILTINS.iter().cloned());
 

yash-cli/tests/pty/mod.rs

@@ -26,7 +26,8 @@ use nix::fcntl::{OFlag, open};
 use nix::libc;
 use nix::pty::{PtyMaster, grantpt, posix_openpt, ptsname, unlockpt};
 use nix::sys::stat::Mode;
-use nix::unistd::{close, getpgrp, setsid, tcgetpgrp};
+use nix::sys::termios::tcgetsid;
+use nix::unistd::{close, getpid, setsid};
 use std::ffi::c_int;
 use std::os::fd::{AsRawFd as _, BorrowedFd, FromRawFd as _, OwnedFd};
 use std::path::{Path, PathBuf};
@@ -94,7 +95,7 @@ fn prepare_as_slave(slave_path: &Path) -> nix::Result<()> {
     unsafe { libc::ioctl(raw_fd, libc::TIOCSCTTY as _, 0 as c_int) };
 
     let fd = unsafe { BorrowedFd::borrow_raw(raw_fd) };
-    if tcgetpgrp(fd) == Ok(getpgrp()) {
+    if tcgetsid(fd) == Ok(getpid()) {
         Ok(())
     } else {
         Err(nix::Error::ENOTSUP)

yash-env/CHANGELOG.md

@@ -15,6 +15,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
       It can be used to store arbitrary data.
 - The `Env` struct now has the `wait_for_subshell_to_halt` method.
     - This method waits for the subshell to terminate or stop.
+- The `Env` struct now has the `ensure_foreground` method.
+    - This method ensures that the shell is in the foreground.
 - The `System` trait now has the `get_sigaction` method.
     - This method returns the current signal handling configuration for a signal.
       This method does not modify anything, so it can be used with an immutable

yash-env/src/lib.rs

@@ -327,6 +327,36 @@ impl Env {
         final_fd
     }
 
+    /// Ensures the shell is in the foreground.
+    ///
+    /// If the current process belongs to the same process group as the session
+    /// leader, this function forces the current process to be in the foreground
+    /// by calling [`SystemEx::tcsetpgrp_with_block`]. Otherwise, this function
+    /// suspends the process until it is resumed in the foreground by another
+    /// job-controlling process (see [`SystemEx::tcsetpgrp_without_block`]).
+    ///
+    /// This function returns an error if the process does not have a controlling
+    /// terminal, that is, [`get_tty`](Self::get_tty) returns `Err(_)`.
+    ///
+    /// # Note on POSIX conformance
+    ///
+    /// This function implements part of the initialization of the job-control
+    /// shell. POSIX says that the shell should bring itself into the foreground
+    /// only if it is started as the controlling process (that is, the session
+    /// leader) for the terminal session. However, this function also brings the
+    /// shell into the foreground if the shell is in the same process group as
+    /// the session leader because it is unlikely that there is another
+    /// job-controlling process that can bring the shell into the foreground.
+    pub fn ensure_foreground(&mut self) -> Result<(), Errno> {
+        let fd = self.get_tty()?;
+
+        if self.system.tcgetpgrp(fd)? == self.main_pgid {
+            self.system.tcsetpgrp_with_block(fd, self.main_pgid)
+        } else {
+            self.system.tcsetpgrp_without_block(fd, self.main_pgid)
+        }
+    }
+
     /// Tests whether the current environment is an interactive shell.
     ///
     /// This function returns true if and only if: