Implement SafeProcessHandle.Open API by Copilot · Pull Request #126705 · dotnet/runtime

Copilot · 2026-04-09T12:38:24Z

Description

Implements the SafeProcessHandle.Open(int processId) API that opens a handle to an existing process by its ID.

Changes

Native layer (pal_process.c, pal_process.h, pal_process_wasi.c):

Added TryOpenPidfd linux-specific helper that uses pidfd_open syscall (works on both Linux and Android since both define __linux__)
Extended SystemNative_ForkAndExecProcess with int32_t* outPidfd output parameter
Added SystemNative_OpenProcess that validates the process exists via kill(pid, 0) and attempts to obtain a pidfd

Managed layer:

Interop.ForkAndExecProcess.cs: Added out int lpPidfd parameter to both the managed wrapper and P/Invoke declaration
SafeProcessHandle.Unix.cs: OpenCore creates a ProcessWaitState.Holder for the target process (ensuring wait/kill operations work on the returned handle) and stores the pidfd when available
SafeProcessHandle.Windows.cs: OpenCore uses OpenProcess with PROCESS_QUERY_LIMITED_INFORMATION | SYNCHRONIZE | PROCESS_TERMINATE
SafeProcessHandle.cs: Public Open(int processId) method with argument validation

Tests (SafeProcessHandleTests.cs):

Open_InvalidProcessId_ThrowsArgumentOutOfRangeException — validates argument checking
Open_NonExistentProcessId_ThrowsWin32Exception — validates error for dead PIDs
Open_RunningProcess_ReturnsValidHandle — validates handle properties
Open_ThenKill_TerminatesProcess — validates kill + wait via the opened handle

Design decisions

On Unix, _waitStateHolder is always present in SafeProcessHandle, enabling wait/kill operations on handles obtained via Open
On platforms without pidfd (e.g., macOS), int.MinValue is used as the handle value to avoid IsInvalid returning true
The pidfd is stored in _pidfd field and closed in ReleaseHandle

Testing

All 617 process tests pass (0 failures, 5 skipped).

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/f57caa85-c528-444f-ab43-a628d6ddd488 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

adamsitnik

@copilot give my feedback a try. Run all process tests and ensure they pass before you push any changes.

dotnet-policy-service · 2026-04-09T13:08:25Z

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

…ss with outPidfd parameter Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/664957d4-993f-4f40-aa79-ce45ec55fd11 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Copilot · 2026-04-09T13:43:29Z

@copilot give my feedback a try. Run all process tests and ensure they pass before you push any changes.

Done in e2bc39d. All 405 process tests pass (0 failures, 5 skipped).

adamsitnik

@copilot address my feedback

…acOS posix_spawn path Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/ed9ebae7-b0ac-4a66-a222-8f7305c55a9b Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Copilot · 2026-04-09T14:42:39Z

@copilot address my feedback

All three review items addressed in a114b09. All 405 process tests pass (0 failures, 5 skipped).

… use int.MinValue for non-pidfd handles Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/99759f32-cf6d-48fa-9f60-1ec4f2630ad2 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Copilot · 2026-04-10T10:47:44Z

@copilot address my feedback

All four review items addressed in b63ecec. All 406 process tests pass (0 failures, 5 skipped).

…shandle-open-api # Conflicts: # src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs # src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs # src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs # src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs # src/native/libs/System.Native/pal_process.c # src/native/libs/System.Native/pal_process.h # src/native/libs/System.Native/pal_process_wasi.c Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/4b27183c-4bf6-4085-9a44-15cce51c2b1f Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

adamsitnik

@copilot please address my feedback

adamsitnik · 2026-05-22T15:43:46Z

