Implement race-free job control

kernel/src/context.rs

@@ -49,9 +49,10 @@ pub struct CurrentUserSpace<'a>(Ref<'a, Option<Vmar<Full>>>);
 /// If you get the access to the [`Context`].
 #[macro_export]
 macro_rules! current_userspace {
-    () => {
+    () => {{
+        use crate::context::CurrentUserSpace;
         CurrentUserSpace::new(&ostd::task::Task::current().unwrap())
-    };
+    }};
 }
 
 impl<'a> CurrentUserSpace<'a> {

kernel/src/device/pty/pty.rs

@@ -183,10 +183,9 @@ impl FileIo for PtyMaster {
             | IoctlCmd::TCSETS
             | IoctlCmd::TIOCGPTN
             | IoctlCmd::TIOCGWINSZ
-            | IoctlCmd::TIOCSWINSZ => self.slave.ioctl(cmd, arg),
+            | IoctlCmd::TIOCSWINSZ => return self.slave.ioctl(cmd, arg),
             IoctlCmd::TIOCSPTLCK => {
                 // TODO: lock/unlock pty
-                Ok(0)
             }
             IoctlCmd::TIOCGPTPEER => {
                 let current_task = Task::current().unwrap();
@@ -217,46 +216,16 @@ impl FileIo for PtyMaster {
                     // TODO: deal with the O_CLOEXEC flag
                     file_table_locked.insert(slave, FdFlags::empty())
                 };
-                Ok(fd)
+                return Ok(fd);
-            }
-            IoctlCmd::TIOCGPGRP => {
-                let Some(foreground) = self.slave.foreground() else {
-                    return_errno_with_message!(
-                        Errno::ESRCH,
-                        "the foreground process group does not exist"
-                    );
-                };
-                let fg_pgid = foreground.pgid();
-                current_userspace!().write_val(arg, &fg_pgid)?;
-                Ok(0)
-            }
-            IoctlCmd::TIOCSPGRP => {
-                let pgid = {
-                    let pgid: i32 = current_userspace!().read_val(arg)?;
-                    if pgid < 0 {
-                        return_errno_with_message!(Errno::EINVAL, "negative pgid");
-                    }
-                    pgid as u32
-                };
-
-                self.slave.set_foreground(&pgid)?;
-                Ok(0)
-            }
-            IoctlCmd::TIOCSCTTY => {
-                self.slave.set_current_session()?;
-                Ok(0)
-            }
-            IoctlCmd::TIOCNOTTY => {
-                self.slave.release_current_session()?;
-                Ok(0)
             }
             IoctlCmd::FIONREAD => {
                 let len = self.input.lock().len() as i32;
                 current_userspace!().write_val(arg, &len)?;
-                Ok(0)
             }
-            _ => Ok(0),
+            _ => (self.slave.clone() as Arc<dyn Terminal>).job_ioctl(cmd, arg, true)?,
         }
+
+        Ok(0)
     }
 }
 
@@ -298,10 +267,6 @@ impl Device for PtySlave {
 }
 
 impl Terminal for PtySlave {
-    fn arc_self(&self) -> Arc<dyn Terminal> {
-        self.weak_self.upgrade().unwrap() as _
-    }
-
     fn job_control(&self) -> &JobControl {
         &self.job_control
     }
@@ -343,70 +308,31 @@ impl FileIo for PtySlave {
             IoctlCmd::TCGETS => {
                 let termios = self.ldisc.termios();
                 current_userspace!().write_val(arg, &termios)?;
-                Ok(0)
             }
             IoctlCmd::TCSETS => {
                 let termios = current_userspace!().read_val(arg)?;
                 self.ldisc.set_termios(termios);
-                Ok(0)
             }
             IoctlCmd::TIOCGPTN => {
                 let idx = self.index();
                 current_userspace!().write_val(arg, &idx)?;
-                Ok(0)
             }
             IoctlCmd::TIOCGWINSZ => {
                 let winsize = self.ldisc.window_size();
                 current_userspace!().write_val(arg, &winsize)?;
-                Ok(0)
             }
             IoctlCmd::TIOCSWINSZ => {
                 let winsize = current_userspace!().read_val(arg)?;
                 self.ldisc.set_window_size(winsize);
-                Ok(0)
-            }
-            IoctlCmd::TIOCGPGRP => {
-                if !self.is_controlling_terminal() {
-                    return_errno_with_message!(Errno::ENOTTY, "slave is not controlling terminal");
-                }
-
-                let Some(foreground) = self.foreground() else {
-                    return_errno_with_message!(
-                        Errno::ESRCH,
-                        "the foreground process group does not exist"
-                    );
-                };
-
-                let fg_pgid = foreground.pgid();
-                current_userspace!().write_val(arg, &fg_pgid)?;
-                Ok(0)
-            }
-            IoctlCmd::TIOCSPGRP => {
-                let pgid = {
-                    let pgid: i32 = current_userspace!().read_val(arg)?;
-                    if pgid < 0 {
-                        return_errno_with_message!(Errno::EINVAL, "negative pgid");
-                    }
-                    pgid as u32
-                };
-
-                self.set_foreground(&pgid)?;
-                Ok(0)
-            }
-            IoctlCmd::TIOCSCTTY => {
-                self.set_current_session()?;
-                Ok(0)
-            }
-            IoctlCmd::TIOCNOTTY => {
-                self.release_current_session()?;
-                Ok(0)
             }
             IoctlCmd::FIONREAD => {
                 let buffer_len = self.ldisc.buffer_len() as i32;
                 current_userspace!().write_val(arg, &buffer_len)?;
-                Ok(0)
             }
-            _ => Ok(0),
+            _ => (self.weak_self.upgrade().unwrap() as Arc<dyn Terminal>)
+                .job_ioctl(cmd, arg, false)?,
         }
+
+        Ok(0)
     }
 }

