Implement SafeProcessHandle.Kill and Signal

[Copilot]

Adds Kill() and Signal(PosixSignal) to SafeProcessHandle, enabling callers to terminate or signal a process via a handle without going through Process.

Description

Public API (SafeProcessHandle.cs + ref assembly)

• Kill() — sends a request to the OS to terminate the process; silently no-ops if already exited (matches Process.Kill semantics). On Windows, the handle must have PROCESS_TERMINATE access.
• Signal(PosixSignal) — sends an arbitrary signal; returns false if process no longer exists (or never existed), true on delivery, throws PlatformNotSupportedException for unsupported signals. On Windows, the handle must have PROCESS_TERMINATE access.

Both methods validate the handle (throw InvalidOperationException on invalid handle), carry the same [UnsupportedOSPlatform("ios/tvos")] / [SupportedOSPlatform("maccatalyst")] annotations as Start, and throw PlatformNotSupportedException at runtime on iOS/tvOS (matching the ProcessUtils.PlatformDoesNotSupportProcessStartAndKill pattern used by Process.Kill).

Windows (SafeProcessHandle.Windows.cs)

• SignalCore: only SIGKILL is supported (maps to TerminateProcess), matching PosixSignalRegistration.Create exception behavior for unsupported signals. Retrieves the error code via Marshal.GetLastWin32Error() before any checks and only constructs Win32Exception(errorCode) when actually throwing. Detects already-exited processes via ERROR_ACCESS_DENIED + GetExitCodeProcess returning non-STILL_ACTIVE.
• No separate KillCore; Kill() calls SignalCore(PosixSignal.SIGKILL) directly from the shared file.

Unix (SafeProcessHandle.Unix.cs)

• SignalCore: checks ProcessUtils.PlatformDoesNotSupportProcessStartAndKill first (throws PlatformNotSupportedException on iOS/tvOS); uses Interop.Sys.GetPlatformSignalNumber(signal) to convert the managed PosixSignal to its platform-native signal number (throws PlatformNotSupportedException if unsupported, matching PosixSignalRegistration.Create behavior); passes the native signal number directly to Interop.Sys.Kill; returns false on ESRCH; uses Interop.Sys.GetLastErrorInfo() and passes errorInfo.RawErrno to the Win32Exception constructor.
• No separate KillCore; Kill() calls SignalCore(PosixSignal.SIGKILL) directly from the shared file.

Native (pal_process.c / pal_signal.c)

• SystemNative_Kill simplified to call kill(pid, signal) directly with no PAL mapping switch — it now takes the platform-native signal number, consistent with other System.Native APIs such as SystemNative_EnablePosixSignalHandling. The old Signals PAL enum (PAL_NONE, PAL_SIGKILL, PAL_SIGSTOP) has been removed from pal_process.h.
• SystemNative_GetPlatformSIGSTOP() moved to pal_signal.h/pal_signal.c (right below SystemNative_GetPlatformSignalNumber) since it is signal-related. Dummy implementations returning 0 are provided in pal_signal_wasm.c for both SystemNative_GetPlatformSIGSTOP and SystemNative_GetPlatformSignalNumber for BROWSER and WASI platforms.
• entrypoints.c: moved the DllImportEntry(SystemNative_GetPlatformSIGSTOP) registration from the process group (between SystemNative_Kill and SystemNative_GetPid) to the signal group (alongside SystemNative_GetPlatformSignalNumber, SystemNative_EnablePosixSignalHandling, etc.), consistent with the function's new home in pal_signal.c/pal_signal.h.

Process.Kill refactoring (Process.Windows.cs)

• Process.Kill() on Windows now delegates to SafeProcessHandle.Kill() after obtaining the handle, eliminating the duplicated TerminateProcess + error-handling logic.

Interop (Interop.Kill.cs / Interop.PosixSignal.cs)

• Removed the Signals managed enum; Interop.Sys.Kill now takes a plain int signal parameter (platform-native number).
• Interop.Sys.GetPlatformSIGSTOP() P/Invoke moved to Interop.PosixSignal.cs alongside GetPlatformSignalNumber, keeping all signal-number interop co-located.
• All existing callers updated: Process.Unix.cs uses GetPlatformSignalNumber(PosixSignal.SIGKILL) and GetPlatformSIGSTOP(); ProcessWaitState.Unix.cs and ProcessManager.Unix.cs pass 0 directly for the "probe" call.

Project / interop

• Added Interop.PosixSignal.cs to the Unix ItemGroup in System.Diagnostics.Process.csproj to expose GetPlatformSignalNumber and GetPlatformSIGSTOP in the main assembly.

Tests

• SafeProcessHandleTests.cs: Invalid handle → InvalidOperationException; Kill / Signal(SIGKILL) on running process terminates it; Kill on exited process does not throw; Signal(SIGKILL) returns false on exited process; Windows: non-SIGKILL signal → PlatformNotSupportedException; Unix: Signal(SIGTERM) on running process returns true and terminates it; Windows: Kill_HandleWithoutTerminatePermission_ThrowsWin32Exception — opens a handle with only PROCESS_QUERY_LIMITED_INFORMATION and verifies Kill() throws Win32Exception.
• ProcessTests.Unix.cs: Updated ChildProcess_WithParentSignalHandler_CanReceiveSignals to use SafeProcessHandle.Signal instead of the raw SendSignal helper. Refactored SendSignal helper to accept a Process parameter — Unix calls process.SafeHandle.Signal(signal) directly (simplified to a single-line Assert.True). Removed the class-level SIGKILL constant and kill P/Invoke declaration; Kill_ExitedNonChildProcess_DoesNotThrow now uses SafeHandle.Signal(PosixSignal.SIGKILL) and Process.HasExited.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

[adamsitnik]

@copilot address my feedback, also do search for other places in the repo where the new Signal API could be used

[Copilot]

@copilot address my feedback, also do search for other places in the repo where the new Signal API could be used

All feedback addressed in 7c03c8c. For other usages: the main candidate in the codebase is the SendSignal helper in tests (ProcessTests.cs:173, :246, :256, :260) which sends signals via kill() directly. However, those tests operate against a Process instance using its Id, while our Signal API requires a SafeProcessHandle — on Unix the handle's ProcessId is only set when the process was started via SafeProcessHandle.Start (or Process.Start). The ChildProcess_WithParentSignalHandler_CanReceiveSignals test was the natural fit since it already uses RemoteInvokeHandle with a started Process, so Process.SafeHandle.Signal() works correctly there.

[Copilot]

Pull request overview

Adds process-termination and signal-sending capabilities directly to SafeProcessHandle, enabling callers to manage processes via handles without going through Process, with platform-specific implementations for Windows and Unix plus corresponding test updates.

Changes:

• Added SafeProcessHandle.Kill() and SafeProcessHandle.Signal(PosixSignal) public APIs (with ref-assembly updates).
• Implemented SignalCore on Windows (via TerminateProcess) and Unix (via kill(2) + GetPlatformSignalNumber).
• Added/updated tests to validate new APIs and switched an existing Unix test to use SafeProcessHandle.Signal.

Reviewed changes

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

Show a summary per file

File	Description
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs	Introduces the new public Kill/Signal APIs and validation path.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs	Implements Windows SignalCore mapping SIGKILL to TerminateProcess with exited-process detection.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs	Implements Unix SignalCore using GetPlatformSignalNumber + kill, returning false on ESRCH.
src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj	Adds Interop.PosixSignal.cs to the Unix compile item group.
src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs	Updates the public ref surface for SafeProcessHandle with Kill/Signal.
src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs	Adds unit tests covering invalid-handle, running-process, exited-process, and platform-specific behaviors.
src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs	Updates signal-delivery test to use SafeProcessHandle.Signal instead of a raw P/Invoke helper.

[github-actions]

🤖 Copilot Code Review — PR #126313

Note

This review was generated by GitHub Copilot using Claude Opus 4.6.

Holistic Assessment

