mirror/moss-kernel
1
0
Fork 0
mirror of https://github.com/hexagonal-sun/moss-kernel.git synced 2026-01-31 01:21:46 -05:00
Code Issues Packages Projects Releases Wiki Activity

process: refactor signal handling and fix action table sharing

Browse Source 
This commit refactors the signal handling subsystem to address a bug in
process creation and to improve the separation of concerns between
signal delivery and signal disposition.

1. Fix Action Table Sharing:
Previously, the signal action table was wrapped in an `Arc`, causing
forked child processes to inadvertently share signal dispositions with
the parent. `SignalActionState` now contains a direct `SigActionSet` and
implements Clone, ensuring that processes receive a private copy of the
action table upon fork/clone.

2. Decouple Signal Selection from Execution:
The logic for selecting a pending signal has been moved from the
disposition state into a new `take_signal` method on `OwnedTask`. This
decouples the "taking" of a signal (respecting masks and priorities)
from the "actioning" of that signal. This is a prerequisite for
implementing ptrace.

3. Various cleanup bits:
- Renamed `SignalState` to `SignalActionState` to more accurately
  reflect that it manages signal dispositions.
- Removed the `pending` signal set out of the action state and directly
  into the `ThreadGroup` and `OwnedTask`.
- Renamed `set_pending` to `set_signal` for consistency.
- Simplified the signal delivery loop in `dispatch_userspace_task`
  using the new `take_signal` API.
This commit is contained in:
Matthew Leach
2026-01-09 06:46:47 +00:00
parent 97c07cd31f
commit 1d79d6b2df
9 changed files with 144 additions and 168 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
src/process/exec.rs
View File

@@ -8,7 +8,7 @@ use crate::{
page::ClaimedPage,
uaccess::{copy_from_user, cstr::UserCStr},
},
process::{ctx::Context, thread_group::signal::SignalState},
process::{ctx::Context, thread_group::signal::SignalActionState},
sched::current::current_task,
};
use alloc::{string::String, vec};
@@ -191,7 +191,7 @@ pub async fn kernel_exec(
current_task.ctx = Context::from_user_ctx(user_ctx);
*current_task.vm.lock_save_irq() = vm;
*current_task.process.signals.lock_save_irq() = SignalState::new_default();
*current_task.process.signals.lock_save_irq() = SignalActionState::new_default();
Ok(())
}

5
src/process/exit.rs
View File