+        [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
+        public void Open_RunningProcess_ReturnsValidHandle()
+        {
+            Process process = CreateProcess(static () =>


Suggested change

Process process = CreateProcess(static () =>

using Process process = CreateProcess(static () =>

Fixed in 9fd4c33.

adamsitnik · 2026-05-22T15:43:53Z

+        [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
+        public void Open_ThenKill_TerminatesProcess()
+        {
+            Process process = CreateProcess(static () =>


Suggested change

Process process = CreateProcess(static () =>

using Process process = CreateProcess(static () =>

Fixed in 9fd4c33.

adamsitnik · 2026-05-22T15:44:43Z

+            using SafeProcessHandle handle = SafeProcessHandle.Open(process.Id);
+            handle.Kill();
+
+            Assert.True(process.WaitForExit(WaitInMS));


We need to use handle here as well

Suggested change

Assert.True(process.WaitForExit(WaitInMS));

Assert.True(handle.WaitForExit(WaitInMS));

Fixed in 9fd4c33. Used handle.TryWaitForExit(TimeSpan.FromMilliseconds(WaitInMS), out _) since SafeProcessHandle doesn't have a WaitForExit(int) overload.

adamsitnik · 2026-05-22T15:45:57Z

+#if defined(__linux__)
+    int pidfd = (int)syscall(SYS_pidfd_open, pid, 0);


@copilot is SYS_pidfd_open available on Android as well? If so, we should do it for both platforms

Yes, SYS_pidfd_open is available on Android. The __linux__ macro is defined on Android, so both the syscall number definitions (lines 43-56) and the TryOpenPidfd function body (line 352) already cover Android — no change needed.

adamsitnik · 2026-05-22T15:47:59Z

+                throw new Win32Exception();
+            }
+
+            return new SafeProcessHandle(pidfd != -1 ? pidfd : int.MinValue, processId);


The SafeProcessHandle that is created here can't be used for waiting for the process to exit. We need to obtain the wait state holder for given process

Fixed in 9fd4c33. OpenCore now creates a ProcessWaitState.Holder for the process, which enables wait operations on the returned handle.

adamsitnik · 2026-05-22T15:48:14Z

                Validate();

-                if (_waitStateHolder is null)
+                if (_waitStateHolder is not null)


_waitStateHolder must always be present

Fixed in 9fd4c33. Removed the _processId field and the private (int pidfd, int processId) constructor. OpenCore now uses new ProcessWaitState.Holder(processId) + the standard constructor, so _waitStateHolder is always present.

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/72aa4a5d-d91c-4ef1-86ff-75322b4676bc Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Copilot · 2026-05-22T16:16:52Z

@copilot please address my feedback

All feedback addressed in 9fd4c33. Key changes:

OpenCore now creates a ProcessWaitState.Holder for the process, ensuring _waitStateHolder is always present
Removed the _processId field and private pidfd constructor — ProcessId always comes from _waitStateHolder
Tests use using Process and handle.TryWaitForExit instead of process.WaitForExit
TryOpenPidfd already works on Android since __linux__ covers it (the SYS_pidfd_open definitions are under #if defined(__linux__))

All 617 process tests pass (0 failures, 5 skipped).

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/3954e392-7975-49be-969f-eceeacf7b568 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Copilot AI and others added 2 commits April 9, 2026 11:51

Implement SafeProcessHandle.Open API with pidfd support on Linux

bad944e

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/f57caa85-c528-444f-ab43-a628d6ddd488 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Add SystemNative_OpenProcess to entrypoints.c

2ed39ab

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/f57caa85-c528-444f-ab43-a628d6ddd488 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Copilot AI self-assigned this Apr 9, 2026

Copilot AI review requested due to automatic review settings April 9, 2026 12:38

Copilot AI assigned adamsitnik Apr 9, 2026

Copilot AI review requested due to automatic review settings April 9, 2026 12:38

Copilot created this pull request from a session on behalf of adamsitnik April 9, 2026 12:38 View session

adamsitnik reviewed Apr 9, 2026

View reviewed changes

Comment thread src/native/libs/System.Native/pal_process.c Outdated

Copilot started work on behalf of adamsitnik April 9, 2026 12:53 View session

github-actions Bot added the area-System.Diagnostics.Process label Apr 9, 2026

Refactor pidfd_open into TryOpenPidfd helper, extend ForkAndExecProce…

e2bc39d

…ss with outPidfd parameter Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/664957d4-993f-4f40-aa79-ce45ec55fd11 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Copilot AI requested review from Copilot and removed request for Copilot April 9, 2026 13:26

Copilot AI requested a review from adamsitnik April 9, 2026 13:44

Copilot finished work on behalf of adamsitnik April 9, 2026 13:44

adamsitnik reviewed Apr 9, 2026

View reviewed changes

Comment thread src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs Outdated

Comment thread src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs Outdated

Comment thread src/native/libs/System.Native/pal_process.c Outdated

Copilot started work on behalf of adamsitnik April 9, 2026 13:58 View session

Address review: use out instead of int* for pidfd params, set -1 on m…

a114b09

…acOS posix_spawn path Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/ed9ebae7-b0ac-4a66-a222-8f7305c55a9b Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Copilot AI requested review from Copilot and removed request for Copilot April 9, 2026 14:36

Copilot AI requested a review from adamsitnik April 9, 2026 15:05

Copilot finished work on behalf of adamsitnik April 9, 2026 15:05

adamsitnik added the NO-REVIEW Experimental/testing PR, do NOT review it label Apr 9, 2026

Copilot started work on behalf of adamsitnik April 10, 2026 10:10 View session

Address review: add outPidfd to assert, remove redundant null checks,…

b63ecec

… use int.MinValue for non-pidfd handles Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/99759f32-cf6d-48fa-9f60-1ec4f2630ad2 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Copilot AI requested review from Copilot and removed request for Copilot April 10, 2026 10:46

Copilot AI requested a review from adamsitnik April 10, 2026 10:49

Copilot finished work on behalf of adamsitnik April 10, 2026 10:49

build-analysis Bot mentioned this pull request Apr 10, 2026

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

Open

Copilot AI mentioned this pull request Apr 11, 2026

Fix WasmTestOnChrome: exclude browser/wasi from gen-debug-dump-docs.py helix post-command #126789

Merged

1 task

Copilot started work on behalf of adamsitnik May 22, 2026 14:16 View session

Copilot AI and others added 2 commits May 22, 2026 14:30

Fix ProcessId property for pidfd-backed handles after merge

a3bf1ae

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/4b27183c-4bf6-4085-9a44-15cce51c2b1f Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Copilot AI requested review from Copilot and removed request for Copilot May 22, 2026 14:37

Copilot finished work on behalf of adamsitnik May 22, 2026 14:37

adamsitnik reviewed May 22, 2026

View reviewed changes

Copilot started work on behalf of adamsitnik May 22, 2026 15:48 View session

Address review: always use waitStateHolder in OpenCore, fix tests

9fd4c33

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/72aa4a5d-d91c-4ef1-86ff-75322b4676bc Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Copilot AI requested review from Copilot and removed request for Copilot May 22, 2026 16:16

Copilot finished work on behalf of adamsitnik May 22, 2026 16:17

Copilot AI requested a review from adamsitnik May 22, 2026 16:17

adamsitnik reviewed May 22, 2026

View reviewed changes

Comment thread ...braries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs Outdated

Copilot started work on behalf of adamsitnik May 22, 2026 17:02 View session

Replace var with explicit type for waitStateHolder

86ce237

Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/3954e392-7975-49be-969f-eceeacf7b568 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>

Copilot AI requested review from Copilot and removed request for Copilot May 22, 2026 17:06

Copilot finished work on behalf of adamsitnik May 22, 2026 17:07

Copilot AI requested a review from adamsitnik May 22, 2026 17:07

	Process process = CreateProcess(static () =>
	using Process process = CreateProcess(static () =>

	Assert.True(process.WaitForExit(WaitInMS));
	Assert.True(handle.WaitForExit(WaitInMS));

		#if defined(__linux__)
		int pidfd = (int)syscall(SYS_pidfd_open, pid, 0);

Conversation

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

Uh oh!

Description

Changes

Design decisions

Testing

Uh oh!

adamsitnik left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dotnet-policy-service Bot commented Apr 9, 2026

Uh oh!

Copilot AI commented Apr 9, 2026

Uh oh!

adamsitnik left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Apr 9, 2026

Uh oh!

Copilot AI commented Apr 10, 2026

Uh oh!

adamsitnik left a comment

Choose a reason for hiding this comment

Uh oh!

adamsitnik May 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 22, 2026

Choose a reason for hiding this comment

Uh oh!

adamsitnik May 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 22, 2026

Choose a reason for hiding this comment

Uh oh!

adamsitnik May 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 22, 2026

Choose a reason for hiding this comment

Uh oh!

adamsitnik May 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 22, 2026

Choose a reason for hiding this comment

Uh oh!

adamsitnik May 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 22, 2026

Choose a reason for hiding this comment

Uh oh!

adamsitnik May 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented May 22, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Copilot AI commented Apr 9, 2026 •

edited

Loading