From 6f3a483be66f40fad6faea3ee16b8295fbf16259 Mon Sep 17 00:00:00 2001 From: Jianfeng Jiang Date: Thu, 9 May 2024 06:37:13 +0000 Subject: [PATCH] Check only not blocked signals in Pauser --- .../aster-nix/src/process/posix_thread/mod.rs | 7 ++-- kernel/aster-nix/src/process/signal/pauser.rs | 32 +++++++++++-------- .../src/process/signal/sig_queues.rs | 15 +++++++++ kernel/aster-nix/src/thread/task.rs | 11 ++----- 4 files changed, 41 insertions(+), 24 deletions(-) diff --git a/kernel/aster-nix/src/process/posix_thread/mod.rs b/kernel/aster-nix/src/process/posix_thread/mod.rs index 35355f391..593a4857b 100644 --- a/kernel/aster-nix/src/process/posix_thread/mod.rs +++ b/kernel/aster-nix/src/process/posix_thread/mod.rs @@ -101,8 +101,11 @@ impl PosixThread { self.sig_queues.sig_pending() } - pub fn has_pending_signal(&self) -> bool { - !self.sig_queues.is_empty() + /// Returns whether the thread has some pending signals + /// that are not blocked. + pub fn has_pending(&self) -> bool { + let blocked = *self.sig_mask().lock(); + self.sig_queues.has_pending(blocked) } /// Returns whether the signal is blocked by the thread. diff --git a/kernel/aster-nix/src/process/signal/pauser.rs b/kernel/aster-nix/src/process/signal/pauser.rs index 6299cdc8f..a1197056f 100644 --- a/kernel/aster-nix/src/process/signal/pauser.rs +++ b/kernel/aster-nix/src/process/signal/pauser.rs @@ -111,25 +111,28 @@ impl Pauser { { self.is_interrupted.store(false, Ordering::Release); - // Register observer on sigqueue - let observer = Arc::downgrade(self) as Weak>; - let filter = { - let sig_mask = { - let current_thread = current_thread!(); - let poxis_thread = current_thread.as_posix_thread().unwrap(); - let mut current_sigmask = *poxis_thread.sig_mask().lock(); - current_sigmask.block(self.sig_mask.as_u64()); - current_sigmask - }; - SigEventsFilter::new(sig_mask) - }; - let current_thread = current_thread!(); let posix_thread = current_thread.as_posix_thread().unwrap(); + + // Block `self.sig_mask` + let (old_mask, filter) = { + let mut current_mask = posix_thread.sig_mask().lock(); + let old_mask = *current_mask; + + let new_mask = { + current_mask.block(self.sig_mask.as_u64()); + *current_mask + }; + + (old_mask, SigEventsFilter::new(new_mask)) + }; + + // Register observer on sigqueue + let observer = Arc::downgrade(self) as Weak>; posix_thread.register_sigqueue_observer(observer.clone(), filter); // Some signal may come before we register observer, so we do another check here. - if posix_thread.has_pending_signal() { + if posix_thread.has_pending_signals() { self.is_interrupted.store(true, Ordering::Release); } @@ -159,6 +162,7 @@ impl Pauser { }; posix_thread.unregiser_sigqueue_observer(&observer); + posix_thread.sig_mask().lock().set(old_mask.as_u64()); match res? { Res::Ok(r) => Ok(r), diff --git a/kernel/aster-nix/src/process/signal/sig_queues.rs b/kernel/aster-nix/src/process/signal/sig_queues.rs index 46d4deb49..d48dad44e 100644 --- a/kernel/aster-nix/src/process/signal/sig_queues.rs +++ b/kernel/aster-nix/src/process/signal/sig_queues.rs @@ -61,11 +61,17 @@ impl SigQueues { signal } + /// Returns the pending signals pub fn sig_pending(&self) -> SigSet { let queues = self.queues.lock(); queues.sig_pending() } + /// Returns whether there's some pending signals that are not blocked + pub fn has_pending(&self, blocked: SigMask) -> bool { + self.queues.lock().has_pending(blocked) + } + pub fn register_observer( &self, observer: Weak>, @@ -189,6 +195,15 @@ impl Queues { None } + /// Returns whether the `SigQueues` has some pending signals which are not blocked + fn has_pending(&self, blocked: SigMask) -> bool { + self.std_queues.iter().any(|signal| { + signal + .as_ref() + .is_some_and(|signal| !blocked.contains(signal.num())) + }) || self.rt_queues.iter().any(|rt_queue| !rt_queue.is_empty()) + } + fn get_std_queue_mut(&mut self, signum: SigNum) -> &mut Option> { debug_assert!(signum.is_std()); let idx = (signum.as_u8() - MIN_STD_SIG_NUM) as usize; diff --git a/kernel/aster-nix/src/thread/task.rs b/kernel/aster-nix/src/thread/task.rs index 50779e959..433334de8 100644 --- a/kernel/aster-nix/src/thread/task.rs +++ b/kernel/aster-nix/src/thread/task.rs @@ -17,11 +17,6 @@ use crate::{ /// create new task with userspace and parent process pub fn create_new_user_task(user_space: Arc, thread_ref: Weak) -> Arc { fn user_task_entry() { - fn has_pending_signal(current_thread: &Arc) -> bool { - let posix_thread = current_thread.as_posix_thread().unwrap(); - posix_thread.has_pending_signal() - } - let current_thread = current_thread!(); let current_task = current_thread.task(); let user_space = current_task @@ -41,8 +36,8 @@ pub fn create_new_user_task(user_space: Arc, thread_ref: Weak user_mode.context().syscall_ret() ); - #[allow(clippy::redundant_closure)] - let has_kernel_event_fn = || has_pending_signal(¤t_thread); + let posix_thread = current_thread.as_posix_thread().unwrap(); + let has_kernel_event_fn = || posix_thread.has_pending(); loop { let return_reason = user_mode.execute(has_kernel_event_fn); let context = user_mode.context_mut(); @@ -52,7 +47,7 @@ pub fn create_new_user_task(user_space: Arc, thread_ref: Weak ReturnReason::UserSyscall => handle_syscall(context), ReturnReason::KernelEvent => {} }; - // should be do this comparison before handle signal? + if current_thread.status().is_exited() { break; }