Add new process management APIs to SafeProcessHandle by Copilot · Pull Request #124375 · dotnet/runtime

Copilot · 2026-02-13T11:57:36Z

Description

Implements the approved API surface for SafeProcessHandle (#123380): process lifecycle management via Start, Open, Kill, Signal, SignalProcessGroup, and WaitForExit overloads. Windows implementation complete; Unix stubs throw NotImplementedException (separate PR).

ProcessStartOptions options = new("cmd.exe") { Arguments = { "/c", "echo hello" } };
using SafeProcessHandle handle = SafeProcessHandle.Start(options, input: null, output: null, error: null);
ProcessExitStatus status = handle.WaitForExit();
// status.ExitCode == 0, status.Canceled == false

Public API

Open(int processId) — opens existing process handle
Start(ProcessStartOptions, SafeFileHandle?, SafeFileHandle?, SafeFileHandle?) — create process with explicit stdio handles
Kill() — terminate process
Signal(PosixSignal) / SignalProcessGroup(PosixSignal) — send signal; accepts raw signal numbers like PosixSignalRegistration.Create
WaitForExit(), TryWaitForExit(TimeSpan, out ProcessExitStatus?), WaitForExitOrKillOnTimeout(TimeSpan) — synchronous wait
WaitForExitAsync(CancellationToken), WaitForExitOrKillOnCancellationAsync(CancellationToken) — async wait

Internal for now (pending follow-up): ProcessId, StartSuspended, Resume, KillProcessGroup.

Windows implementation

STARTUPINFOEX + PROC_THREAD_ATTRIBUTE_HANDLE_LIST for explicit handle inheritance
Job objects for KillOnParentExit and process group termination (best-effort emulation of Unix process groups)
DangerousAddRef/DangerousRelease on InheritedHandles in PrepareHandleAllowList and on this in KillCore to prevent use-after-free races with concurrent Dispose
Interlocked.Exchange for _threadHandle and _processGroupJobHandle in ReleaseHandle; Volatile.Read for _processGroupJobHandle access elsewhere
Sync wait methods use WaitForSingleObject(SafeProcessHandle, ...) directly — no handle duplication via ProcessWaitHandle
Async wait methods retain ProcessWaitHandle for RegisterWaitForSingleObject
NativeMemory.Alloc with two-arg overflow-safe version; typed pointers (IntPtr*, void*)
OOM-safe handle cleanup in finally; error check immediately after CreateProcess

Architecture — `ProcessUtils` as shared dependency

ProcessUtils.cs holds s_processStartLock (ReaderWriterLockSlim) — avoids Process ↔ SafeProcessHandle dependency cycle
ProcessUtils.Windows.cs holds BuildArgs(string, IList<string>?) and GetEnvironmentVariablesBlock
Process.Windows.cs acquires write lock; SafeProcessHandle.Windows.cs acquires read lock

Interop additions

Interop.JobObjects.cs, Interop.ProcThreadAttributeList.cs, Interop.ConsoleCtrl.cs (consolidated), Interop.HandleInformation.cs (extended), Interop.ResumeThread_IntPtr.cs, Interop.WaitForSingleObject_SafeProcessHandle.cs

Tests

SafeProcessHandleTests.cs — start/kill/wait/timeout/signal tests with Nano Server support (ping fallback); signal tests as [Theory] with [InlineData]

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

…, Theory for utils Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

…h for sh Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

…syntax Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

… remove dead code Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

…names Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

…s.Process.csproj

…artoptions-class

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

…s, and Windows implementation Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

jkotas · 2026-03-03T16:17:17Z

@adamsitnik Let me know once you believe my feedback is addressed properly, and this is ready for another review

stephentoub · 2026-03-04T13:58:02Z

@copilot please invoke the code-review skill and post the analysis/comments as a comment on this PR

- use SafeHandles instead of raw handles - use DangerousAddRef and DangerousRelease for handles we don't control - simplify comments - call SetConsoleCtrlHandler in the test process to avoid weird pop up window - make ProcessId public - add tests for KillOnParentExit

adamsitnik · 2026-03-04T22:26:01Z

@copilot please invoke the code-review skill and post the analysis/comments as a comment on this PR

@copilot please do it

...ries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs

src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs

jkotas · 2026-03-04T18:12:24Z

src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs

+        // - powershell is not available on Nano. We can't always use it.
+        // - ping seems to be a workaround, but it's simple and work everywhere. The arguments are set to make it sleep for approximately 10 seconds.
+        private static ProcessStartOptions CreateTenSecondSleep() => OperatingSystem.IsWindows()
+            ? new("ping") { Arguments = { "127.0.0.1", "-n", "11" } }


0xc0000142 is STATUS_DLL_INIT_FAILED.

How does missing ReEnableCtrlHandlerIfNeeded lead to this error?

I would expect that missing ReEnableCtrlHandlerIfNeeded will make it impossible to send the signal to the process. I do not see how it can lead to STATUS_DLL_INIT_FAILED.

src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs

...ries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs

src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs

tmds · 2026-03-05T13:35:08Z

src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs

+            }
+            finally
+            {
+                nullHandle?.Dispose();


I wonder if it wouldn't make sense to dispose all handles in this finally block that are intended for the child process?

I think it helps on the caller side since usually you won't have a block that sits around just the Start call, so with a using these handles will be open longer than they should be.

@adamsitnik thoughts?

@tmds I really like this idea, it would simplify the implementation and make it more reliable (#124375 (comment)). At the same time, it would help us to avoid confusion like in my prototype, where only pipes were getting disposed.

And if somebody does not like it, they can pass a SafeFileHandle with ownsHandle: false

@jkotas @stephentoub thoughts?

This design would mean the one has to create a fresh set of SafeHandle instances for each process start. For example, this would not work:

var stdout = OpenStandardOutputHandle(); SafeProcessHandle.Start(... , output: stdout); SafeProcessHandle.Start(... , output: stdout);

And one would have to do this instead:

SafeProcessHandle.Start(... , output: OpenStandardOutputHandle()); SafeProcessHandle.Start(... , output: OpenStandardOutputHandle());

Is my understanding correct?

And if somebody does not like it, they can pass a SafeFileHandle with ownsHandle: false

ownsHandle: false would still have the problem with the handle being marked as inheritable by the process start.

Right, for some use-cases you want this and for others you don't.

The question also applies to the InheritedHandles.

It might make sense add a bool leaveOpen = false similar to other APIs.

It might make sense add a bool leaveOpen = false similar to other APIs.

But this would lead to another question: should we allow for setting leaveOpen per handle?

Since we provide InheritedHandles for completeness and I expect it to be a very niche API (used by users who use anonymous pipes for communicating with their child processes), I think it's OK to document it as required to be disposed by the users.

If we're considering an API, I'd consider a bool leaveOpen = false arg to Start and ProcessStartOptions.CloseInheritedHandlesOnStart = true, so that all handles are disposed by default and a user can opt-out and do things manually.

Co-authored-by: Jan Kotas <jkotas@microsoft.com>

- improve test coverage - make InheritedHandles and non-inheritable after process is started so other processes don't inherit these handles - use RemoteExecutor to test the new APIs - stop using WaitForSingleObject

...ries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs

src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs

jkotas · 2026-03-07T05:52:35Z

src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs

+        // - powershell is not available on Nano. We can't always use it.
+        // - ping seems to be a workaround, but it's simple and work everywhere. The arguments are set to make it sleep for approximately 10 seconds.
+        private static ProcessStartOptions CreateTenSecondSleep() => OperatingSystem.IsWindows()
+            ? new("ping") { Arguments = { "127.0.0.1", "-n", "11" } }


My best guess is some race

It was 100% repro. It is very unlikely to be a race.

This error typically means that the process environment is badly corrupted. We should try to understand what lead to it. There may be a real problem that only repros with some processes, and RemoteExecutor is lucky to not repro it.

Co-authored-by: Jan Kotas <jkotas@microsoft.com>

src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs

Co-authored-by: Adam Sitnik <adam.sitnik@gmail.com>

Copilot AI and others added 22 commits February 11, 2026 13:33

Initial plan

4f3f950

Add ProcessStartOptions class with path resolution and tests

92de299

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Fix build errors and test failures in ProcessStartOptions

0c9d3e2

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Improve code comments based on review feedback

794a8a2

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Fix exception documentation and add caching comment

a1b3426

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Address PR feedback: use Process.ResolvePath, fix tests, add null checks

10ff95a

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Refactor: ProcessStartOptions.ResolvePath as main impl, Process calls it

fc0fd30

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

address my own feedback

f687eb8

Simplify Windows tests: remove ConditionalFact, fix nested try-finally

5459ec9

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Refactor tests: partial classes, remove Test prefix, use new() syntax…

c3517ee

…, Theory for utils Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Fix test conditions: remove redundant IsWindows check, use GetFullPat…

bbea91e

…h for sh Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Fix test failures: use ResolveTarget helper for symlinks, fix lambda …

d9d381f

…syntax Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Fix Unix test failures: use EndsWith instead of exact path comparison

b4d6e64

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Address code review: add file check, fix .exe logic, update comments,…

98d4201

… remove dead code Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Cache Windows directory value for performance

5b024d4

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Remove DictionaryWrapper and fix Unix path test for macOS symlinks

add4af6

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Use Environment.SystemDirectory and fix Unix path test for macOS

b3939ad

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Inline GetSystemDirectory, fix Unix path test, use EndsWith for util …

63e7fc0

…names Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Update src/libraries/System.Diagnostics.Process/src/System.Diagnostic…

9ceaad1

…s.Process.csproj

fix the ResolvePath_UsesCurrentDirectory test

82e2cec

Merge remote-tracking branch 'origin/main' into copilot/add-processst…

0f37ab7

…artoptions-class

Initial plan

27ad99b

Copilot AI assigned Copilot and adamsitnik Feb 13, 2026

Copilot started work on behalf of adamsitnik February 13, 2026 11:57 View session

github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Feb 13, 2026

Copilot AI and others added 3 commits February 13, 2026 12:24

Add ProcessExitStatus type, Windows interop files, and ProcessUtils

90ba788

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Add SafeProcessHandle new APIs, ProcessExitStatus, interop definition…

23a457a

…s, and Windows implementation Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Add test files for SafeProcessHandle APIs

3017731

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Add new APIs to SafeProcessHandle class~~ Add new process management APIs to SafeProcessHandle Feb 13, 2026

Copilot finished work on behalf of adamsitnik March 3, 2026 15:02

build-analysis bot mentioned this pull request Mar 3, 2026

[android] Android.Device_Emulator.JIT.Test failing on emulators with CoreCLR #112633

Open

Merge branch 'main' into copilot/add-new-apis-to-safeprocesshandle

6fe2f5b

address feedback

c7c7176

- use SafeHandles instead of raw handles - use DangerousAddRef and DangerousRelease for handles we don't control - simplify comments - call SetConsoleCtrlHandler in the test process to avoid weird pop up window - make ProcessId public - add tests for KillOnParentExit

adamsitnik requested a review from jkotas March 4, 2026 19:17

jkotas reviewed Mar 4, 2026

View reviewed changes

tmds reviewed Mar 5, 2026

View reviewed changes

Apply suggestions from code review

f7ff001

Co-authored-by: Jan Kotas <jkotas@microsoft.com>

adamsitnik mentioned this pull request Mar 6, 2026

[API Proposal]: Introduce SafeHandle.DangerousAddRef() #125265

Closed

address code review feedback:

8a1d8c8

- improve test coverage - make InheritedHandles and non-inheritable after process is started so other processes don't inherit these handles - use RemoteExecutor to test the new APIs - stop using WaitForSingleObject

adamsitnik requested a review from jkotas March 6, 2026 16:33

fix test failure

399c84a

build-analysis bot mentioned this pull request Mar 6, 2026

slow macOS - "##[error]The job running on agent Azure Pipelines 9 ran longer than the maximum time of 60 minutes." dotnet/dnceng#1883

Open

3 tasks

jkotas reviewed Mar 7, 2026

View reviewed changes

adamsitnik and others added 2 commits March 7, 2026 07:51

Apply suggestions from code review

7b1f334

Co-authored-by: Jan Kotas <jkotas@microsoft.com>

take writer lock when InheritedHandles are provided

6999b01

Copilot AI mentioned this pull request Mar 7, 2026

Add C++ repro for SetConsoleCtrlHandler + CREATE_NEW_PROCESS_GROUP signal delivery behavior adamsitnik/ProcessRepro#1

Merged

This was referenced Mar 7, 2026

Unable to pull image from mcr.microsoft.com #117164

Open

[Android][CoreCLR] System.Security.Cryptography.Tests killed by lowmemorykiller #118603

Open

CryptographicException in NonPowerOfTwoKeySizeOaepRoundtrip test #120606

Open

adamsitnik reviewed Mar 7, 2026

View reviewed changes

src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs Outdated Show resolved Hide resolved

adamsitnik added 2 commits March 7, 2026 16:43

Apply suggestions from code review

2ec139f

Co-authored-by: Adam Sitnik <adam.sitnik@gmail.com>

address code review feedback: update XML docs about InheritedHandles

0a35881

This was referenced Mar 9, 2026

Cannot find 'arm64-v8a' device dotnet/dnceng#2284

Open

[mono] mono_thread_info_install_interrupt: previous_token should be INTERRUPT_STATE #122669

Open

Conversation

Copilot AI commented Feb 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Public API

Windows implementation

Architecture — ProcessUtils as shared dependency

Interop additions

Tests

Uh oh!

jkotas commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stephentoub commented Mar 4, 2026

Uh oh!

adamsitnik commented Mar 4, 2026

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Copilot AI commented Feb 13, 2026 •

edited

Loading

Architecture — `ProcessUtils` as shared dependency

jkotas commented Mar 3, 2026 •

edited

Loading