feat: Implement process state machine for stop/continue in support for SIGSTOP, SIGCONT, and more waitpid features #4

TomGoh · 2025-11-17T09:54:46Z

Introduced ProcessState Enum:

Replaced the is_zombie boolean with a ProcessState enum that includes Running, Stopped, Continued, and Zombie states.
This allows the kernel to accurately track the lifecycle of a process, especially when it's affected by job control signals.

Enhanced State Management:

Added new methods to the Process struct to manage transitions between these states (e.g., stop_by_signal, continue_from_stop).
Introduced an acknowledgment mechanism (stopped_unacked, continued_unacked) to ensure that state changes like Stopped and Continued are reported only once to a waiting parent via waitpid.

Detailed Zombie Information:

The Zombie state now contains a ZombieInfo struct, which captures the process's exit code, whether it was terminated by a signal, and if a core dump occurred.
This enables waitpid to provide detailed and accurate status information to the parent process.

New exit_with_signal Method:

A dedicated exit_with_signal function was added to handle cases where a process is terminated by a signal, ensuring the correct exit code and signal information are recorded.

guoweikang · 2025-11-21T01:51:37Z

I'd like to add a license declaration to this PR as well.

src/process.rs

Improved process state management with composite state model and type safety. Key changes: - Introduced ProcessStateKind + ProcessStateFlags composite state - Encapsulated all transitions in ProcessState methods - Added atomic try_consume_* methods for waitpid event consumption - Introduced ExitCode type to replace magic number (128 + signal) - Extracted reaper_children helper to eliminate code duplication Addresses code review feedback from PR Starry-OS#4.

…d waitpid

Improved process state management with composite state model and type safety. Key changes: - Introduced ProcessStateKind + ProcessStateFlags composite state - Encapsulated all transitions in ProcessState methods - Added atomic try_consume_* methods for waitpid event consumption - Introduced ExitCode type to replace magic number (128 + signal) - Extracted reaper_children helper to eliminate code duplication Addresses code review feedback from PR Starry-OS#4.

TomGoh · 2025-11-24T06:55:02Z

I'd like to add a license declaration to this PR as well.

There is a license type declared in the Cargo.toml after rebase:

license = "MIT OR Apache-2.0"

src/process.rs

\- Fix license header in by removing placeholder authors. \- Fix to accept to handle both signal and ptrace continuations correctly without code duplication. \- Fix lint in by using a match expression. \- Add comprehensive integration tests in for: \- Stop/Continue lifecycle (SIGSTOP/SIGCONT). \- Ptrace stop/resume (ensuring no continued event is generated). \- Exit with signal (verifying exit code and signal info).

TomGoh · 2025-11-25T03:18:13Z

I have also updated some unit tests and integrated tests for these added process state machine methods in the tests directory.

AsakuraMizu · 2025-11-25T05:38:55Z

src/process.rs

@@ -1,13 +1,93 @@
+// SPDX-License-Identifier: Apache-2.0
+// Copyright (C) 2025 KylinSoft Co., Ltd. https://www.kylinos.cn/
+// Copyright (C) 2025 Azure-stars Azure_stars@126.com


No this crate was not written by @Azure-stars 🤣

My bad.
Now I would change the copyright declaration from Azure-stars to AsakuraMizu based on the author information in Cargo.toml. My apologies.

Copilot

Pull request overview

This PR implements a comprehensive process state machine to support SIGSTOP/SIGCONT signals and enhanced waitpid features. The implementation replaces a simple boolean is_zombie flag with a rich state system that tracks process lifecycle (Running, Stopped, Zombie) and provides event acknowledgment mechanisms for proper parent notification.

Key Changes