Motivation: Justified. This is the next step in the approved API expansion (#125838, api-approved by @bartonjs on 2026-03-24). Following the merged Start/ProcessId PR (#126192), Kill and Signal are the natural additions to enable callers to manage process lifecycle via SafeProcessHandle without requiring a Process object. The motivation is clear and real.

Approach: Sound. The implementation follows the approved API shape exactly, builds on the existing SafeProcessHandle partial class pattern, and properly separates platform-specific behavior. The native code simplification (removing the Signals PAL enum and having SystemNative_Kill accept platform-native signal numbers directly) is a good cleanup that aligns with how other System.Native APIs like SystemNative_EnablePosixSignalHandling work. The decision to have Kill() delegate to SignalCore(SIGKILL) keeps the code DRY while the void return type correctly matches the "fire-and-forget" semantics of Process.Kill().

Summary: ✅ LGTM. The implementation matches the approved API shape, all prior maintainer review feedback (from @jkotas, @stephentoub, @adamsitnik) has been addressed through multiple iterations, and the code is correct and well-tested. No blocking issues found. Two minor follow-up observations below.

---

Detailed Findings

✅ API Approval Verification — Matches approved shape

The approved API from #125838 (comment) by @bartonjs specifies:

publicpartialclassSafeProcessHandle{publicvoidKill();publicboolSignal(PosixSignalsignal);}

The ref assembly implementation matches exactly:

• Kill() → void return ✅
• Signal(PosixSignal signal) → bool return ✅
• Parameter name signal matches ✅
• No extra unapproved public API ✅
• No missing approved API from this PR's scope ✅
• Platform attributes ([UnsupportedOSPlatform("ios")], [UnsupportedOSPlatform("tvos")], [SupportedOSPlatform("maccatalyst")]) are consistent with Start ✅

✅ Correctness — Edge cases handled properly

Both platform implementations handle key edge cases correctly:

• Already-exited process: Windows detects via ERROR_ACCESS_DENIED + GetExitCodeProcess returning non-STILL_ACTIVE; Unix detects via ESRCH. Both return false from SignalCore, which Kill() silently discards (matching Process.Kill semantics).
• Unsupported signals: Windows throws PlatformNotSupportedException for non-SIGKILL (matching PosixSignalRegistration.Create behavior); Unix throws PlatformNotSupportedException when GetPlatformSignalNumber returns 0.
• iOS/tvOS: Unix SignalCore checks ProcessUtils.PlatformDoesNotSupportProcessStartAndKill first, matching the pattern in Process.Kill().
• Error code capture: Windows captures Marshal.GetLastWin32Error() immediately after TerminateProcess fails (before any other calls); Unix uses Interop.Sys.GetLastErrorInfo() and passes RawErrno to Win32Exception.
• Process.Kill delegation: On Windows, Process.Kill() now correctly delegates to SafeProcessHandle.Kill(), reducing code duplication.

✅ Native Code — Clean simplification

• SystemNative_Kill stripped to a thin wrapper around kill(pid, signal) — all PAL mapping removed since callers now pass platform-native signal numbers. This is consistent with SystemNative_EnablePosixSignalHandling and other System.Native APIs.
• SystemNative_GetPlatformSIGSTOP() correctly placed in pal_signal.c/pal_signal.h (next to GetPlatformSignalNumber), with a WASM stub returning 0.
• The old Signals PAL enum (PAL_NONE, PAL_SIGKILL, PAL_SIGSTOP) removed cleanly.

✅ Test Coverage — Comprehensive

Tests cover all key scenarios:

Test	Scenario
Kill_InvalidHandle_ThrowsInvalidOperationException	Error path — invalid handle
Signal_InvalidHandle_ThrowsInvalidOperationException	Error path — invalid handle
Kill_RunningProcess_Terminates	Happy path — kill via SafeProcessHandle.Start
Kill_AlreadyExited_DoesNotThrow	Already-exited process — no exception
Signal_SIGKILL_RunningProcess_ReturnsTrue	Signal delivery confirmation
Signal_SIGKILL_AlreadyExited_ReturnsFalse	Already-exited returns false
Signal_NonSIGKILL_OnWindows_ThrowsPlatformNotSupportedException	Windows platform limitation
Signal_SIGTERM_RunningProcess_ReturnsTrue	Unix signal support
Kill_HandleWithoutTerminatePermission_ThrowsWin32Exception	Windows permissions

Existing tests also updated to use the new API (Kill_ExitedNonChildProcess_DoesNotThrow, ChildProcess_WithParentSignalHandler_CanReceiveSignals).

💡 entrypoints.c — Minor organizational note (follow-up)

SystemNative_GetPlatformSIGSTOP was moved to pal_signal.c/pal_signal.h (alongside GetPlatformSignalNumber), but its entrypoint entry in entrypoints.c (line 219) remains in the pal_process group between SystemNative_Kill and SystemNative_GetPid. The other signal entries (GetPlatformSignalNumber, EnablePosixSignalHandling, etc.) are at lines 286-291. Moving the entrypoint to the signal group would be more consistent, but this is purely organizational and not a blocker.

💡 Review feedback fully addressed

All prior review comments from @jkotas, @stephentoub, and @adamsitnik have been addressed across 9+ follow-up commits, including:

• Removing Unix implementation details from Kill() docs (stephentoub)
• Adding PROCESS_TERMINATE to docs + permission test (jkotas/adamsitnik)
• Delegating Process.Kill() on Windows to SafeProcessHandle.Kill() (jkotas)
• Adding PlatformDoesNotSupportProcessStartAndKill check (jkotas)
• Simplifying native SystemNative_Kill to pass-through (jkotas)
• Updating SendSignal test helper to accept Process (jkotas/adamsitnik)
• Using short Win32Exception name in XML docs (jkotas)

Generated by Code Review for issue #126313 · ◷

[jkotas]

SystemNative_GetPlatformSIGSTOP was moved to pal_signal.c/pal_signal.h (alongside GetPlatformSignalNumber), but its entrypoint entry in entrypoints.c (line 219) remains in the pal_process group between SystemNative_Kill and SystemNative_GetPid. The other signal entries (GetPlatformSignalNumber, EnablePosixSignalHandling, etc.) are at lines 286-291. Moving the entrypoint to the signal group would be more consistent

@copilot Address this feedback

[Copilot]

SystemNative_GetPlatformSIGSTOP was moved to pal_signal.c/pal_signal.h (alongside GetPlatformSignalNumber), but its entrypoint entry in entrypoints.c (line 219) remains in the pal_process group betw...

Done in 493466c — moved DllImportEntry(SystemNative_GetPlatformSIGSTOP) from the process group (after SystemNative_Kill) to the signal group (after SystemNative_GetPlatformSignalNumber).

[Copilot]

Adds Kill() and Signal(PosixSignal) to SafeProcessHandle, enabling callers to terminate or signal a process via a handle without going through Process.

Description

Public API (SafeProcessHandle.cs + ref assembly)

• Kill() — sends a request to the OS to terminate the process; silently no-ops if already exited (matches Process.Kill semantics). On Windows, the handle must have PROCESS_TERMINATE access.
• Signal(PosixSignal) — sends an arbitrary signal; returns false if process no longer exists (or never existed), true on delivery, throws PlatformNotSupportedException for unsupported signals. On Windows, the handle must have PROCESS_TERMINATE access.

Both methods validate the handle (throw InvalidOperationException on invalid handle), carry the same [UnsupportedOSPlatform("ios/tvos")] / [SupportedOSPlatform("maccatalyst")] annotations as Start, and throw PlatformNotSupportedException at runtime on iOS/tvOS (matching the ProcessUtils.PlatformDoesNotSupportProcessStartAndKill pattern used by Process.Kill).

Windows (SafeProcessHandle.Windows.cs)

• SignalCore: only SIGKILL is supported (maps to TerminateProcess), matching PosixSignalRegistration.Create exception behavior for unsupported signals. Retrieves the error code via Marshal.GetLastWin32Error() before any checks and only constructs Win32Exception(errorCode) when actually throwing. Detects already-exited processes via ERROR_ACCESS_DENIED + GetExitCodeProcess returning non-STILL_ACTIVE.
• No separate KillCore; Kill() calls SignalCore(PosixSignal.SIGKILL) directly from the shared file.

Unix (SafeProcessHandle.Unix.cs)

• SignalCore: checks ProcessUtils.PlatformDoesNotSupportProcessStartAndKill first (throws PlatformNotSupportedException on iOS/tvOS); uses Interop.Sys.GetPlatformSignalNumber(signal) to convert the managed PosixSignal to its platform-native signal number (throws PlatformNotSupportedException if unsupported, matching PosixSignalRegistration.Create behavior); passes the native signal number directly to Interop.Sys.Kill; returns false on ESRCH; uses Interop.Sys.GetLastErrorInfo() and passes errorInfo.RawErrno to the Win32Exception constructor.
• No separate KillCore; Kill() calls SignalCore(PosixSignal.SIGKILL) directly from the shared file.

Native (pal_process.c / pal_signal.c)

• SystemNative_Kill simplified to call kill(pid, signal) directly with no PAL mapping switch — it now takes the platform-native signal number, consistent with other System.Native APIs such as SystemNative_EnablePosixSignalHandling. The old Signals PAL enum (PAL_NONE, PAL_SIGKILL, PAL_SIGSTOP) has been removed from pal_process.h.
• SystemNative_GetPlatformSIGSTOP() moved to pal_signal.h/pal_signal.c (right below SystemNative_GetPlatformSignalNumber) since it is signal-related. Dummy implementations returning 0 are provided in pal_signal_wasm.c for both SystemNative_GetPlatformSIGSTOP and SystemNative_GetPlatformSignalNumber for BROWSER and WASI platforms.
• entrypoints.c: moved the DllImportEntry(SystemNative_GetPlatformSIGSTOP) registration from the process group (between SystemNative_Kill and SystemNative_GetPid) to the signal group (alongside SystemNative_GetPlatformSignalNumber, SystemNative_EnablePosixSignalHandling, etc.), consistent with the function's new home in pal_signal.c/pal_signal.h.

Process.Kill refactoring (Process.Windows.cs)

• Process.Kill() on Windows now delegates to SafeProcessHandle.Kill() after obtaining the handle, eliminating the duplicated TerminateProcess + error-handling logic.

Interop (Interop.Kill.cs / Interop.PosixSignal.cs)

• Removed the Signals managed enum; Interop.Sys.Kill now takes a plain int signal parameter (platform-native number).
• Interop.Sys.GetPlatformSIGSTOP() P/Invoke moved to Interop.PosixSignal.cs alongside GetPlatformSignalNumber, keeping all signal-number interop co-located.
• All existing callers updated: Process.Unix.cs uses GetPlatformSignalNumber(PosixSignal.SIGKILL) and GetPlatformSIGSTOP(); ProcessWaitState.Unix.cs and ProcessManager.Unix.cs pass 0 directly for the "probe" call.

Project / interop

• Added Interop.PosixSignal.cs to the Unix ItemGroup in System.Diagnostics.Process.csproj to expose GetPlatformSignalNumber and GetPlatformSIGSTOP in the main assembly.

Tests

• SafeProcessHandleTests.cs: Invalid handle → InvalidOperationException; Kill / Signal(SIGKILL) on running process terminates it; Kill on exited process does not throw; Signal(SIGKILL) returns false on exited process; Windows: non-SIGKILL signal → PlatformNotSupportedException; Unix: Signal(SIGTERM) on running process returns true and terminates it; Windows: Kill_HandleWithoutTerminatePermission_ThrowsWin32Exception — opens a handle with only PROCESS_QUERY_LIMITED_INFORMATION and verifies Kill() throws Win32Exception.
• ProcessTests.Unix.cs: Updated ChildProcess_WithParentSignalHandler_CanReceiveSignals to use SafeProcessHandle.Signal instead of the raw SendSignal helper. Refactored SendSignal helper to accept a Process parameter — Unix calls process.SafeHandle.Signal(signal) directly (simplified to a single-line Assert.True). Removed the class-level SIGKILL constant and kill P/Invoke declaration; Kill_ExitedNonChildProcess_DoesNotThrow now uses SafeHandle.Signal(PosixSignal.SIGKILL) and Process.HasExited.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

[adamsitnik]

@copilot address my feedback, also do search for other places in the repo where the new Signal API could be used

[Copilot]

@copilot address my feedback, also do search for other places in the repo where the new Signal API could be used

All feedback addressed in 7c03c8c. For other usages: the main candidate in the codebase is the SendSignal helper in tests (ProcessTests.cs:173, :246, :256, :260) which sends signals via kill() directly. However, those tests operate against a Process instance using its Id, while our Signal API requires a SafeProcessHandle — on Unix the handle's ProcessId is only set when the process was started via SafeProcessHandle.Start (or Process.Start). The ChildProcess_WithParentSignalHandler_CanReceiveSignals test was the natural fit since it already uses RemoteInvokeHandle with a started Process, so Process.SafeHandle.Signal() works correctly there.

[Copilot]

Pull request overview

Adds process-termination and signal-sending capabilities directly to SafeProcessHandle, enabling callers to manage processes via handles without going through Process, with platform-specific implementations for Windows and Unix plus corresponding test updates.

Changes:

• Added SafeProcessHandle.Kill() and SafeProcessHandle.Signal(PosixSignal) public APIs (with ref-assembly updates).
• Implemented SignalCore on Windows (via TerminateProcess) and Unix (via kill(2) + GetPlatformSignalNumber).
• Added/updated tests to validate new APIs and switched an existing Unix test to use SafeProcessHandle.Signal.

Reviewed changes

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

Show a summary per file

File	Description
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs	Introduces the new public Kill/Signal APIs and validation path.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs	Implements Windows SignalCore mapping SIGKILL to TerminateProcess with exited-process detection.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs	Implements Unix SignalCore using GetPlatformSignalNumber + kill, returning false on ESRCH.
src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj	Adds Interop.PosixSignal.cs to the Unix compile item group.
src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs	Updates the public ref surface for SafeProcessHandle with Kill/Signal.
src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs	Adds unit tests covering invalid-handle, running-process, exited-process, and platform-specific behaviors.
src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs	Updates signal-delivery test to use SafeProcessHandle.Signal instead of a raw P/Invoke helper.

[github-actions]

🤖 Copilot Code Review — PR #126313

Note

This review was generated by GitHub Copilot using Claude Opus 4.6.

Holistic Assessment

Motivation: Justified. This is the next step in the approved API expansion (#125838, api-approved by @bartonjs on 2026-03-24). Following the merged Start/ProcessId PR (#126192), Kill and Signal are the natural additions to enable callers to manage process lifecycle via SafeProcessHandle without requiring a Process object. The motivation is clear and real.

Approach: Sound. The implementation follows the approved API shape exactly, builds on the existing SafeProcessHandle partial class pattern, and properly separates platform-specific behavior. The native code simplification (removing the Signals PAL enum and having SystemNative_Kill accept platform-native signal numbers directly) is a good cleanup that aligns with how other System.Native APIs like SystemNative_EnablePosixSignalHandling work. The decision to have Kill() delegate to SignalCore(SIGKILL) keeps the code DRY while the void return type correctly matches the "fire-and-forget" semantics of Process.Kill().

Summary: ✅ LGTM. The implementation matches the approved API shape, all prior maintainer review feedback (from @jkotas, @stephentoub, @adamsitnik) has been addressed through multiple iterations, and the code is correct and well-tested. No blocking issues found. Two minor follow-up observations below.

---

Detailed Findings

✅ API Approval Verification — Matches approved shape

The approved API from #125838 (comment) by @bartonjs specifies:

publicpartialclassSafeProcessHandle{publicvoidKill();publicboolSignal(PosixSignalsignal);}

The ref assembly implementation matches exactly:

• Kill() → void return ✅
• Signal(PosixSignal signal) → bool return ✅
• Parameter name signal matches ✅
• No extra unapproved public API ✅
• No missing approved API from this PR's scope ✅
• Platform attributes ([UnsupportedOSPlatform("ios")], [UnsupportedOSPlatform("tvos")], [SupportedOSPlatform("maccatalyst")]) are consistent with Start ✅

✅ Correctness — Edge cases handled properly

Both platform implementations handle key edge cases correctly:

• Already-exited process: Windows detects via ERROR_ACCESS_DENIED + GetExitCodeProcess returning non-STILL_ACTIVE; Unix detects via ESRCH. Both return false from SignalCore, which Kill() silently discards (matching Process.Kill semantics).
• Unsupported signals: Windows throws PlatformNotSupportedException for non-SIGKILL (matching PosixSignalRegistration.Create behavior); Unix throws PlatformNotSupportedException when GetPlatformSignalNumber returns 0.
• iOS/tvOS: Unix SignalCore checks ProcessUtils.PlatformDoesNotSupportProcessStartAndKill first, matching the pattern in Process.Kill().
• Error code capture: Windows captures Marshal.GetLastWin32Error() immediately after TerminateProcess fails (before any other calls); Unix uses Interop.Sys.GetLastErrorInfo() and passes RawErrno to Win32Exception.
• Process.Kill delegation: On Windows, Process.Kill() now correctly delegates to SafeProcessHandle.Kill(), reducing code duplication.

✅ Native Code — Clean simplification

• SystemNative_Kill stripped to a thin wrapper around kill(pid, signal) — all PAL mapping removed since callers now pass platform-native signal numbers. This is consistent with SystemNative_EnablePosixSignalHandling and other System.Native APIs.
• SystemNative_GetPlatformSIGSTOP() correctly placed in pal_signal.c/pal_signal.h (next to GetPlatformSignalNumber), with a WASM stub returning 0.
• The old Signals PAL enum (PAL_NONE, PAL_SIGKILL, PAL_SIGSTOP) removed cleanly.

✅ Test Coverage — Comprehensive

Tests cover all key scenarios:

Test	Scenario
Kill_InvalidHandle_ThrowsInvalidOperationException	Error path — invalid handle
Signal_InvalidHandle_ThrowsInvalidOperationException	Error path — invalid handle
Kill_RunningProcess_Terminates	Happy path — kill via SafeProcessHandle.Start
Kill_AlreadyExited_DoesNotThrow	Already-exited process — no exception
Signal_SIGKILL_RunningProcess_ReturnsTrue	Signal delivery confirmation
Signal_SIGKILL_AlreadyExited_ReturnsFalse	Already-exited returns false
Signal_NonSIGKILL_OnWindows_ThrowsPlatformNotSupportedException	Windows platform limitation
Signal_SIGTERM_RunningProcess_ReturnsTrue	Unix signal support
Kill_HandleWithoutTerminatePermission_ThrowsWin32Exception	Windows permissions

Existing tests also updated to use the new API (Kill_ExitedNonChildProcess_DoesNotThrow, ChildProcess_WithParentSignalHandler_CanReceiveSignals).

💡 entrypoints.c — Minor organizational note (follow-up)

SystemNative_GetPlatformSIGSTOP was moved to pal_signal.c/pal_signal.h (alongside GetPlatformSignalNumber), but its entrypoint entry in entrypoints.c (line 219) remains in the pal_process group between SystemNative_Kill and SystemNative_GetPid. The other signal entries (GetPlatformSignalNumber, EnablePosixSignalHandling, etc.) are at lines 286-291. Moving the entrypoint to the signal group would be more consistent, but this is purely organizational and not a blocker.

💡 Review feedback fully addressed

All prior review comments from @jkotas, @stephentoub, and @adamsitnik have been addressed across 9+ follow-up commits, including:

• Removing Unix implementation details from Kill() docs (stephentoub)
• Adding PROCESS_TERMINATE to docs + permission test (jkotas/adamsitnik)
• Delegating Process.Kill() on Windows to SafeProcessHandle.Kill() (jkotas)
• Adding PlatformDoesNotSupportProcessStartAndKill check (jkotas)
• Simplifying native SystemNative_Kill to pass-through (jkotas)
• Updating SendSignal test helper to accept Process (jkotas/adamsitnik)
• Using short Win32Exception name in XML docs (jkotas)

Generated by Code Review for issue #126313 · ◷

[jkotas]

SystemNative_GetPlatformSIGSTOP was moved to pal_signal.c/pal_signal.h (alongside GetPlatformSignalNumber), but its entrypoint entry in entrypoints.c (line 219) remains in the pal_process group between SystemNative_Kill and SystemNative_GetPid. The other signal entries (GetPlatformSignalNumber, EnablePosixSignalHandling, etc.) are at lines 286-291. Moving the entrypoint to the signal group would be more consistent

@copilot Address this feedback

[Copilot]

SystemNative_GetPlatformSIGSTOP was moved to pal_signal.c/pal_signal.h (alongside GetPlatformSignalNumber), but its entrypoint entry in entrypoints.c (line 219) remains in the pal_process group betw...

Done in 493466c — moved DllImportEntry(SystemNative_GetPlatformSIGSTOP) from the process group (after SystemNative_Kill) to the signal group (after SystemNative_GetPlatformSignalNumber).

[Copilot]

Adds Kill() and Signal(PosixSignal) to SafeProcessHandle, enabling callers to terminate or signal a process via a handle without going through Process.

Description

Public API (SafeProcessHandle.cs + ref assembly)

• Kill() — sends a request to the OS to terminate the process; silently no-ops if already exited (matches Process.Kill semantics). On Windows, the handle must have PROCESS_TERMINATE access.
• Signal(PosixSignal) — sends an arbitrary signal; returns false if process no longer exists (or never existed), true on delivery, throws PlatformNotSupportedException for unsupported signals. On Windows, the handle must have PROCESS_TERMINATE access.

Both methods validate the handle (throw InvalidOperationException on invalid handle), carry the same [UnsupportedOSPlatform("ios/tvos")] / [SupportedOSPlatform("maccatalyst")] annotations as Start, and throw PlatformNotSupportedException at runtime on iOS/tvOS (matching the ProcessUtils.PlatformDoesNotSupportProcessStartAndKill pattern used by Process.Kill).

Windows (SafeProcessHandle.Windows.cs)

• SignalCore: only SIGKILL is supported (maps to TerminateProcess), matching PosixSignalRegistration.Create exception behavior for unsupported signals. Retrieves the error code via Marshal.GetLastWin32Error() before any checks and only constructs Win32Exception(errorCode) when actually throwing. Detects already-exited processes via ERROR_ACCESS_DENIED + GetExitCodeProcess returning non-STILL_ACTIVE.
• No separate KillCore; Kill() calls SignalCore(PosixSignal.SIGKILL) directly from the shared file.

Unix (SafeProcessHandle.Unix.cs)

• SignalCore: checks ProcessUtils.PlatformDoesNotSupportProcessStartAndKill first (throws PlatformNotSupportedException on iOS/tvOS); uses Interop.Sys.GetPlatformSignalNumber(signal) to convert the managed PosixSignal to its platform-native signal number (throws PlatformNotSupportedException if unsupported, matching PosixSignalRegistration.Create behavior); passes the native signal number directly to Interop.Sys.Kill; returns false on ESRCH; uses Interop.Sys.GetLastErrorInfo() and passes errorInfo.RawErrno to the Win32Exception constructor.
• No separate KillCore; Kill() calls SignalCore(PosixSignal.SIGKILL) directly from the shared file.

Native (pal_process.c / pal_signal.c)

• SystemNative_Kill simplified to call kill(pid, signal) directly with no PAL mapping switch — it now takes the platform-native signal number, consistent with other System.Native APIs such as SystemNative_EnablePosixSignalHandling. The old Signals PAL enum (PAL_NONE, PAL_SIGKILL, PAL_SIGSTOP) has been removed from pal_process.h.
• SystemNative_GetPlatformSIGSTOP() moved to pal_signal.h/pal_signal.c (right below SystemNative_GetPlatformSignalNumber) since it is signal-related. Dummy implementations returning 0 are provided in pal_signal_wasm.c for both SystemNative_GetPlatformSIGSTOP and SystemNative_GetPlatformSignalNumber for BROWSER and WASI platforms.
• entrypoints.c: moved the DllImportEntry(SystemNative_GetPlatformSIGSTOP) registration from the process group (between SystemNative_Kill and SystemNative_GetPid) to the signal group (alongside SystemNative_GetPlatformSignalNumber, SystemNative_EnablePosixSignalHandling, etc.), consistent with the function's new home in pal_signal.c/pal_signal.h.

Process.Kill refactoring (Process.Windows.cs)

• Process.Kill() on Windows now delegates to SafeProcessHandle.Kill() after obtaining the handle, eliminating the duplicated TerminateProcess + error-handling logic.

Interop (Interop.Kill.cs / Interop.PosixSignal.cs)

• Removed the Signals managed enum; Interop.Sys.Kill now takes a plain int signal parameter (platform-native number).
• Interop.Sys.GetPlatformSIGSTOP() P/Invoke moved to Interop.PosixSignal.cs alongside GetPlatformSignalNumber, keeping all signal-number interop co-located.
• All existing callers updated: Process.Unix.cs uses GetPlatformSignalNumber(PosixSignal.SIGKILL) and GetPlatformSIGSTOP(); ProcessWaitState.Unix.cs and ProcessManager.Unix.cs pass 0 directly for the "probe" call.

Project / interop

• Added Interop.PosixSignal.cs to the Unix ItemGroup in System.Diagnostics.Process.csproj to expose GetPlatformSignalNumber and GetPlatformSIGSTOP in the main assembly.

Tests

• SafeProcessHandleTests.cs: Invalid handle → InvalidOperationException; Kill / Signal(SIGKILL) on running process terminates it; Kill on exited process does not throw; Signal(SIGKILL) returns false on exited process; Windows: non-SIGKILL signal → PlatformNotSupportedException; Unix: Signal(SIGTERM) on running process returns true and terminates it; Windows: Kill_HandleWithoutTerminatePermission_ThrowsWin32Exception — opens a handle with only PROCESS_QUERY_LIMITED_INFORMATION and verifies Kill() throws Win32Exception.
• ProcessTests.Unix.cs: Updated ChildProcess_WithParentSignalHandler_CanReceiveSignals to use SafeProcessHandle.Signal instead of the raw SendSignal helper. Refactored SendSignal helper to accept a Process parameter — Unix calls process.SafeHandle.Signal(signal) directly (simplified to a single-line Assert.True). Removed the class-level SIGKILL constant and kill P/Invoke declaration; Kill_ExitedNonChildProcess_DoesNotThrow now uses SafeHandle.Signal(PosixSignal.SIGKILL) and Process.HasExited.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

[adamsitnik]

@copilot address my feedback, also do search for other places in the repo where the new Signal API could be used

[Copilot]

@copilot address my feedback, also do search for other places in the repo where the new Signal API could be used

All feedback addressed in 7c03c8c. For other usages: the main candidate in the codebase is the SendSignal helper in tests (ProcessTests.cs:173, :246, :256, :260) which sends signals via kill() directly. However, those tests operate against a Process instance using its Id, while our Signal API requires a SafeProcessHandle — on Unix the handle's ProcessId is only set when the process was started via SafeProcessHandle.Start (or Process.Start). The ChildProcess_WithParentSignalHandler_CanReceiveSignals test was the natural fit since it already uses RemoteInvokeHandle with a started Process, so Process.SafeHandle.Signal() works correctly there.

[Copilot]

Pull request overview

Adds process-termination and signal-sending capabilities directly to SafeProcessHandle, enabling callers to manage processes via handles without going through Process, with platform-specific implementations for Windows and Unix plus corresponding test updates.

Changes:

• Added SafeProcessHandle.Kill() and SafeProcessHandle.Signal(PosixSignal) public APIs (with ref-assembly updates).
• Implemented SignalCore on Windows (via TerminateProcess) and Unix (via kill(2) + GetPlatformSignalNumber).
• Added/updated tests to validate new APIs and switched an existing Unix test to use SafeProcessHandle.Signal.

Reviewed changes

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

Show a summary per file

File	Description
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs	Introduces the new public Kill/Signal APIs and validation path.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs	Implements Windows SignalCore mapping SIGKILL to TerminateProcess with exited-process detection.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs	Implements Unix SignalCore using GetPlatformSignalNumber + kill, returning false on ESRCH.
src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj	Adds Interop.PosixSignal.cs to the Unix compile item group.
src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs	Updates the public ref surface for SafeProcessHandle with Kill/Signal.
src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs	Adds unit tests covering invalid-handle, running-process, exited-process, and platform-specific behaviors.
src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs	Updates signal-delivery test to use SafeProcessHandle.Signal instead of a raw P/Invoke helper.

[github-actions]

🤖 Copilot Code Review — PR #126313

Note

This review was generated by GitHub Copilot using Claude Opus 4.6.

Holistic Assessment

Motivation: Justified. This is the next step in the approved API expansion (#125838, api-approved by @bartonjs on 2026-03-24). Following the merged Start/ProcessId PR (#126192), Kill and Signal are the natural additions to enable callers to manage process lifecycle via SafeProcessHandle without requiring a Process object. The motivation is clear and real.

Approach: Sound. The implementation follows the approved API shape exactly, builds on the existing SafeProcessHandle partial class pattern, and properly separates platform-specific behavior. The native code simplification (removing the Signals PAL enum and having SystemNative_Kill accept platform-native signal numbers directly) is a good cleanup that aligns with how other System.Native APIs like SystemNative_EnablePosixSignalHandling work. The decision to have Kill() delegate to SignalCore(SIGKILL) keeps the code DRY while the void return type correctly matches the "fire-and-forget" semantics of Process.Kill().

Summary: ✅ LGTM. The implementation matches the approved API shape, all prior maintainer review feedback (from @jkotas, @stephentoub, @adamsitnik) has been addressed through multiple iterations, and the code is correct and well-tested. No blocking issues found. Two minor follow-up observations below.

---

Detailed Findings

✅ API Approval Verification — Matches approved shape

The approved API from #125838 (comment) by @bartonjs specifies:

publicpartialclassSafeProcessHandle{publicvoidKill();publicboolSignal(PosixSignalsignal);}

The ref assembly implementation matches exactly:

• Kill() → void return ✅
• Signal(PosixSignal signal) → bool return ✅
• Parameter name signal matches ✅
• No extra unapproved public API ✅
• No missing approved API from this PR's scope ✅
• Platform attributes ([UnsupportedOSPlatform("ios")], [UnsupportedOSPlatform("tvos")], [SupportedOSPlatform("maccatalyst")]) are consistent with Start ✅

✅ Correctness — Edge cases handled properly

Both platform implementations handle key edge cases correctly:

• Already-exited process: Windows detects via ERROR_ACCESS_DENIED + GetExitCodeProcess returning non-STILL_ACTIVE; Unix detects via ESRCH. Both return false from SignalCore, which Kill() silently discards (matching Process.Kill semantics).
• Unsupported signals: Windows throws PlatformNotSupportedException for non-SIGKILL (matching PosixSignalRegistration.Create behavior); Unix throws PlatformNotSupportedException when GetPlatformSignalNumber returns 0.
• iOS/tvOS: Unix SignalCore checks ProcessUtils.PlatformDoesNotSupportProcessStartAndKill first, matching the pattern in Process.Kill().
• Error code capture: Windows captures Marshal.GetLastWin32Error() immediately after TerminateProcess fails (before any other calls); Unix uses Interop.Sys.GetLastErrorInfo() and passes RawErrno to Win32Exception.
• Process.Kill delegation: On Windows, Process.Kill() now correctly delegates to SafeProcessHandle.Kill(), reducing code duplication.

✅ Native Code — Clean simplification

• SystemNative_Kill stripped to a thin wrapper around kill(pid, signal) — all PAL mapping removed since callers now pass platform-native signal numbers. This is consistent with SystemNative_EnablePosixSignalHandling and other System.Native APIs.
• SystemNative_GetPlatformSIGSTOP() correctly placed in pal_signal.c/pal_signal.h (next to GetPlatformSignalNumber), with a WASM stub returning 0.
• The old Signals PAL enum (PAL_NONE, PAL_SIGKILL, PAL_SIGSTOP) removed cleanly.

✅ Test Coverage — Comprehensive

Tests cover all key scenarios:

Test	Scenario
Kill_InvalidHandle_ThrowsInvalidOperationException	Error path — invalid handle
Signal_InvalidHandle_ThrowsInvalidOperationException	Error path — invalid handle
Kill_RunningProcess_Terminates	Happy path — kill via SafeProcessHandle.Start
Kill_AlreadyExited_DoesNotThrow	Already-exited process — no exception
Signal_SIGKILL_RunningProcess_ReturnsTrue	Signal delivery confirmation
Signal_SIGKILL_AlreadyExited_ReturnsFalse	Already-exited returns false
Signal_NonSIGKILL_OnWindows_ThrowsPlatformNotSupportedException	Windows platform limitation
Signal_SIGTERM_RunningProcess_ReturnsTrue	Unix signal support
Kill_HandleWithoutTerminatePermission_ThrowsWin32Exception	Windows permissions

Existing tests also updated to use the new API (Kill_ExitedNonChildProcess_DoesNotThrow, ChildProcess_WithParentSignalHandler_CanReceiveSignals).

💡 entrypoints.c — Minor organizational note (follow-up)

SystemNative_GetPlatformSIGSTOP was moved to pal_signal.c/pal_signal.h (alongside GetPlatformSignalNumber), but its entrypoint entry in entrypoints.c (line 219) remains in the pal_process group between SystemNative_Kill and SystemNative_GetPid. The other signal entries (GetPlatformSignalNumber, EnablePosixSignalHandling, etc.) are at lines 286-291. Moving the entrypoint to the signal group would be more consistent, but this is purely organizational and not a blocker.

💡 Review feedback fully addressed

All prior review comments from @jkotas, @stephentoub, and @adamsitnik have been addressed across 9+ follow-up commits, including:

• Removing Unix implementation details from Kill() docs (stephentoub)
• Adding PROCESS_TERMINATE to docs + permission test (jkotas/adamsitnik)
• Delegating Process.Kill() on Windows to SafeProcessHandle.Kill() (jkotas)
• Adding PlatformDoesNotSupportProcessStartAndKill check (jkotas)
• Simplifying native SystemNative_Kill to pass-through (jkotas)
• Updating SendSignal test helper to accept Process (jkotas/adamsitnik)
• Using short Win32Exception name in XML docs (jkotas)

Generated by Code Review for issue #126313 · ◷

[jkotas]

SystemNative_GetPlatformSIGSTOP was moved to pal_signal.c/pal_signal.h (alongside GetPlatformSignalNumber), but its entrypoint entry in entrypoints.c (line 219) remains in the pal_process group between SystemNative_Kill and SystemNative_GetPid. The other signal entries (GetPlatformSignalNumber, EnablePosixSignalHandling, etc.) are at lines 286-291. Moving the entrypoint to the signal group would be more consistent

@copilot Address this feedback

[Copilot]

SystemNative_GetPlatformSIGSTOP was moved to pal_signal.c/pal_signal.h (alongside GetPlatformSignalNumber), but its entrypoint entry in entrypoints.c (line 219) remains in the pal_process group betw...

Done in 493466c — moved DllImportEntry(SystemNative_GetPlatformSIGSTOP) from the process group (after SystemNative_Kill) to the signal group (after SystemNative_GetPlatformSignalNumber).

[Copilot]

Adds Kill() and Signal(PosixSignal) to SafeProcessHandle, enabling callers to terminate or signal a process via a handle without going through Process.

Description

Public API (SafeProcessHandle.cs + ref assembly)

• Kill() — sends a request to the OS to terminate the process; silently no-ops if already exited (matches Process.Kill semantics). On Windows, the handle must have PROCESS_TERMINATE access.
• Signal(PosixSignal) — sends an arbitrary signal; returns false if process no longer exists (or never existed), true on delivery, throws PlatformNotSupportedException for unsupported signals. On Windows, the handle must have PROCESS_TERMINATE access.

Both methods validate the handle (throw InvalidOperationException on invalid handle), carry the same [UnsupportedOSPlatform("ios/tvos")] / [SupportedOSPlatform("maccatalyst")] annotations as Start, and throw PlatformNotSupportedException at runtime on iOS/tvOS (matching the ProcessUtils.PlatformDoesNotSupportProcessStartAndKill pattern used by Process.Kill).

Windows (SafeProcessHandle.Windows.cs)

• SignalCore: only SIGKILL is supported (maps to TerminateProcess), matching PosixSignalRegistration.Create exception behavior for unsupported signals. Retrieves the error code via Marshal.GetLastWin32Error() before any checks and only constructs Win32Exception(errorCode) when actually throwing. Detects already-exited processes via ERROR_ACCESS_DENIED + GetExitCodeProcess returning non-STILL_ACTIVE.
• No separate KillCore; Kill() calls SignalCore(PosixSignal.SIGKILL) directly from the shared file.

Unix (SafeProcessHandle.Unix.cs)

• SignalCore: checks ProcessUtils.PlatformDoesNotSupportProcessStartAndKill first (throws PlatformNotSupportedException on iOS/tvOS); uses Interop.Sys.GetPlatformSignalNumber(signal) to convert the managed PosixSignal to its platform-native signal number (throws PlatformNotSupportedException if unsupported, matching PosixSignalRegistration.Create behavior); passes the native signal number directly to Interop.Sys.Kill; returns false on ESRCH; uses Interop.Sys.GetLastErrorInfo() and passes errorInfo.RawErrno to the Win32Exception constructor.
• No separate KillCore; Kill() calls SignalCore(PosixSignal.SIGKILL) directly from the shared file.

Native (pal_process.c / pal_signal.c)

• SystemNative_Kill simplified to call kill(pid, signal) directly with no PAL mapping switch — it now takes the platform-native signal number, consistent with other System.Native APIs such as SystemNative_EnablePosixSignalHandling. The old Signals PAL enum (PAL_NONE, PAL_SIGKILL, PAL_SIGSTOP) has been removed from pal_process.h.
• SystemNative_GetPlatformSIGSTOP() moved to pal_signal.h/pal_signal.c (right below SystemNative_GetPlatformSignalNumber) since it is signal-related. Dummy implementations returning 0 are provided in pal_signal_wasm.c for both SystemNative_GetPlatformSIGSTOP and SystemNative_GetPlatformSignalNumber for BROWSER and WASI platforms.
• entrypoints.c: moved the DllImportEntry(SystemNative_GetPlatformSIGSTOP) registration from the process group (between SystemNative_Kill and SystemNative_GetPid) to the signal group (alongside SystemNative_GetPlatformSignalNumber, SystemNative_EnablePosixSignalHandling, etc.), consistent with the function's new home in pal_signal.c/pal_signal.h.

Process.Kill refactoring (Process.Windows.cs)

• Process.Kill() on Windows now delegates to SafeProcessHandle.Kill() after obtaining the handle, eliminating the duplicated TerminateProcess + error-handling logic.

Interop (Interop.Kill.cs / Interop.PosixSignal.cs)

• Removed the Signals managed enum; Interop.Sys.Kill now takes a plain int signal parameter (platform-native number).
• Interop.Sys.GetPlatformSIGSTOP() P/Invoke moved to Interop.PosixSignal.cs alongside GetPlatformSignalNumber, keeping all signal-number interop co-located.
• All existing callers updated: Process.Unix.cs uses GetPlatformSignalNumber(PosixSignal.SIGKILL) and GetPlatformSIGSTOP(); ProcessWaitState.Unix.cs and ProcessManager.Unix.cs pass 0 directly for the "probe" call.

Project / interop

• Added Interop.PosixSignal.cs to the Unix ItemGroup in System.Diagnostics.Process.csproj to expose GetPlatformSignalNumber and GetPlatformSIGSTOP in the main assembly.

Tests

• SafeProcessHandleTests.cs: Invalid handle → InvalidOperationException; Kill / Signal(SIGKILL) on running process terminates it; Kill on exited process does not throw; Signal(SIGKILL) returns false on exited process; Windows: non-SIGKILL signal → PlatformNotSupportedException; Unix: Signal(SIGTERM) on running process returns true and terminates it; Windows: Kill_HandleWithoutTerminatePermission_ThrowsWin32Exception — opens a handle with only PROCESS_QUERY_LIMITED_INFORMATION and verifies Kill() throws Win32Exception.
• ProcessTests.Unix.cs: Updated ChildProcess_WithParentSignalHandler_CanReceiveSignals to use SafeProcessHandle.Signal instead of the raw SendSignal helper. Refactored SendSignal helper to accept a Process parameter — Unix calls process.SafeHandle.Signal(signal) directly (simplified to a single-line Assert.True). Removed the class-level SIGKILL constant and kill P/Invoke declaration; Kill_ExitedNonChildProcess_DoesNotThrow now uses SafeHandle.Signal(PosixSignal.SIGKILL) and Process.HasExited.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

[adamsitnik]

@copilot address my feedback, also do search for other places in the repo where the new Signal API could be used

[Copilot]

@copilot address my feedback, also do search for other places in the repo where the new Signal API could be used

All feedback addressed in 7c03c8c. For other usages: the main candidate in the codebase is the SendSignal helper in tests (ProcessTests.cs:173, :246, :256, :260) which sends signals via kill() directly. However, those tests operate against a Process instance using its Id, while our Signal API requires a SafeProcessHandle — on Unix the handle's ProcessId is only set when the process was started via SafeProcessHandle.Start (or Process.Start). The ChildProcess_WithParentSignalHandler_CanReceiveSignals test was the natural fit since it already uses RemoteInvokeHandle with a started Process, so Process.SafeHandle.Signal() works correctly there.

[Copilot]

Pull request overview

Adds process-termination and signal-sending capabilities directly to SafeProcessHandle, enabling callers to manage processes via handles without going through Process, with platform-specific implementations for Windows and Unix plus corresponding test updates.

Changes:

• Added SafeProcessHandle.Kill() and SafeProcessHandle.Signal(PosixSignal) public APIs (with ref-assembly updates).
• Implemented SignalCore on Windows (via TerminateProcess) and Unix (via kill(2) + GetPlatformSignalNumber).
• Added/updated tests to validate new APIs and switched an existing Unix test to use SafeProcessHandle.Signal.

Reviewed changes

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

Show a summary per file

File	Description
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs	Introduces the new public Kill/Signal APIs and validation path.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs	Implements Windows SignalCore mapping SIGKILL to TerminateProcess with exited-process detection.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs	Implements Unix SignalCore using GetPlatformSignalNumber + kill, returning false on ESRCH.
src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj	Adds Interop.PosixSignal.cs to the Unix compile item group.
src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs	Updates the public ref surface for SafeProcessHandle with Kill/Signal.
src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs	Adds unit tests covering invalid-handle, running-process, exited-process, and platform-specific behaviors.
src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs	Updates signal-delivery test to use SafeProcessHandle.Signal instead of a raw P/Invoke helper.

[github-actions]

🤖 Copilot Code Review — PR #126313

Note

This review was generated by GitHub Copilot using Claude Opus 4.6.

Holistic Assessment

Motivation: Justified. This is the next step in the approved API expansion (#125838, api-approved by @bartonjs on 2026-03-24). Following the merged Start/ProcessId PR (#126192), Kill and Signal are the natural additions to enable callers to manage process lifecycle via SafeProcessHandle without requiring a Process object. The motivation is clear and real.

Approach: Sound. The implementation follows the approved API shape exactly, builds on the existing SafeProcessHandle partial class pattern, and properly separates platform-specific behavior. The native code simplification (removing the Signals PAL enum and having SystemNative_Kill accept platform-native signal numbers directly) is a good cleanup that aligns with how other System.Native APIs like SystemNative_EnablePosixSignalHandling work. The decision to have Kill() delegate to SignalCore(SIGKILL) keeps the code DRY while the void return type correctly matches the "fire-and-forget" semantics of Process.Kill().

Summary: ✅ LGTM. The implementation matches the approved API shape, all prior maintainer review feedback (from @jkotas, @stephentoub, @adamsitnik) has been addressed through multiple iterations, and the code is correct and well-tested. No blocking issues found. Two minor follow-up observations below.

---

Detailed Findings

✅ API Approval Verification — Matches approved shape

The approved API from #125838 (comment) by @bartonjs specifies:

publicpartialclassSafeProcessHandle{publicvoidKill();publicboolSignal(PosixSignalsignal);}

The ref assembly implementation matches exactly:

• Kill() → void return ✅
• Signal(PosixSignal signal) → bool return ✅
• Parameter name signal matches ✅
• No extra unapproved public API ✅
• No missing approved API from this PR's scope ✅
• Platform attributes ([UnsupportedOSPlatform("ios")], [UnsupportedOSPlatform("tvos")], [SupportedOSPlatform("maccatalyst")]) are consistent with Start ✅

✅ Correctness — Edge cases handled properly

Both platform implementations handle key edge cases correctly:

• Already-exited process: Windows detects via ERROR_ACCESS_DENIED + GetExitCodeProcess returning non-STILL_ACTIVE; Unix detects via ESRCH. Both return false from SignalCore, which Kill() silently discards (matching Process.Kill semantics).
• Unsupported signals: Windows throws PlatformNotSupportedException for non-SIGKILL (matching PosixSignalRegistration.Create behavior); Unix throws PlatformNotSupportedException when GetPlatformSignalNumber returns 0.
• iOS/tvOS: Unix SignalCore checks ProcessUtils.PlatformDoesNotSupportProcessStartAndKill first, matching the pattern in Process.Kill().
• Error code capture: Windows captures Marshal.GetLastWin32Error() immediately after TerminateProcess fails (before any other calls); Unix uses Interop.Sys.GetLastErrorInfo() and passes RawErrno to Win32Exception.
• Process.Kill delegation: On Windows, Process.Kill() now correctly delegates to SafeProcessHandle.Kill(), reducing code duplication.

✅ Native Code — Clean simplification

• SystemNative_Kill stripped to a thin wrapper around kill(pid, signal) — all PAL mapping removed since callers now pass platform-native signal numbers. This is consistent with SystemNative_EnablePosixSignalHandling and other System.Native APIs.
• SystemNative_GetPlatformSIGSTOP() correctly placed in pal_signal.c/pal_signal.h (next to GetPlatformSignalNumber), with a WASM stub returning 0.
• The old Signals PAL enum (PAL_NONE, PAL_SIGKILL, PAL_SIGSTOP) removed cleanly.

✅ Test Coverage — Comprehensive

Tests cover all key scenarios:

Test	Scenario
Kill_InvalidHandle_ThrowsInvalidOperationException	Error path — invalid handle
Signal_InvalidHandle_ThrowsInvalidOperationException	Error path — invalid handle
Kill_RunningProcess_Terminates	Happy path — kill via SafeProcessHandle.Start
Kill_AlreadyExited_DoesNotThrow	Already-exited process — no exception
Signal_SIGKILL_RunningProcess_ReturnsTrue	Signal delivery confirmation
Signal_SIGKILL_AlreadyExited_ReturnsFalse	Already-exited returns false
Signal_NonSIGKILL_OnWindows_ThrowsPlatformNotSupportedException	Windows platform limitation
Signal_SIGTERM_RunningProcess_ReturnsTrue	Unix signal support
Kill_HandleWithoutTerminatePermission_ThrowsWin32Exception	Windows permissions

Existing tests also updated to use the new API (Kill_ExitedNonChildProcess_DoesNotThrow, ChildProcess_WithParentSignalHandler_CanReceiveSignals).

💡 entrypoints.c — Minor organizational note (follow-up)

SystemNative_GetPlatformSIGSTOP was moved to pal_signal.c/pal_signal.h (alongside GetPlatformSignalNumber), but its entrypoint entry in entrypoints.c (line 219) remains in the pal_process group between SystemNative_Kill and SystemNative_GetPid. The other signal entries (GetPlatformSignalNumber, EnablePosixSignalHandling, etc.) are at lines 286-291. Moving the entrypoint to the signal group would be more consistent, but this is purely organizational and not a blocker.

💡 Review feedback fully addressed

All prior review comments from @jkotas, @stephentoub, and @adamsitnik have been addressed across 9+ follow-up commits, including:

• Removing Unix implementation details from Kill() docs (stephentoub)
• Adding PROCESS_TERMINATE to docs + permission test (jkotas/adamsitnik)
• Delegating Process.Kill() on Windows to SafeProcessHandle.Kill() (jkotas)
• Adding PlatformDoesNotSupportProcessStartAndKill check (jkotas)
• Simplifying native SystemNative_Kill to pass-through (jkotas)
• Updating SendSignal test helper to accept Process (jkotas/adamsitnik)
• Using short Win32Exception name in XML docs (jkotas)

Generated by Code Review for issue #126313 · ◷

[jkotas]

SystemNative_GetPlatformSIGSTOP was moved to pal_signal.c/pal_signal.h (alongside GetPlatformSignalNumber), but its entrypoint entry in entrypoints.c (line 219) remains in the pal_process group between SystemNative_Kill and SystemNative_GetPid. The other signal entries (GetPlatformSignalNumber, EnablePosixSignalHandling, etc.) are at lines 286-291. Moving the entrypoint to the signal group would be more consistent

@copilot Address this feedback

[Copilot]

SystemNative_GetPlatformSIGSTOP was moved to pal_signal.c/pal_signal.h (alongside GetPlatformSignalNumber), but its entrypoint entry in entrypoints.c (line 219) remains in the pal_process group betw...

Done in 493466c — moved DllImportEntry(SystemNative_GetPlatformSIGSTOP) from the process group (after SystemNative_Kill) to the signal group (after SystemNative_GetPlatformSignalNumber).

[Copilot]

Adds Kill() and Signal(PosixSignal) to SafeProcessHandle, enabling callers to terminate or signal a process via a handle without going through Process.

Description

Public API (SafeProcessHandle.cs + ref assembly)

• Kill() — sends a request to the OS to terminate the process; silently no-ops if already exited (matches Process.Kill semantics). On Windows, the handle must have PROCESS_TERMINATE access.
• Signal(PosixSignal) — sends an arbitrary signal; returns false if process no longer exists (or never existed), true on delivery, throws PlatformNotSupportedException for unsupported signals. On Windows, the handle must have PROCESS_TERMINATE access.

Both methods validate the handle (throw InvalidOperationException on invalid handle), carry the same [UnsupportedOSPlatform("ios/tvos")] / [SupportedOSPlatform("maccatalyst")] annotations as Start, and throw PlatformNotSupportedException at runtime on iOS/tvOS (matching the ProcessUtils.PlatformDoesNotSupportProcessStartAndKill pattern used by Process.Kill).

Windows (SafeProcessHandle.Windows.cs)

• SignalCore: only SIGKILL is supported (maps to TerminateProcess), matching PosixSignalRegistration.Create exception behavior for unsupported signals. Retrieves the error code via Marshal.GetLastWin32Error() before any checks and only constructs Win32Exception(errorCode) when actually throwing. Detects already-exited processes via ERROR_ACCESS_DENIED + GetExitCodeProcess returning non-STILL_ACTIVE.
• No separate KillCore; Kill() calls SignalCore(PosixSignal.SIGKILL) directly from the shared file.

Unix (SafeProcessHandle.Unix.cs)

• SignalCore: checks ProcessUtils.PlatformDoesNotSupportProcessStartAndKill first (throws PlatformNotSupportedException on iOS/tvOS); uses Interop.Sys.GetPlatformSignalNumber(signal) to convert the managed PosixSignal to its platform-native signal number (throws PlatformNotSupportedException if unsupported, matching PosixSignalRegistration.Create behavior); passes the native signal number directly to Interop.Sys.Kill; returns false on ESRCH; uses Interop.Sys.GetLastErrorInfo() and passes errorInfo.RawErrno to the Win32Exception constructor.
• No separate KillCore; Kill() calls SignalCore(PosixSignal.SIGKILL) directly from the shared file.

Native (pal_process.c / pal_signal.c)

• SystemNative_Kill simplified to call kill(pid, signal) directly with no PAL mapping switch — it now takes the platform-native signal number, consistent with other System.Native APIs such as SystemNative_EnablePosixSignalHandling. The old Signals PAL enum (PAL_NONE, PAL_SIGKILL, PAL_SIGSTOP) has been removed from pal_process.h.
• SystemNative_GetPlatformSIGSTOP() moved to pal_signal.h/pal_signal.c (right below SystemNative_GetPlatformSignalNumber) since it is signal-related. Dummy implementations returning 0 are provided in pal_signal_wasm.c for both SystemNative_GetPlatformSIGSTOP and SystemNative_GetPlatformSignalNumber for BROWSER and WASI platforms.
• entrypoints.c: moved the DllImportEntry(SystemNative_GetPlatformSIGSTOP) registration from the process group (between SystemNative_Kill and SystemNative_GetPid) to the signal group (alongside SystemNative_GetPlatformSignalNumber, SystemNative_EnablePosixSignalHandling, etc.), consistent with the function's new home in pal_signal.c/pal_signal.h.

Process.Kill refactoring (Process.Windows.cs)

• Process.Kill() on Windows now delegates to SafeProcessHandle.Kill() after obtaining the handle, eliminating the duplicated TerminateProcess + error-handling logic.

Interop (Interop.Kill.cs / Interop.PosixSignal.cs)

• Removed the Signals managed enum; Interop.Sys.Kill now takes a plain int signal parameter (platform-native number).
• Interop.Sys.GetPlatformSIGSTOP() P/Invoke moved to Interop.PosixSignal.cs alongside GetPlatformSignalNumber, keeping all signal-number interop co-located.
• All existing callers updated: Process.Unix.cs uses GetPlatformSignalNumber(PosixSignal.SIGKILL) and GetPlatformSIGSTOP(); ProcessWaitState.Unix.cs and ProcessManager.Unix.cs pass 0 directly for the "probe" call.

Project / interop

• Added Interop.PosixSignal.cs to the Unix ItemGroup in System.Diagnostics.Process.csproj to expose GetPlatformSignalNumber and GetPlatformSIGSTOP in the main assembly.

Tests

• SafeProcessHandleTests.cs: Invalid handle → InvalidOperationException; Kill / Signal(SIGKILL) on running process terminates it; Kill on exited process does not throw; Signal(SIGKILL) returns false on exited process; Windows: non-SIGKILL signal → PlatformNotSupportedException; Unix: Signal(SIGTERM) on running process returns true and terminates it; Windows: Kill_HandleWithoutTerminatePermission_ThrowsWin32Exception — opens a handle with only PROCESS_QUERY_LIMITED_INFORMATION and verifies Kill() throws Win32Exception.
• ProcessTests.Unix.cs: Updated ChildProcess_WithParentSignalHandler_CanReceiveSignals to use SafeProcessHandle.Signal instead of the raw SendSignal helper. Refactored SendSignal helper to accept a Process parameter — Unix calls process.SafeHandle.Signal(signal) directly (simplified to a single-line Assert.True). Removed the class-level SIGKILL constant and kill P/Invoke declaration; Kill_ExitedNonChildProcess_DoesNotThrow now uses SafeHandle.Signal(PosixSignal.SIGKILL) and Process.HasExited.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

[adamsitnik]

@copilot address my feedback, also do search for other places in the repo where the new Signal API could be used

[Copilot]

@copilot address my feedback, also do search for other places in the repo where the new Signal API could be used

All feedback addressed in 7c03c8c. For other usages: the main candidate in the codebase is the SendSignal helper in tests (ProcessTests.cs:173, :246, :256, :260) which sends signals via kill() directly. However, those tests operate against a Process instance using its Id, while our Signal API requires a SafeProcessHandle — on Unix the handle's ProcessId is only set when the process was started via SafeProcessHandle.Start (or Process.Start). The ChildProcess_WithParentSignalHandler_CanReceiveSignals test was the natural fit since it already uses RemoteInvokeHandle with a started Process, so Process.SafeHandle.Signal() works correctly there.

[Copilot]

Pull request overview

Adds process-termination and signal-sending capabilities directly to SafeProcessHandle, enabling callers to manage processes via handles without going through Process, with platform-specific implementations for Windows and Unix plus corresponding test updates.

Changes:

• Added SafeProcessHandle.Kill() and SafeProcessHandle.Signal(PosixSignal) public APIs (with ref-assembly updates).
• Implemented SignalCore on Windows (via TerminateProcess) and Unix (via kill(2) + GetPlatformSignalNumber).
• Added/updated tests to validate new APIs and switched an existing Unix test to use SafeProcessHandle.Signal.

Reviewed changes

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

Show a summary per file

File	Description
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs	Introduces the new public Kill/Signal APIs and validation path.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs	Implements Windows SignalCore mapping SIGKILL to TerminateProcess with exited-process detection.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs	Implements Unix SignalCore using GetPlatformSignalNumber + kill, returning false on ESRCH.
src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj	Adds Interop.PosixSignal.cs to the Unix compile item group.
src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs	Updates the public ref surface for SafeProcessHandle with Kill/Signal.
src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs	Adds unit tests covering invalid-handle, running-process, exited-process, and platform-specific behaviors.
src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs	Updates signal-delivery test to use SafeProcessHandle.Signal instead of a raw P/Invoke helper.

[github-actions]

🤖 Copilot Code Review — PR #126313

Note

This review was generated by GitHub Copilot using Claude Opus 4.6.

Holistic Assessment

Motivation: Justified. This is the next step in the approved API expansion (#125838, api-approved by @bartonjs on 2026-03-24). Following the merged Start/ProcessId PR (#126192), Kill and Signal are the natural additions to enable callers to manage process lifecycle via SafeProcessHandle without requiring a Process object. The motivation is clear and real.

Approach: Sound. The implementation follows the approved API shape exactly, builds on the existing SafeProcessHandle partial class pattern, and properly separates platform-specific behavior. The native code simplification (removing the Signals PAL enum and having SystemNative_Kill accept platform-native signal numbers directly) is a good cleanup that aligns with how other System.Native APIs like SystemNative_EnablePosixSignalHandling work. The decision to have Kill() delegate to SignalCore(SIGKILL) keeps the code DRY while the void return type correctly matches the "fire-and-forget" semantics of Process.Kill().

Summary: ✅ LGTM. The implementation matches the approved API shape, all prior maintainer review feedback (from @jkotas, @stephentoub, @adamsitnik) has been addressed through multiple iterations, and the code is correct and well-tested. No blocking issues found. Two minor follow-up observations below.

---

Detailed Findings

✅ API Approval Verification — Matches approved shape

The approved API from #125838 (comment) by @bartonjs specifies:

publicpartialclassSafeProcessHandle{publicvoidKill();publicboolSignal(PosixSignalsignal);}

The ref assembly implementation matches exactly:

• Kill() → void return ✅
• Signal(PosixSignal signal) → bool return ✅
• Parameter name signal matches ✅
• No extra unapproved public API ✅
• No missing approved API from this PR's scope ✅
• Platform attributes ([UnsupportedOSPlatform("ios")], [UnsupportedOSPlatform("tvos")], [SupportedOSPlatform("maccatalyst")]) are consistent with Start ✅

✅ Correctness — Edge cases handled properly

Both platform implementations handle key edge cases correctly:

• Already-exited process: Windows detects via ERROR_ACCESS_DENIED + GetExitCodeProcess returning non-STILL_ACTIVE; Unix detects via ESRCH. Both return false from SignalCore, which Kill() silently discards (matching Process.Kill semantics).
• Unsupported signals: Windows throws PlatformNotSupportedException for non-SIGKILL (matching PosixSignalRegistration.Create behavior); Unix throws PlatformNotSupportedException when GetPlatformSignalNumber returns 0.
• iOS/tvOS: Unix SignalCore checks ProcessUtils.PlatformDoesNotSupportProcessStartAndKill first, matching the pattern in Process.Kill().
• Error code capture: Windows captures Marshal.GetLastWin32Error() immediately after TerminateProcess fails (before any other calls); Unix uses Interop.Sys.GetLastErrorInfo() and passes RawErrno to Win32Exception.
• Process.Kill delegation: On Windows, Process.Kill() now correctly delegates to SafeProcessHandle.Kill(), reducing code duplication.

✅ Native Code — Clean simplification

• SystemNative_Kill stripped to a thin wrapper around kill(pid, signal) — all PAL mapping removed since callers now pass platform-native signal numbers. This is consistent with SystemNative_EnablePosixSignalHandling and other System.Native APIs.
• SystemNative_GetPlatformSIGSTOP() correctly placed in pal_signal.c/pal_signal.h (next to GetPlatformSignalNumber), with a WASM stub returning 0.
• The old Signals PAL enum (PAL_NONE, PAL_SIGKILL, PAL_SIGSTOP) removed cleanly.

✅ Test Coverage — Comprehensive

Tests cover all key scenarios:

Test	Scenario
Kill_InvalidHandle_ThrowsInvalidOperationException	Error path — invalid handle
Signal_InvalidHandle_ThrowsInvalidOperationException	Error path — invalid handle
Kill_RunningProcess_Terminates	Happy path — kill via SafeProcessHandle.Start
Kill_AlreadyExited_DoesNotThrow	Already-exited process — no exception
Signal_SIGKILL_RunningProcess_ReturnsTrue	Signal delivery confirmation
Signal_SIGKILL_AlreadyExited_ReturnsFalse	Already-exited returns false
Signal_NonSIGKILL_OnWindows_ThrowsPlatformNotSupportedException	Windows platform limitation
Signal_SIGTERM_RunningProcess_ReturnsTrue	Unix signal support
Kill_HandleWithoutTerminatePermission_ThrowsWin32Exception	Windows permissions

Existing tests also updated to use the new API (Kill_ExitedNonChildProcess_DoesNotThrow, ChildProcess_WithParentSignalHandler_CanReceiveSignals).

💡 entrypoints.c — Minor organizational note (follow-up)

SystemNative_GetPlatformSIGSTOP was moved to pal_signal.c/pal_signal.h (alongside GetPlatformSignalNumber), but its entrypoint entry in entrypoints.c (line 219) remains in the pal_process group between SystemNative_Kill and SystemNative_GetPid. The other signal entries (GetPlatformSignalNumber, EnablePosixSignalHandling, etc.) are at lines 286-291. Moving the entrypoint to the signal group would be more consistent, but this is purely organizational and not a blocker.

💡 Review feedback fully addressed

All prior review comments from @jkotas, @stephentoub, and @adamsitnik have been addressed across 9+ follow-up commits, including:

• Removing Unix implementation details from Kill() docs (stephentoub)
• Adding PROCESS_TERMINATE to docs + permission test (jkotas/adamsitnik)
• Delegating Process.Kill() on Windows to SafeProcessHandle.Kill() (jkotas)
• Adding PlatformDoesNotSupportProcessStartAndKill check (jkotas)
• Simplifying native SystemNative_Kill to pass-through (jkotas)
• Updating SendSignal test helper to accept Process (jkotas/adamsitnik)
• Using short Win32Exception name in XML docs (jkotas)

Generated by Code Review for issue #126313 · ◷

[jkotas]

SystemNative_GetPlatformSIGSTOP was moved to pal_signal.c/pal_signal.h (alongside GetPlatformSignalNumber), but its entrypoint entry in entrypoints.c (line 219) remains in the pal_process group between SystemNative_Kill and SystemNative_GetPid. The other signal entries (GetPlatformSignalNumber, EnablePosixSignalHandling, etc.) are at lines 286-291. Moving the entrypoint to the signal group would be more consistent

@copilot Address this feedback

[Copilot]

SystemNative_GetPlatformSIGSTOP was moved to pal_signal.c/pal_signal.h (alongside GetPlatformSignalNumber), but its entrypoint entry in entrypoints.c (line 219) remains in the pal_process group betw...

Done in 493466c — moved DllImportEntry(SystemNative_GetPlatformSIGSTOP) from the process group (after SystemNative_Kill) to the signal group (after SystemNative_GetPlatformSignalNumber).

[Copilot]

Adds Kill() and Signal(PosixSignal) to SafeProcessHandle, enabling callers to terminate or signal a process via a handle without going through Process.

Description

Public API (SafeProcessHandle.cs + ref assembly)

• Kill() — sends a request to the OS to terminate the process; silently no-ops if already exited (matches Process.Kill semantics). On Windows, the handle must have PROCESS_TERMINATE access.
• Signal(PosixSignal) — sends an arbitrary signal; returns false if process no longer exists (or never existed), true on delivery, throws PlatformNotSupportedException for unsupported signals. On Windows, the handle must have PROCESS_TERMINATE access.

Both methods validate the handle (throw InvalidOperationException on invalid handle), carry the same [UnsupportedOSPlatform("ios/tvos")] / [SupportedOSPlatform("maccatalyst")] annotations as Start, and throw PlatformNotSupportedException at runtime on iOS/tvOS (matching the ProcessUtils.PlatformDoesNotSupportProcessStartAndKill pattern used by Process.Kill).

Windows (SafeProcessHandle.Windows.cs)

• SignalCore: only SIGKILL is supported (maps to TerminateProcess), matching PosixSignalRegistration.Create exception behavior for unsupported signals. Retrieves the error code via Marshal.GetLastWin32Error() before any checks and only constructs Win32Exception(errorCode) when actually throwing. Detects already-exited processes via ERROR_ACCESS_DENIED + GetExitCodeProcess returning non-STILL_ACTIVE.
• No separate KillCore; Kill() calls SignalCore(PosixSignal.SIGKILL) directly from the shared file.

Unix (SafeProcessHandle.Unix.cs)

• SignalCore: checks ProcessUtils.PlatformDoesNotSupportProcessStartAndKill first (throws PlatformNotSupportedException on iOS/tvOS); uses Interop.Sys.GetPlatformSignalNumber(signal) to convert the managed PosixSignal to its platform-native signal number (throws PlatformNotSupportedException if unsupported, matching PosixSignalRegistration.Create behavior); passes the native signal number directly to Interop.Sys.Kill; returns false on ESRCH; uses Interop.Sys.GetLastErrorInfo() and passes errorInfo.RawErrno to the Win32Exception constructor.
• No separate KillCore; Kill() calls SignalCore(PosixSignal.SIGKILL) directly from the shared file.

Native (pal_process.c / pal_signal.c)

• SystemNative_Kill simplified to call kill(pid, signal) directly with no PAL mapping switch — it now takes the platform-native signal number, consistent with other System.Native APIs such as SystemNative_EnablePosixSignalHandling. The old Signals PAL enum (PAL_NONE, PAL_SIGKILL, PAL_SIGSTOP) has been removed from pal_process.h.
• SystemNative_GetPlatformSIGSTOP() moved to pal_signal.h/pal_signal.c (right below SystemNative_GetPlatformSignalNumber) since it is signal-related. Dummy implementations returning 0 are provided in pal_signal_wasm.c for both SystemNative_GetPlatformSIGSTOP and SystemNative_GetPlatformSignalNumber for BROWSER and WASI platforms.
• entrypoints.c: moved the DllImportEntry(SystemNative_GetPlatformSIGSTOP) registration from the process group (between SystemNative_Kill and SystemNative_GetPid) to the signal group (alongside SystemNative_GetPlatformSignalNumber, SystemNative_EnablePosixSignalHandling, etc.), consistent with the function's new home in pal_signal.c/pal_signal.h.

Process.Kill refactoring (Process.Windows.cs)

• Process.Kill() on Windows now delegates to SafeProcessHandle.Kill() after obtaining the handle, eliminating the duplicated TerminateProcess + error-handling logic.

Interop (Interop.Kill.cs / Interop.PosixSignal.cs)

• Removed the Signals managed enum; Interop.Sys.Kill now takes a plain int signal parameter (platform-native number).
• Interop.Sys.GetPlatformSIGSTOP() P/Invoke moved to Interop.PosixSignal.cs alongside GetPlatformSignalNumber, keeping all signal-number interop co-located.
• All existing callers updated: Process.Unix.cs uses GetPlatformSignalNumber(PosixSignal.SIGKILL) and GetPlatformSIGSTOP(); ProcessWaitState.Unix.cs and ProcessManager.Unix.cs pass 0 directly for the "probe" call.

Project / interop

• Added Interop.PosixSignal.cs to the Unix ItemGroup in System.Diagnostics.Process.csproj to expose GetPlatformSignalNumber and GetPlatformSIGSTOP in the main assembly.

Tests

• SafeProcessHandleTests.cs: Invalid handle → InvalidOperationException; Kill / Signal(SIGKILL) on running process terminates it; Kill on exited process does not throw; Signal(SIGKILL) returns false on exited process; Windows: non-SIGKILL signal → PlatformNotSupportedException; Unix: Signal(SIGTERM) on running process returns true and terminates it; Windows: Kill_HandleWithoutTerminatePermission_ThrowsWin32Exception — opens a handle with only PROCESS_QUERY_LIMITED_INFORMATION and verifies Kill() throws Win32Exception.
• ProcessTests.Unix.cs: Updated ChildProcess_WithParentSignalHandler_CanReceiveSignals to use SafeProcessHandle.Signal instead of the raw SendSignal helper. Refactored SendSignal helper to accept a Process parameter — Unix calls process.SafeHandle.Signal(signal) directly (simplified to a single-line Assert.True). Removed the class-level SIGKILL constant and kill P/Invoke declaration; Kill_ExitedNonChildProcess_DoesNotThrow now uses SafeHandle.Signal(PosixSignal.SIGKILL) and Process.HasExited.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

[adamsitnik]

@copilot address my feedback, also do search for other places in the repo where the new Signal API could be used

[Copilot]

@copilot address my feedback, also do search for other places in the repo where the new Signal API could be used

All feedback addressed in 7c03c8c. For other usages: the main candidate in the codebase is the SendSignal helper in tests (ProcessTests.cs:173, :246, :256, :260) which sends signals via kill() directly. However, those tests operate against a Process instance using its Id, while our Signal API requires a SafeProcessHandle — on Unix the handle's ProcessId is only set when the process was started via SafeProcessHandle.Start (or Process.Start). The ChildProcess_WithParentSignalHandler_CanReceiveSignals test was the natural fit since it already uses RemoteInvokeHandle with a started Process, so Process.SafeHandle.Signal() works correctly there.

[Copilot]

Pull request overview

Adds process-termination and signal-sending capabilities directly to SafeProcessHandle, enabling callers to manage processes via handles without going through Process, with platform-specific implementations for Windows and Unix plus corresponding test updates.

Changes:

• Added SafeProcessHandle.Kill() and SafeProcessHandle.Signal(PosixSignal) public APIs (with ref-assembly updates).
• Implemented SignalCore on Windows (via TerminateProcess) and Unix (via kill(2) + GetPlatformSignalNumber).
• Added/updated tests to validate new APIs and switched an existing Unix test to use SafeProcessHandle.Signal.

Reviewed changes

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

Show a summary per file

File	Description
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs	Introduces the new public Kill/Signal APIs and validation path.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs	Implements Windows SignalCore mapping SIGKILL to TerminateProcess with exited-process detection.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs	Implements Unix SignalCore using GetPlatformSignalNumber + kill, returning false on ESRCH.
src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj	Adds Interop.PosixSignal.cs to the Unix compile item group.
src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs	Updates the public ref surface for SafeProcessHandle with Kill/Signal.
src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs	Adds unit tests covering invalid-handle, running-process, exited-process, and platform-specific behaviors.
src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs	Updates signal-delivery test to use SafeProcessHandle.Signal instead of a raw P/Invoke helper.

[github-actions]

🤖 Copilot Code Review — PR #126313

Note

This review was generated by GitHub Copilot using Claude Opus 4.6.

Holistic Assessment

Motivation: Justified. This is the next step in the approved API expansion (#125838, api-approved by @bartonjs on 2026-03-24). Following the merged Start/ProcessId PR (#126192), Kill and Signal are the natural additions to enable callers to manage process lifecycle via SafeProcessHandle without requiring a Process object. The motivation is clear and real.

Approach: Sound. The implementation follows the approved API shape exactly, builds on the existing SafeProcessHandle partial class pattern, and properly separates platform-specific behavior. The native code simplification (removing the Signals PAL enum and having SystemNative_Kill accept platform-native signal numbers directly) is a good cleanup that aligns with how other System.Native APIs like SystemNative_EnablePosixSignalHandling work. The decision to have Kill() delegate to SignalCore(SIGKILL) keeps the code DRY while the void return type correctly matches the "fire-and-forget" semantics of Process.Kill().

Summary: ✅ LGTM. The implementation matches the approved API shape, all prior maintainer review feedback (from @jkotas, @stephentoub, @adamsitnik) has been addressed through multiple iterations, and the code is correct and well-tested. No blocking issues found. Two minor follow-up observations below.

---

Detailed Findings

✅ API Approval Verification — Matches approved shape

The approved API from #125838 (comment) by @bartonjs specifies:

publicpartialclassSafeProcessHandle{publicvoidKill();publicboolSignal(PosixSignalsignal);}

The ref assembly implementation matches exactly:

• Kill() → void return ✅
• Signal(PosixSignal signal) → bool return ✅
• Parameter name signal matches ✅
• No extra unapproved public API ✅
• No missing approved API from this PR's scope ✅
• Platform attributes ([UnsupportedOSPlatform("ios")], [UnsupportedOSPlatform("tvos")], [SupportedOSPlatform("maccatalyst")]) are consistent with Start ✅

✅ Correctness — Edge cases handled properly

Both platform implementations handle key edge cases correctly:

• Already-exited process: Windows detects via ERROR_ACCESS_DENIED + GetExitCodeProcess returning non-STILL_ACTIVE; Unix detects via ESRCH. Both return false from SignalCore, which Kill() silently discards (matching Process.Kill semantics).
• Unsupported signals: Windows throws PlatformNotSupportedException for non-SIGKILL (matching PosixSignalRegistration.Create behavior); Unix throws PlatformNotSupportedException when GetPlatformSignalNumber returns 0.
• iOS/tvOS: Unix SignalCore checks ProcessUtils.PlatformDoesNotSupportProcessStartAndKill first, matching the pattern in Process.Kill().
• Error code capture: Windows captures Marshal.GetLastWin32Error() immediately after TerminateProcess fails (before any other calls); Unix uses Interop.Sys.GetLastErrorInfo() and passes RawErrno to Win32Exception.
• Process.Kill delegation: On Windows, Process.Kill() now correctly delegates to SafeProcessHandle.Kill(), reducing code duplication.

✅ Native Code — Clean simplification

• SystemNative_Kill stripped to a thin wrapper around kill(pid, signal) — all PAL mapping removed since callers now pass platform-native signal numbers. This is consistent with SystemNative_EnablePosixSignalHandling and other System.Native APIs.
• SystemNative_GetPlatformSIGSTOP() correctly placed in pal_signal.c/pal_signal.h (next to GetPlatformSignalNumber), with a WASM stub returning 0.
• The old Signals PAL enum (PAL_NONE, PAL_SIGKILL, PAL_SIGSTOP) removed cleanly.

✅ Test Coverage — Comprehensive

Tests cover all key scenarios:

Test	Scenario
Kill_InvalidHandle_ThrowsInvalidOperationException	Error path — invalid handle
Signal_InvalidHandle_ThrowsInvalidOperationException	Error path — invalid handle
Kill_RunningProcess_Terminates	Happy path — kill via SafeProcessHandle.Start
Kill_AlreadyExited_DoesNotThrow	Already-exited process — no exception
Signal_SIGKILL_RunningProcess_ReturnsTrue	Signal delivery confirmation
Signal_SIGKILL_AlreadyExited_ReturnsFalse	Already-exited returns false
Signal_NonSIGKILL_OnWindows_ThrowsPlatformNotSupportedException	Windows platform limitation
Signal_SIGTERM_RunningProcess_ReturnsTrue	Unix signal support
Kill_HandleWithoutTerminatePermission_ThrowsWin32Exception	Windows permissions

Existing tests also updated to use the new API (Kill_ExitedNonChildProcess_DoesNotThrow, ChildProcess_WithParentSignalHandler_CanReceiveSignals).

💡 entrypoints.c — Minor organizational note (follow-up)

SystemNative_GetPlatformSIGSTOP was moved to pal_signal.c/pal_signal.h (alongside GetPlatformSignalNumber), but its entrypoint entry in entrypoints.c (line 219) remains in the pal_process group between SystemNative_Kill and SystemNative_GetPid. The other signal entries (GetPlatformSignalNumber, EnablePosixSignalHandling, etc.) are at lines 286-291. Moving the entrypoint to the signal group would be more consistent, but this is purely organizational and not a blocker.

💡 Review feedback fully addressed

All prior review comments from @jkotas, @stephentoub, and @adamsitnik have been addressed across 9+ follow-up commits, including:

• Removing Unix implementation details from Kill() docs (stephentoub)
• Adding PROCESS_TERMINATE to docs + permission test (jkotas/adamsitnik)
• Delegating Process.Kill() on Windows to SafeProcessHandle.Kill() (jkotas)
• Adding PlatformDoesNotSupportProcessStartAndKill check (jkotas)
• Simplifying native SystemNative_Kill to pass-through (jkotas)
• Updating SendSignal test helper to accept Process (jkotas/adamsitnik)
• Using short Win32Exception name in XML docs (jkotas)

Generated by Code Review for issue #126313 · ◷

[jkotas]

SystemNative_GetPlatformSIGSTOP was moved to pal_signal.c/pal_signal.h (alongside GetPlatformSignalNumber), but its entrypoint entry in entrypoints.c (line 219) remains in the pal_process group between SystemNative_Kill and SystemNative_GetPid. The other signal entries (GetPlatformSignalNumber, EnablePosixSignalHandling, etc.) are at lines 286-291. Moving the entrypoint to the signal group would be more consistent

@copilot Address this feedback

[Copilot]

SystemNative_GetPlatformSIGSTOP was moved to pal_signal.c/pal_signal.h (alongside GetPlatformSignalNumber), but its entrypoint entry in entrypoints.c (line 219) remains in the pal_process group betw...

Done in 493466c — moved DllImportEntry(SystemNative_GetPlatformSIGSTOP) from the process group (after SystemNative_Kill) to the signal group (after SystemNative_GetPlatformSignalNumber).

[Copilot]

Adds Kill() and Signal(PosixSignal) to SafeProcessHandle, enabling callers to terminate or signal a process via a handle without going through Process.

Description

Public API (SafeProcessHandle.cs + ref assembly)

• Kill() — sends a request to the OS to terminate the process; silently no-ops if already exited (matches Process.Kill semantics). On Windows, the handle must have PROCESS_TERMINATE access.
• Signal(PosixSignal) — sends an arbitrary signal; returns false if process no longer exists (or never existed), true on delivery, throws PlatformNotSupportedException for unsupported signals. On Windows, the handle must have PROCESS_TERMINATE access.

Both methods validate the handle (throw InvalidOperationException on invalid handle), carry the same [UnsupportedOSPlatform("ios/tvos")] / [SupportedOSPlatform("maccatalyst")] annotations as Start, and throw PlatformNotSupportedException at runtime on iOS/tvOS (matching the ProcessUtils.PlatformDoesNotSupportProcessStartAndKill pattern used by Process.Kill).

Windows (SafeProcessHandle.Windows.cs)

• SignalCore: only SIGKILL is supported (maps to TerminateProcess), matching PosixSignalRegistration.Create exception behavior for unsupported signals. Retrieves the error code via Marshal.GetLastWin32Error() before any checks and only constructs Win32Exception(errorCode) when actually throwing. Detects already-exited processes via ERROR_ACCESS_DENIED + GetExitCodeProcess returning non-STILL_ACTIVE.
• No separate KillCore; Kill() calls SignalCore(PosixSignal.SIGKILL) directly from the shared file.

Unix (SafeProcessHandle.Unix.cs)

• SignalCore: checks ProcessUtils.PlatformDoesNotSupportProcessStartAndKill first (throws PlatformNotSupportedException on iOS/tvOS); uses Interop.Sys.GetPlatformSignalNumber(signal) to convert the managed PosixSignal to its platform-native signal number (throws PlatformNotSupportedException if unsupported, matching PosixSignalRegistration.Create behavior); passes the native signal number directly to Interop.Sys.Kill; returns false on ESRCH; uses Interop.Sys.GetLastErrorInfo() and passes errorInfo.RawErrno to the Win32Exception constructor.
• No separate KillCore; Kill() calls SignalCore(PosixSignal.SIGKILL) directly from the shared file.

Native (pal_process.c / pal_signal.c)

• SystemNative_Kill simplified to call kill(pid, signal) directly with no PAL mapping switch — it now takes the platform-native signal number, consistent with other System.Native APIs such as SystemNative_EnablePosixSignalHandling. The old Signals PAL enum (PAL_NONE, PAL_SIGKILL, PAL_SIGSTOP) has been removed from pal_process.h.
• SystemNative_GetPlatformSIGSTOP() moved to pal_signal.h/pal_signal.c (right below SystemNative_GetPlatformSignalNumber) since it is signal-related. Dummy implementations returning 0 are provided in pal_signal_wasm.c for both SystemNative_GetPlatformSIGSTOP and SystemNative_GetPlatformSignalNumber for BROWSER and WASI platforms.
• entrypoints.c: moved the DllImportEntry(SystemNative_GetPlatformSIGSTOP) registration from the process group (between SystemNative_Kill and SystemNative_GetPid) to the signal group (alongside SystemNative_GetPlatformSignalNumber, SystemNative_EnablePosixSignalHandling, etc.), consistent with the function's new home in pal_signal.c/pal_signal.h.

Process.Kill refactoring (Process.Windows.cs)

• Process.Kill() on Windows now delegates to SafeProcessHandle.Kill() after obtaining the handle, eliminating the duplicated TerminateProcess + error-handling logic.

Interop (Interop.Kill.cs / Interop.PosixSignal.cs)

• Removed the Signals managed enum; Interop.Sys.Kill now takes a plain int signal parameter (platform-native number).
• Interop.Sys.GetPlatformSIGSTOP() P/Invoke moved to Interop.PosixSignal.cs alongside GetPlatformSignalNumber, keeping all signal-number interop co-located.
• All existing callers updated: Process.Unix.cs uses GetPlatformSignalNumber(PosixSignal.SIGKILL) and GetPlatformSIGSTOP(); ProcessWaitState.Unix.cs and ProcessManager.Unix.cs pass 0 directly for the "probe" call.

Project / interop

• Added Interop.PosixSignal.cs to the Unix ItemGroup in System.Diagnostics.Process.csproj to expose GetPlatformSignalNumber and GetPlatformSIGSTOP in the main assembly.

Tests

• SafeProcessHandleTests.cs: Invalid handle → InvalidOperationException; Kill / Signal(SIGKILL) on running process terminates it; Kill on exited process does not throw; Signal(SIGKILL) returns false on exited process; Windows: non-SIGKILL signal → PlatformNotSupportedException; Unix: Signal(SIGTERM) on running process returns true and terminates it; Windows: Kill_HandleWithoutTerminatePermission_ThrowsWin32Exception — opens a handle with only PROCESS_QUERY_LIMITED_INFORMATION and verifies Kill() throws Win32Exception.
• ProcessTests.Unix.cs: Updated ChildProcess_WithParentSignalHandler_CanReceiveSignals to use SafeProcessHandle.Signal instead of the raw SendSignal helper. Refactored SendSignal helper to accept a Process parameter — Unix calls process.SafeHandle.Signal(signal) directly (simplified to a single-line Assert.True). Removed the class-level SIGKILL constant and kill P/Invoke declaration; Kill_ExitedNonChildProcess_DoesNotThrow now uses SafeHandle.Signal(PosixSignal.SIGKILL) and Process.HasExited.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

[adamsitnik]

@copilot address my feedback, also do search for other places in the repo where the new Signal API could be used

[Copilot]

@copilot address my feedback, also do search for other places in the repo where the new Signal API could be used

All feedback addressed in 7c03c8c. For other usages: the main candidate in the codebase is the SendSignal helper in tests (ProcessTests.cs:173, :246, :256, :260) which sends signals via kill() directly. However, those tests operate against a Process instance using its Id, while our Signal API requires a SafeProcessHandle — on Unix the handle's ProcessId is only set when the process was started via SafeProcessHandle.Start (or Process.Start). The ChildProcess_WithParentSignalHandler_CanReceiveSignals test was the natural fit since it already uses RemoteInvokeHandle with a started Process, so Process.SafeHandle.Signal() works correctly there.

[Copilot]

Pull request overview

Adds process-termination and signal-sending capabilities directly to SafeProcessHandle, enabling callers to manage processes via handles without going through Process, with platform-specific implementations for Windows and Unix plus corresponding test updates.

Changes:

• Added SafeProcessHandle.Kill() and SafeProcessHandle.Signal(PosixSignal) public APIs (with ref-assembly updates).
• Implemented SignalCore on Windows (via TerminateProcess) and Unix (via kill(2) + GetPlatformSignalNumber).
• Added/updated tests to validate new APIs and switched an existing Unix test to use SafeProcessHandle.Signal.

Reviewed changes

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

Show a summary per file

File	Description
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs	Introduces the new public Kill/Signal APIs and validation path.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs	Implements Windows SignalCore mapping SIGKILL to TerminateProcess with exited-process detection.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs	Implements Unix SignalCore using GetPlatformSignalNumber + kill, returning false on ESRCH.
src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj	Adds Interop.PosixSignal.cs to the Unix compile item group.
src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs	Updates the public ref surface for SafeProcessHandle with Kill/Signal.
src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs	Adds unit tests covering invalid-handle, running-process, exited-process, and platform-specific behaviors.
src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs	Updates signal-delivery test to use SafeProcessHandle.Signal instead of a raw P/Invoke helper.

[github-actions]

🤖 Copilot Code Review — PR #126313

Note

This review was generated by GitHub Copilot using Claude Opus 4.6.

Holistic Assessment

Motivation: Justified. This is the next step in the approved API expansion (#125838, api-approved by @bartonjs on 2026-03-24). Following the merged Start/ProcessId PR (#126192), Kill and Signal are the natural additions to enable callers to manage process lifecycle via SafeProcessHandle without requiring a Process object. The motivation is clear and real.

Approach: Sound. The implementation follows the approved API shape exactly, builds on the existing SafeProcessHandle partial class pattern, and properly separates platform-specific behavior. The native code simplification (removing the Signals PAL enum and having SystemNative_Kill accept platform-native signal numbers directly) is a good cleanup that aligns with how other System.Native APIs like SystemNative_EnablePosixSignalHandling work. The decision to have Kill() delegate to SignalCore(SIGKILL) keeps the code DRY while the void return type correctly matches the "fire-and-forget" semantics of Process.Kill().

Summary: ✅ LGTM. The implementation matches the approved API shape, all prior maintainer review feedback (from @jkotas, @stephentoub, @adamsitnik) has been addressed through multiple iterations, and the code is correct and well-tested. No blocking issues found. Two minor follow-up observations below.

---

Detailed Findings

✅ API Approval Verification — Matches approved shape

The approved API from #125838 (comment) by @bartonjs specifies:

publicpartialclassSafeProcessHandle{publicvoidKill();publicboolSignal(PosixSignalsignal);}

The ref assembly implementation matches exactly:

• Kill() → void return ✅
• Signal(PosixSignal signal) → bool return ✅
• Parameter name signal matches ✅
• No extra unapproved public API ✅
• No missing approved API from this PR's scope ✅
• Platform attributes ([UnsupportedOSPlatform("ios")], [UnsupportedOSPlatform("tvos")], [SupportedOSPlatform("maccatalyst")]) are consistent with Start ✅

✅ Correctness — Edge cases handled properly

Both platform implementations handle key edge cases correctly:

• Already-exited process: Windows detects via ERROR_ACCESS_DENIED + GetExitCodeProcess returning non-STILL_ACTIVE; Unix detects via ESRCH. Both return false from SignalCore, which Kill() silently discards (matching Process.Kill semantics).
• Unsupported signals: Windows throws PlatformNotSupportedException for non-SIGKILL (matching PosixSignalRegistration.Create behavior); Unix throws PlatformNotSupportedException when GetPlatformSignalNumber returns 0.
• iOS/tvOS: Unix SignalCore checks ProcessUtils.PlatformDoesNotSupportProcessStartAndKill first, matching the pattern in Process.Kill().
• Error code capture: Windows captures Marshal.GetLastWin32Error() immediately after TerminateProcess fails (before any other calls); Unix uses Interop.Sys.GetLastErrorInfo() and passes RawErrno to Win32Exception.
• Process.Kill delegation: On Windows, Process.Kill() now correctly delegates to SafeProcessHandle.Kill(), reducing code duplication.

✅ Native Code — Clean simplification

• SystemNative_Kill stripped to a thin wrapper around kill(pid, signal) — all PAL mapping removed since callers now pass platform-native signal numbers. This is consistent with SystemNative_EnablePosixSignalHandling and other System.Native APIs.
• SystemNative_GetPlatformSIGSTOP() correctly placed in pal_signal.c/pal_signal.h (next to GetPlatformSignalNumber), with a WASM stub returning 0.
• The old Signals PAL enum (PAL_NONE, PAL_SIGKILL, PAL_SIGSTOP) removed cleanly.

✅ Test Coverage — Comprehensive

Tests cover all key scenarios:

Test	Scenario
Kill_InvalidHandle_ThrowsInvalidOperationException	Error path — invalid handle
Signal_InvalidHandle_ThrowsInvalidOperationException	Error path — invalid handle
Kill_RunningProcess_Terminates	Happy path — kill via SafeProcessHandle.Start
Kill_AlreadyExited_DoesNotThrow	Already-exited process — no exception
Signal_SIGKILL_RunningProcess_ReturnsTrue	Signal delivery confirmation
Signal_SIGKILL_AlreadyExited_ReturnsFalse	Already-exited returns false
Signal_NonSIGKILL_OnWindows_ThrowsPlatformNotSupportedException	Windows platform limitation
Signal_SIGTERM_RunningProcess_ReturnsTrue	Unix signal support
Kill_HandleWithoutTerminatePermission_ThrowsWin32Exception	Windows permissions

Existing tests also updated to use the new API (Kill_ExitedNonChildProcess_DoesNotThrow, ChildProcess_WithParentSignalHandler_CanReceiveSignals).

💡 entrypoints.c — Minor organizational note (follow-up)

SystemNative_GetPlatformSIGSTOP was moved to pal_signal.c/pal_signal.h (alongside GetPlatformSignalNumber), but its entrypoint entry in entrypoints.c (line 219) remains in the pal_process group between SystemNative_Kill and SystemNative_GetPid. The other signal entries (GetPlatformSignalNumber, EnablePosixSignalHandling, etc.) are at lines 286-291. Moving the entrypoint to the signal group would be more consistent, but this is purely organizational and not a blocker.

💡 Review feedback fully addressed

All prior review comments from @jkotas, @stephentoub, and @adamsitnik have been addressed across 9+ follow-up commits, including:

• Removing Unix implementation details from Kill() docs (stephentoub)
• Adding PROCESS_TERMINATE to docs + permission test (jkotas/adamsitnik)
• Delegating Process.Kill() on Windows to SafeProcessHandle.Kill() (jkotas)
• Adding PlatformDoesNotSupportProcessStartAndKill check (jkotas)
• Simplifying native SystemNative_Kill to pass-through (jkotas)
• Updating SendSignal test helper to accept Process (jkotas/adamsitnik)
• Using short Win32Exception name in XML docs (jkotas)

Generated by Code Review for issue #126313 · ◷

[jkotas]

SystemNative_GetPlatformSIGSTOP was moved to pal_signal.c/pal_signal.h (alongside GetPlatformSignalNumber), but its entrypoint entry in entrypoints.c (line 219) remains in the pal_process group between SystemNative_Kill and SystemNative_GetPid. The other signal entries (GetPlatformSignalNumber, EnablePosixSignalHandling, etc.) are at lines 286-291. Moving the entrypoint to the signal group would be more consistent

@copilot Address this feedback

[Copilot]

SystemNative_GetPlatformSIGSTOP was moved to pal_signal.c/pal_signal.h (alongside GetPlatformSignalNumber), but its entrypoint entry in entrypoints.c (line 219) remains in the pal_process group betw...

Done in 493466c — moved DllImportEntry(SystemNative_GetPlatformSIGSTOP) from the process group (after SystemNative_Kill) to the signal group (after SystemNative_GetPlatformSignalNumber).

[Copilot]

Adds Kill() and Signal(PosixSignal) to SafeProcessHandle, enabling callers to terminate or signal a process via a handle without going through Process.

Description

Public API (SafeProcessHandle.cs + ref assembly)

• Kill() — sends a request to the OS to terminate the process; silently no-ops if already exited (matches Process.Kill semantics). On Windows, the handle must have PROCESS_TERMINATE access.
• Signal(PosixSignal) — sends an arbitrary signal; returns false if process no longer exists (or never existed), true on delivery, throws PlatformNotSupportedException for unsupported signals. On Windows, the handle must have PROCESS_TERMINATE access.

Both methods validate the handle (throw InvalidOperationException on invalid handle), carry the same [UnsupportedOSPlatform("ios/tvos")] / [SupportedOSPlatform("maccatalyst")] annotations as Start, and throw PlatformNotSupportedException at runtime on iOS/tvOS (matching the ProcessUtils.PlatformDoesNotSupportProcessStartAndKill pattern used by Process.Kill).

Windows (SafeProcessHandle.Windows.cs)

• SignalCore: only SIGKILL is supported (maps to TerminateProcess), matching PosixSignalRegistration.Create exception behavior for unsupported signals. Retrieves the error code via Marshal.GetLastWin32Error() before any checks and only constructs Win32Exception(errorCode) when actually throwing. Detects already-exited processes via ERROR_ACCESS_DENIED + GetExitCodeProcess returning non-STILL_ACTIVE.
• No separate KillCore; Kill() calls SignalCore(PosixSignal.SIGKILL) directly from the shared file.

Unix (SafeProcessHandle.Unix.cs)

• SignalCore: checks ProcessUtils.PlatformDoesNotSupportProcessStartAndKill first (throws PlatformNotSupportedException on iOS/tvOS); uses Interop.Sys.GetPlatformSignalNumber(signal) to convert the managed PosixSignal to its platform-native signal number (throws PlatformNotSupportedException if unsupported, matching PosixSignalRegistration.Create behavior); passes the native signal number directly to Interop.Sys.Kill; returns false on ESRCH; uses Interop.Sys.GetLastErrorInfo() and passes errorInfo.RawErrno to the Win32Exception constructor.
• No separate KillCore; Kill() calls SignalCore(PosixSignal.SIGKILL) directly from the shared file.

Native (pal_process.c / pal_signal.c)

• SystemNative_Kill simplified to call kill(pid, signal) directly with no PAL mapping switch — it now takes the platform-native signal number, consistent with other System.Native APIs such as SystemNative_EnablePosixSignalHandling. The old Signals PAL enum (PAL_NONE, PAL_SIGKILL, PAL_SIGSTOP) has been removed from pal_process.h.
• SystemNative_GetPlatformSIGSTOP() moved to pal_signal.h/pal_signal.c (right below SystemNative_GetPlatformSignalNumber) since it is signal-related. Dummy implementations returning 0 are provided in pal_signal_wasm.c for both SystemNative_GetPlatformSIGSTOP and SystemNative_GetPlatformSignalNumber for BROWSER and WASI platforms.
• entrypoints.c: moved the DllImportEntry(SystemNative_GetPlatformSIGSTOP) registration from the process group (between SystemNative_Kill and SystemNative_GetPid) to the signal group (alongside SystemNative_GetPlatformSignalNumber, SystemNative_EnablePosixSignalHandling, etc.), consistent with the function's new home in pal_signal.c/pal_signal.h.

Process.Kill refactoring (Process.Windows.cs)

• Process.Kill() on Windows now delegates to SafeProcessHandle.Kill() after obtaining the handle, eliminating the duplicated TerminateProcess + error-handling logic.

Interop (Interop.Kill.cs / Interop.PosixSignal.cs)

• Removed the Signals managed enum; Interop.Sys.Kill now takes a plain int signal parameter (platform-native number).
• Interop.Sys.GetPlatformSIGSTOP() P/Invoke moved to Interop.PosixSignal.cs alongside GetPlatformSignalNumber, keeping all signal-number interop co-located.
• All existing callers updated: Process.Unix.cs uses GetPlatformSignalNumber(PosixSignal.SIGKILL) and GetPlatformSIGSTOP(); ProcessWaitState.Unix.cs and ProcessManager.Unix.cs pass 0 directly for the "probe" call.

Project / interop

• Added Interop.PosixSignal.cs to the Unix ItemGroup in System.Diagnostics.Process.csproj to expose GetPlatformSignalNumber and GetPlatformSIGSTOP in the main assembly.

Tests

• SafeProcessHandleTests.cs: Invalid handle → InvalidOperationException; Kill / Signal(SIGKILL) on running process terminates it; Kill on exited process does not throw; Signal(SIGKILL) returns false on exited process; Windows: non-SIGKILL signal → PlatformNotSupportedException; Unix: Signal(SIGTERM) on running process returns true and terminates it; Windows: Kill_HandleWithoutTerminatePermission_ThrowsWin32Exception — opens a handle with only PROCESS_QUERY_LIMITED_INFORMATION and verifies Kill() throws Win32Exception.
• ProcessTests.Unix.cs: Updated ChildProcess_WithParentSignalHandler_CanReceiveSignals to use SafeProcessHandle.Signal instead of the raw SendSignal helper. Refactored SendSignal helper to accept a Process parameter — Unix calls process.SafeHandle.Signal(signal) directly (simplified to a single-line Assert.True). Removed the class-level SIGKILL constant and kill P/Invoke declaration; Kill_ExitedNonChildProcess_DoesNotThrow now uses SafeHandle.Signal(PosixSignal.SIGKILL) and Process.HasExited.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

[adamsitnik]

@copilot address my feedback, also do search for other places in the repo where the new Signal API could be used

[Copilot]

@copilot address my feedback, also do search for other places in the repo where the new Signal API could be used

All feedback addressed in 7c03c8c. For other usages: the main candidate in the codebase is the SendSignal helper in tests (ProcessTests.cs:173, :246, :256, :260) which sends signals via kill() directly. However, those tests operate against a Process instance using its Id, while our Signal API requires a SafeProcessHandle — on Unix the handle's ProcessId is only set when the process was started via SafeProcessHandle.Start (or Process.Start). The ChildProcess_WithParentSignalHandler_CanReceiveSignals test was the natural fit since it already uses RemoteInvokeHandle with a started Process, so Process.SafeHandle.Signal() works correctly there.

[Copilot]

Pull request overview

Adds process-termination and signal-sending capabilities directly to SafeProcessHandle, enabling callers to manage processes via handles without going through Process, with platform-specific implementations for Windows and Unix plus corresponding test updates.

Changes:

• Added SafeProcessHandle.Kill() and SafeProcessHandle.Signal(PosixSignal) public APIs (with ref-assembly updates).
• Implemented SignalCore on Windows (via TerminateProcess) and Unix (via kill(2) + GetPlatformSignalNumber).
• Added/updated tests to validate new APIs and switched an existing Unix test to use SafeProcessHandle.Signal.

Reviewed changes

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

Show a summary per file

File	Description
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs	Introduces the new public Kill/Signal APIs and validation path.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs	Implements Windows SignalCore mapping SIGKILL to TerminateProcess with exited-process detection.
src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs	Implements Unix SignalCore using GetPlatformSignalNumber + kill, returning false on ESRCH.
src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj	Adds Interop.PosixSignal.cs to the Unix compile item group.
src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs	Updates the public ref surface for SafeProcessHandle with Kill/Signal.
src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs	Adds unit tests covering invalid-handle, running-process, exited-process, and platform-specific behaviors.
src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs	Updates signal-delivery test to use SafeProcessHandle.Signal instead of a raw P/Invoke helper.

[github-actions]

🤖 Copilot Code Review — PR #126313

Note

This review was generated by GitHub Copilot using Claude Opus 4.6.

Holistic Assessment

Motivation: Justified. This is the next step in the approved API expansion (#125838, api-approved by @bartonjs on 2026-03-24). Following the merged Start/ProcessId PR (#126192), Kill and Signal are the natural additions to enable callers to manage process lifecycle via SafeProcessHandle without requiring a Process object. The motivation is clear and real.

Approach: Sound. The implementation follows the approved API shape exactly, builds on the existing SafeProcessHandle partial class pattern, and properly separates platform-specific behavior. The native code simplification (removing the Signals PAL enum and having SystemNative_Kill accept platform-native signal numbers directly) is a good cleanup that aligns with how other System.Native APIs like SystemNative_EnablePosixSignalHandling work. The decision to have Kill() delegate to SignalCore(SIGKILL) keeps the code DRY while the void return type correctly matches the "fire-and-forget" semantics of Process.Kill().

Summary: ✅ LGTM. The implementation matches the approved API shape, all prior maintainer review feedback (from @jkotas, @stephentoub, @adamsitnik) has been addressed through multiple iterations, and the code is correct and well-tested. No blocking issues found. Two minor follow-up observations below.

---

Detailed Findings

✅ API Approval Verification — Matches approved shape

The approved API from #125838 (comment) by @bartonjs specifies:

publicpartialclassSafeProcessHandle{publicvoidKill();publicboolSignal(PosixSignalsignal);}

The ref assembly implementation matches exactly:

• Kill() → void return ✅
• Signal(PosixSignal signal) → bool return ✅
• Parameter name signal matches ✅
• No extra unapproved public API ✅
• No missing approved API from this PR's scope ✅
• Platform attributes ([UnsupportedOSPlatform("ios")], [UnsupportedOSPlatform("tvos")], [SupportedOSPlatform("maccatalyst")]) are consistent with Start ✅

✅ Correctness — Edge cases handled properly

Both platform implementations handle key edge cases correctly:

• Already-exited process: Windows detects via ERROR_ACCESS_DENIED + GetExitCodeProcess returning non-STILL_ACTIVE; Unix detects via ESRCH. Both return false from SignalCore, which Kill() silently discards (matching Process.Kill semantics).
• Unsupported signals: Windows throws PlatformNotSupportedException for non-SIGKILL (matching PosixSignalRegistration.Create behavior); Unix throws PlatformNotSupportedException when GetPlatformSignalNumber returns 0.
• iOS/tvOS: Unix SignalCore checks ProcessUtils.PlatformDoesNotSupportProcessStartAndKill first, matching the pattern in Process.Kill().
• Error code capture: Windows captures Marshal.GetLastWin32Error() immediately after TerminateProcess fails (before any other calls); Unix uses Interop.Sys.GetLastErrorInfo() and passes RawErrno to Win32Exception.
• Process.Kill delegation: On Windows, Process.Kill() now correctly delegates to SafeProcessHandle.Kill(), reducing code duplication.

✅ Native Code — Clean simplification

• SystemNative_Kill stripped to a thin wrapper around kill(pid, signal) — all PAL mapping removed since callers now pass platform-native signal numbers. This is consistent with SystemNative_EnablePosixSignalHandling and other System.Native APIs.
• SystemNative_GetPlatformSIGSTOP() correctly placed in pal_signal.c/pal_signal.h (next to GetPlatformSignalNumber), with a WASM stub returning 0.
• The old Signals PAL enum (PAL_NONE, PAL_SIGKILL, PAL_SIGSTOP) removed cleanly.

✅ Test Coverage — Comprehensive

Tests cover all key scenarios:

Test	Scenario
Kill_InvalidHandle_ThrowsInvalidOperationException	Error path — invalid handle
Signal_InvalidHandle_ThrowsInvalidOperationException	Error path — invalid handle
Kill_RunningProcess_Terminates	Happy path — kill via SafeProcessHandle.Start
Kill_AlreadyExited_DoesNotThrow	Already-exited process — no exception
Signal_SIGKILL_RunningProcess_ReturnsTrue	Signal delivery confirmation
Signal_SIGKILL_AlreadyExited_ReturnsFalse	Already-exited returns false
Signal_NonSIGKILL_OnWindows_ThrowsPlatformNotSupportedException	Windows platform limitation
Signal_SIGTERM_RunningProcess_ReturnsTrue	Unix signal support
Kill_HandleWithoutTerminatePermission_ThrowsWin32Exception	Windows permissions

Existing tests also updated to use the new API (Kill_ExitedNonChildProcess_DoesNotThrow, ChildProcess_WithParentSignalHandler_CanReceiveSignals).

💡 entrypoints.c — Minor organizational note (follow-up)

SystemNative_GetPlatformSIGSTOP was moved to pal_signal.c/pal_signal.h (alongside GetPlatformSignalNumber), but its entrypoint entry in entrypoints.c (line 219) remains in the pal_process group between SystemNative_Kill and SystemNative_GetPid. The other signal entries (GetPlatformSignalNumber, EnablePosixSignalHandling, etc.) are at lines 286-291. Moving the entrypoint to the signal group would be more consistent, but this is purely organizational and not a blocker.

💡 Review feedback fully addressed

All prior review comments from @jkotas, @stephentoub, and @adamsitnik have been addressed across 9+ follow-up commits, including:

• Removing Unix implementation details from Kill() docs (stephentoub)
• Adding PROCESS_TERMINATE to docs + permission test (jkotas/adamsitnik)
• Delegating Process.Kill() on Windows to SafeProcessHandle.Kill() (jkotas)
• Adding PlatformDoesNotSupportProcessStartAndKill check (jkotas)
• Simplifying native SystemNative_Kill to pass-through (jkotas)
• Updating SendSignal test helper to accept Process (jkotas/adamsitnik)
• Using short Win32Exception name in XML docs (jkotas)

Generated by Code Review for issue #126313 · ◷

[jkotas]

SystemNative_GetPlatformSIGSTOP was moved to pal_signal.c/pal_signal.h (alongside GetPlatformSignalNumber), but its entrypoint entry in entrypoints.c (line 219) remains in the pal_process group between SystemNative_Kill and SystemNative_GetPid. The other signal entries (GetPlatformSignalNumber, EnablePosixSignalHandling, etc.) are at lines 286-291. Moving the entrypoint to the signal group would be more consistent

@copilot Address this feedback

[Copilot]

SystemNative_GetPlatformSIGSTOP was moved to pal_signal.c/pal_signal.h (alongside GetPlatformSignalNumber), but its entrypoint entry in entrypoints.c (line 219) remains in the pal_process group betw...

Done in 493466c — moved DllImportEntry(SystemNative_GetPlatformSIGSTOP) from the process group (after SystemNative_Kill) to the signal group (after SystemNative_GetPlatformSignalNumber).