kernel/src/device/tty/device.rs

@@ -16,13 +16,10 @@ pub struct TtyDevice;
 
 impl Device for TtyDevice {
     fn open(&self) -> Result<Option<Arc<dyn FileIo>>> {
-        let current = current!();
+        let Some(terminal) = current!().terminal() else {
-        let session = current.session().unwrap();
-
-        let Some(terminal) = session.terminal() else {
             return_errno_with_message!(
                 Errno::ENOTTY,
-                "the session does not have controlling terminal"
+                "the process does not have a controlling terminal"
             );
         };
 

kernel/src/device/tty/mod.rs

@@ -103,43 +103,18 @@ impl FileIo for Tty {
                 let termios = self.ldisc.termios();
                 trace!("get termios = {:?}", termios);
                 current_userspace!().write_val(arg, &termios)?;
-                Ok(0)
-            }
-            IoctlCmd::TIOCGPGRP => {
-                let Some(foreground) = self.foreground() else {
-                    return_errno_with_message!(Errno::ESRCH, "No fg process group")
-                };
-                let fg_pgid = foreground.pgid();
-                debug!("fg_pgid = {}", fg_pgid);
-                current_userspace!().write_val(arg, &fg_pgid)?;
-                Ok(0)
-            }
-            IoctlCmd::TIOCSPGRP => {
-                // Set the process group id of fg progress group
-                let pgid = {
-                    let pgid: i32 = current_userspace!().read_val(arg)?;
-                    if pgid < 0 {
-                        return_errno_with_message!(Errno::EINVAL, "negative pgid");
-                    }
-                    pgid as u32
-                };
-
-                self.set_foreground(&pgid)?;
-                Ok(0)
             }
             IoctlCmd::TCSETS => {
                 // Set terminal attributes
                 let termios = current_userspace!().read_val(arg)?;
                 debug!("set termios = {:?}", termios);
                 self.ldisc.set_termios(termios);
-                Ok(0)
             }
             IoctlCmd::TCSETSW => {
                 let termios = current_userspace!().read_val(arg)?;
                 debug!("set termios = {:?}", termios);
                 self.ldisc.set_termios(termios);
                 // TODO: drain output buffer
-                Ok(0)
             }
             IoctlCmd::TCSETSF => {
                 let termios = current_userspace!().read_val(arg)?;
@@ -147,32 +122,24 @@ impl FileIo for Tty {
                 self.ldisc.set_termios(termios);
                 self.ldisc.drain_input();
                 // TODO: drain output buffer
-                Ok(0)
             }
             IoctlCmd::TIOCGWINSZ => {
                 let winsize = self.ldisc.window_size();
                 current_userspace!().write_val(arg, &winsize)?;
-                Ok(0)
             }
             IoctlCmd::TIOCSWINSZ => {
                 let winsize = current_userspace!().read_val(arg)?;
                 self.ldisc.set_window_size(winsize);
-                Ok(0)
             }
-            IoctlCmd::TIOCSCTTY => {
+            _ => (self.weak_self.upgrade().unwrap() as Arc<dyn Terminal>)
-                self.set_current_session()?;
+                .job_ioctl(cmd, arg, false)?,
-                Ok(0)
-            }
-            _ => todo!(),
         }
+
+        Ok(0)
     }
 }
 
 impl Terminal for Tty {
-    fn arc_self(&self) -> Arc<dyn Terminal> {
-        self.weak_self.upgrade().unwrap() as _
-    }
-
     fn job_control(&self) -> &JobControl {
         &self.job_control
     }

kernel/src/fs/utils/ioctl.rs

@@ -27,6 +27,8 @@ pub enum IoctlCmd {
     FIONBIO = 0x5421,
     /// the calling process gives up this controlling terminal
     TIOCNOTTY = 0x5422,
+    /// Return the session ID of FD
+    TIOCGSID = 0x5429,
     /// Clear the close on exec flag on a file descriptor
     FIONCLEX = 0x5450,
     /// Set the close on exec flag on a file descriptor

kernel/src/process/process/init_proc.rs

@@ -6,7 +6,6 @@ use ostd::{cpu::context::UserContext, task::Task, user::UserContextApi};
 
 use super::{Process, Terminal};
 use crate::{
-    device::tty::get_n_tty,
     fs::{
         fs_resolver::{FsPath, AT_FDCWD},
         thread_info::ThreadFsInfo,
@@ -37,7 +36,8 @@ pub fn spawn_init_process(
 
     set_session_and_group(&process);
 
-    open_ntty_as_controlling_terminal(&process)?;
+    // FIXME: This should be done by the userspace init process.
+    (crate::device::tty::get_n_tty().clone() as Arc<dyn Terminal>).set_control(&process)?;
 
     process.run();
 
@@ -127,20 +127,3 @@ fn create_init_task(
         .fs(Arc::new(fs));
     Ok(thread_builder.build())
 }
-
-/// Opens `N_TTY` as the controlling terminal for the process.
-fn open_ntty_as_controlling_terminal(process: &Process) -> Result<()> {
-    let tty = get_n_tty();
-
-    let session = &process.session().unwrap();
-    let process_group = process.process_group().unwrap();
-
-    session.set_terminal(|| {
-        tty.job_control().set_session(session);
-        Ok(tty.clone())
-    })?;
-
-    tty.job_control().set_foreground(Some(&process_group))?;
-
-    Ok(())
-}

kernel/src/process/process/job_control.rs

@@ -1,169 +1,155 @@
 // SPDX-License-Identifier: MPL-2.0
 
-#![expect(unused_variables)]
+use ostd::sync::{LocalIrqDisabled, WaitQueue};
 
-use ostd::sync::WaitQueue;
+use super::{ProcessGroup, Session};
+use crate::prelude::*;
 
-use crate::{
+/// The job control for terminals like TTY and PTY.
-    prelude::*,
-    process::{
-        signal::{
-            constants::{SIGCONT, SIGHUP},
-            signals::kernel::KernelSignal,
-        },
-        ProcessGroup, Session,
-    },
-};
-
-/// The job control for terminals like tty and pty.
 ///
-/// This struct is used to support shell job control, which allows users to
+/// This structure is used to support the shell job control, allowing users to
-/// run commands in the foreground or in the background. This struct manages
+/// run commands in the foreground or in the background. To achieve this, this
-/// the session and foreground process group for a terminal.
+/// structure internally manages the session and the foreground process group
+/// for a terminal.
 pub struct JobControl {
-    foreground: SpinLock<Weak<ProcessGroup>>,
+    inner: SpinLock<Inner, LocalIrqDisabled>,
-    session: SpinLock<Weak<Session>>,
     wait_queue: WaitQueue,
 }
 
+#[derive(Default)]
+struct Inner {
+    session: Weak<Session>,
+    foreground: Weak<ProcessGroup>,
+}
+
 impl JobControl {
-    /// Creates a new `TtyJobControl`
+    /// Creates a new `JobControl`.
     pub fn new() -> Self {
         Self {
-            foreground: SpinLock::new(Weak::new()),
+            inner: SpinLock::new(Inner::default()),
-            session: SpinLock::new(Weak::new()),
             wait_queue: WaitQueue::new(),
         }
     }
 
     // *************** Session ***************
 
-    /// Returns the session whose controlling terminal is the terminal.
+    /// Returns the session whose controlling terminal is this terminal.
-    fn session(&self) -> Option<Arc<Session>> {
+    pub(super) fn session(&self) -> Option<Arc<Session>> {
-        self.session.lock().upgrade()
+        self.inner.lock().session.upgrade()
     }
 
-    /// Sets the terminal as the controlling terminal of the `session`.
+    /// Sets the session whose controlling terminal is this terminal.
+    ///
+    /// # Errors
+    ///
+    /// This method will fail with `EPERM` if the terminal is already a controlling terminal of
+    /// another session.
     ///
     /// # Panics
     ///
-    /// This terminal should not belong to any session.
+    /// The caller needs to ensure that the foreground process group actually belongs to the
-    pub fn set_session(&self, session: &Arc<Session>) {
+    /// session. Otherwise, this method may panic.
-        debug_assert!(self.session().is_none());
+    pub(super) fn set_session(
-        *self.session.lock() = Arc::downgrade(session);
+        &self,
-    }
+        session: &Arc<Session>,
+        foreground: &Arc<ProcessGroup>,
+    ) -> Result<()> {
+        let mut inner = self.inner.lock();
 
-    /// Sets the terminal as the controlling terminal of the session of current process.
+        if inner.session.upgrade().is_some() {
-    ///
-    /// # Panics
-    ///
-    /// This function should only be called in process context.
-    pub fn set_current_session(&self) -> Result<()> {
-        if self.session().is_some() {
             return_errno_with_message!(
                 Errno::EPERM,
-                "the terminal is already controlling terminal of another session"
+                "the terminal is already a controlling terminal of another session"
             );
         }
 
-        let current = current!();
+        *inner = Inner {
+            session: Arc::downgrade(session),
+            foreground: Arc::downgrade(foreground),
+        };
 
-        let process_group = current.process_group().unwrap();
-        *self.foreground.lock() = Arc::downgrade(&process_group);
-
-        let session = current.session().unwrap();
-        *self.session.lock() = Arc::downgrade(&session);
-
-        self.wait_queue.wake_all();
         Ok(())
     }
 
-    /// Releases the current session from this terminal.
+    /// Unsets the session because its controlling terminal is no longer this terminal.
-    pub fn release_current_session(&self) -> Result<()> {
+    ///
-        let Some(session) = self.session() else {
+    /// This method will return the foreground process group before the session is cleared.
-            return_errno_with_message!(
+    ///
-                Errno::ENOTTY,
+    /// # Panics
-                "the terminal is not controlling terminal now"
+    ///
-            );
+    /// The caller needs to ensure that the session was previously set. Otherwise this method may
-        };
+    /// panic.
+    pub(super) fn unset_session(&self) -> Option<Arc<ProcessGroup>> {
+        let mut inner = self.inner.lock();
 
-        if let Some(foreground) = self.foreground() {
+        debug_assert!(inner.session.upgrade().is_some());
-            foreground.broadcast_signal(KernelSignal::new(SIGHUP));
-            foreground.broadcast_signal(KernelSignal::new(SIGCONT));
-        }
 
-        Ok(())
+        let foreground = inner.foreground.upgrade();
+        *inner = Inner::default();
+
+        foreground
     }
 
     // *************** Foreground process group ***************
 
-    /// Returns the foreground process group
+    /// Returns the foreground process group.
     pub fn foreground(&self) -> Option<Arc<ProcessGroup>> {
-        self.foreground.lock().upgrade()
+        self.inner.lock().foreground.upgrade()
     }
 
     /// Sets the foreground process group.
     ///
     /// # Panics
     ///
-    /// The process group should belong to one session.
+    /// The caller needs to ensure that foreground process group actually belongs to the session
-    pub fn set_foreground(&self, process_group: Option<&Arc<ProcessGroup>>) -> Result<()> {
+    /// whose controlling terminal is this terminal. Otherwise this method may panic.
-        let Some(process_group) = process_group else {
+    pub(super) fn set_foreground(&self, process_group: &Arc<ProcessGroup>) {
-            // FIXME: should we allow this branch?
+        let mut inner = self.inner.lock();
-            *self.foreground.lock() = Weak::new();
-            return Ok(());
-        };
 
-        let session = process_group.session().unwrap();
+        debug_assert!(Arc::ptr_eq(
-        let Some(terminal_session) = self.session() else {
+            &process_group.session().unwrap(),
-            return_errno_with_message!(
+            &inner.session.upgrade().unwrap()
-                Errno::EPERM,
+        ));
-                "the terminal does not become controlling terminal of one session."
+        inner.foreground = Arc::downgrade(process_group);
-            );
-        };
 
-        if !Arc::ptr_eq(&terminal_session, &session) {
-            return_errno_with_message!(
-                Errno::EPERM,
-                "the process proup belongs to different session"
-            );
-        }
-
-        *self.foreground.lock() = Arc::downgrade(process_group);
         self.wait_queue.wake_all();
-        Ok(())
     }
 
-    /// Wait until the current process is the foreground process group. If
+    /// Waits until the current process is in the foreground process group.
-    /// the foreground process group is None, returns true.
+    ///
+    /// Note that we only wait if the terminal is the our controlling terminal. If it isn't, the
+    /// method returns immediately without an error. This should match the Linux behavior where the
+    /// SIGTTIN won't be sent if we're reading the terminal that is the controlling terminal of
+    /// another session.
     ///
     /// # Panics
     ///
-    /// This function should only be called in process context.
+    /// This method will panic if it is not called in the process context.
     pub fn wait_until_in_foreground(&self) -> Result<()> {
-        // Fast path
+        let current = current!();
-        if self.current_belongs_to_foreground() {
-            return Ok(());
-        }
 
-        // Slow path
         self.wait_queue.pause_until(|| {
-            if self.current_belongs_to_foreground() {
+            let process_group_mut = current.process_group.lock();
-                Some(())
+            let process_group = process_group_mut.upgrade().unwrap();
-            } else {
+            let session = process_group.session().unwrap();
-                None
+
+            let inner = self.inner.lock();
+            if !inner
+                .session
+                .upgrade()
+                .is_some_and(|terminal_session| Arc::ptr_eq(&terminal_session, &session))
+            {
+                // The terminal is not our controlling terminal. Don't wait.
+                return Some(());
             }
+
+            inner
+                .foreground
+                .upgrade()
+                .is_some_and(|terminal_foregroup| Arc::ptr_eq(&terminal_foregroup, &process_group))
+                .then_some(())
         })
     }
-
-    fn current_belongs_to_foreground(&self) -> bool {
-        let Some(foreground) = self.foreground() else {
-            return true;
-        };
-
-        let foreground_inner = foreground.lock();
-        foreground_inner.contains_process(&current!().pid())
-    }
 }
 
 impl Default for JobControl {

kernel/src/process/process/mod.rs

@@ -295,47 +295,55 @@ impl Process {
 
     /// Returns the process group ID of the process.
     //
-    // FIXME: If we call this method without holding the process table lock, it may return zero if
+    // FIXME: If we call this method on a non-current process without holding the process table
-    // the process is reaped at the same time.
+    // lock, it may return zero if the process is reaped at the same time.
     pub fn pgid(&self) -> Pgid {
-        self.process_group().map_or(0, |group| group.pgid())
+        self.process_group
+            .lock()
+            .upgrade()
+            .map_or(0, |group| group.pgid())
     }
 
     /// Returns the session ID of the process.
     //
-    // FIXME: If we call this method without holding the process table lock, it may return zero if
+    // FIXME: If we call this method on a non-current process without holding the process table
-    // the process is reaped at the same time.
+    // lock, it may return zero if the process is reaped at the same time.
     pub fn sid(&self) -> Sid {
-        self.session().map_or(0, |session| session.sid())
-    }
-
-    /// Returns the process group to which the process belongs.
-    ///
-    /// **Deprecated:** This is a very poorly designed API. Almost every use of this API is wrong
-    /// because some race condition is not handled correctly. Such usages are being refactored and
-    /// this API will be removed soon. So DO NOT ATTEMPT TO USE THIS API IN NEW CODE unless you
-    /// know exactly what you're doing.
-    pub fn process_group(&self) -> Option<Arc<ProcessGroup>> {
-        self.process_group.lock().upgrade()
-    }
-
-    /// Returns the session to which the process belongs.
-    ///
-    /// **Deprecated:** This is a very poorly designed API. Almost every use of this API is wrong
-    /// because some race condition is not handled correctly. Such usages are being refactored and
-    /// this API will be removed soon. So DO NOT ATTEMPT TO USE THIS API IN NEW CODE unless you
-    /// know exactly what you're doing.
-    pub fn session(&self) -> Option<Arc<Session>> {
         self.process_group
             .lock()
             .upgrade()
             .and_then(|group| group.session())
+            .map_or(0, |session| session.sid())
     }
 
-    /// Returns whether the process is session leader.
+    /// Returns the process group and the session if the process is a session leader.
-    pub fn is_session_leader(&self) -> bool {
+    ///
-        self.session()
+    /// Note that once a process becomes a session leader, it remains a session leader forever.
-            .is_some_and(|session| session.sid() == self.pid)
+    /// Specifically, it can no longer be moved to a new process group or a new session. So this
+    /// method is race-free.
+    //
+    // FIXME: If we call this method on a non-current process without holding the process table
+    // lock, the session may be dead if the process is reaped at the same time.
+    fn leading_session(&self) -> Option<(Arc<ProcessGroup>, Arc<Session>)> {
+        let process_group_mut = self.process_group.lock();
+
+        let process_group = process_group_mut.upgrade().unwrap();
+        let session = process_group.session().unwrap();
+
+        if session.is_leader(self) {
+            Some((process_group, session))
+        } else {
+            None
+        }
+    }
+
+    /// Returns the controlling terminal of the process, if any.
+    pub fn terminal(&self) -> Option<Arc<dyn Terminal>> {
+        self.process_group
+            .lock()
+            .upgrade()
+            .and_then(|group| group.session())
+            .and_then(|session| session.lock().terminal().cloned())
     }
 
     /// Moves the process to the new session.

kernel/src/process/process/process_group.rs

@@ -88,11 +88,6 @@ impl ProcessGroupGuard<'_> {
         }
     }
 
-    /// Returns whether the process group contains the process.
-    pub(super) fn contains_process(&self, pid: &Pid) -> bool {
-        self.inner.processes.contains_key(pid)
-    }
-
     /// Inserts a process into the process group.
     ///
     /// The caller needs to ensure that the process didn't previously belong to the process group,

kernel/src/process/process/session.rs

@@ -64,60 +64,17 @@ impl Session {
         self.sid
     }
 
+    /// Returns whether the process is the session leader.
+    pub(super) fn is_leader(&self, process: &Process) -> bool {
+        self.sid == process.pid()
+    }
+
     /// Acquires a lock on the session.
     pub fn lock(&self) -> SessionGuard {
         SessionGuard {
             inner: self.inner.lock(),
         }
     }
-
-    /// Sets terminal as the controlling terminal of the session. The `get_terminal` method
-    /// should set the session for the terminal and returns the session.
-    ///
-    /// If the session already has controlling terminal, this method will return `Err(EPERM)`.
-    pub fn set_terminal<F>(&self, get_terminal: F) -> Result<()>
-    where
-        F: Fn() -> Result<Arc<dyn Terminal>>,
-    {
-        let mut inner = self.inner.lock();
-
-        if inner.terminal.is_some() {
-            return_errno_with_message!(
-                Errno::EPERM,
-                "current session already has controlling terminal"
-            );
-        }
-
-        let terminal = get_terminal()?;
-        inner.terminal = Some(terminal);
-        Ok(())
-    }
-
-    /// Releases the controlling terminal of the session.
-    ///
-    /// If the session does not have controlling terminal, this method will return `ENOTTY`.
-    pub fn release_terminal<F>(&self, release_session: F) -> Result<()>
-    where
-        F: Fn(&Arc<dyn Terminal>) -> Result<()>,
-    {
-        let mut inner = self.inner.lock();
-        if inner.terminal.is_none() {
-            return_errno_with_message!(
-                Errno::ENOTTY,
-                "current session does not has controlling terminal"
-            );
-        }
-
-        let terminal = inner.terminal.as_ref().unwrap();
-        release_session(terminal)?;
-        inner.terminal = None;
-        Ok(())
-    }
-
-    /// Returns the controlling terminal of `self`.
-    pub fn terminal(&self) -> Option<Arc<dyn Terminal>> {
-        self.inner.lock().terminal.clone()
-    }
 }
 
 /// A scoped lock guard for a session.
@@ -130,6 +87,19 @@ pub struct SessionGuard<'a> {
 }
 
 impl SessionGuard<'_> {
+    /// Sets the controlling terminal of the session.
+    ///
+    /// The caller needs to ensure that the job control of the old and new controlling terminals
+    /// are properly updated and synchronized.
+    pub(super) fn set_terminal(&mut self, terminal: Option<Arc<dyn Terminal>>) {
+        self.inner.terminal = terminal;
+    }
+
+    /// Returns the controlling terminal of the session.
+    pub fn terminal(&self) -> Option<&Arc<dyn Terminal>> {
+        self.inner.terminal.as_ref()
+    }
+
     /// Inserts a process group into the session.
     ///
     /// The caller needs to ensure that the process group didn't previously belong to the session,

kernel/src/process/process/terminal.rs

@@ -1,107 +1,193 @@
 // SPDX-License-Identifier: MPL-2.0
 
-use super::JobControl;
+use alloc::sync::Arc;
+
+use super::{session::SessionGuard, JobControl, Pgid, Process, Session, Sid};
 use crate::{
-    fs::inode_handle::FileIo,
+    current_userspace,
-    prelude::*,
+    fs::{inode_handle::FileIo, utils::IoctlCmd},
-    process::{process_table, Pgid, ProcessGroup},
+    prelude::{current, return_errno_with_message, warn, Errno, Error, Result},
+    process::process_table,
 };
 
-/// A terminal is used to interact with system. A terminal can support the shell
+/// A terminal.
-/// job control.
 ///
-/// We currently support two kinds of terminal, the tty and pty.
+/// We currently support two kinds of terminal, the TTY and PTY. They're associated with a
+/// `JobControl` to track the session and the foreground process group.
 pub trait Terminal: FileIo {
-    // *************** Foreground ***************
-
-    /// Returns the foreground process group
-    fn foreground(&self) -> Option<Arc<ProcessGroup>> {
-        self.job_control().foreground()
-    }
-
-    /// Sets the foreground process group of this terminal.
-    ///
-    /// If the terminal is not controlling terminal, this method returns `ENOTTY`.
-    ///
-    /// # Panics
-    ///
-    /// This method should be called in process context.
-    fn set_foreground(&self, pgid: &Pgid) -> Result<()> {
-        if !self.is_controlling_terminal() {
-            return_errno_with_message!(Errno::ENOTTY, "self is not controlling terminal");
-        }
-
-        let foreground = process_table::get_process_group(pgid);
-
-        self.job_control().set_foreground(foreground.as_ref())
-    }
-
-    // *************** Session and controlling terminal ***************
-
-    /// Returns whether the terminal is the controlling terminal of current process.
-    ///
-    /// # Panics
-    ///
-    /// This method should be called in process context.
-    fn is_controlling_terminal(&self) -> bool {
-        let session = current!().session().unwrap();
-        let Some(terminal) = session.terminal() else {
-            return false;
-        };
-
-        let arc_self = self.arc_self();
-        Arc::ptr_eq(&terminal, &arc_self)
-    }
-
-    /// Sets the terminal as the controlling terminal of the session of current process.
-    ///
-    /// If self is not session leader, or the terminal is controlling terminal of other session,
-    /// or the session already has controlling terminal, this method returns `EPERM`.
-    ///
-    /// # Panics
-    ///
-    /// This method should only be called in process context.
-    fn set_current_session(&self) -> Result<()> {
-        if !current!().is_session_leader() {
-            return_errno_with_message!(Errno::EPERM, "current process is not session leader");
-        }
-
-        let get_terminal = || {
-            self.job_control().set_current_session()?;
-            Ok(self.arc_self())
-        };
-
-        let session = current!().session().unwrap();
-        session.set_terminal(get_terminal)
-    }
-
-    /// Releases the terminal from the session of current process if the terminal is the controlling
-    /// terminal of the session.
-    ///
-    /// If the terminal is not the controlling terminal of the session, this method will return `ENOTTY`.
-    ///
-    /// # Panics
-    ///
-    /// This method should only be called in process context.
-    fn release_current_session(&self) -> Result<()> {
-        if !self.is_controlling_terminal() {
-            return_errno_with_message!(Errno::ENOTTY, "release wrong tty");
-        }
-
-        let current = current!();
-        if !current.is_session_leader() {
-            warn!("TODO: release tty for process that is not session leader");
-            return Ok(());
-        }
-
-        let release_session = |_: &Arc<dyn Terminal>| self.job_control().release_current_session();
-
-        let session = current.session().unwrap();
-        session.release_terminal(release_session)
-    }
-
     /// Returns the job control of the terminal.
     fn job_control(&self) -> &JobControl;
-
+}
-    fn arc_self(&self) -> Arc<dyn Terminal>;
+
+impl dyn Terminal {
+    pub fn job_ioctl(self: Arc<Self>, cmd: IoctlCmd, arg: usize, via_master: bool) -> Result<()> {
+        match cmd {
+            // Commands about foreground process groups
+            IoctlCmd::TIOCSPGRP => {
+                let pgid = current_userspace!().read_val::<Pgid>(arg)?;
+                if pgid.cast_signed() < 0 {
+                    return_errno_with_message!(Errno::EINVAL, "negative PGIDs are not valid");
+                }
+
+                self.set_foreground(pgid, &current!())
+            }
+            IoctlCmd::TIOCGPGRP => {
+                let operate = || {
+                    self.job_control()
+                        .foreground()
+                        .map_or(0, |foreground| foreground.pgid())
+                };
+
+                let pgid = if via_master {
+                    operate()
+                } else {
+                    self.is_control_and(&current!(), |_, _| Ok(operate()))?
+                };
+
+                current_userspace!().write_val::<Pgid>(arg, &pgid)
+            }
+
+            // Commands about sessions
+            IoctlCmd::TIOCSCTTY => {
+                if arg == 1 {
+                    warn!("stealing TTY from another session is not supported");
+                }
+
+                self.set_control(&current!())
+            }
+            IoctlCmd::TIOCNOTTY => {
+                if via_master {
+                    return_errno_with_message!(
+                        Errno::ENOTTY,
+                        "the terminal to operate is not our controlling terminal"
+                    );
+                }
+
+                self.unset_control(&current!())
+            }
+            IoctlCmd::TIOCGSID => {
+                let sid = if via_master {
+                    self.job_control()
+                        .session()
+                        .ok_or_else(|| {
+                            Error::with_message(
+                                Errno::ENOTTY,
+                                "the terminal is not a controlling termainal of any session",
+                            )
+                        })?
+                        .sid()
+                } else {
+                    self.is_control_and(&current!(), |session, _| Ok(session.sid()))?
+                };
+
+                current_userspace!().write_val::<Sid>(arg, &sid)
+            }
+
+            // Commands that are invalid or not supported
+            _ => {
+                return_errno_with_message!(Errno::EINVAL, "the `ioctl` command is invalid")
+            }
+        }
+    }
+
+    /// Sets the terminal to be the controlling terminal of the process.
+    pub(super) fn set_control(self: Arc<Self>, process: &Process) -> Result<()> {
+        let Some((process_group, session)) = process.leading_session() else {
+            return_errno_with_message!(
+                Errno::EPERM,
+                "the process who sets the controlling terminal is not a session leader"
+            );
+        };
+
+        let mut session_inner = session.lock();
+
+        if let Some(session_terminal) = session_inner.terminal() {
+            if Arc::ptr_eq(&session_terminal, &self) {
+                return Ok(());
+            }
+            return_errno_with_message!(
+                Errno::EPERM,
+                "the session already has a controlling terminal"
+            );
+        }
+
+        self.job_control().set_session(&session, &process_group)?;
+        session_inner.set_terminal(Some(self));
+
+        Ok(())
+    }
+
+    /// Unsets the terminal from the controlling terminal of the process.
+    fn unset_control(self: Arc<Self>, process: &Process) -> Result<()> {
+        self.is_control_and(process, |_, session_inner| {
+            session_inner.set_terminal(None);
+            if let Some(foreground) = self.job_control().unset_session() {
+                use crate::process::signal::{
+                    constants::{SIGCONT, SIGHUP},
+                    signals::kernel::KernelSignal,
+                };
+                foreground.broadcast_signal(KernelSignal::new(SIGHUP));
+                foreground.broadcast_signal(KernelSignal::new(SIGCONT));
+            }
+
+            Ok(())
+        })
+    }
+
+    /// Sets the foreground process group of the terminal.
+    fn set_foreground(self: Arc<Self>, pgid: Pgid, process: &Process) -> Result<()> {
+        // Take this lock before calling `is_control_and` to avoid dead locks.
+        let group_table_mut = process_table::group_table_mut();
+
+        self.is_control_and(process, |session, _| {
+            let Some(process_group) = group_table_mut.get(&pgid) else {
+                return_errno_with_message!(
+                    Errno::ESRCH,
+                    "the process group to be foreground does not exist"
+                );
+            };
+
+            if !Arc::ptr_eq(session, &process_group.session().unwrap()) {
+                return_errno_with_message!(
+                    Errno::EPERM,
+                    "the process group to be foreground belongs to a different session"
+                );
+            }
+
+            self.job_control().set_foreground(process_group);
+
+            Ok(())
+        })
+    }
+
+    /// Runs `op` when the process controls the terminal.
+    ///
+    /// Note that this requires that _both_ of the following two conditions are met:
+    /// * the process is a session leader, and
+    /// * the terminal is the controlling terminal of the session.
+    fn is_control_and<F, R>(self: &Arc<Self>, process: &Process, op: F) -> Result<R>
+    where
+        F: FnOnce(&Arc<Session>, &mut SessionGuard) -> Result<R>,
+    {
+        let Some((_, session)) = process.leading_session() else {
+            return_errno_with_message!(
+                Errno::ENOTTY,
+                "the process who operates the terminal is not a session leader"
+            );
+        };
+
+        let mut session_inner = session.lock();
+
+        if !session_inner
+            .terminal()
+            .is_some_and(|session_terminal| Arc::ptr_eq(session_terminal, &self))
+        {
+            return_errno_with_message!(
+                Errno::ENOTTY,
+                "the terminal to operate is not our controlling terminal"
+            );
+        }
+
+        op(&session, &mut session_inner)
+    }
 }

test/syscall_test/blocklists/pty_test

@@ -24,5 +24,9 @@ PtyTest.SwitchTwiceMultiline
 JobControlTest.SetTTYBadArg
 JobControlTest.SetTTYDifferentSession
 JobControlTest.ReleaseTTYSignals
+# FIXME: `ReleaseTTYNonLeader` does pass in Linux, but is disabled because we
+# cannot mimic the exact Linux behavior due to some design differences.
+# See <https://github.com/asterinas/asterinas/pull/2066#issuecomment-2865177068>.
+JobControlTest.ReleaseTTYNonLeader
 JobControlTest.SetForegroundProcessGroup
-JobControlTest.SetForegroundProcessGroupEmptyProcessGroup
+JobControlTest.SetForegroundProcessGroupEmptyProcessGroup