Introduced ProcessState with ProcessStateKind enum (Running/Stopped/Zombie) and ProcessStateFlags bitflags for event tracking
Added detailed zombie information (ZombieInfo) capturing exit codes, terminating signals, and core dump status
Implemented ptrace stop support with distinction between signal-stops and ptrace-stops

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
src/process.rs	Core implementation of the state machine with ProcessState, ProcessStateKind, ProcessStateFlags, ExitCode, ZombieInfo types; added state transition methods, stop/continue/ptrace APIs, and comprehensive module documentation
tests/process.rs	Added integration tests for stop/continue transitions, ptrace stops, and signal-based process termination
tests/session.rs	Minor import statement consolidation
src/lib.rs	Exported new public types (ProcessState, ZombieInfo)
Cargo.toml	Added bitflags dependency for state flag management
LICENSES	Added Apache License 2.0 text

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/process.rs

src/lib.rs

src/process.rs

…t code \- Correct license header in with correct author information. \- Export in . \- Remove redundant check in as it is implied by the state. \- Fix various typos in documentation comments (e.g., stoppade, ptrace.s). \- Correct documentation for to accurately reflect that it checks for unacked continuations.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/process.rs

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 12 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-25T06:46:04Z

src/process.rs

+        // has not been acked by its parent
+        const STOPPED_UNACKED = 1 << 0;
+        // A status of a process who has just continued but its continuation
+        // has not been acked by its parent


Inconsistent terminology: The comment uses "acked" (abbreviated form) instead of "acknowledged", which is used throughout the rest of the module documentation (e.g., lines 25, 57, 634, 656). For consistency and formality, use "acknowledged" instead of "acked".

Suggested change

// has not been acked by its parent

const STOPPED_UNACKED = 1 << 0;

// A status of a process who has just continued but its continuation

// has not been acked by its parent

// has not been acknowledged by its parent

const STOPPED_UNACKED = 1 << 0;

// A status of a process who has just continued but its continuation

// has not been acknowledged by its parent

Copilot · 2025-11-25T06:46:05Z

src/process.rs