@@ -80,7 +80,10 @@ pub fn do_exit_group(exit_code: ChildState) {
parent.child_notifiers.child_update(process.tgid, exit_code);
parent.signals.lock_save_irq().set_pending(SigId::SIGCHLD);
parent
.pending_signals
.lock_save_irq()
.set_signal(SigId::SIGCHLD);
// 5. This thread is now finished.
*task.state.lock_save_irq() = TaskState::Finished;

15
src/process/owned.rs
View File

@@ -8,7 +8,7 @@ use super::{
thread_group::{
Tgid,
builder::ThreadGroupBuilder,
signal::{SigId, SigSet, SignalState},
signal::{SigId, SigSet, SignalActionState},
},
threading::RobustListHead,
};
@@ -62,7 +62,7 @@ impl OwnedTask {
let thread_group_builder = ThreadGroupBuilder::new(Tgid::idle())
.with_priority(i8::MIN)
.with_sigstate(Arc::new(SpinLock::new(SignalState::new_ignore())));
.with_sigstate(Arc::new(SpinLock::new(SignalActionState::new_ignore())));
let task = Task {
tid: Tid::idle_for_cpu(),
@@ -130,4 +130,15 @@ impl OwnedTask {
pub fn raise_task_signal(&mut self, signal: SigId) {
self.pending_signals.insert(signal.into());
}
/// Take a pending signal from this task's pending signal queue, or the
/// process's pending signal queue, while repsecting the signal mask.
pub fn take_signal(&mut self) -> Option<SigId> {
self.pending_signals.take_signal(self.sig_mask).or_else(|| {
self.process
.pending_signals
.lock_save_irq()
.take_signal(self.sig_mask)
})
}
}

4
src/process/thread_group.rs
View File

@@ -12,7 +12,7 @@ use core::{
};
use pid::PidT;
use rsrc_lim::ResourceLimits;
use signal::{SigSet, SignalState};
use signal::{SigSet, SignalActionState};
use wait::ChildNotifiers;
pub mod builder;
@@ -96,7 +96,7 @@ pub struct ThreadGroup {
pub parent: SpinLock<Option<Weak<ThreadGroup>>>,
pub children: SpinLock<BTreeMap<Tgid, Arc<ThreadGroup>>>,
pub tasks: SpinLock<BTreeMap<Tid, Weak<Task>>>,
pub signals: Arc<SpinLock<SignalState>>,
pub signals: Arc<SpinLock<SignalActionState>>,
pub rsrc_lim: Arc<SpinLock<ResourceLimits>>,
pub pending_signals: SpinLock<SigSet>,
pub priority: SpinLock<i8>,

8
src/process/thread_group/builder.rs
View File

@@ -7,7 +7,7 @@ use crate::sync::SpinLock;
use super::{
Pgid, ProcessState, Sid, TG_LIST, Tgid, ThreadGroup,
rsrc_lim::ResourceLimits,
signal::{SigSet, SignalState},
signal::{SigSet, SignalActionState},
wait::ChildNotifiers,
};
@@ -17,7 +17,7 @@ pub struct ThreadGroupBuilder {
parent: Option<Arc<ThreadGroup>>,
umask: Option<u32>,
pri: Option<i8>,
sigstate: Option<Arc<SpinLock<SignalState>>>,
sigstate: Option<Arc<SpinLock<SignalActionState>>>,
rsrc_lim: Option<Arc<SpinLock<ResourceLimits>>>,
}
@@ -41,7 +41,7 @@ impl ThreadGroupBuilder {
}
/// Sets the signal state of the thread group.
pub fn with_sigstate(mut self, sigstate: Arc<SpinLock<SignalState>>) -> Self {
pub fn with_sigstate(mut self, sigstate: Arc<SpinLock<SignalActionState>>) -> Self {
self.sigstate = Some(sigstate);
self
}
@@ -69,7 +69,7 @@ impl ThreadGroupBuilder {
children: SpinLock::new(BTreeMap::new()),
signals: self
.sigstate
.unwrap_or_else(|| Arc::new(SpinLock::new(SignalState::new_default()))),
.unwrap_or_else(|| Arc::new(SpinLock::new(SignalActionState::new_default()))),
rsrc_lim: self
.rsrc_lim
.unwrap_or_else(|| Arc::new(SpinLock::new(ResourceLimits::default()))),

102
src/process/thread_group/signal.rs
View File

@@ -8,9 +8,8 @@ use core::{
use bitflags::bitflags;
use ksigaction::{KSignalAction, UserspaceSigAction};
use libkernel::memory::{address::UA, region::UserMemoryRegion};
use ringbuf::Arc;
use crate::{memory::uaccess::UserCopyable, sync::SpinLock};
use crate::memory::uaccess::UserCopyable;
pub mod kill;
pub mod ksigaction;
@@ -81,6 +80,23 @@ impl From<SigSet> for SigId {
}
}
impl SigSet {
/// Set the signal with id `signal` to true in the set.
pub fn set_signal(&mut self, signal: SigId) {
*self = self.union(signal.into());
}
/// Remove a set signal from the set, setting it to false, while respecting
/// `mask`. Returns the ID of the removed signal.
pub fn take_signal(&mut self, mask: SigSet) -> Option<SigId> {
let signal = self.difference(mask).iter().next()?;
self.remove(signal);
Some(signal.into())
}
}
#[repr(u32)]
#[derive(Clone, Copy, PartialEq, Eq, Debug)]
#[allow(clippy::upper_case_acronyms)]
@@ -196,91 +212,33 @@ impl AltSigStack {
}
}
pub struct SignalState {
action: Arc<SpinLock<SigActionSet>>,
pending: SigSet,
#[derive(Clone)]
pub struct SignalActionState {
action: SigActionSet,
pub alt_stack: Option<AltSigStack>,
}
impl Clone for SignalState {
fn clone(&self) -> Self {
Self {
action: self.action.clone(),
pending: SigSet::empty(),
alt_stack: None,
}
}
}
impl SignalState {
impl SignalActionState {
pub fn new_ignore() -> Self {
Self {
action: Arc::new(SpinLock::new(SigActionSet([SigActionState::Ignore; 64]))),
pending: SigSet::empty(),
action: SigActionSet([SigActionState::Ignore; 64]),
alt_stack: None,
}
}
pub fn new_default() -> Self {
Self {
action: Arc::new(SpinLock::new(SigActionSet([SigActionState::Default; 64]))),
pending: SigSet::empty(),
action: SigActionSet([SigActionState::Default; 64]),
alt_stack: None,
}
}
pub fn clone_sharing_action_table(&self) -> Self {
Self {
action: self.action.clone(),
pending: SigSet::empty(),
alt_stack: None,
}
}
pub fn clone_copying_action_table(&self) -> Self {
Self {
action: Arc::new(SpinLock::new(self.action.lock_save_irq().clone())),
pending: SigSet::empty(),
alt_stack: None,
}
}
pub fn set_pending(&mut self, signal: SigId) {
self.pending.insert(signal.into());
}
pub fn action_signal(
&mut self,
mask: SigSet,
task_pending: &mut SigSet,
) -> Option<(SigId, KSignalAction)> {
loop {
let signal = self
.pending
.union(*task_pending)
.difference(mask)
.iter()
.next()?;
// Consume the signal we are about to action.
self.pending.remove(signal);
task_pending.remove(signal);
let id: SigId = signal.into();
match self.action.lock_save_irq()[id] {
SigActionState::Ignore => continue, // look for another signal,
SigActionState::Default => {
let action = KSignalAction::default_action(id);
if let Some(action) = action {
return Some((id, action));
}
// Signal is ignored by default. Look for another signal.
}
SigActionState::Action(userspace_sig_action) => {
return Some((id, KSignalAction::Userspace(id, userspace_sig_action)));
}
pub fn action_signal(&self, id: SigId) -> Option<KSignalAction> {
match self.action[id] {
SigActionState::Ignore => None, // look for another signal,
SigActionState::Default => KSignalAction::default_action(id),
SigActionState::Action(userspace_sig_action) => {
Some(KSignalAction::Userspace(id, userspace_sig_action))
}
}
}

21
src/process/thread_group/signal/kill.rs
View File

@@ -18,16 +18,16 @@ pub fn sys_kill(pid: PidT, signal: UserSigId) -> Result<usize> {
if pid == current_task.process.tgid.value() as PidT {
current_task
.process
.signals
.pending_signals
.lock_save_irq()
.set_pending(signal);
.set_signal(signal);
return Ok(0);
}
match pid {
p if p > 0 => {
let target_tg = ThreadGroup::get(Tgid(p as _)).ok_or(KernelError::NoProcess)?;
target_tg.signals.lock_save_irq().set_pending(signal);
target_tg.pending_signals.lock_save_irq().set_signal(signal);
}
0 => {
@@ -41,7 +41,7 @@ pub fn sys_kill(pid: PidT, signal: UserSigId) -> Result<usize> {
if let Some(tg) = tg_weak.upgrade()
&& *tg.pgid.lock_save_irq() == our_pgid
{
tg.signals.lock_save_irq().set_pending(signal);
tg.pending_signals.lock_save_irq().set_signal(signal);
}
}
}
@@ -55,7 +55,7 @@ pub fn sys_kill(pid: PidT, signal: UserSigId) -> Result<usize> {
if let Some(tg) = tg_weak.upgrade()
&& *tg.pgid.lock_save_irq() == target_pgid
{
tg.signals.lock_save_irq().set_pending(signal);
tg.pending_signals.lock_save_irq().set_signal(signal);
}
}
}
@@ -76,9 +76,9 @@ pub fn sys_tkill(tid: PidT, signal: UserSigId) -> Result<usize> {
if current_task.tid == target_tid {
current_task
.process
.signals
.pending_signals
.lock_save_irq()
.set_pending(signal);
.set_signal(signal);
} else {
let task = current_task
.process
@@ -88,7 +88,10 @@ pub fn sys_tkill(tid: PidT, signal: UserSigId) -> Result<usize> {
.and_then(|t| t.upgrade())
.ok_or(KernelError::NoProcess)?;
task.process.signals.lock_save_irq().set_pending(signal);
task.process
.pending_signals
.lock_save_irq()
.set_signal(signal);
}
Ok(0)
@@ -99,7 +102,7 @@ pub fn send_signal_to_pg(pgid: Pgid, signal: SigId) {
if let Some(tg) = tg_weak.upgrade()
&& *tg.pgid.lock_save_irq() == pgid
{
tg.signals.lock_save_irq().set_pending(signal);
tg.pending_signals.lock_save_irq().set_signal(signal);
}
}
}

7
src/process/thread_group/signal/sigaction.rs
View File

@@ -112,12 +112,11 @@ pub async fn sys_rt_sigaction(
let old_action = {
let task = current_task();
let sigstate = task.process.signals.lock_save_irq();
let mut action_table = sigstate.action.lock_save_irq();
let old_action = action_table[sig];
let mut sigstate = task.process.signals.lock_save_irq();
let old_action = sigstate.action[sig];
if let Some(new_action) = new_act {
action_table[sig] = new_action.into();
sigstate.action[sig] = new_action.into();
}
old_action

146
src/sched/uspc_ret.rs
View File

@@ -74,7 +74,7 @@ enum State {
pub fn dispatch_userspace_task(ctx: *mut UserCtx) {
let mut state = State::PickNewTask;
loop {
'dispatch: loop {
match state {
State::PickNewTask => {
// Pick a new task, potentially context switching to a new task.
@@ -228,91 +228,93 @@ pub fn dispatch_userspace_task(ctx: *mut UserCtx) {
continue;
}
// See if there are any signals we need to action.
if let Some((id, action)) = {
let task = current_task();
let mut pending_task_sigs = task.pending_signals;
{
let mut task = current_task();
let mask = task.sig_mask;
task.process
.signals
.lock_save_irq()
.action_signal(mask, &mut pending_task_sigs)
} {
match action {
KSignalAction::Term | KSignalAction::Core => {
// Terminate the process, and find a new task.
kernel_exit_with_signal(id, false);
while let Some(signal) = task.take_signal() {
let sigaction = task.process.signals.lock_save_irq().action_signal(signal);
state = State::PickNewTask;
continue;
}
KSignalAction::Stop => {
let task = current_task();
match sigaction {
// Signal ignored, look for another.
None => continue,
Some(KSignalAction::Term | KSignalAction::Core) => {
// Terminate the process, and find a new task.
kernel_exit_with_signal(signal, false);
// Default action: stop (suspend) the entire process.
let process = &task.process;
// Notify the parent that we have stopped (SIGCHLD).
if let Some(parent) = process
.parent
.lock_save_irq()
.as_ref()
.and_then(|p| p.upgrade())
{
parent
.child_notifiers
.child_update(process.tgid, ChildState::Stop { signal: id });
parent.signals.lock_save_irq().set_pending(SigId::SIGCHLD);
state = State::PickNewTask;
continue 'dispatch;
}
Some(KSignalAction::Stop) => {
// Default action: stop (suspend) the entire process.
let process = &task.process;
for thr_weak in process.tasks.lock_save_irq().values() {
if let Some(thr) = thr_weak.upgrade() {
*thr.state.lock_save_irq() = TaskState::Stopped;
// Notify the parent that we have stopped (SIGCHLD).
if let Some(parent) = process
.parent
.lock_save_irq()
.as_ref()
.and_then(|p| p.upgrade())
{
parent
.child_notifiers
.child_update(process.tgid, ChildState::Stop { signal });
parent
.pending_signals
.lock_save_irq()
.set_signal(SigId::SIGCHLD);
}
}
state = State::PickNewTask;
continue;
}
KSignalAction::Continue => {
let task = current_task();
let process = &task.process;
// Wake up all sleeping threads in the process.
for thr_weak in process.tasks.lock_save_irq().values() {
if let Some(thr) = thr_weak.upgrade() {
let mut st = thr.state.lock_save_irq();
if *st == TaskState::Sleeping {
*st = TaskState::Runnable;
for thr_weak in process.tasks.lock_save_irq().values() {
if let Some(thr) = thr_weak.upgrade() {
*thr.state.lock_save_irq() = TaskState::Stopped;
}
}
state = State::PickNewTask;
continue 'dispatch;
}
Some(KSignalAction::Continue) => {
let process = &task.process;
// Notify the parent that we have continued (SIGCHLD).
if let Some(parent) = process
.parent
.lock_save_irq()
.as_ref()
.and_then(|p| p.upgrade())
{
parent
.child_notifiers
.child_update(process.tgid, ChildState::Continue);
parent.signals.lock_save_irq().set_pending(SigId::SIGCHLD);
// Wake up all sleeping threads in the process.
for thr_weak in process.tasks.lock_save_irq().values() {
if let Some(thr) = thr_weak.upgrade() {
let mut st = thr.state.lock_save_irq();
if *st == TaskState::Sleeping {
*st = TaskState::Runnable;
}
}
}
// Notify the parent that we have continued (SIGCHLD).
if let Some(parent) = process
.parent
.lock_save_irq()
.as_ref()
.and_then(|p| p.upgrade())
{
parent
.child_notifiers
.child_update(process.tgid, ChildState::Continue);
parent
.pending_signals
.lock_save_irq()
.set_signal(SigId::SIGCHLD);
}
// Re-process kernel work for this task (there may be more to do).
state = State::ProcessKernelWork;
continue 'dispatch;
}
Some(KSignalAction::Userspace(id, action)) => {
let fut = ArchImpl::do_signal(id, action);
// Re-process kernel work for this task (there may be more to do).
state = State::ProcessKernelWork;
continue;
}
KSignalAction::Userspace(id, action) => {
let fut = ArchImpl::do_signal(id, action);
task.ctx.put_signal_work(Box::pin(fut));
current_task().ctx.put_signal_work(Box::pin(fut));
state = State::ProcessKernelWork;
continue;
state = State::ProcessKernelWork;
continue 'dispatch;
}
}
}
}