diff --git a/services/libs/jinux-std/src/device/pty/pty.rs b/services/libs/jinux-std/src/device/pty/pty.rs index 7c7d5780d..748edb3ec 100644 --- a/services/libs/jinux-std/src/device/pty/pty.rs +++ b/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, input: SpinLock>, - job_control: JobControl, + job_control: Arc, /// The state of input buffer pollee: Pollee, weak_self: Weak, @@ -33,12 +33,13 @@ pub struct PtyMaster { impl PtyMaster { pub fn new(ptmx: Arc, index: u32) -> Arc { - 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 { - 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) - }; - let mut input = self.input.lock(); for character in buf { - self.output - .push_char(*character, may_send_signal, |content| { - for byte in content.as_bytes() { - input.push_overwrite(*byte); - } - }); + self.output.push_char(*character, |content| { + for byte in content.as_bytes() { + 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 { - self.master() - .output - .read(buf, || self.job_control.current_belongs_to_foreground()) + self.job_control.wait_until_in_foreground()?; + self.master().output.read(buf) } fn write(&self, buf: &[u8]) -> Result { diff --git a/services/libs/jinux-std/src/device/tty/device.rs b/services/libs/jinux-std/src/device/tty/device.rs index 661ac3e3f..e5f8afb79 100644 --- a/services/libs/jinux-std/src/device/tty/device.rs +++ b/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 { - unreachable!() + return_errno_with_message!(Errno::EINVAL, "cannot read tty device"); } fn write(&self, buf: &[u8]) -> Result { - unreachable!() + return_errno_with_message!(Errno::EINVAL, "cannot write tty device"); } fn poll(&self, mask: IoEvents, poller: Option<&Poller>) -> IoEvents { - unreachable!() + IoEvents::empty() } } diff --git a/services/libs/jinux-std/src/device/tty/line_discipline.rs b/services/libs/jinux-std/src/device/tty/line_discipline.rs index c127befd4..7764f4eb5 100644 --- a/services/libs/jinux-std/src/device/tty/line_discipline.rs +++ b/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; + pub struct LineDiscipline { /// current line current_line: SpinLock, @@ -27,6 +29,8 @@ pub struct LineDiscipline { winsize: SpinLock, /// Pollee pollee: Pollee, + /// Used to send signal for foreground processes, when some char comes. + send_signal: LdiscSignalSender, /// work item work_item: Arc, /// Parameters used by a work item. @@ -71,8 +75,8 @@ impl CurrentLine { impl LineDiscipline { /// Create a new line discipline - pub fn new() -> Arc { - Arc::new_cyclic(|line_ref: &Weak| { + pub fn new(send_signal: LdiscSignalSender) -> Arc { + Arc::new_cyclic(move |line_ref: &Weak| { 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< - F1: Fn() -> Option>, - F2: FnMut(&str), - >( - &self, - ch: u8, - may_send_signal: F1, - echo_callback: F2, - ) { + pub fn push_char(&self, ch: u8, 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 Option>>( - &self, - termios: &KernelTermios, - ch: u8, - may_send_signal: F, - ) -> bool { + fn may_send_signal(&self, termios: &KernelTermios, ch: u8) -> 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)) = - self.work_item_para.lock_irq_disabled().kernel_signal.take() - { - send_signal(signal); + if let Some(signal) = self.work_item_para.lock_irq_disabled().kernel_signal.take() { + (self.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 bool>(&self, buf: &mut [u8], belongs_to_foreground: F) -> Result { - let mut poller = None; + pub fn read(&self, buf: &mut [u8]) -> Result { 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) { - 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, KernelSignal)>, + kernel_signal: Option, pollee_type: Option, } diff --git a/services/libs/jinux-std/src/device/tty/mod.rs b/services/libs/jinux-std/src/device/tty/mod.rs index 9bbf0448c..1aea18dad 100644 --- a/services/libs/jinux-std/src/device/tty/mod.rs +++ b/services/libs/jinux-std/src/device/tty/mod.rs @@ -33,7 +33,7 @@ pub struct Tty { name: CString, /// line discipline ldisc: Arc, - job_control: JobControl, + job_control: Arc, /// driver driver: SpinLock>, weak_self: Weak, @@ -41,10 +41,11 @@ pub struct Tty { impl Tty { pub fn new(name: CString) -> Arc { - 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(), - job_control: JobControl::new(), + ldisc, + 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 = || { - 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) - }; - - self.ldisc - .push_char(ch, may_send_signal, |content| print!("{}", content)); + self.ldisc.push_char(ch, |content| print!("{}", content)); } } impl FileIo for Tty { fn read(&self, buf: &mut [u8]) -> Result { - self.ldisc - .read(buf, || self.job_control.current_belongs_to_foreground()) + self.job_control.wait_until_in_foreground()?; + self.ldisc.read(buf) } fn write(&self, buf: &[u8]) -> Result { @@ -187,6 +175,25 @@ impl Device for Tty { } } +pub fn new_job_control_and_ldisc() -> (Arc, Arc) { + 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 { 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(()) } diff --git a/services/libs/jinux-std/src/fs/devpts/ptmx.rs b/services/libs/jinux-std/src/fs/devpts/ptmx.rs index ba7ed4828..c8031d997 100644 --- a/services/libs/jinux-std/src/fs/devpts/ptmx.rs +++ b/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 { - unreachable!() + return_errno_with_message!(Errno::EINVAL, "cannot read ptmx"); } fn write(&self, buf: &[u8]) -> Result { - unreachable!() + return_errno_with_message!(Errno::EINVAL, "cannot write ptmx"); } fn poll(&self, mask: IoEvents, poller: Option<&Poller>) -> IoEvents { - unreachable!() + IoEvents::empty() } } diff --git a/services/libs/jinux-std/src/process/clone.rs b/services/libs/jinux-std/src/process/clone.rs index eab642640..fa38ab0c2 100644 --- a/services/libs/jinux-std/src/process/clone.rs +++ b/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(¤t, &child); + Ok(child) } @@ -430,3 +409,19 @@ fn clone_sysvsem(clone_flags: CloneFlags) -> Result<()> { } Ok(()) } + +fn set_parent_and_group(parent: &Arc, child: &Arc) { + 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()); +} diff --git a/services/libs/jinux-std/src/process/process/job_control.rs b/services/libs/jinux-std/src/process/process/job_control.rs index d3e81ecee..d5af056c6 100644 --- a/services/libs/jinux-std/src/process/process/job_control.rs +++ b/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>, session: SpinLock>, + pauser: Arc, } 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.kernel_signal(KernelSignal::new(SIGCONT)); + foreground.broadcast_signal(KernelSignal::new(SIGHUP)); + 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; }; diff --git a/services/libs/jinux-std/src/process/process/mod.rs b/services/libs/jinux-std/src/process/process/mod.rs index e6cefda48..ee12e3483 100644 --- a/services/libs/jinux-std/src/process/process/mod.rs +++ b/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(); - 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 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) -> Result<()> { let session = self.session().unwrap(); - let mut session_inner = session.inner.lock(); - let mut self_group_mut = self.process_group.lock(); + // Lock order: group table -> group of process -> group inner -> session inner 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, group: &Arc) -> 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 = old_group.inner.lock(); - group_inner.remove_process(&self.pid); + let mut group_inner = if let Some(old_group) = self_group_mut.upgrade() { + // Lock order: group with smaller pgid first + 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 { - 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) { + // 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() { diff --git a/services/libs/jinux-std/src/process/process/process_group.rs b/services/libs/jinux-std/src/process/process/process_group.rs index 4da6c5d15..3cc819050 100644 --- a/services/libs/jinux-std/src/process/process/process_group.rs +++ b/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::user::UserSignal; +use crate::process::signal::signals::Signal; /// `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) -> Arc { + pub(in crate::process) fn new(process: Arc) -> Arc { 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 - pub fn kernel_signal(&self, signal: KernelSignal) { + /// Broadcasts signal to all processes in the group. + pub fn broadcast_signal(&self, signal: impl Signal + Clone + 'static) { for process in self.inner.lock().processes.values() { - process.enqueue_signal(Box::new(signal)); - } - } - - /// 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)); + process.enqueue_signal(Box::new(signal.clone())); } } /// Returns the leader process. - pub(super) fn leader(&self) -> Option> { + pub fn leader(&self) -> Option> { self.inner.lock().leader.clone() } diff --git a/services/libs/jinux-std/src/process/process/session.rs b/services/libs/jinux-std/src/process/process/session.rs index 6c956b657..18ff44c4f 100644 --- a/services/libs/jinux-std/src/process/process/session.rs +++ b/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) -> Arc { + pub(in crate::process) fn new(group: Arc) -> Arc { 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> { + pub fn leader(&self) -> Option> { 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, process_group: &Arc, ) -> 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 Result>>(&self, terminal: F) -> Result<()> { + pub fn set_terminal(&self, get_terminal: F) -> Result<()> + where + F: Fn() -> Result>, + { 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 Result<()>>(&self, release_session: F) -> Result<()> { + pub fn release_terminal(&self, release_session: F) -> Result<()> + where + F: Fn(&Arc) -> 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(()) } diff --git a/services/libs/jinux-std/src/process/process/terminal.rs b/services/libs/jinux-std/src/process/process/terminal.rs index 309dd9574..94d66b319 100644 --- a/services/libs/jinux-std/src/process/process/terminal.rs +++ b/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| self.job_control().release_current_session(); let session = current.session().unwrap(); session.release_terminal(release_session) diff --git a/services/libs/jinux-std/src/process/wait.rs b/services/libs/jinux-std/src/process/wait.rs index 8db08d16f..99e4d4572 100644 --- a/services/libs/jinux-std/src/process/wait.rs +++ b/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(); - let mut group_table_mut = process_table::group_table_mut(); + // 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(); + 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(); diff --git a/services/libs/jinux-std/src/syscall/kill.rs b/services/libs/jinux-std/src/syscall/kill.rs index 8ca7e7fe8..e0c82be0b 100644 --- a/services/libs/jinux-std/src/syscall/kill.rs +++ b/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"); } diff --git a/services/libs/jinux-std/src/syscall/tgkill.rs b/services/libs/jinux-std/src/syscall/tgkill.rs index c02236dd8..014fa0d64 100644 --- a/services/libs/jinux-std/src/syscall/tgkill.rs +++ b/services/libs/jinux-std/src/syscall/tgkill.rs @@ -16,8 +16,8 @@ pub fn sys_tgkill(tgid: Pid, tid: Tid, sig_num: u8) -> Result { 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) - .ok_or(Error::with_message(Errno::EINVAL, "Invalid pid"))?; + let target_thread = + 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 { diff --git a/services/libs/jinux-std/src/thread/thread_table.rs b/services/libs/jinux-std/src/thread/thread_table.rs index b1ae4463f..59e5facf8 100644 --- a/services/libs/jinux-std/src/thread/thread_table.rs +++ b/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> { +pub fn get_thread(tid: Tid) -> Option> { THREAD_TABLE.lock().get(&tid).cloned() }