Refactor session & tty code

services/libs/jinux-std/src/device/pty/pty.rs

@@ -2,6 +2,7 @@ use alloc::format;
 use ringbuf::{ring_buffer::RbBase, HeapRb, Rb};
 
 use crate::device::tty::line_discipline::LineDiscipline;
+use crate::device::tty::new_job_control_and_ldisc;
 use crate::events::IoEvents;
 use crate::fs::device::{Device, DeviceId, DeviceType};
 use crate::fs::devpts::DevPts;
@@ -9,7 +10,6 @@ use crate::fs::fs_resolver::FsPath;
 use crate::fs::inode_handle::FileIo;
 use crate::fs::utils::{AccessMode, Inode, InodeMode, IoctlCmd};
 use crate::prelude::*;
-use crate::process::signal::signals::kernel::KernelSignal;
 use crate::process::signal::{Pollee, Poller};
 use crate::process::{JobControl, Terminal};
 use crate::util::{read_val_from_user, write_val_to_user};
@@ -25,7 +25,7 @@ pub struct PtyMaster {
     index: u32,
     output: Arc<LineDiscipline>,
     input: SpinLock<HeapRb<u8>>,
-    job_control: JobControl,
+    job_control: Arc<JobControl>,
     /// The state of input buffer
     pollee: Pollee,
     weak_self: Weak<Self>,
@@ -33,12 +33,13 @@ pub struct PtyMaster {
 
 impl PtyMaster {
     pub fn new(ptmx: Arc<dyn Inode>, index: u32) -> Arc<Self> {
-        Arc::new_cyclic(|weak_ref| PtyMaster {
+        let (job_control, ldisc) = new_job_control_and_ldisc();
+        Arc::new_cyclic(move |weak_ref| PtyMaster {
             ptmx,
             index,
-            output: LineDiscipline::new(),
+            output: ldisc,
             input: SpinLock::new(HeapRb::new(BUFFER_CAPACITY)),
-            job_control: JobControl::new(),
+            job_control,
             pollee: Pollee::new(IoEvents::OUT),
             weak_self: weak_ref.clone(),
         })
@@ -123,26 +124,13 @@ impl FileIo for PtyMaster {
     }
 
     fn write(&self, buf: &[u8]) -> Result<usize> {
-        let may_send_signal = || {
-            let Some(foreground) = self.foreground() else {
-                return None;
-            };
-
-            let send_signal = move |signal: KernelSignal| {
-                foreground.kernel_signal(signal);
-            };
-
-            Some(Arc::new(send_signal) as Arc<dyn Fn(KernelSignal) + Send + Sync>)
-        };
-
         let mut input = self.input.lock();
         for character in buf {
-            self.output
+            self.output.push_char(*character, |content| {
-                .push_char(*character, may_send_signal, |content| {
+                for byte in content.as_bytes() {
-                    for byte in content.as_bytes() {
+                    input.push_overwrite(*byte);
-                        input.push_overwrite(*byte);
+                }
-                    }
+            });
-                });
         }
 
         self.update_state(&input);
@@ -332,9 +320,8 @@ impl Terminal for PtySlave {
 
 impl FileIo for PtySlave {
     fn read(&self, buf: &mut [u8]) -> Result<usize> {
-        self.master()
+        self.job_control.wait_until_in_foreground()?;
-            .output
+        self.master().output.read(buf)
-            .read(buf, || self.job_control.current_belongs_to_foreground())
     }
 
     fn write(&self, buf: &[u8]) -> Result<usize> {

services/libs/jinux-std/src/device/tty/device.rs

@@ -34,14 +34,14 @@ impl Device for TtyDevice {
 
 impl FileIo for TtyDevice {
     fn read(&self, buf: &mut [u8]) -> Result<usize> {
-        unreachable!()
+        return_errno_with_message!(Errno::EINVAL, "cannot read tty device");
     }
 
     fn write(&self, buf: &[u8]) -> Result<usize> {
-        unreachable!()
+        return_errno_with_message!(Errno::EINVAL, "cannot write tty device");
     }
 
     fn poll(&self, mask: IoEvents, poller: Option<&Poller>) -> IoEvents {
-        unreachable!()
+        IoEvents::empty()
     }
 }

services/libs/jinux-std/src/device/tty/line_discipline.rs

@@ -16,6 +16,8 @@ use super::termio::{KernelTermios, WinSize, CC_C_CHAR};
 
 const BUFFER_CAPACITY: usize = 4096;
 
+pub type LdiscSignalSender = Arc<dyn Fn(KernelSignal) + Send + Sync + 'static>;
+
 pub struct LineDiscipline {
     /// current line
     current_line: SpinLock<CurrentLine>,
@@ -27,6 +29,8 @@ pub struct LineDiscipline {
     winsize: SpinLock<WinSize>,
     /// Pollee
     pollee: Pollee,
+    /// Used to send signal for foreground processes, when some char comes.
+    send_signal: LdiscSignalSender,
     /// work item
     work_item: Arc<WorkItem>,
     /// Parameters used by a work item.
@@ -71,8 +75,8 @@ impl CurrentLine {
 
 impl LineDiscipline {
     /// Create a new line discipline
-    pub fn new() -> Arc<Self> {
+    pub fn new(send_signal: LdiscSignalSender) -> Arc<Self> {
-        Arc::new_cyclic(|line_ref: &Weak<LineDiscipline>| {
+        Arc::new_cyclic(move |line_ref: &Weak<LineDiscipline>| {
             let line_discipline = line_ref.clone();
             let work_item = Arc::new(WorkItem::new(Box::new(move || {
                 if let Some(line_discipline) = line_discipline.upgrade() {
@@ -85,6 +89,7 @@ impl LineDiscipline {
                 termios: SpinLock::new(KernelTermios::default()),
                 winsize: SpinLock::new(WinSize::default()),
                 pollee: Pollee::new(IoEvents::empty()),
+                send_signal,
                 work_item,
                 work_item_para: Arc::new(SpinLock::new(LineDisciplineWorkPara::new())),
             }
@@ -92,15 +97,7 @@ impl LineDiscipline {
     }
 
     /// Push char to line discipline.
-    pub fn push_char<
+    pub fn push_char<F2: FnMut(&str)>(&self, ch: u8, echo_callback: F2) {
-        F1: Fn() -> Option<Arc<dyn Fn(KernelSignal) + Send + Sync>>,
-        F2: FnMut(&str),
-    >(
-        &self,
-        ch: u8,
-        may_send_signal: F1,
-        echo_callback: F2,
-    ) {
         let termios = self.termios.lock_irq_disabled();
 
         let ch = if termios.contains_icrnl() && ch == b'\r' {
@@ -109,7 +106,7 @@ impl LineDiscipline {
             ch
         };
 
-        if self.may_send_signal(&termios, ch, may_send_signal) {
+        if self.may_send_signal(&termios, ch) {
             // The char is already dealt with, so just return
             return;
         }
@@ -160,27 +157,18 @@ impl LineDiscipline {
         self.update_readable_state_deferred();
     }
 
-    fn may_send_signal<F: Fn() -> Option<Arc<dyn Fn(KernelSignal) + Send + Sync>>>(
+    fn may_send_signal(&self, termios: &KernelTermios, ch: u8) -> bool {
-        &self,
-        termios: &KernelTermios,
-        ch: u8,
-        may_send_signal: F,
-    ) -> bool {
         if !termios.is_canonical_mode() || !termios.contains_isig() {
             return false;
         }
 
-        let Some(send_signal) = may_send_signal() else {
-            return false;
-        };
-
         let signal = match ch {
             ch if ch == *termios.get_special_char(CC_C_CHAR::VINTR) => KernelSignal::new(SIGINT),
             ch if ch == *termios.get_special_char(CC_C_CHAR::VQUIT) => KernelSignal::new(SIGQUIT),
             _ => return false,
         };
         // `kernel_signal()` may cause sleep, so only construct parameters here.
-        self.work_item_para.lock_irq_disabled().kernel_signal = Some((send_signal, signal));
+        self.work_item_para.lock_irq_disabled().kernel_signal = Some(signal);
 
         true
     }
@@ -207,10 +195,8 @@ impl LineDiscipline {
 
     /// include all operations that may cause sleep, and processes by a work queue.
     fn update_readable_state_after(&self) {
-        if let Some((send_signal, signal)) =
+        if let Some(signal) = self.work_item_para.lock_irq_disabled().kernel_signal.take() {
-            self.work_item_para.lock_irq_disabled().kernel_signal.take()
+            (self.send_signal)(signal);
-        {
-            send_signal(signal);
         };
         if let Some(pollee_type) = self.work_item_para.lock_irq_disabled().pollee_type.take() {
             match pollee_type {
@@ -243,23 +229,14 @@ impl LineDiscipline {
         }
     }
 
-    pub fn read<F: Fn() -> bool>(&self, buf: &mut [u8], belongs_to_foreground: F) -> Result<usize> {
+    pub fn read(&self, buf: &mut [u8]) -> Result<usize> {
-        let mut poller = None;
         loop {
-            if !belongs_to_foreground() {
-                init_poller(&mut poller);
-                if self.poll(IoEvents::IN, poller.as_ref()).is_empty() {
-                    poller.as_ref().unwrap().wait()?
-                }
-                continue;
-            }
-
             let res = self.try_read(buf);
             match res {
                 Ok(len) => return Ok(len),
                 Err(e) if e.error() != Errno::EAGAIN => return Err(e),
                 Err(_) => {
-                    init_poller(&mut poller);
+                    let poller = Some(Poller::new());
                     if self.poll(IoEvents::IN, poller.as_ref()).is_empty() {
                         poller.as_ref().unwrap().wait()?
                     }
@@ -427,14 +404,6 @@ fn get_printable_char(ctrl_char: u8) -> u8 {
     ctrl_char + b'A' - 1
 }
 
-fn init_poller(poller: &mut Option<Poller>) {
-    if poller.is_some() {
-        return;
-    }
-
-    *poller = Some(Poller::new());
-}
-
 enum PolleeType {
     Add,
     Del,
@@ -442,7 +411,7 @@ enum PolleeType {
 
 struct LineDisciplineWorkPara {
     #[allow(clippy::type_complexity)]
-    kernel_signal: Option<(Arc<dyn Fn(KernelSignal) + Send + Sync>, KernelSignal)>,
+    kernel_signal: Option<KernelSignal>,
     pollee_type: Option<PolleeType>,
 }
 

services/libs/jinux-std/src/device/tty/mod.rs

@@ -33,7 +33,7 @@ pub struct Tty {
     name: CString,
     /// line discipline
     ldisc: Arc<LineDiscipline>,
-    job_control: JobControl,
+    job_control: Arc<JobControl>,
     /// driver
     driver: SpinLock<Weak<TtyDriver>>,
     weak_self: Weak<Self>,
@@ -41,10 +41,11 @@ pub struct Tty {
 
 impl Tty {
     pub fn new(name: CString) -> Arc<Self> {
-        Arc::new_cyclic(|weak_ref| Tty {
+        let (job_control, ldisc) = new_job_control_and_ldisc();
+        Arc::new_cyclic(move |weak_ref| Tty {
             name,
-            ldisc: LineDiscipline::new(),
+            ldisc,
-            job_control: JobControl::new(),
+            job_control,
             driver: SpinLock::new(Weak::new()),
             weak_self: weak_ref.clone(),
         })
@@ -55,27 +56,14 @@ impl Tty {
     }
 
     pub fn receive_char(&self, ch: u8) {
-        let may_send_signal = || {
+        self.ldisc.push_char(ch, |content| print!("{}", content));
-            let Some(foreground) = self.foreground() else {
-                return None;
-            };
-
-            let send_signal = move |signal: KernelSignal| {
-                foreground.kernel_signal(signal);
-            };
-
-            Some(Arc::new(send_signal) as Arc<dyn Fn(KernelSignal) + Send + Sync>)
-        };
-
-        self.ldisc
-            .push_char(ch, may_send_signal, |content| print!("{}", content));
     }
 }
 
 impl FileIo for Tty {
     fn read(&self, buf: &mut [u8]) -> Result<usize> {
-        self.ldisc
+        self.job_control.wait_until_in_foreground()?;
-            .read(buf, || self.job_control.current_belongs_to_foreground())
+        self.ldisc.read(buf)
     }
 
     fn write(&self, buf: &[u8]) -> Result<usize> {
@@ -187,6 +175,25 @@ impl Device for Tty {
     }
 }
 
+pub fn new_job_control_and_ldisc() -> (Arc<JobControl>, Arc<LineDiscipline>) {
+    let job_control = Arc::new(JobControl::new());
+
+    let send_signal = {
+        let cloned_job_control = job_control.clone();
+        move |signal: KernelSignal| {
+            let Some(foreground) = cloned_job_control.foreground() else {
+                return;
+            };
+
+            foreground.broadcast_signal(signal);
+        }
+    };
+
+    let ldisc = LineDiscipline::new(Arc::new(send_signal));
+
+    (job_control, ldisc)
+}
+
 pub fn get_n_tty() -> &'static Arc<Tty> {
     N_TTY.get().unwrap()
 }
@@ -198,8 +205,13 @@ pub fn open_ntty_as_controlling_terminal(process: &Process) -> Result<()> {
 
     let session = &process.session().unwrap();
     let process_group = process.process_group().unwrap();
-    tty.job_control.set_session(session);
+
+    session.set_terminal(|| {
+        tty.job_control.set_session(session);
+        Ok(tty.clone())
+    })?;
+
     tty.job_control.set_foreground(Some(&process_group))?;
 
-    session.set_terminal(|| Ok(tty.clone()))
+    Ok(())
 }

services/libs/jinux-std/src/fs/devpts/ptmx.rs

@@ -148,14 +148,14 @@ impl Device for Inner {
 
 impl FileIo for Inner {
     fn read(&self, buf: &mut [u8]) -> Result<usize> {
-        unreachable!()
+        return_errno_with_message!(Errno::EINVAL, "cannot read ptmx");
     }
 
     fn write(&self, buf: &[u8]) -> Result<usize> {
-        unreachable!()
+        return_errno_with_message!(Errno::EINVAL, "cannot write ptmx");
     }
 
     fn poll(&self, mask: IoEvents, poller: Option<&Poller>) -> IoEvents {
-        unreachable!()
+        IoEvents::empty()
     }
 }

services/libs/jinux-std/src/process/clone.rs

@@ -1,26 +1,20 @@
-use jinux_frame::{cpu::UserContext, user::UserSpace, vm::VmIo};
+use super::posix_thread::{PosixThread, PosixThreadBuilder, PosixThreadExt, ThreadName};
-
+use super::process_vm::ProcessVm;
+use super::signal::sig_disposition::SigDispositions;
+use super::{process_table, Process, ProcessBuilder};
+use crate::current_thread;
+use crate::fs::file_table::FileTable;
+use crate::fs::fs_resolver::FsResolver;
+use crate::fs::utils::FileCreationMask;
+use crate::prelude::*;
+use crate::thread::{allocate_tid, thread_table, Thread, Tid};
+use crate::util::write_val_to_user;
+use crate::vm::vmar::Vmar;
+use jinux_frame::cpu::UserContext;
+use jinux_frame::user::UserSpace;
+use jinux_frame::vm::VmIo;
 use jinux_rights::Full;
 
-use crate::{
-    current_thread,
-    fs::file_table::FileTable,
-    fs::{fs_resolver::FsResolver, utils::FileCreationMask},
-    prelude::*,
-    process::{
-        posix_thread::{PosixThreadBuilder, PosixThreadExt, ThreadName},
-        process_table,
-    },
-    thread::{allocate_tid, thread_table, Thread, Tid},
-    util::write_val_to_user,
-    vm::vmar::Vmar,
-};
-
-use super::{
-    posix_thread::PosixThread, process_vm::ProcessVm, signal::sig_disposition::SigDispositions,
-    Process, ProcessBuilder,
-};
-
 bitflags! {
     pub struct CloneFlags: u32 {
         const CLONE_VM      = 0x00000100;       /* Set if VM shared between processes.  */
@@ -280,27 +274,8 @@ fn clone_child_process(parent_context: UserContext, clone_args: CloneArgs) -> Re
         process_builder.build()?
     };
 
-    // Adds child to parent's process group, and parent's child processes.
-    let process_group = current!().process_group().unwrap();
-
-    let mut process_table_mut = process_table::process_table_mut();
-    let mut group_inner = process_group.inner.lock();
-    let mut child_group_mut = child.process_group.lock();
-    let mut children_mut = current.children().lock();
-
-    children_mut.insert(child.pid(), child.clone());
-
-    group_inner.processes.insert(child.pid(), child.clone());
-    *child_group_mut = Arc::downgrade(&process_group);
-
-    process_table_mut.insert(child.pid(), child.clone());
-    drop(process_table_mut);
-    drop(group_inner);
-    drop(child_group_mut);
-    drop(children_mut);
-
     // Deals with clone flags
-    let child_thread = thread_table::tid_to_thread(child_tid).unwrap();
+    let child_thread = thread_table::get_thread(child_tid).unwrap();
     let child_posix_thread = child_thread.as_posix_thread().unwrap();
     clone_parent_settid(child_tid, clone_args.parent_tidptr, clone_flags)?;
     clone_child_cleartid(child_posix_thread, clone_args.child_tidptr, clone_flags)?;
@@ -312,6 +287,10 @@ fn clone_child_process(parent_context: UserContext, clone_args: CloneArgs) -> Re
         clone_args.child_tidptr,
         clone_flags,
     )?;
+
+    // Sets parent process and group for child process.
+    set_parent_and_group(&current, &child);
+
     Ok(child)
 }
 
@@ -430,3 +409,19 @@ fn clone_sysvsem(clone_flags: CloneFlags) -> Result<()> {
     }
     Ok(())
 }
+
+fn set_parent_and_group(parent: &Arc<Process>, child: &Arc<Process>) {
+    let process_group = parent.process_group().unwrap();
+
+    let mut process_table_mut = process_table::process_table_mut();
+    let mut group_inner = process_group.inner.lock();
+    let mut child_group_mut = child.process_group.lock();
+    let mut children_mut = parent.children().lock();
+
+    children_mut.insert(child.pid(), child.clone());
+
+    group_inner.processes.insert(child.pid(), child.clone());
+    *child_group_mut = Arc::downgrade(&process_group);
+
+    process_table_mut.insert(child.pid(), child.clone());
+}

services/libs/jinux-std/src/process/process/job_control.rs

@@ -1,6 +1,7 @@
 use crate::prelude::*;
 use crate::process::signal::constants::{SIGCONT, SIGHUP};
 use crate::process::signal::signals::kernel::KernelSignal;
+use crate::process::signal::Pauser;
 use crate::process::{ProcessGroup, Session};
 
 /// The job control for terminals like tty and pty.
@@ -11,6 +12,7 @@ use crate::process::{ProcessGroup, Session};
 pub struct JobControl {
     foreground: SpinLock<Weak<ProcessGroup>>,
     session: SpinLock<Weak<Session>>,
+    pauser: Arc<Pauser>,
 }
 
 impl JobControl {
@@ -19,6 +21,7 @@ impl JobControl {
         Self {
             foreground: SpinLock::new(Weak::new()),
             session: SpinLock::new(Weak::new()),
+            pauser: Pauser::new(),
         }
     }
 
@@ -60,6 +63,7 @@ impl JobControl {
         let session = current.session().unwrap();
         *self.session.lock() = Arc::downgrade(&session);
 
+        self.pauser.resume_all();
         Ok(())
     }
 
@@ -73,8 +77,8 @@ impl JobControl {
         };
 
         if let Some(foreground) = self.foreground() {
-            foreground.kernel_signal(KernelSignal::new(SIGHUP));
+            foreground.broadcast_signal(KernelSignal::new(SIGHUP));
-            foreground.kernel_signal(KernelSignal::new(SIGCONT));
+            foreground.broadcast_signal(KernelSignal::new(SIGCONT));
         }
 
         Ok(())
@@ -115,16 +119,33 @@ impl JobControl {
         }
 
         *self.foreground.lock() = Arc::downgrade(process_group);
+        self.pauser.resume_all();
         Ok(())
     }
 
-    /// Determines whether current process belongs to the foreground process group. If
+    /// Wait until the current process is the foreground process group. If
     /// the foreground process group is None, returns true.
     ///
     /// # Panic
     ///
     /// This function should only be called in process context.
-    pub fn current_belongs_to_foreground(&self) -> bool {
+    pub fn wait_until_in_foreground(&self) -> Result<()> {
+        // Fast path
+        if self.current_belongs_to_foreground() {
+            return Ok(());
+        }
+
+        // Slow path
+        self.pauser.pause_until(|| {
+            if self.current_belongs_to_foreground() {
+                Some(())
+            } else {
+                None
+            }
+        })
+    }
+
+    fn current_belongs_to_foreground(&self) -> bool {
         let Some(foreground) = self.foreground() else {
             return true;
         };

services/libs/jinux-std/src/process/process/mod.rs

@@ -151,6 +151,8 @@ impl Process {
 
         let process = process_builder.build()?;
 
+        // Lock order: session table -> group table -> process table -> group of process
+        // -> group inner -> session inner
         let mut session_table_mut = process_table::session_table_mut();
         let mut group_table_mut = process_table::group_table_mut();
         let mut process_table_mut = process_table::process_table_mut();
@@ -290,10 +292,11 @@ impl Process {
         }
 
         let session = self.session().unwrap();
-        let mut session_inner = session.inner.lock();
+
-        let mut self_group_mut = self.process_group.lock();
+        // Lock order: session table -> group table -> group of process -> group inner -> session inner
-        let mut group_table_mut = process_table::group_table_mut();
         let mut session_table_mut = process_table::session_table_mut();
+        let mut group_table_mut = process_table::group_table_mut();
+        let mut self_group_mut = self.process_group.lock();
 
         if session_table_mut.contains_key(&self.pid) {
             return_errno_with_message!(Errno::EPERM, "cannot create new session");
@@ -303,12 +306,10 @@ impl Process {
             return_errno_with_message!(Errno::EPERM, "cannot create process group");
         }
 
-        // Removes the process from session.
-        session_inner.remove_process(self);
-
         // Removes the process from old group
-        if let Some(old_group) = self.process_group() {
+        if let Some(old_group) = self_group_mut.upgrade() {
             let mut group_inner = old_group.inner.lock();
+            let mut session_inner = session.inner.lock();
             group_inner.remove_process(&self.pid);
             *self_group_mut = Weak::new();
 
@@ -335,6 +336,10 @@ impl Process {
         new_session.inner.lock().leader = Some(self.clone());
         session_table_mut.insert(new_session.sid(), new_session.clone());
 
+        // Removes the process from session.
+        let mut session_inner = session.inner.lock();
+        session_inner.remove_process(self);
+
         Ok(new_session)
     }
 
@@ -386,13 +391,14 @@ impl Process {
     /// The new group will be added to the same session as the process.
     fn to_new_group(self: &Arc<Self>) -> Result<()> {
         let session = self.session().unwrap();
-        let mut session_inner = session.inner.lock();
+        // Lock order: group table -> group of process -> group inner -> session inner
-        let mut self_group_mut = self.process_group.lock();
         let mut group_table_mut = process_table::group_table_mut();
+        let mut self_group_mut = self.process_group.lock();
 
         // Removes the process from old group
-        if let Some(old_group) = self.process_group() {
+        if let Some(old_group) = self_group_mut.upgrade() {
             let mut group_inner = old_group.inner.lock();
+            let mut session_inner = session.inner.lock();
             group_inner.remove_process(&self.pid);
             *self_group_mut = Weak::new();
 
@@ -406,15 +412,18 @@ impl Process {
 
         // Creates a new process group. Adds the new group to group table and session.
         let new_group = ProcessGroup::new(self.clone());
+
+        let mut new_group_inner = new_group.inner.lock();
+        let mut session_inner = session.inner.lock();
+
         *self_group_mut = Arc::downgrade(&new_group);
 
         group_table_mut.insert(new_group.pgid(), new_group.clone());
 
+        new_group_inner.session = Arc::downgrade(&session);
         session_inner
             .process_groups
             .insert(new_group.pgid(), new_group.clone());
-        let mut new_group_inner = new_group.inner.lock();
-        new_group_inner.session = Arc::downgrade(&session);
 
         Ok(())
     }
@@ -423,20 +432,33 @@ impl Process {
     ///
     /// The caller needs to ensure that the process and the group belongs to the same session.
     fn to_specified_group(self: &Arc<Process>, group: &Arc<ProcessGroup>) -> Result<()> {
-        let mut self_group_mut = self.process_group.lock();
+        // Lock order: group table -> group of process -> group inner (small pgid -> big pgid)
         let mut group_table_mut = process_table::group_table_mut();
-        let mut group_inner = group.inner.lock();
+        let mut self_group_mut = self.process_group.lock();
 
         // Removes the process from old group
-        if let Some(old_group) = self.process_group() {
+        let mut group_inner = if let Some(old_group) = self_group_mut.upgrade() {
-            let mut group_inner = old_group.inner.lock();
+            // Lock order: group with smaller pgid first
-            group_inner.remove_process(&self.pid);
+            let (mut old_group_inner, group_inner) = match old_group.pgid().cmp(&group.pgid()) {
+                core::cmp::Ordering::Equal => return Ok(()),
+                core::cmp::Ordering::Less => (old_group.inner.lock(), group.inner.lock()),
+                core::cmp::Ordering::Greater => {
+                    let group_inner = group.inner.lock();
+                    let old_group_inner = old_group.inner.lock();
+                    (old_group_inner, group_inner)
+                }
+            };
+            old_group_inner.remove_process(&self.pid);
             *self_group_mut = Weak::new();
 
-            if group_inner.is_empty() {
+            if old_group_inner.is_empty() {
                 group_table_mut.remove(&old_group.pgid());
             }
-        }
+
+            group_inner
+        } else {
+            group.inner.lock()
+        };
 
         // Adds the process to the specified group
         group_inner.processes.insert(self.pid, self.clone());
@@ -567,8 +589,10 @@ mod test {
     }
 
     fn new_process_in_session(parent: Option<Arc<Process>>) -> Arc<Process> {
-        let mut group_table_mut = process_table::group_table_mut();
+        // Lock order: session table -> group table -> group of process -> group inner
+        // -> session inner
         let mut session_table_mut = process_table::session_table_mut();
+        let mut group_table_mut = process_table::group_table_mut();
 
         let process = new_process(parent);
         // Creates new group
@@ -587,6 +611,7 @@ mod test {
     }
 
     fn remove_session_and_group(process: Arc<Process>) {
+        // Lock order: session table -> group table
         let mut session_table_mut = process_table::session_table_mut();
         let mut group_table_mut = process_table::group_table_mut();
         if let Some(sess) = process.session() {

services/libs/jinux-std/src/process/process/process_group.rs

@@ -1,7 +1,6 @@
 use super::{Pgid, Pid, Process, Session};
 use crate::prelude::*;
-use crate::process::signal::signals::kernel::KernelSignal;
+use crate::process::signal::signals::Signal;
-use crate::process::signal::signals::user::UserSignal;
 
 /// `ProcessGroup` represents a set of processes. Each `ProcessGroup` has a unique
 /// identifier `pgid`.
@@ -37,7 +36,7 @@ impl ProcessGroup {
     /// id. The process will become the leading process of the new process group.
     ///
     /// The caller needs to ensure that the process does not belong to any group.
-    pub(super) fn new(process: Arc<Process>) -> Arc<Self> {
+    pub(in crate::process) fn new(process: Arc<Process>) -> Arc<Self> {
         let pid = process.pid();
 
         let inner = {
@@ -57,7 +56,7 @@ impl ProcessGroup {
     }
 
     /// Returns whether self contains a process with `pid`.
-    pub(super) fn contains_process(&self, pid: Pid) -> bool {
+    pub(in crate::process) fn contains_process(&self, pid: Pid) -> bool {
         self.inner.lock().processes.contains_key(&pid)
     }
 
@@ -66,22 +65,15 @@ impl ProcessGroup {
         self.pgid
     }
 
-    /// Sends kernel signal to all processes in the group
+    /// Broadcasts signal to all processes in the group.
-    pub fn kernel_signal(&self, signal: KernelSignal) {
+    pub fn broadcast_signal(&self, signal: impl Signal + Clone + 'static) {
         for process in self.inner.lock().processes.values() {
-            process.enqueue_signal(Box::new(signal));
+            process.enqueue_signal(Box::new(signal.clone()));
-        }
-    }
-
-    /// Sends user signal to all processes in the group
-    pub fn user_signal(&self, signal: UserSignal) {
-        for process in self.inner.lock().processes.values() {
-            process.enqueue_signal(Box::new(signal));
         }
     }
 
     /// Returns the leader process.
-    pub(super) fn leader(&self) -> Option<Arc<Process>> {
+    pub fn leader(&self) -> Option<Arc<Process>> {
         self.inner.lock().leader.clone()
     }
 

services/libs/jinux-std/src/process/process/session.rs

@@ -44,7 +44,7 @@ impl Session {
     ///
     /// The caller needs to ensure that the group does not belong to any session, and the caller
     /// should set the leader process after creating the session.
-    pub(super) fn new(group: Arc<ProcessGroup>) -> Arc<Self> {
+    pub(in crate::process) fn new(group: Arc<ProcessGroup>) -> Arc<Self> {
         let sid = group.pgid();
         let inner = {
             let mut process_groups = BTreeMap::new();
@@ -68,12 +68,12 @@ impl Session {
     }
 
     /// Returns the leader process.
-    pub(super) fn leader(&self) -> Option<Arc<Process>> {
+    pub fn leader(&self) -> Option<Arc<Process>> {
         self.inner.lock().leader.clone()
     }
 
     /// Returns whether `self` contains the `process_group`
-    pub(super) fn contains_process_group(
+    pub(in crate::process) fn contains_process_group(
         self: &Arc<Self>,
         process_group: &Arc<ProcessGroup>,
     ) -> bool {
@@ -83,10 +83,14 @@ impl Session {
             .contains_key(&process_group.pgid())
     }
 
-    /// Sets terminal as the controlling terminal of the session.
+    /// 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: Fn() -> Result<Arc<dyn Terminal>>>(&self, terminal: F) -> Result<()> {
+    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() {
@@ -96,7 +100,7 @@ impl Session {
             );
         }
 
-        let terminal = terminal()?;
+        let terminal = get_terminal()?;
         inner.terminal = Some(terminal);
         Ok(())
     }
@@ -104,7 +108,10 @@ impl Session {
     /// 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: Fn() -> Result<()>>(&self, release_session: F) -> Result<()> {
+    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!(
@@ -113,7 +120,8 @@ impl Session {
             );
         }
 
-        release_session()?;
+        let terminal = inner.terminal.as_ref().unwrap();
+        release_session(terminal)?;
         inner.terminal = None;
         Ok(())
     }

services/libs/jinux-std/src/process/process/terminal.rs

@@ -63,13 +63,13 @@ pub trait Terminal: Send + Sync + FileIo {
             return_errno_with_message!(Errno::EPERM, "current process is not session leader");
         }
 
-        let terminal = || {
+        let get_terminal = || {
             self.job_control().set_current_session()?;
             Ok(self.arc_self())
         };
 
         let session = current!().session().unwrap();
-        session.set_terminal(terminal)
+        session.set_terminal(get_terminal)
     }
 
     /// Releases the terminal from the session of current process if the terminal is the controlling
@@ -91,7 +91,7 @@ pub trait Terminal: Send + Sync + FileIo {
             return Ok(());
         }
 
-        let release_session = || self.job_control().release_current_session();
+        let release_session = |_: &Arc<dyn Terminal>| self.job_control().release_current_session();
 
         let session = current.session().unwrap();
         session.release_terminal(release_session)

services/libs/jinux-std/src/process/wait.rs

@@ -81,12 +81,15 @@ fn reap_zombie_child(process: &Process, pid: Pid) -> u32 {
         thread_table::remove_thread(thread.tid());
     }
 
-    let mut process_table_mut = process_table::process_table_mut();
+    // Lock order: session table -> group table -> process table -> group of process
-    let mut group_table_mut = process_table::group_table_mut();
+    // -> group inner -> session inner
     let mut session_table_mut = process_table::session_table_mut();
+    let mut group_table_mut = process_table::group_table_mut();
+    let mut process_table_mut = process_table::process_table_mut();
+
     let mut child_group_mut = child_process.process_group.lock();
 
-    let process_group = child_process.process_group().unwrap();
+    let process_group = child_group_mut.upgrade().unwrap();
     let mut group_inner = process_group.inner.lock();
     let session = group_inner.session.upgrade().unwrap();
     let mut session_inner = session.inner.lock();

services/libs/jinux-std/src/syscall/kill.rs

@@ -42,7 +42,7 @@ pub fn do_sys_kill(filter: ProcessFilter, sig_num: SigNum) -> Result<()> {
         }
         ProcessFilter::WithPgid(pgid) => {
             if let Some(process_group) = process_table::get_process_group(&pgid) {
-                process_group.user_signal(signal);
+                process_group.broadcast_signal(signal);
             } else {
                 return_errno_with_message!(Errno::ESRCH, "No such process group in process table");
             }

services/libs/jinux-std/src/syscall/tgkill.rs

@@ -16,8 +16,8 @@ pub fn sys_tgkill(tgid: Pid, tid: Tid, sig_num: u8) -> Result<SyscallReturn> {
     log_syscall_entry!(SYS_TGKILL);
     let sig_num = SigNum::from_u8(sig_num);
     info!("tgid = {}, pid = {}, sig_num = {:?}", tgid, tid, sig_num);
-    let target_thread = thread_table::tid_to_thread(tid)
+    let target_thread =
-        .ok_or(Error::with_message(Errno::EINVAL, "Invalid pid"))?;
+        thread_table::get_thread(tid).ok_or(Error::with_message(Errno::EINVAL, "Invalid pid"))?;
     let posix_thread = target_thread.as_posix_thread().unwrap();
     let pid = posix_thread.process().pid();
     if pid != tgid {

services/libs/jinux-std/src/thread/thread_table.rs

@@ -15,6 +15,6 @@ pub fn remove_thread(tid: Tid) {
     THREAD_TABLE.lock().remove(&tid);
 }
 
-pub fn tid_to_thread(tid: Tid) -> Option<Arc<Thread>> {
+pub fn get_thread(tid: Tid) -> Option<Arc<Thread>> {
     THREAD_TABLE.lock().get(&tid).cloned()
 }