+    pub fn transition_to_running(&mut self, with_continued_flag: bool) {
+        if let ProcessStateKind::Stopped { .. } = self.kind {
+            self.kind = ProcessStateKind::Running;
+            if with_continued_flag {
+                self.flags.insert(ProcessStateFlags::CONTINUED_UNACKED);
+            }
+            self.flags.remove(ProcessStateFlags::STOPPED_UNACKED);


The transition_to_running method only transitions from Stopped to Running, silently ignoring calls when the process is already Running or Zombie. This could mask bugs where the method is called on a process in an unexpected state.

For instance, if a process is already running and continue_from_stop is called, this silently does nothing, which might not be the intended behavior. Consider either:

Returning a Result or bool to indicate whether the transition succeeded

Adding an assertion or debug warning for invalid transitions

Documenting this silent-fail behavior more explicitly in the public API methods that call this

This would help callers detect incorrect usage patterns.

Suggested change

pub fn transition_to_running(&mut self, with_continued_flag: bool) {

if let ProcessStateKind::Stopped { .. } = self.kind {

self.kind = ProcessStateKind::Running;

if with_continued_flag {

self.flags.insert(ProcessStateFlags::CONTINUED_UNACKED);

}

self.flags.remove(ProcessStateFlags::STOPPED_UNACKED);

/// Returns `true` if the transition from `Stopped` to `Running` occurred,

/// or `false` if the process was not in the `Stopped` state and no transition was made.

pub fn transition_to_running(&mut self, with_continued_flag: bool) -> bool {

if let ProcessStateKind::Stopped { .. } = self.kind {

self.kind = ProcessStateKind::Running;

if with_continued_flag {

self.flags.insert(ProcessStateFlags::CONTINUED_UNACKED);

}

self.flags.remove(ProcessStateFlags::STOPPED_UNACKED);

true

} else {

false

Copilot · 2025-11-25T06:46:05Z

src/process.rs

+    /// including both the case which the process just stopped without acked by
+    /// its parent and already acked by its parent


The documentation states "the case which the process just stopped without acked" - this should be "acknowledged" not "acked" for consistency with other documentation in this module (e.g., line 25 uses "acknowledged", line 634 uses "acknowledged").

Additionally, "the case which" should be "the case where" or "the case in which" for better grammar.

Suggested change

/// including both the case which the process just stopped without acked by

/// its parent and already acked by its parent

/// including both the case where the process just stopped without being acknowledged by

/// its parent and the case where it has already been acknowledged by its parent

Copilot · 2025-11-25T06:46:05Z

src/process.rs

+            && state.flags.contains(ProcessStateFlags::CONTINUED_UNACKED)
+    }
+
+    /// Updating the status of a process continued from stoppage


Typo in method name and documentation: "Updating" should be "Updates" to match the imperative style used in other doc comments (e.g., "Terminates" at line 429, "Stops" at line 489, "Transitions" at line 629).

Suggested change

/// Updating the status of a process continued from stoppage

/// Updates the status of a process continued from stoppage

Copilot · 2025-11-25T06:46:06Z

src/process.rs

+/// `CONTINUED_UNACKED`).
+///
+/// For a `Stopped` process, if its stoppage has not been acked by its parent,
+/// i.e., the parent has not been notified for the child's stoppage, the


The documentation mentions "the parent has not been notified for the child's stoppage" - this should be "the parent has not been notified of the child's stoppage" for correct grammar. The preposition "of" is more appropriate than "for" in this context.

Suggested change

/// i.e., the parent has not been notified for the child's stoppage, the

/// i.e., the parent has not been notified of the child's stoppage, the

Copilot · 2025-11-25T06:46:06Z

src/process.rs

+    pub struct ProcessStateFlags: u8 {
+        // A status of a process who has stopped but its stoppage
+        // has not been acked by its parent
+        const STOPPED_UNACKED = 1 << 0;
+        // A status of a process who has just continued but its continuation
+        // has not been acked by its parent
+        const CONTINUED_UNACKED = 1 << 1;
+    }
+}


ProcessStateFlags is marked as pub but is not exported in src/lib.rs. This makes it inaccessible to external users. Similar to ProcessStateKind, if this is intended to be internal only, it should be marked as pub(crate) instead of pub to clarify the design intent.

Copilot · 2025-11-25T06:46:07Z

src/process.rs

+        // A status of a process who has just continued but its continuation
+        // has not been acked by its parent
+        const CONTINUED_UNACKED = 1 << 1;


Inconsistent terminology: The comment uses "acked" (abbreviated form) instead of "acknowledged". For consistency with the rest of the module documentation, use "acknowledged" instead of "acked".

Copilot · 2025-11-25T06:46:07Z

src/process.rs

+
+        self.kind = ProcessStateKind::Stopped { signal, ptraced };
+        self.flags.insert(ProcessStateFlags::STOPPED_UNACKED);


The transition_to_stopped method always sets the STOPPED_UNACKED flag even if the process is already stopped with a different signal. This means that calling stop_by_signal multiple times will keep resetting the unacked flag, potentially causing the same stop to be reported multiple times via waitpid.

For example:

Process receives SIGSTOP (signal 19) → sets STOPPED_UNACKED

Parent hasn't called waitpid yet

Process receives SIGTSTP (signal 20) → sets STOPPED_UNACKED again (overwrites the signal)

This might be intentional behavior, but it should be documented. Alternatively, consider whether the flag should only be set on the first stop transition, not on subsequent re-stops.

Suggested change

self.kind = ProcessStateKind::Stopped { signal, ptraced };

self.flags.insert(ProcessStateFlags::STOPPED_UNACKED);

// Only transition to stopped if not already stopped

match self.kind {

ProcessStateKind::Stopped { .. } => {

// Already stopped, do not reset STOPPED_UNACKED or overwrite signal

return;

}

_ => {

self.kind = ProcessStateKind::Stopped { signal, ptraced };

self.flags.insert(ProcessStateFlags::STOPPED_UNACKED);

}

}

Copilot · 2025-11-25T06:46:07Z

src/process.rs

+
+/// The full process state machine.
+#[derive(Debug, Clone, Copy)]
+pub struct ProcessState {


ProcessState is marked as pub but is not exported in src/lib.rs, making it inaccessible to external users. The struct's fields are private, and only internal state transition methods are implemented on it.

If ProcessState is meant to be an internal implementation detail (which appears to be the case based on the API design), it should be marked as pub(crate) instead of pub to make this clear and prevent accidental public exposure.

Suggested change

pub struct ProcessState {

pub(crate) struct ProcessState {

Copilot · 2025-11-25T06:46:08Z

src/process.rs

+    /// including both the case which the process just stopped without acked by
+    /// its parent and already acked by its parent
+    pub fn is_stopped(&self) -> bool {
+        let state = self.state.lock();
+        matches!(state.kind, ProcessStateKind::Stopped { .. })
+    }
+
+    /// Check whether the process has continued from the stoppage,
+    /// but its continuation has not been acked by its parent


The documentation states "acked by its parent" - for consistency with the rest of the module documentation, this should use "acknowledged" instead of "acked". The term "acknowledged" is used in other doc comments (lines 25, 57, 634, 656) and is more formal.

Suggested change

/// including both the case which the process just stopped without acked by

/// its parent and already acked by its parent

pub fn is_stopped(&self) -> bool {

let state = self.state.lock();

matches!(state.kind, ProcessStateKind::Stopped { .. })

}

/// Check whether the process has continued from the stoppage,

/// but its continuation has not been acked by its parent

/// including both the case which the process just stopped without being acknowledged by

/// its parent and already acknowledged by its parent

pub fn is_stopped(&self) -> bool {

let state = self.state.lock();

matches!(state.kind, ProcessStateKind::Stopped { .. })

}

/// Check whether the process has continued from the stoppage,

/// but its continuation has not been acknowledged by its parent

TomGoh · 2025-11-25T08:27:35Z

For the ZombieInfo struct, a potential better approach IMO could be taking only vanilla exit code in Zombie information and signal:

/// Information about a zombie (terminated) process.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct ZombieInfo {
    /// The exit code value passed to exit().
    pub exit_code: i32,
    /// The signal that terminated the process, if any.
    pub signal: Option<i32>,
    /// Whether a core dump was produced.
    pub core_dumped: bool,
}

and implement a function to encode these information, along with those inside ProcessState into a POSIX-compatible i32:

impl ProcessState {
    ...
    /// Returns the POSIX wait status word for the current state if there is a
    /// reportable event.
    ///
    /// - Stopped + STOPPED_UNACKED: Returns `(signal << 8) | 0x7f`
    /// - Running + CONTINUED_UNACKED: Returns `0xffff`
    /// - Zombie: Returns encoded exit status per POSIX
    /// - Otherwise: Returns `None`
    pub fn wait_status(&self) -> Option<i32> {
        if self.flags.contains(ProcessStateFlags::STOPPED_UNACKED) {
            if let ProcessStateKind::Stopped { signal, .. } = self.kind {
                return Some((signal << 8) | 0x7f);
            }
        }

        if self.flags.contains(ProcessStateFlags::CONTINUED_UNACKED) {
            if let ProcessStateKind::Running = self.kind {
                return Some(0xffff);
            }
        }

        if let ProcessStateKind::Zombie { info } = self.kind {
            if let Some(sig) = info.signal {
                // WIFSIGNALED: Bits 0-6 are signal, Bit 7 is core dump.
                let core_bit = if info.core_dumped { 0x80 } else { 0 };
                return Some((sig & 0x7f) | core_bit);
            } else {
                // WIFEXITED: Bits 8-15 are exit code.
                return Some((info.exit_code & 0xff) << 8);
            }
        }

        None
    }
}

And thus, for the Process we may have:

impl Process {
    ...
    /// Returns the POSIX wait status word for the current state if there is a
    /// reportable event.
    pub fn wait_status(&self) -> Option<i32> {
        self.state.lock().wait_status()
    }
}

The reason why we may do so is that we will leave all the wait status related stuff to wait system call, and taking the original status information of a process here.
Would this be an improvement from the current kinda redundant ZombieInfo?

…for future improvements.

guoweikang reviewed Nov 21, 2025

View reviewed changes

src/process.rs Outdated Show resolved Hide resolved

src/process.rs Outdated Show resolved Hide resolved

src/process.rs Outdated Show resolved Hide resolved

TomGoh requested a review from guoweikang November 24, 2025 06:04

TomGoh added 4 commits November 24, 2025 14:46

feat: add process state management in support of SIGSTOP, SIGCONT, an…

ddeaad3

…d waitpid

ci: fix clippy errors

e1080dd

feat: Update daat structure support for ptrace

239d21a

TomGoh force-pushed the feat/sigstop-cont branch from 0991c5b to 8391dc5 Compare November 24, 2025 06:46

guoweikang approved these changes Nov 25, 2025

View reviewed changes

guoweikang reviewed Nov 25, 2025

View reviewed changes

src/process.rs Show resolved Hide resolved

TomGoh added 2 commits November 25, 2025 10:18

license: Updated licenses for starry-process. Format with cargo fmt

4bc75de

AsakuraMizu reviewed Nov 25, 2025

View reviewed changes

AsakuraMizu requested review from Mivik and Copilot November 25, 2025 05:44

TomGoh had a problem deploying to github-pages November 25, 2025 05:44 — with GitHub Actions Failure

Copilot started reviewing on behalf of AsakuraMizu November 25, 2025 05:44 View session

Copilot finished reviewing on behalf of AsakuraMizu November 25, 2025 05:49

Copilot AI reviewed Nov 25, 2025

View reviewed changes

TomGoh requested review from AsakuraMizu and Copilot November 25, 2025 06:26

Copilot started reviewing on behalf of TomGoh November 25, 2025 06:26 View session

Copilot finished reviewing on behalf of TomGoh November 25, 2025 06:29

Copilot AI reviewed Nov 25, 2025

View reviewed changes

src/process.rs Outdated Show resolved Hide resolved

src/process.rs Outdated Show resolved Hide resolved

TomGoh added 2 commits November 25, 2025 14:37

fix: update comments

4c371b6

fix: update comments

e1f70e9

TomGoh requested a review from Copilot November 25, 2025 06:39

Copilot started reviewing on behalf of TomGoh November 25, 2025 06:39 View session

Copilot finished reviewing on behalf of TomGoh November 25, 2025 06:44

Copilot AI reviewed Nov 25, 2025

View reviewed changes

feat: updated Process State management to preserve raw info and todo …

ad525db

…for future improvements.

TomGoh closed this Dec 2, 2025

		/// including both the case which the process just stopped without acked by
		/// its parent and already acked by its parent

	/// Updating the status of a process continued from stoppage
	/// Updates the status of a process continued from stoppage

	/// i.e., the parent has not been notified for the child's stoppage, the
	/// i.e., the parent has not been notified of the child's stoppage, the


		self.kind = ProcessStateKind::Stopped { signal, ptraced };
		self.flags.insert(ProcessStateFlags::STOPPED_UNACKED);

feat: Implement process state machine for stop/continue in support for SIGSTOP, SIGCONT, and more waitpid features #4

feat: Implement process state machine for stop/continue in support for SIGSTOP, SIGCONT, and more waitpid features #4

Uh oh!

Conversation

TomGoh commented Nov 17, 2025

Uh oh!

guoweikang commented Nov 21, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TomGoh commented Nov 24, 2025

Uh oh!

Uh oh!

TomGoh commented Nov 25, 2025

Uh oh!

AsakuraMizu Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

TomGoh Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

TomGoh commented Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

TomGoh commented Nov 25, 2025 •

edited

Loading