Skip to content

Replace ProcessUtilities.SendPosixSignal with SafeProcessHandle.Signal API#53920

Merged
adamsitnik merged 7 commits intomainfrom
copilot/replace-processutilities-sigkill-sigterm
Apr 27, 2026
Merged

Replace ProcessUtilities.SendPosixSignal with SafeProcessHandle.Signal API#53920
adamsitnik merged 7 commits intomainfrom
copilot/replace-processutilities-sigkill-sigterm

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 16, 2026

Migrate from the manual P/Invoke kill wrapper (ProcessUtilities.SendPosixSignal) to the new SafeProcessHandle.Signal(PosixSignal) API introduced in dotnet/runtime#126313.

Changes

  • ProcessRunner.cs: TerminateUnixProcess now takes a Process and calls process.SafeHandle.Signal(PosixSignal.SIGKILL / SIGTERM). Annotated [UnsupportedOSPlatform("windows")] to satisfy CA1416.
  • PhysicalConsole.cs: Resolves process by ID via Process.GetProcessById to obtain a handle for Signal. Catches ArgumentException for already-exited processes.
  • ProcessUtilities.cs: Removed SendPosixSignal, SIGKILL, and SIGTERM constants. SendWindowsCtrlCEvent remains.

Before:

var error = ProcessUtilities.SendPosixSignal(state.ProcessId,
    signal: force ? ProcessUtilities.SIGKILL : ProcessUtilities.SIGTERM);

After:

process.SafeHandle.Signal(force ? PosixSignal.SIGKILL : PosixSignal.SIGTERM);

Copilot AI and others added 2 commits April 16, 2026 14:13
Copy link
Copy Markdown
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot please address my feedback

Comment thread src/Dotnet.Watch/Watch/Process/ProcessRunner.cs Outdated
Comment thread src/Dotnet.Watch/Watch/UI/PhysicalConsole.cs
@adamsitnik adamsitnik marked this pull request as ready for review April 27, 2026 11:42
Copilot AI review requested due to automatic review settings April 27, 2026 11:42
@adamsitnik adamsitnik requested review from a team and tmat as code owners April 27, 2026 11:42
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR migrates dotnet-watch’s Unix process termination from a custom kill P/Invoke (ProcessUtilities.SendPosixSignal) to the newer SafeProcessHandle.Signal(PosixSignal) API, aligning the implementation with the runtime-provided signaling mechanism.

Changes:

  • Remove the SendPosixSignal P/Invoke wrapper and related SIG* constants from ProcessUtilities.
  • Update ProcessRunner to signal the existing Process via process.SafeHandle.Signal(...) on Unix.
  • Update PhysicalConsole to resolve processes by PID (Process.GetProcessById) in order to obtain a SafeHandle for signaling.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
src/Dotnet.Watch/Watch/Utilities/ProcessUtilities.cs Removes the custom kill P/Invoke signal-sending helper and constants, leaving only the Windows Ctrl+C path.
src/Dotnet.Watch/Watch/UI/PhysicalConsole.cs Uses Process.GetProcessById(...).SafeHandle.Signal(PosixSignal.SIGTERM) to propagate Ctrl+C/SIGTERM behavior to child processes on Unix.
src/Dotnet.Watch/Watch/Process/ProcessRunner.cs Uses process.SafeHandle.Signal(...) for Unix termination, and adds platform annotation to satisfy platform compatibility analysis.

Comment thread src/Dotnet.Watch/Watch/UI/PhysicalConsole.cs Outdated
Comment thread src/Dotnet.Watch/Watch/UI/PhysicalConsole.cs Outdated
Comment thread src/Dotnet.Watch/Watch/Process/ProcessRunner.cs Outdated
Comment thread src/Dotnet.Watch/Watch/Process/ProcessRunner.cs Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@adamsitnik adamsitnik enabled auto-merge (squash) April 27, 2026 12:54
@adamsitnik adamsitnik merged commit a773c33 into main Apr 27, 2026
24 checks passed
@adamsitnik adamsitnik deleted the copilot/replace-processutilities-sigkill-sigterm branch April 27, 2026 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants