diff --git a/Cargo.toml b/Cargo.toml index 90e1744..c2e49e5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,6 +9,7 @@ license = "MIT OR Apache-2.0" repository = "https://github.com/Starry-OS/starry-process" [dependencies] +bitflags = "2.6" kspin = "0.1" lazyinit = "0.2.1" weak-map = "0.1" diff --git a/src/process.rs b/src/process.rs index 3ccff29..89971e3 100644 --- a/src/process.rs +++ b/src/process.rs @@ -5,9 +5,10 @@ use alloc::{ }; use core::{ fmt, - sync::atomic::{AtomicBool, Ordering}, + sync::atomic::{AtomicU8, Ordering}, }; +use bitflags::bitflags; use kspin::SpinNoIrq; use lazyinit::LazyInit; use weak_map::StrongMap; @@ -21,10 +22,19 @@ pub(crate) struct ThreadGroup { pub(crate) group_exited: bool, } +bitflags! { + #[derive(Debug, Clone, Copy, PartialEq, Eq)] + pub(crate) struct ProcessState: u8 { + const RUNNING = 1 << 0; + const STOPPED = 1 << 1; + const ZOMBIE = 1 << 2; + } +} + /// A process. pub struct Process { pid: Pid, - is_zombie: AtomicBool, + state: AtomicU8, pub(crate) tg: SpinNoIrq, // TODO: child subreaper9 @@ -191,10 +201,110 @@ impl Process { impl Process { /// Returns `true` if the [`Process`] is a zombie process. pub fn is_zombie(&self) -> bool { - self.is_zombie.load(Ordering::Acquire) + let bits = self.state.load(Ordering::Acquire); + ProcessState::from_bits_truncate(bits).contains(ProcessState::ZOMBIE) + } + + /// Returns `true` if the [`Process`] is running. + pub fn is_running(&self) -> bool { + let bits = self.state.load(Ordering::Acquire); + ProcessState::from_bits_truncate(bits).contains(ProcessState::RUNNING) + } + + /// Returns `true` if the [`Process`] is stopped. + pub fn is_stopped(&self) -> bool { + let bits = self.state.load(Ordering::Acquire); + ProcessState::from_bits_truncate(bits).contains(ProcessState::STOPPED) + } + + /// Change the [`Process`] from `Running` to `Stopped`. + /// + /// This method atomically transitions the process state to STOPPED using + /// CAS, ensuring the state is either successfully changed or already in + /// ZOMBIE state (in which case no change occurs). + /// + /// # Memory Ordering + /// + /// Uses `Release` ordering on success to synchronize with `Acquire` loads + /// in `is_stopped()`. This ensures that any writes before this + /// transition, such as setting `stop_signal` in the + /// `ProcessSignalManager`, are visible to threads that observe the + /// `STOPPED` state transition. + pub fn transition_to_stopped(&self) { + let _ = self.state.fetch_update( + Ordering::Release, // Success: synchronize with is_stopped() + Ordering::Relaxed, // Failure: no synchronization needed + |curr| { + let mut flags = ProcessState::from_bits_truncate(curr); + if flags.contains(ProcessState::ZOMBIE) || !flags.contains(ProcessState::RUNNING) { + None // Already zombie or not running, don't transition + } else { + flags.remove(ProcessState::RUNNING); + flags.insert(ProcessState::STOPPED); + Some(flags.bits()) + } + }, + ); + } + + /// Change the [`Process`] from `Stopped` to `Running`. + /// + /// This method atomically transitions the process state to RUNNING using + /// CAS. The transition succeeds if and only if the current state is + /// `STOPPED` and not `ZOMBIE`. + /// + /// # Memory Ordering + /// + /// Uses `Release` ordering on success to synchronize with `Acquire` loads + /// in `is_running()`. This ensures that any writes before this + /// transition, for example, setting `cont_signal` in the + /// `ProcessSignalManager`, are visible to threads that observe the + /// `RUNNING` state transition. + pub fn transition_to_running(&self) { + let _ = self.state.fetch_update( + Ordering::Release, // Success: synchronize with is_running() + Ordering::Relaxed, // Failure: no synchronization needed + |curr| { + let mut flags = ProcessState::from_bits_truncate(curr); + if !flags.contains(ProcessState::STOPPED) || flags.contains(ProcessState::ZOMBIE) { + None // Not stopped or already zombie, don't transition + } else { + flags.remove(ProcessState::STOPPED); + flags.insert(ProcessState::RUNNING); + Some(flags.bits()) + } + }, + ); + } + + /// Change the [`Process`] from any non-ZOMBIE state to `Zombie`. + /// + /// This is a terminal state transition - once a process becomes a zombie, + /// it cannot transition to any other state (it can only be freed via + /// `free()`). + /// + /// # Memory Ordering + /// + /// Uses `Release` ordering to synchronize with `Acquire` loads in + /// `is_zombie()`, ensuring that when a parent process's `wait()` observes + /// the ZOMBIE state, it also observes all writes that happened before + /// the transition, particularly the exit_code set by `exit_thread()`. + pub fn transition_to_zombie(&self) { + let _ = self + .state + .fetch_update(Ordering::Release, Ordering::Relaxed, |curr| { + let mut flags = ProcessState::from_bits_truncate(curr); + if flags.contains(ProcessState::ZOMBIE) { + None // Already zombie + } else { + flags.remove(ProcessState::RUNNING | ProcessState::STOPPED); + flags.insert(ProcessState::ZOMBIE); + Some(flags.bits()) + } + }); } - /// Terminates the [`Process`], marking it as a zombie process. + /// Terminates the [`Process`]. /// /// Child processes are inherited by the init process or by the nearest /// subreaper process. @@ -209,7 +319,6 @@ impl Process { } let mut children = self.children.lock(); // Acquire the lock first - self.is_zombie.store(true, Ordering::Release); let mut reaper_children = reaper.children.lock(); let reaper = Arc::downgrade(reaper); @@ -266,7 +375,7 @@ impl Process { let process = Arc::new(Process { pid, - is_zombie: AtomicBool::new(false), + state: AtomicU8::new(ProcessState::RUNNING.bits()), tg: SpinNoIrq::new(ThreadGroup::default()), children: SpinNoIrq::new(StrongMap::new()), parent: SpinNoIrq::new(parent.as_ref().map(Arc::downgrade).unwrap_or_default()), diff --git a/tests/group.rs b/tests/group.rs index 2e1b7ec..50f1778 100644 --- a/tests/group.rs +++ b/tests/group.rs @@ -59,6 +59,7 @@ fn cleanup() { assert!(group.upgrade().is_some()); child.exit(); + child.transition_to_zombie(); child.free(); drop(child); assert!(group.upgrade().is_none()); @@ -134,6 +135,7 @@ fn cleanup_processes() { let group = parent.create_group().unwrap(); parent.exit(); + parent.transition_to_zombie(); parent.free(); drop(parent); diff --git a/tests/process.rs b/tests/process.rs index b91eb0d..3e782b5 100644 --- a/tests/process.rs +++ b/tests/process.rs @@ -1,4 +1,8 @@ -use std::sync::Arc; +use std::{ + sync::{Arc, Barrier}, + thread, + time::Duration, +}; use starry_process::init_proc; @@ -18,6 +22,7 @@ fn exit() { let parent = init_proc(); let child = parent.new_child(); child.exit(); + child.transition_to_zombie(); assert!(child.is_zombie()); assert!(parent.children().iter().any(|c| Arc::ptr_eq(c, &child))); } @@ -33,6 +38,7 @@ fn free() { let parent = init_proc().new_child(); let child = parent.new_child(); child.exit(); + child.transition_to_zombie(); child.free(); assert!(parent.children().is_empty()); } @@ -70,4 +76,129 @@ fn thread_exit() { let last2 = child.exit_thread(102, 3); assert!(last2); assert_eq!(child.exit_code(), 7); -} \ No newline at end of file +} + +#[test] +fn state_lifecycle() { + let process = init_proc().new_child(); + + // Initial state should be RUNNING + assert!(process.is_running()); + assert!(!process.is_stopped()); + assert!(!process.is_zombie()); + + // RUNNING -> STOPPED + process.transition_to_stopped(); + assert!(!process.is_running()); + assert!(process.is_stopped()); + assert!(!process.is_zombie()); + + // STOPPED -> RUNNING + process.transition_to_running(); + assert!(process.is_running()); + assert!(!process.is_stopped()); + assert!(!process.is_zombie()); + + // RUNNING -> ZOMBIE + process.transition_to_zombie(); + assert!(!process.is_running()); + assert!(!process.is_stopped()); + assert!(process.is_zombie()); +} + +#[test] +fn invalid_transitions() { + let process = init_proc().new_child(); + + // STOPPED -> STOPPED (idempotent) + process.transition_to_stopped(); + assert!(process.is_stopped()); + process.transition_to_stopped(); + assert!(process.is_stopped()); + + // STOPPED -> ZOMBIE + process.transition_to_zombie(); + assert!(process.is_zombie()); + + // ZOMBIE -> RUNNING (Invalid) + process.transition_to_running(); + assert!(process.is_zombie()); + assert!(!process.is_running()); + + // ZOMBIE -> STOPPED (Invalid) + process.transition_to_stopped(); + assert!(process.is_zombie()); + assert!(!process.is_stopped()); +} + +#[test] +fn concurrent_transitions() { + let process = init_proc().new_child(); + let barrier = Arc::new(Barrier::new(4)); + + // Strategy: + // Spawn 4 threads to change the state concurrently. + // All of them start at the same time after all 4 barriers are reached, + // simulating multiple kernel threads accessing and changing the process state + // simultaneously. + let mut handles = vec![]; + + // Thread 1: Tries to stop + let p1 = process.clone(); + let b1 = barrier.clone(); + handles.push(thread::spawn(move || { + b1.wait(); + for _ in 0..1000 { + p1.transition_to_stopped(); + thread::yield_now(); + } + })); + + // Thread 2: Tries to continue + let p2 = process.clone(); + let b2 = barrier.clone(); + handles.push(thread::spawn(move || { + b2.wait(); + for _ in 0..1000 { + p2.transition_to_running(); + thread::yield_now(); + } + })); + + // Thread 3: Validates terminal state consistency + // validate the terminal state property: + // once a process is a zombie, it should always be a zombie. + let p3 = process.clone(); + let b3 = barrier.clone(); + handles.push(thread::spawn(move || { + b3.wait(); + let mut observed_zombie = false; + for _ in 0..1000 { + if p3.is_zombie() { + observed_zombie = true; + } else if observed_zombie { + // If we previously saw zombie, we should never see non-zombie + panic!("Process transitioned from zombie to non-zombie state!"); + } + thread::yield_now(); + } + })); + + // Thread 4: The killer + let p4 = process.clone(); + let b4 = barrier.clone(); + handles.push(thread::spawn(move || { + b4.wait(); + thread::sleep(Duration::from_millis(50)); + p4.transition_to_zombie(); + })); + + for h in handles { + h.join().unwrap(); + } + + // Must be zombie at the end + assert!(process.is_zombie()); + assert!(!process.is_running()); + assert!(!process.is_stopped()); +} diff --git a/tests/session.rs b/tests/session.rs index 71269ee..f268bff 100644 --- a/tests/session.rs +++ b/tests/session.rs @@ -1,5 +1,4 @@ -use std::sync::Arc; -use std::any::Any; +use std::{any::Any, sync::Arc}; use starry_process::init_proc; @@ -59,6 +58,7 @@ fn cleanup() { assert!(session.upgrade().is_some()); child.exit(); + child.transition_to_zombie(); child.free(); drop(child); assert!(session.upgrade().is_none()); @@ -102,6 +102,7 @@ fn cleanup_groups() { let (session, _) = child.create_session().unwrap(); child.exit(); + child.transition_to_zombie(); child.free(); drop(child); @@ -129,4 +130,4 @@ fn terminal_set_unset() { assert!(session.unset_terminal(&term)); assert!(session.terminal().is_none()); -} \ No newline at end of file +}