From dc2f85bda2b76df76f910d23082cf2bdb3e6d1c5 Mon Sep 17 00:00:00 2001 From: Haoze Wu Date: Thu, 27 Nov 2025 17:34:46 +0800 Subject: [PATCH 1/7] feat: update state machine for process --- Cargo.toml | 1 + src/process.rs | 89 +++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 85 insertions(+), 5 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 90e1744..556e9e1 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.workspace = true kspin = "0.1" lazyinit = "0.2.1" weak-map = "0.1" diff --git a/src/process.rs b/src/process.rs index 3ccff29..5659d99 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,31 @@ pub(crate) struct ThreadGroup { pub(crate) group_exited: bool, } +bitflags! { + #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] + pub(crate) struct ProcessState: u8 { + const RUNNING = 1 << 0; + const STOPPED = 1 << 1; + const ZOMBIE = 1 << 2; + } +} + +impl PartialEq for ProcessState { + fn eq(&self, other: &u8) -> bool { + self.bits() == *other + } +} + +impl PartialEq for u8 { + fn eq(&self, other: &ProcessState) -> bool { + *self == other.bits() + } +} + /// A process. pub struct Process { pid: Pid, - is_zombie: AtomicBool, + state: AtomicU8, pub(crate) tg: SpinNoIrq, // TODO: child subreaper9 @@ -191,7 +213,64 @@ 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) + self.state.load(Ordering::Acquire) == ProcessState::ZOMBIE + } + + /// Returns `true` if the [`Process`] is running. + pub fn is_running(&self) -> bool { + self.state.load(Ordering::Acquire) == ProcessState::RUNNING + } + + /// Returns `true` if the [`Process`] is stopped. + pub fn is_stopped(&self) -> bool { + self.state.load(Ordering::Acquire) == ProcessState::STOPPED + } + + /// Change the [`Process`] from Running to `Stopped` with a causing signal. + /// + /// This method checks for the process state atomically first using the CAS + /// `fetch_update`, ensuring only the state has successfully changed + /// into `STOPPED` or not changed. + pub fn transition_to_stopped(&self) { + let _ = self + .state + .fetch_update(Ordering::SeqCst, Ordering::SeqCst, |curr| { + if curr == ProcessState::ZOMBIE.bits() { + None + } else { + Some(ProcessState::STOPPED.bits()) + } + }); + } + + /// Change the [`Process`] from `Stopped` to `Running`. + /// + /// Since the only signal that can trigger this state change is `SIGCONT`, + /// we omit the signal here. + /// + /// This method checks for the process state atomically first using the CAS + /// `fetch_update`, ensuring only the state has successfully changed + /// into `RUNNING` or not changed. + pub fn transition_to_running(&self) { + let _ = self + .state + .fetch_update(Ordering::SeqCst, Ordering::SeqCst, |curr| { + if curr != ProcessState::STOPPED { + None + } else { + Some(ProcessState::RUNNING.bits()) + } + }); + } + + /// Change the [`Process`] from `Stopped` or `Running` to `Zombie`. + pub fn transition_to_zombie(&self) { + if self.is_zombie() { + return; + } + + self.state + .store(ProcessState::ZOMBIE.bits(), Ordering::Relaxed); } /// Terminates the [`Process`], marking it as a zombie process. @@ -209,7 +288,7 @@ impl Process { } let mut children = self.children.lock(); // Acquire the lock first - self.is_zombie.store(true, Ordering::Release); + self.transition_to_zombie(); let mut reaper_children = reaper.children.lock(); let reaper = Arc::downgrade(reaper); @@ -266,7 +345,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()), From a7411b9cd6b2c4831cd16d0bdc5a7d4f31394a28 Mon Sep 17 00:00:00 2001 From: Haoze Wu Date: Wed, 3 Dec 2025 15:18:43 +0800 Subject: [PATCH 2/7] fix: fix memory ordering issue and update comments Changes: - Fix the memory ordering issue in transition_to_zombie function, change from Relaxed to Release to achieve Acquire-Release ordering with zombie state setting and getting - Remove the state transition in exit function of a process, bring the transition to do_exit for single resposibility of reparenting children in exit - Add some comments on memory ordering --- src/process.rs | 77 ++++++++++++++++++++++++++++++++++---------------- 1 file changed, 53 insertions(+), 24 deletions(-) diff --git a/src/process.rs b/src/process.rs index 5659d99..04a3f80 100644 --- a/src/process.rs +++ b/src/process.rs @@ -226,54 +226,84 @@ impl Process { self.state.load(Ordering::Acquire) == ProcessState::STOPPED } - /// Change the [`Process`] from Running to `Stopped` with a causing signal. + /// Change the [`Process`] from Running to `Stopped`. /// - /// This method checks for the process state atomically first using the CAS - /// `fetch_update`, ensuring only the state has successfully changed - /// into `STOPPED` or not changed. + /// 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::SeqCst, Ordering::SeqCst, |curr| { + let _ = self.state.fetch_update( + Ordering::Release, // Success: synchronize with is_stopped() + Ordering::Relaxed, // Failure: no synchronization needed + |curr| { if curr == ProcessState::ZOMBIE.bits() { - None + None // Already zombie, don't transition } else { Some(ProcessState::STOPPED.bits()) } - }); + }, + ); } /// Change the [`Process`] from `Stopped` to `Running`. /// - /// Since the only signal that can trigger this state change is `SIGCONT`, - /// we omit the signal here. + /// This method atomically transitions the process state to RUNNING using + /// CAS. The transition succeeds if and only if the current state is + /// `STOPPED`. + /// + /// # Memory Ordering /// - /// This method checks for the process state atomically first using the CAS - /// `fetch_update`, ensuring only the state has successfully changed - /// into `RUNNING` or not changed. + /// 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::SeqCst, Ordering::SeqCst, |curr| { - if curr != ProcessState::STOPPED { - None + let _ = self.state.fetch_update( + Ordering::Release, // Success: synchronize with is_running() + Ordering::Relaxed, // Failure: no synchronization needed + |curr| { + if curr != ProcessState::STOPPED.bits() { + None // Not stopped, don't transition } else { Some(ProcessState::RUNNING.bits()) } - }); + }, + ); } /// Change the [`Process`] from `Stopped` or `Running` 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) { if self.is_zombie() { return; } - self.state - .store(ProcessState::ZOMBIE.bits(), Ordering::Relaxed); + self.state.store( + ProcessState::ZOMBIE.bits(), + Ordering::Release, // Synchronize with is_zombie() + ); } - /// 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. @@ -288,7 +318,6 @@ impl Process { } let mut children = self.children.lock(); // Acquire the lock first - self.transition_to_zombie(); let mut reaper_children = reaper.children.lock(); let reaper = Arc::downgrade(reaper); From 2f2bb17e05f3cf7eab5d76f5492bbaef6c0ec6a5 Mon Sep 17 00:00:00 2001 From: Haoze Wu Date: Thu, 4 Dec 2025 20:42:13 +0800 Subject: [PATCH 3/7] feat: add wait event tracking to Process for POSIX-compliant waitpid Changes: - Add WaitEventFlags bitflags to ThreadGroup - Add wait_events and last_stop_signal fields to ThreadGroup - Implement set_stop_signal, set_cont_signal on Process - Implement peek/consume methods for stop/continue events - Update ThreadGroup::default() to initialize new fields Rationale: Events persist until zombie reaping, ensuring proper WUNTRACED/WCONTINUED reporting --- src/process.rs | 111 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 110 insertions(+), 1 deletion(-) diff --git a/src/process.rs b/src/process.rs index 04a3f80..939f1f8 100644 --- a/src/process.rs +++ b/src/process.rs @@ -15,11 +15,27 @@ use weak_map::StrongMap; use crate::{Pid, ProcessGroup, Session}; -#[derive(Default)] pub(crate) struct ThreadGroup { pub(crate) threads: BTreeSet, pub(crate) exit_code: i32, pub(crate) group_exited: bool, + /// Unreported stop/continue events for waitpid() with WUNTRACED/WCONTINUED. + /// Persists until the zombie is reaped. + wait_events: AtomicU8, + /// The signal number that most recently stopped the process. + last_stop_signal: AtomicU8, +} + +impl Default for ThreadGroup { + fn default() -> Self { + Self { + threads: BTreeSet::new(), + exit_code: 0, + group_exited: false, + wait_events: AtomicU8::new(0), + last_stop_signal: AtomicU8::new(0), + } + } } bitflags! { @@ -31,6 +47,14 @@ bitflags! { } } +bitflags! { + #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] + pub(crate) struct WaitEventFlags: u8 { + const PENDING_STOP_EVENT = 1 << 0; + const PENDING_CONT_EVENT = 1 << 1; + } +} + impl PartialEq for ProcessState { fn eq(&self, other: &u8) -> bool { self.bits() == *other @@ -338,6 +362,91 @@ impl Process { parent.children.lock().remove(&self.pid); } } + + /// Records a stop signal effect for waitpid reporting. + /// + /// Sets the PENDING_STOP_EVENT flag and records which signal caused the + /// stop. Atomically clears any pending continue event. + pub fn set_stop_signal(&self, signal: u8) { + let tg = self.tg.lock(); + tg.last_stop_signal.store(signal, Ordering::Release); + + // Atomically: clear CONT event, set STOP event + tg.wait_events.fetch_update( + Ordering::Release, + Ordering::Acquire, + |current_flags| { + Some( + (current_flags & !WaitEventFlags::PENDING_CONT_EVENT.bits()) + | WaitEventFlags::PENDING_STOP_EVENT.bits(), + ) + }, + ).ok(); + } + + /// Records a continue signal effect for waitpid reporting. + /// + /// Sets the PENDING_CONT_EVENT flag and clears the recorded stop signal. + /// Atomically clears any pending stop event. + pub fn set_cont_signal(&self) { + let tg = self.tg.lock(); + tg.last_stop_signal.store(0, Ordering::Release); + + // Atomically: clear STOP event, set CONT event + tg.wait_events.fetch_update( + Ordering::Release, + Ordering::Acquire, + |current_flags| { + Some( + (current_flags & !WaitEventFlags::PENDING_STOP_EVENT.bits()) + | WaitEventFlags::PENDING_CONT_EVENT.bits(), + ) + }, + ).ok(); + } + + /// Peeks at a pending stop signal event without consuming it. + /// + /// Returns the signal that caused the stop if there is an unreported stop event. + pub fn peek_pending_stop_event(&self) -> Option { + let tg = self.tg.lock(); + let flags = tg.wait_events.load(Ordering::Acquire); + + if (flags & WaitEventFlags::PENDING_STOP_EVENT.bits()) != 0 { + let signal = tg.last_stop_signal.load(Ordering::Acquire); + if signal != 0 { Some(signal) } else { None } + } else { + None + } + } + + /// Consumes (clears) the pending stop signal event. + pub fn consume_stop_event(&self) { + let tg = self.tg.lock(); + tg.last_stop_signal.store(0, Ordering::Release); + tg.wait_events.fetch_and( + !WaitEventFlags::PENDING_STOP_EVENT.bits(), + Ordering::Release, + ); + } + + /// Peeks at a pending continue signal event without consuming it. + /// + /// Returns true if there is an unreported continue event. + pub fn peek_pending_cont_event(&self) -> bool { + let tg = self.tg.lock(); + let flags = tg.wait_events.load(Ordering::Acquire); + (flags & WaitEventFlags::PENDING_CONT_EVENT.bits()) != 0 + } + + /// Consumes (clears) the pending continue signal event. + pub fn consume_cont_event(&self) { + let tg = self.tg.lock(); + tg.wait_events.fetch_and( + !WaitEventFlags::PENDING_CONT_EVENT.bits(), + Ordering::Release, + ); + } } impl fmt::Debug for Process { From 5586ee1ed79706e60162d2db27b9f6b1abb1ce29 Mon Sep 17 00:00:00 2001 From: Haoze Wu Date: Fri, 5 Dec 2025 09:45:06 +0800 Subject: [PATCH 4/7] Revert "feat: add wait event tracking to Process for POSIX-compliant waitpid" This reverts commit 2f2bb17e05f3cf7eab5d76f5492bbaef6c0ec6a5. --- src/process.rs | 111 +------------------------------------------------ 1 file changed, 1 insertion(+), 110 deletions(-) diff --git a/src/process.rs b/src/process.rs index 939f1f8..04a3f80 100644 --- a/src/process.rs +++ b/src/process.rs @@ -15,27 +15,11 @@ use weak_map::StrongMap; use crate::{Pid, ProcessGroup, Session}; +#[derive(Default)] pub(crate) struct ThreadGroup { pub(crate) threads: BTreeSet, pub(crate) exit_code: i32, pub(crate) group_exited: bool, - /// Unreported stop/continue events for waitpid() with WUNTRACED/WCONTINUED. - /// Persists until the zombie is reaped. - wait_events: AtomicU8, - /// The signal number that most recently stopped the process. - last_stop_signal: AtomicU8, -} - -impl Default for ThreadGroup { - fn default() -> Self { - Self { - threads: BTreeSet::new(), - exit_code: 0, - group_exited: false, - wait_events: AtomicU8::new(0), - last_stop_signal: AtomicU8::new(0), - } - } } bitflags! { @@ -47,14 +31,6 @@ bitflags! { } } -bitflags! { - #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] - pub(crate) struct WaitEventFlags: u8 { - const PENDING_STOP_EVENT = 1 << 0; - const PENDING_CONT_EVENT = 1 << 1; - } -} - impl PartialEq for ProcessState { fn eq(&self, other: &u8) -> bool { self.bits() == *other @@ -362,91 +338,6 @@ impl Process { parent.children.lock().remove(&self.pid); } } - - /// Records a stop signal effect for waitpid reporting. - /// - /// Sets the PENDING_STOP_EVENT flag and records which signal caused the - /// stop. Atomically clears any pending continue event. - pub fn set_stop_signal(&self, signal: u8) { - let tg = self.tg.lock(); - tg.last_stop_signal.store(signal, Ordering::Release); - - // Atomically: clear CONT event, set STOP event - tg.wait_events.fetch_update( - Ordering::Release, - Ordering::Acquire, - |current_flags| { - Some( - (current_flags & !WaitEventFlags::PENDING_CONT_EVENT.bits()) - | WaitEventFlags::PENDING_STOP_EVENT.bits(), - ) - }, - ).ok(); - } - - /// Records a continue signal effect for waitpid reporting. - /// - /// Sets the PENDING_CONT_EVENT flag and clears the recorded stop signal. - /// Atomically clears any pending stop event. - pub fn set_cont_signal(&self) { - let tg = self.tg.lock(); - tg.last_stop_signal.store(0, Ordering::Release); - - // Atomically: clear STOP event, set CONT event - tg.wait_events.fetch_update( - Ordering::Release, - Ordering::Acquire, - |current_flags| { - Some( - (current_flags & !WaitEventFlags::PENDING_STOP_EVENT.bits()) - | WaitEventFlags::PENDING_CONT_EVENT.bits(), - ) - }, - ).ok(); - } - - /// Peeks at a pending stop signal event without consuming it. - /// - /// Returns the signal that caused the stop if there is an unreported stop event. - pub fn peek_pending_stop_event(&self) -> Option { - let tg = self.tg.lock(); - let flags = tg.wait_events.load(Ordering::Acquire); - - if (flags & WaitEventFlags::PENDING_STOP_EVENT.bits()) != 0 { - let signal = tg.last_stop_signal.load(Ordering::Acquire); - if signal != 0 { Some(signal) } else { None } - } else { - None - } - } - - /// Consumes (clears) the pending stop signal event. - pub fn consume_stop_event(&self) { - let tg = self.tg.lock(); - tg.last_stop_signal.store(0, Ordering::Release); - tg.wait_events.fetch_and( - !WaitEventFlags::PENDING_STOP_EVENT.bits(), - Ordering::Release, - ); - } - - /// Peeks at a pending continue signal event without consuming it. - /// - /// Returns true if there is an unreported continue event. - pub fn peek_pending_cont_event(&self) -> bool { - let tg = self.tg.lock(); - let flags = tg.wait_events.load(Ordering::Acquire); - (flags & WaitEventFlags::PENDING_CONT_EVENT.bits()) != 0 - } - - /// Consumes (clears) the pending continue signal event. - pub fn consume_cont_event(&self) { - let tg = self.tg.lock(); - tg.wait_events.fetch_and( - !WaitEventFlags::PENDING_CONT_EVENT.bits(), - Ordering::Release, - ); - } } impl fmt::Debug for Process { From e9c410d6e46858c10c2929e103fb2130b0c5693f Mon Sep 17 00:00:00 2001 From: Haoze Wu Date: Mon, 8 Dec 2025 12:41:41 +0800 Subject: [PATCH 5/7] fix: resolve bitflag comparison issue. Changes: - Remove PartialEq impl for ProcessState - Update process state comparison functions to use bitflags contains function - Update transition_to_x function logic for state safety --- src/process.rs | 65 +++++++++++++++++++++++++------------------------- 1 file changed, 33 insertions(+), 32 deletions(-) diff --git a/src/process.rs b/src/process.rs index 04a3f80..ff02723 100644 --- a/src/process.rs +++ b/src/process.rs @@ -23,7 +23,7 @@ pub(crate) struct ThreadGroup { } bitflags! { - #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] + #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) struct ProcessState: u8 { const RUNNING = 1 << 0; const STOPPED = 1 << 1; @@ -31,18 +31,6 @@ bitflags! { } } -impl PartialEq for ProcessState { - fn eq(&self, other: &u8) -> bool { - self.bits() == *other - } -} - -impl PartialEq for u8 { - fn eq(&self, other: &ProcessState) -> bool { - *self == other.bits() - } -} - /// A process. pub struct Process { pid: Pid, @@ -213,17 +201,20 @@ impl Process { impl Process { /// Returns `true` if the [`Process`] is a zombie process. pub fn is_zombie(&self) -> bool { - self.state.load(Ordering::Acquire) == ProcessState::ZOMBIE + 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 { - self.state.load(Ordering::Acquire) == ProcessState::RUNNING + let bits = self.state.load(Ordering::Acquire); + ProcessState::from_bits_truncate(bits).contains(ProcessState::RUNNING) } - /// Returns `true` if the [`Process`] is stopped. + /// Returns `true` if the [`Process`] is stopped. pub fn is_stopped(&self) -> bool { - self.state.load(Ordering::Acquire) == ProcessState::STOPPED + let bits = self.state.load(Ordering::Acquire); + ProcessState::from_bits_truncate(bits).contains(ProcessState::STOPPED) } /// Change the [`Process`] from Running to `Stopped`. @@ -244,10 +235,13 @@ impl Process { Ordering::Release, // Success: synchronize with is_stopped() Ordering::Relaxed, // Failure: no synchronization needed |curr| { - if curr == ProcessState::ZOMBIE.bits() { - None // Already zombie, don't transition + 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 { - Some(ProcessState::STOPPED.bits()) + flags.remove(ProcessState::RUNNING); + flags.insert(ProcessState::STOPPED); + Some(flags.bits()) } }, ); @@ -257,7 +251,7 @@ impl Process { /// /// This method atomically transitions the process state to RUNNING using /// CAS. The transition succeeds if and only if the current state is - /// `STOPPED`. + /// `STOPPED` and not `ZOMBIE`. /// /// # Memory Ordering /// @@ -271,10 +265,13 @@ impl Process { Ordering::Release, // Success: synchronize with is_running() Ordering::Relaxed, // Failure: no synchronization needed |curr| { - if curr != ProcessState::STOPPED.bits() { - None // Not stopped, don't transition + 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 { - Some(ProcessState::RUNNING.bits()) + flags.remove(ProcessState::STOPPED); + flags.insert(ProcessState::RUNNING); + Some(flags.bits()) } }, ); @@ -293,14 +290,18 @@ impl Process { /// 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) { - if self.is_zombie() { - return; - } - - self.state.store( - ProcessState::ZOMBIE.bits(), - Ordering::Release, // Synchronize with is_zombie() - ); + 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`]. From c40e98f04584c00a840dd71586cc6b87aaf39e9d Mon Sep 17 00:00:00 2001 From: Haoze Wu Date: Mon, 8 Dec 2025 15:44:27 +0800 Subject: [PATCH 6/7] test: Add test for process state management. Update group, process, and session test based on new state machine. Update cargo.toml --- Cargo.toml | 2 +- test_debug.rs | 42 +++++++++++++++ tests/group.rs | 2 + tests/process.rs | 135 ++++++++++++++++++++++++++++++++++++++++++++++- tests/session.rs | 7 +-- 5 files changed, 182 insertions(+), 6 deletions(-) create mode 100644 test_debug.rs diff --git a/Cargo.toml b/Cargo.toml index 556e9e1..c2e49e5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,7 +9,7 @@ license = "MIT OR Apache-2.0" repository = "https://github.com/Starry-OS/starry-process" [dependencies] -bitflags.workspace = true +bitflags = "2.6" kspin = "0.1" lazyinit = "0.2.1" weak-map = "0.1" diff --git a/test_debug.rs b/test_debug.rs new file mode 100644 index 0000000..f14bb20 --- /dev/null +++ b/test_debug.rs @@ -0,0 +1,42 @@ +use std::sync::atomic::{AtomicU8, Ordering}; +use bitflags::bitflags; + +bitflags\! { + #[derive(Debug, Clone, Copy, PartialEq, Eq)] + struct ProcessState: u8 { + const RUNNING = 1 << 0; + const STOPPED = 1 << 1; + const ZOMBIE = 1 << 2; + } +} + +fn main() { + let state = AtomicU8::new(ProcessState::RUNNING.bits()); + + // Simulate what happens during concurrent transitions + println\!("Initial: {:08b}", state.load(Ordering::Acquire)); + + // Thread 1: transition_to_stopped + let result = state.fetch_update(Ordering::Release, Ordering::Relaxed, |curr| { + let mut flags = ProcessState::from_bits_truncate(curr); + println\!("Thread 1 sees: {:?} ({:08b})", flags, curr); + if flags.contains(ProcessState::ZOMBIE) || \!flags.contains(ProcessState::RUNNING) { + None + } else { + flags.remove(ProcessState::RUNNING); + flags.insert(ProcessState::STOPPED); + println\!("Thread 1 setting to: {:?} ({:08b})", flags, flags.bits()); + Some(flags.bits()) + } + }); + + println\!("After T1: {:08b}, result: {:?}", state.load(Ordering::Acquire), result); + + // Check the state + let bits = state.load(Ordering::Acquire); + let flags = ProcessState::from_bits_truncate(bits); + println\!("Checking: running={}, stopped={}, zombie={}", + flags.contains(ProcessState::RUNNING), + flags.contains(ProcessState::STOPPED), + flags.contains(ProcessState::ZOMBIE)); +} 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 +} From ed9212288c12704185cd2a2aa545bcc1e88d1399 Mon Sep 17 00:00:00 2001 From: Haoze Wu Date: Mon, 8 Dec 2025 15:52:30 +0800 Subject: [PATCH 7/7] chore: update comments. remove test scratch files --- src/process.rs | 4 ++-- test_debug.rs | 42 ------------------------------------------ 2 files changed, 2 insertions(+), 44 deletions(-) delete mode 100644 test_debug.rs diff --git a/src/process.rs b/src/process.rs index ff02723..89971e3 100644 --- a/src/process.rs +++ b/src/process.rs @@ -217,7 +217,7 @@ impl Process { ProcessState::from_bits_truncate(bits).contains(ProcessState::STOPPED) } - /// Change the [`Process`] from Running to `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 @@ -277,7 +277,7 @@ impl Process { ); } - /// Change the [`Process`] from `Stopped` or `Running` to `Zombie`. + /// 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 diff --git a/test_debug.rs b/test_debug.rs deleted file mode 100644 index f14bb20..0000000 --- a/test_debug.rs +++ /dev/null @@ -1,42 +0,0 @@ -use std::sync::atomic::{AtomicU8, Ordering}; -use bitflags::bitflags; - -bitflags\! { - #[derive(Debug, Clone, Copy, PartialEq, Eq)] - struct ProcessState: u8 { - const RUNNING = 1 << 0; - const STOPPED = 1 << 1; - const ZOMBIE = 1 << 2; - } -} - -fn main() { - let state = AtomicU8::new(ProcessState::RUNNING.bits()); - - // Simulate what happens during concurrent transitions - println\!("Initial: {:08b}", state.load(Ordering::Acquire)); - - // Thread 1: transition_to_stopped - let result = state.fetch_update(Ordering::Release, Ordering::Relaxed, |curr| { - let mut flags = ProcessState::from_bits_truncate(curr); - println\!("Thread 1 sees: {:?} ({:08b})", flags, curr); - if flags.contains(ProcessState::ZOMBIE) || \!flags.contains(ProcessState::RUNNING) { - None - } else { - flags.remove(ProcessState::RUNNING); - flags.insert(ProcessState::STOPPED); - println\!("Thread 1 setting to: {:?} ({:08b})", flags, flags.bits()); - Some(flags.bits()) - } - }); - - println\!("After T1: {:08b}, result: {:?}", state.load(Ordering::Acquire), result); - - // Check the state - let bits = state.load(Ordering::Acquire); - let flags = ProcessState::from_bits_truncate(bits); - println\!("Checking: running={}, stopped={}, zombie={}", - flags.contains(ProcessState::RUNNING), - flags.contains(ProcessState::STOPPED), - flags.contains(ProcessState::ZOMBIE)); -}