From 943b669e2a654b6a01bc47dbbb6527dc18291caa Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 28 May 2026 12:20:27 +0000 Subject: [PATCH 1/8] Add Process.TryGetProcessById and SafeProcessHandle.TryOpen APIs Implement SafeProcessHandle.TryOpen that attempts to open a process by ID without throwing when the process doesn't exist (returns false instead). Throws Win32Exception only for permission errors. Implement Process.TryGetProcessById that delegates to SafeProcessHandle.TryOpen. Refactor SafeProcessHandle.Open to delegate to TryOpenCore internally. Add ProcessOpenTests.cs with tests for the new APIs and moved tests for Open/GetProcessById from SafeProcessHandleTests.cs and ProcessTests.cs. Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../ref/System.Diagnostics.Process.cs | 10 +- .../SafeHandles/SafeProcessHandle.Unix.cs | 15 +- .../SafeHandles/SafeProcessHandle.Windows.cs | 14 +- .../Win32/SafeHandles/SafeProcessHandle.cs | 38 ++- .../src/System/Diagnostics/Process.cs | 24 ++ .../tests/ProcessOpenTests.cs | 255 ++++++++++++++++++ .../tests/ProcessTests.cs | 22 -- .../tests/SafeProcessHandleTests.cs | 78 +----- .../System.Diagnostics.Process.Tests.csproj | 1 + 9 files changed, 356 insertions(+), 101 deletions(-) create mode 100644 src/libraries/System.Diagnostics.Process/tests/ProcessOpenTests.cs diff --git a/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs b/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs index 6a0abcb60fe39d..80d19c89303c73 100644 --- a/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs +++ b/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs @@ -27,6 +27,10 @@ public void Kill() { } [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] + public static bool TryOpen(int processId, [System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] out Microsoft.Win32.SafeHandles.SafeProcessHandle? processHandle) { throw null; } + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] + [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] public static Microsoft.Win32.SafeHandles.SafeProcessHandle Start(System.Diagnostics.ProcessStartInfo startInfo) { throw null; } public bool TryWaitForExit(System.TimeSpan timeout, [System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] out System.Diagnostics.ProcessExitStatus? exitStatus) { throw null; } public System.Diagnostics.ProcessExitStatus WaitForExit() { throw null; } @@ -148,7 +152,7 @@ public static void EnterDebugMode() { } public static System.Diagnostics.Process GetProcessById(int processId, string machineName) { throw null; } [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] - [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] // this needs to come after the ios attribute due to limitations in the platform analyzer + [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] public static System.Diagnostics.Process[] GetProcesses() { throw null; } [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] @@ -246,6 +250,10 @@ public void Refresh() { } [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] // this needs to come after the ios attribute due to limitations in the platform analyzer public static int StartAndForget(string fileName, System.Collections.Generic.IList? arguments = null) { throw null; } public override string ToString() { throw null; } + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] + [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] + public static bool TryGetProcessById(int processId, [System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] out System.Diagnostics.Process? process) { throw null; } public void WaitForExit() { } public bool WaitForExit(int milliseconds) { throw null; } public bool WaitForExit(System.TimeSpan timeout) { throw null; } diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs index cf7afba459e35d..c56129280192f1 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs @@ -76,17 +76,26 @@ protected override bool ReleaseHandle() return true; } - private static SafeProcessHandle OpenCore(int processId) + private static bool TryOpenCore(int processId, [NotNullWhen(true)] out SafeProcessHandle? processHandle) { int result = Interop.Sys.OpenProcess(processId); if (result == -1) { - throw new Win32Exception(); + Interop.Error error = Interop.Sys.GetLastError(); + + if (error == Interop.Error.EPERM) + { + throw new Win32Exception(); + } + + processHandle = null; + return false; } ProcessWaitState.Holder waitStateHolder = new(processId); - return new SafeProcessHandle(waitStateHolder); + processHandle = new SafeProcessHandle(waitStateHolder); + return true; } diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs index 60679088bb156f..886197327fe61f 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs @@ -51,7 +51,7 @@ protected override bool ReleaseHandle() return Interop.Kernel32.CloseHandle(handle); } - private static SafeProcessHandle OpenCore(int processId) + private static bool TryOpenCore(int processId, [NotNullWhen(true)] out SafeProcessHandle? processHandle) { const int desiredAccess = Interop.Advapi32.ProcessOptions.PROCESS_QUERY_LIMITED_INFORMATION | Interop.Advapi32.ProcessOptions.SYNCHRONIZE @@ -63,11 +63,19 @@ private static SafeProcessHandle OpenCore(int processId) { int error = Marshal.GetLastPInvokeError(); safeHandle.Dispose(); - throw new Win32Exception(error); + + if (error == Interop.Errors.ERROR_ACCESS_DENIED) + { + throw new Win32Exception(error); + } + + processHandle = null; + return false; } safeHandle.ProcessId = processId; - return safeHandle; + processHandle = safeHandle; + return true; } private static unsafe Interop.Kernel32.SafeJobHandle CreateKillOnParentExitJob() diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs index 10e9da16d6ea35..96ba4fc1bc585e 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs @@ -83,7 +83,43 @@ public static SafeProcessHandle Open(int processId) throw new PlatformNotSupportedException(); } - return OpenCore(processId); + if (!TryOpenCore(processId, out SafeProcessHandle? handle)) + { + throw new Win32Exception(); + } + + return handle; + } + + /// + /// Attempts to open an existing process by its process ID. + /// + /// The process ID of the process to open. + /// When this method returns , contains the for the opened process; otherwise, . + /// if the process was successfully opened; otherwise, . + /// Thrown when is negative or zero. + /// Thrown when the process exists but the caller does not have permissions to open it. + /// + /// + /// This method does not throw when the process does not exist. Instead, it returns . + /// + /// + /// On Windows, if the process has already exited, the method may still succeed and return a valid handle representing the terminated process. + /// + /// + [UnsupportedOSPlatform("ios")] + [UnsupportedOSPlatform("tvos")] + [SupportedOSPlatform("maccatalyst")] + public static bool TryOpen(int processId, [NotNullWhen(true)] out SafeProcessHandle? processHandle) + { + ArgumentOutOfRangeException.ThrowIfLessThanOrEqual(processId, 0); + + if (!ProcessUtils.PlatformSupportsProcessStartAndKill) + { + throw new PlatformNotSupportedException(); + } + + return TryOpenCore(processId, out processHandle); } /// diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs index 8966e578389e92..6432e6a63048bf 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs @@ -936,6 +936,30 @@ public static Process GetProcessById(int processId) return new Process(".", false, processId, null); } + /// + /// Attempts to get a instance for an existing process with the specified process ID. + /// + /// The process ID of the process to open. + /// When this method returns , contains a representing the opened process; otherwise, . + /// if the process was found and opened successfully; otherwise, . + /// Thrown when is negative or zero. + /// Thrown when the process exists but the caller does not have permissions to open it. + [UnsupportedOSPlatform("ios")] + [UnsupportedOSPlatform("tvos")] + [SupportedOSPlatform("maccatalyst")] + public static bool TryGetProcessById(int processId, [NotNullWhen(true)] out Process? process) + { + if (!SafeProcessHandle.TryOpen(processId, out SafeProcessHandle? processHandle)) + { + process = null; + return false; + } + + process = new Process(".", false, processId, null); + processHandle.Dispose(); + return true; + } + /// /// /// Creates an array of components that are diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessOpenTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessOpenTests.cs new file mode 100644 index 00000000000000..5ca2ea7a428c16 --- /dev/null +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessOpenTests.cs @@ -0,0 +1,255 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.ComponentModel; +using System.Runtime.InteropServices; +using Microsoft.DotNet.RemoteExecutor; +using Microsoft.Win32.SafeHandles; +using Xunit; + +namespace System.Diagnostics.Tests +{ + public class ProcessOpenTests : ProcessTestBase + { + [Theory] + [InlineData(0)] + [InlineData(-1)] + public void Open_InvalidProcessId_ThrowsArgumentOutOfRangeException(int processId) + { + Assert.Throws(() => SafeProcessHandle.Open(processId)); + } + + [Theory] + [InlineData(0)] + [InlineData(-1)] + public void TryOpen_InvalidProcessId_ThrowsArgumentOutOfRangeException(int processId) + { + Assert.Throws(() => SafeProcessHandle.TryOpen(processId, out _)); + } + + [Theory] + [InlineData(0)] + [InlineData(-1)] + public void TryGetProcessById_InvalidProcessId_ThrowsArgumentOutOfRangeException(int processId) + { + Assert.Throws(() => Process.TryGetProcessById(processId, out _)); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void Open_NonExistentProcessId_ThrowsWin32Exception() + { + // Use an unlikely process ID that should not exist. + Assert.Throws(() => SafeProcessHandle.Open(int.MaxValue)); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void TryOpen_NonExistentProcessId_ReturnsFalse() + { + bool result = SafeProcessHandle.TryOpen(int.MaxValue, out SafeProcessHandle? handle); + Assert.False(result); + Assert.Null(handle); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void TryGetProcessById_NonExistentProcessId_ReturnsFalse() + { + bool result = Process.TryGetProcessById(int.MaxValue, out Process? process); + Assert.False(result); + Assert.Null(process); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void Open_RunningProcess_ReturnsValidHandle() + { + using Process process = CreateProcess(static () => + { + System.Threading.Thread.Sleep(System.Threading.Timeout.Infinite); + return RemoteExecutor.SuccessExitCode; + }); + process.Start(); + + try + { + using SafeProcessHandle handle = SafeProcessHandle.Open(process.Id); + Assert.False(handle.IsInvalid); + Assert.Equal(process.Id, handle.ProcessId); + } + finally + { + process.Kill(); + process.WaitForExit(); + } + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void TryOpen_RunningProcess_ReturnsTrue() + { + using Process process = CreateProcess(static () => + { + System.Threading.Thread.Sleep(System.Threading.Timeout.Infinite); + return RemoteExecutor.SuccessExitCode; + }); + process.Start(); + + try + { + bool result = SafeProcessHandle.TryOpen(process.Id, out SafeProcessHandle? handle); + Assert.True(result); + Assert.NotNull(handle); + Assert.False(handle.IsInvalid); + Assert.Equal(process.Id, handle.ProcessId); + handle.Dispose(); + } + finally + { + process.Kill(); + process.WaitForExit(); + } + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void TryGetProcessById_RunningProcess_ReturnsTrue() + { + using Process process = CreateProcess(static () => + { + System.Threading.Thread.Sleep(System.Threading.Timeout.Infinite); + return RemoteExecutor.SuccessExitCode; + }); + process.Start(); + + try + { + bool result = Process.TryGetProcessById(process.Id, out Process? found); + Assert.True(result); + Assert.NotNull(found); + Assert.Equal(process.Id, found.Id); + found.Dispose(); + } + finally + { + process.Kill(); + process.WaitForExit(); + } + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void Open_ExitedProcess_BehaviorDependsOnPlatform() + { + using Process process = CreateProcess(static () => RemoteExecutor.SuccessExitCode); + process.Start(); + process.WaitForExit(); + + if (OperatingSystem.IsWindows()) + { + // On Windows, the kernel process object persists as long as at least one handle is open. + // Since Process.WaitForExit() doesn't release the handle, OpenProcess succeeds and returns + // a valid handle to the terminated process. + using SafeProcessHandle handle = SafeProcessHandle.Open(process.Id); + Assert.False(handle.IsInvalid); + } + else + { + // On Unix, once the process has been waited for (reaped), it is removed from the process + // table and its PID may be reused. Open throws because the process no longer exists. + Assert.Throws(() => SafeProcessHandle.Open(process.Id)); + } + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void TryOpen_ExitedProcess_BehaviorDependsOnPlatform() + { + using Process process = CreateProcess(static () => RemoteExecutor.SuccessExitCode); + process.Start(); + process.WaitForExit(); + + if (OperatingSystem.IsWindows()) + { + bool result = SafeProcessHandle.TryOpen(process.Id, out SafeProcessHandle? handle); + Assert.True(result); + Assert.NotNull(handle); + Assert.False(handle.IsInvalid); + handle.Dispose(); + } + else + { + bool result = SafeProcessHandle.TryOpen(process.Id, out SafeProcessHandle? handle); + Assert.False(result); + Assert.Null(handle); + } + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void TryGetProcessById_ExitedProcess_BehaviorDependsOnPlatform() + { + using Process process = CreateProcess(static () => RemoteExecutor.SuccessExitCode); + process.Start(); + process.WaitForExit(); + + if (OperatingSystem.IsWindows()) + { + bool result = Process.TryGetProcessById(process.Id, out Process? found); + Assert.True(result); + Assert.NotNull(found); + found.Dispose(); + } + else + { + bool result = Process.TryGetProcessById(process.Id, out Process? found); + Assert.False(result); + Assert.Null(found); + } + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void Open_ThenKill_TerminatesProcess() + { + using Process process = CreateProcess(static () => + { + System.Threading.Thread.Sleep(System.Threading.Timeout.Infinite); + return RemoteExecutor.SuccessExitCode; + }); + process.Start(); + + using SafeProcessHandle handle = SafeProcessHandle.Open(process.Id); + handle.Kill(); + + Assert.True(handle.TryWaitForExit(TimeSpan.FromMilliseconds(WaitInMS), out _)); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void TestGetProcessById() + { + CreateDefaultProcess(); + + Process p = Process.GetProcessById(_process.Id); + Assert.Equal(_process.Id, p.Id); + Assert.Equal(_process.ProcessName, p.ProcessName); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void GetProcessById_KilledProcess_ThrowsArgumentException() + { + Process process = CreateDefaultProcess(); + SafeProcessHandle handle = process.SafeHandle; + int processId = process.Id; + process.Kill(); + process.WaitForExit(WaitInMS); + Assert.Throws(() => Process.GetProcessById(processId)); + GC.KeepAlive(handle); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void TryGetProcessById_KilledProcess_ReturnsFalse() + { + Process process = CreateDefaultProcess(); + SafeProcessHandle handle = process.SafeHandle; + int processId = process.Id; + process.Kill(); + process.WaitForExit(WaitInMS); + bool result = Process.TryGetProcessById(processId, out Process? found); + Assert.False(result); + Assert.Null(found); + GC.KeepAlive(handle); + } + } +} diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs index f7129caacd3fb5..367c5646bc46c8 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs @@ -1446,28 +1446,6 @@ public void TestGetCurrentProcess() Assert.Equal(currentProcessId, current.Id); } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void TestGetProcessById() - { - CreateDefaultProcess(); - - Process p = Process.GetProcessById(_process.Id); - Assert.Equal(_process.Id, p.Id); - Assert.Equal(_process.ProcessName, p.ProcessName); - } - - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void GetProcessById_KilledProcess_ThrowsArgumentException() - { - Process process = CreateDefaultProcess(); - var handle = process.SafeHandle; - int processId = process.Id; - process.Kill(); - process.WaitForExit(WaitInMS); - Assert.Throws(() => Process.GetProcessById(processId)); - GC.KeepAlive(handle); - } - [Fact] [SkipOnPlatform(TestPlatforms.iOS | TestPlatforms.tvOS, "libproc is not supported on iOS/tvOS")] public void TestGetProcesses() diff --git a/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs b/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs index cd749a2d22d418..4b237719bc6577 100644 --- a/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs @@ -270,87 +270,23 @@ public void Kill_HandleWithoutTerminatePermission_ThrowsWin32Exception() } } - [Theory] - [InlineData(0)] - [InlineData(-1)] - public void Open_InvalidProcessId_ThrowsArgumentOutOfRangeException(int processId) - { - Assert.Throws(() => SafeProcessHandle.Open(processId)); - } - - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void Open_NonExistentProcessId_ThrowsWin32Exception() - { - // Use an unlikely process ID that should not exist. - Assert.Throws(() => SafeProcessHandle.Open(int.MaxValue)); - } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void Open_RunningProcess_ReturnsValidHandle() + public void Signal_SIGKILL_RunningProcess_ReturnsTrue() { - using Process process = CreateProcess(static () => + Process process = CreateProcess(static () => { Thread.Sleep(Timeout.Infinite); return RemoteExecutor.SuccessExitCode; }); - process.Start(); - - try - { - using SafeProcessHandle handle = SafeProcessHandle.Open(process.Id); - Assert.False(handle.IsInvalid); - Assert.Equal(process.Id, handle.ProcessId); - } - finally - { - process.Kill(); - process.WaitForExit(); - } - } - - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void Open_ExitedProcess_BehaviorDependsOnPlatform() - { - using Process process = CreateProcess(static () => RemoteExecutor.SuccessExitCode); - process.Start(); - process.WaitForExit(); - - if (OperatingSystem.IsWindows()) - { - // On Windows, the kernel process object persists as long as at least one handle is open. - // Since Process.WaitForExit() doesn't release the handle, OpenProcess succeeds and returns - // a valid handle to the terminated process. - using SafeProcessHandle handle = SafeProcessHandle.Open(process.Id); - Assert.False(handle.IsInvalid); - } - else - { - // On Unix, once the process has been waited for (reaped), it is removed from the process - // table and its PID may be reused. Open throws because the process no longer exists. - Assert.Throws(() => SafeProcessHandle.Open(process.Id)); - } - } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void Open_ThenKill_TerminatesProcess() - { - using Process process = CreateProcess(static () => - { - Thread.Sleep(Timeout.Infinite); - return RemoteExecutor.SuccessExitCode; - }); - process.Start(); + using SafeProcessHandle processHandle = SafeProcessHandle.Start(process.StartInfo); + using Process fetchedProcess = Process.GetProcessById(processHandle.ProcessId); - using SafeProcessHandle handle = SafeProcessHandle.Open(process.Id); - handle.Kill(); + bool delivered = processHandle.Signal(PosixSignal.SIGKILL); - Assert.True(handle.TryWaitForExit(TimeSpan.FromMilliseconds(WaitInMS), out _)); + Assert.True(delivered); + Assert.True(fetchedProcess.WaitForExit(WaitInMS)); } - - [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - [InlineData(true)] - [InlineData(false)] - public async Task WaitForExit_ProcessExitsNormally_ReturnsExitCode(bool useAsync) { Process process = CreateProcess(static () => RemoteExecutor.SuccessExitCode); diff --git a/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj b/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj index f1ed6055933db3..33132969ff9ec7 100644 --- a/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj +++ b/src/libraries/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj @@ -43,6 +43,7 @@ + From e12fb178d4e399b92e1c33f022db36af85f3d7b3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 28 May 2026 12:21:47 +0000 Subject: [PATCH 2/8] Fix test file: add using System.Threading and remove fully-qualified names Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../tests/ProcessOpenTests.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessOpenTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessOpenTests.cs index 5ca2ea7a428c16..6f0d51eb132792 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessOpenTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessOpenTests.cs @@ -2,7 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.ComponentModel; -using System.Runtime.InteropServices; +using System.Threading; using Microsoft.DotNet.RemoteExecutor; using Microsoft.Win32.SafeHandles; using Xunit; @@ -63,7 +63,7 @@ public void Open_RunningProcess_ReturnsValidHandle() { using Process process = CreateProcess(static () => { - System.Threading.Thread.Sleep(System.Threading.Timeout.Infinite); + Thread.Sleep(Timeout.Infinite); return RemoteExecutor.SuccessExitCode; }); process.Start(); @@ -86,7 +86,7 @@ public void TryOpen_RunningProcess_ReturnsTrue() { using Process process = CreateProcess(static () => { - System.Threading.Thread.Sleep(System.Threading.Timeout.Infinite); + Thread.Sleep(Timeout.Infinite); return RemoteExecutor.SuccessExitCode; }); process.Start(); @@ -112,7 +112,7 @@ public void TryGetProcessById_RunningProcess_ReturnsTrue() { using Process process = CreateProcess(static () => { - System.Threading.Thread.Sleep(System.Threading.Timeout.Infinite); + Thread.Sleep(Timeout.Infinite); return RemoteExecutor.SuccessExitCode; }); process.Start(); @@ -205,7 +205,7 @@ public void Open_ThenKill_TerminatesProcess() { using Process process = CreateProcess(static () => { - System.Threading.Thread.Sleep(System.Threading.Timeout.Infinite); + Thread.Sleep(Timeout.Infinite); return RemoteExecutor.SuccessExitCode; }); process.Start(); From 6528b5e6b0f999fae454fa77ef26aa3690551432 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 28 May 2026 12:54:09 +0000 Subject: [PATCH 3/8] Address reviewer feedback: delegate Open to TryOpen, pass error code explicitly, add Process(SafeProcessHandle) ctor, consolidate tests with Theory Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../ref/System.Diagnostics.Process.cs | 2 +- .../SafeHandles/SafeProcessHandle.Unix.cs | 2 +- .../Win32/SafeHandles/SafeProcessHandle.cs | 9 +- .../src/System/Diagnostics/Process.cs | 9 +- .../tests/ProcessOpenTests.cs | 114 ++++++++---------- 5 files changed, 63 insertions(+), 73 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs b/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs index 80d19c89303c73..c2ef66f997772b 100644 --- a/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs +++ b/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs @@ -152,7 +152,7 @@ public static void EnterDebugMode() { } public static System.Diagnostics.Process GetProcessById(int processId, string machineName) { throw null; } [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] - [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] + [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] // this needs to come after the ios attribute due to limitations in the platform analyzer public static System.Diagnostics.Process[] GetProcesses() { throw null; } [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs index c56129280192f1..48e2d740ce3e18 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs @@ -86,7 +86,7 @@ private static bool TryOpenCore(int processId, [NotNullWhen(true)] out SafeProce if (error == Interop.Error.EPERM) { - throw new Win32Exception(); + throw new Win32Exception((int)error); } processHandle = null; diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs index 96ba4fc1bc585e..a1ef91c6dec4fd 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs @@ -76,14 +76,7 @@ public SafeProcessHandle(IntPtr existingHandle, bool ownsHandle) [SupportedOSPlatform("maccatalyst")] public static SafeProcessHandle Open(int processId) { - ArgumentOutOfRangeException.ThrowIfLessThanOrEqual(processId, 0); - - if (!ProcessUtils.PlatformSupportsProcessStartAndKill) - { - throw new PlatformNotSupportedException(); - } - - if (!TryOpenCore(processId, out SafeProcessHandle? handle)) + if (!TryOpen(processId, out SafeProcessHandle? handle)) { throw new Win32Exception(); } diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs index 6432e6a63048bf..45499e1068f0ee 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs @@ -119,6 +119,12 @@ private Process(string machineName, bool isRemoteMachine, int processId, Process _errorStreamReadMode = StreamReadMode.Undefined; } + private Process(int processId, SafeProcessHandle processHandle) : this(".", false, processId, null) + { + _processHandle = processHandle; + _haveProcessHandle = true; + } + public SafeProcessHandle SafeHandle { get @@ -955,8 +961,7 @@ public static bool TryGetProcessById(int processId, [NotNullWhen(true)] out Proc return false; } - process = new Process(".", false, processId, null); - processHandle.Dispose(); + process = new Process(processId, processHandle); return true; } diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessOpenTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessOpenTests.cs index 6f0d51eb132792..7424c7b29f13e4 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessOpenTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessOpenTests.cs @@ -58,8 +58,10 @@ public void TryGetProcessById_NonExistentProcessId_ReturnsFalse() Assert.Null(process); } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void Open_RunningProcess_ReturnsValidHandle() + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(true)] + [InlineData(false)] + public void Open_RunningProcess_ReturnsValidHandle(bool tryOpen) { using Process process = CreateProcess(static () => { @@ -68,47 +70,33 @@ public void Open_RunningProcess_ReturnsValidHandle() }); process.Start(); + SafeProcessHandle? handle = null; try { - using SafeProcessHandle handle = SafeProcessHandle.Open(process.Id); - Assert.False(handle.IsInvalid); - Assert.Equal(process.Id, handle.ProcessId); - } - finally - { - process.Kill(); - process.WaitForExit(); - } - } - - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void TryOpen_RunningProcess_ReturnsTrue() - { - using Process process = CreateProcess(static () => - { - Thread.Sleep(Timeout.Infinite); - return RemoteExecutor.SuccessExitCode; - }); - process.Start(); + if (tryOpen) + { + Assert.True(SafeProcessHandle.TryOpen(process.Id, out handle)); + } + else + { + handle = SafeProcessHandle.Open(process.Id); + } - try - { - bool result = SafeProcessHandle.TryOpen(process.Id, out SafeProcessHandle? handle); - Assert.True(result); - Assert.NotNull(handle); Assert.False(handle.IsInvalid); Assert.Equal(process.Id, handle.ProcessId); - handle.Dispose(); } finally { process.Kill(); process.WaitForExit(); + handle?.Dispose(); } } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void TryGetProcessById_RunningProcess_ReturnsTrue() + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(true)] + [InlineData(false)] + public void GetProcessById_RunningProcess_ReturnsProcess(bool tryGet) { using Process process = CreateProcess(static () => { @@ -119,11 +107,20 @@ public void TryGetProcessById_RunningProcess_ReturnsTrue() try { - bool result = Process.TryGetProcessById(process.Id, out Process? found); - Assert.True(result); - Assert.NotNull(found); - Assert.Equal(process.Id, found.Id); - found.Dispose(); + if (tryGet) + { + bool result = Process.TryGetProcessById(process.Id, out Process? found); + Assert.True(result); + Assert.NotNull(found); + Assert.Equal(process.Id, found.Id); + found.Dispose(); + } + else + { + Process p = Process.GetProcessById(process.Id); + Assert.Equal(process.Id, p.Id); + Assert.Equal(process.ProcessName, p.ProcessName); + } } finally { @@ -216,39 +213,34 @@ public void Open_ThenKill_TerminatesProcess() Assert.True(handle.TryWaitForExit(TimeSpan.FromMilliseconds(WaitInMS), out _)); } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void TestGetProcessById() + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(true)] + [InlineData(false)] + public void GetProcessById_KilledProcess_Fails(bool tryGet) { - CreateDefaultProcess(); - - Process p = Process.GetProcessById(_process.Id); - Assert.Equal(_process.Id, p.Id); - Assert.Equal(_process.ProcessName, p.ProcessName); - } + using Process process = CreateProcess(static () => + { + Thread.Sleep(Timeout.Infinite); + return RemoteExecutor.SuccessExitCode; + }); + process.Start(); - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void GetProcessById_KilledProcess_ThrowsArgumentException() - { - Process process = CreateDefaultProcess(); SafeProcessHandle handle = process.SafeHandle; int processId = process.Id; process.Kill(); process.WaitForExit(WaitInMS); - Assert.Throws(() => Process.GetProcessById(processId)); - GC.KeepAlive(handle); - } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void TryGetProcessById_KilledProcess_ReturnsFalse() - { - Process process = CreateDefaultProcess(); - SafeProcessHandle handle = process.SafeHandle; - int processId = process.Id; - process.Kill(); - process.WaitForExit(WaitInMS); - bool result = Process.TryGetProcessById(processId, out Process? found); - Assert.False(result); - Assert.Null(found); + if (tryGet) + { + bool result = Process.TryGetProcessById(processId, out Process? found); + Assert.False(result); + Assert.Null(found); + } + else + { + Assert.Throws(() => Process.GetProcessById(processId)); + } + GC.KeepAlive(handle); } } From a8bdfffd29850cf8a70317799ac9055a4a7cea1c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 28 May 2026 12:55:09 +0000 Subject: [PATCH 4/8] Dispose Process object in test to avoid resource leak Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../System.Diagnostics.Process/tests/ProcessOpenTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessOpenTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessOpenTests.cs index 7424c7b29f13e4..4498749f24c6d3 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessOpenTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessOpenTests.cs @@ -120,6 +120,7 @@ public void GetProcessById_RunningProcess_ReturnsProcess(bool tryGet) Process p = Process.GetProcessById(process.Id); Assert.Equal(process.Id, p.Id); Assert.Equal(process.ProcessName, p.ProcessName); + p.Dispose(); } } finally From ce3c6cbfbf04ac430451ab7780d498c3ac26cfd8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 28 May 2026 17:59:00 +0000 Subject: [PATCH 5/8] Address reviewer feedback: UnauthorizedAccessException, simplify Process ctor Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../SafeHandles/SafeProcessHandle.Unix.cs | 2 +- .../SafeHandles/SafeProcessHandle.Windows.cs | 2 +- .../Win32/SafeHandles/SafeProcessHandle.cs | 2 +- .../src/System/Diagnostics/Process.cs | 6 +++--- .../tests/SafeProcessHandleTests.cs | 21 ++++--------------- 5 files changed, 10 insertions(+), 23 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs index 48e2d740ce3e18..32ff0c3c7a122f 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs @@ -86,7 +86,7 @@ private static bool TryOpenCore(int processId, [NotNullWhen(true)] out SafeProce if (error == Interop.Error.EPERM) { - throw new Win32Exception((int)error); + throw new UnauthorizedAccessException(); } processHandle = null; diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs index 886197327fe61f..e2ed071d25184e 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs @@ -66,7 +66,7 @@ private static bool TryOpenCore(int processId, [NotNullWhen(true)] out SafeProce if (error == Interop.Errors.ERROR_ACCESS_DENIED) { - throw new Win32Exception(error); + throw new UnauthorizedAccessException(); } processHandle = null; diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs index a1ef91c6dec4fd..a957c11cc757d8 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.cs @@ -91,7 +91,7 @@ public static SafeProcessHandle Open(int processId) /// When this method returns , contains the for the opened process; otherwise, . /// if the process was successfully opened; otherwise, . /// Thrown when is negative or zero. - /// Thrown when the process exists but the caller does not have permissions to open it. + /// Thrown when the process exists but the caller does not have permissions to open it. /// /// /// This method does not throw when the process does not exist. Instead, it returns . diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs index 45499e1068f0ee..c13176e6912af2 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs @@ -119,7 +119,7 @@ private Process(string machineName, bool isRemoteMachine, int processId, Process _errorStreamReadMode = StreamReadMode.Undefined; } - private Process(int processId, SafeProcessHandle processHandle) : this(".", false, processId, null) + private Process(SafeProcessHandle processHandle) : this(".", false, processHandle.ProcessId, null) { _processHandle = processHandle; _haveProcessHandle = true; @@ -949,7 +949,7 @@ public static Process GetProcessById(int processId) /// When this method returns , contains a representing the opened process; otherwise, . /// if the process was found and opened successfully; otherwise, . /// Thrown when is negative or zero. - /// Thrown when the process exists but the caller does not have permissions to open it. + /// Thrown when the process exists but the caller does not have permissions to open it. [UnsupportedOSPlatform("ios")] [UnsupportedOSPlatform("tvos")] [SupportedOSPlatform("maccatalyst")] @@ -961,7 +961,7 @@ public static bool TryGetProcessById(int processId, [NotNullWhen(true)] out Proc return false; } - process = new Process(processId, processHandle); + process = new Process(processHandle); return true; } diff --git a/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs b/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs index 4b237719bc6577..a9dacea578c48f 100644 --- a/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs @@ -270,23 +270,10 @@ public void Kill_HandleWithoutTerminatePermission_ThrowsWin32Exception() } } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void Signal_SIGKILL_RunningProcess_ReturnsTrue() - { - Process process = CreateProcess(static () => - { - Thread.Sleep(Timeout.Infinite); - return RemoteExecutor.SuccessExitCode; - }); - - using SafeProcessHandle processHandle = SafeProcessHandle.Start(process.StartInfo); - using Process fetchedProcess = Process.GetProcessById(processHandle.ProcessId); - - bool delivered = processHandle.Signal(PosixSignal.SIGKILL); - - Assert.True(delivered); - Assert.True(fetchedProcess.WaitForExit(WaitInMS)); - } + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(true)] + [InlineData(false)] + public async Task WaitForExit_ProcessExitsNormally_ReturnsExitCode(bool useAsync) { Process process = CreateProcess(static () => RemoteExecutor.SuccessExitCode); From 2548a912e6f429392924958d4ae270335d103281 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 28 May 2026 18:23:15 +0000 Subject: [PATCH 6/8] Update tests: add UnauthorizedAccessException tests, merge Theory patterns, remove redundant test Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../tests/ProcessOpenTests.cs | 159 +++++++++--------- 1 file changed, 77 insertions(+), 82 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessOpenTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessOpenTests.cs index 4498749f24c6d3..7f21de2f89c1dd 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessOpenTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessOpenTests.cs @@ -58,6 +58,34 @@ public void TryGetProcessById_NonExistentProcessId_ReturnsFalse() Assert.Null(process); } + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows), nameof(PlatformDetection.IsNotPrivilegedProcess))] + public void Open_ProtectedProcess_ThrowsUnauthorizedAccessException() + { + int pid = Assert.Single(Process.GetProcessesByName("lsass")).Id; + Assert.Throws(() => SafeProcessHandle.Open(pid)); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows), nameof(PlatformDetection.IsNotPrivilegedProcess))] + public void TryOpen_ProtectedProcess_ThrowsUnauthorizedAccessException() + { + int pid = Assert.Single(Process.GetProcessesByName("lsass")).Id; + Assert.Throws(() => SafeProcessHandle.TryOpen(pid, out _)); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsLinux), nameof(PlatformDetection.IsNotPrivilegedProcess))] + public void Open_ProcessOwnedByRoot_ThrowsUnauthorizedAccessException() + { + // PID 1 (init/systemd) is always owned by root + Assert.Throws(() => SafeProcessHandle.Open(1)); + } + + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsLinux), nameof(PlatformDetection.IsNotPrivilegedProcess))] + public void TryOpen_ProcessOwnedByRoot_ThrowsUnauthorizedAccessException() + { + // PID 1 (init/systemd) is always owned by root + Assert.Throws(() => SafeProcessHandle.TryOpen(1, out _)); + } + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] [InlineData(true)] [InlineData(false)] @@ -84,6 +112,9 @@ public void Open_RunningProcess_ReturnsValidHandle(bool tryOpen) Assert.False(handle.IsInvalid); Assert.Equal(process.Id, handle.ProcessId); + + handle.Kill(); + Assert.True(handle.TryWaitForExit(TimeSpan.FromMilliseconds(WaitInMS), out _)); } finally { @@ -105,33 +136,37 @@ public void GetProcessById_RunningProcess_ReturnsProcess(bool tryGet) }); process.Start(); + Process? found = null; try { if (tryGet) { - bool result = Process.TryGetProcessById(process.Id, out Process? found); - Assert.True(result); - Assert.NotNull(found); - Assert.Equal(process.Id, found.Id); - found.Dispose(); + Assert.True(Process.TryGetProcessById(process.Id, out found)); } else { - Process p = Process.GetProcessById(process.Id); - Assert.Equal(process.Id, p.Id); - Assert.Equal(process.ProcessName, p.ProcessName); - p.Dispose(); + found = Process.GetProcessById(process.Id); } + + Assert.NotNull(found); + Assert.Equal(process.Id, found.Id); + Assert.Equal(process.ProcessName, found.ProcessName); + + found.Kill(); + found.WaitForExit(); } finally { process.Kill(); process.WaitForExit(); + found?.Dispose(); } } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void Open_ExitedProcess_BehaviorDependsOnPlatform() + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(true)] + [InlineData(false)] + public void Open_ExitedProcess_BehaviorDependsOnPlatform(bool tryOpen) { using Process process = CreateProcess(static () => RemoteExecutor.SuccessExitCode); process.Start(); @@ -139,81 +174,44 @@ public void Open_ExitedProcess_BehaviorDependsOnPlatform() if (OperatingSystem.IsWindows()) { - // On Windows, the kernel process object persists as long as at least one handle is open. - // Since Process.WaitForExit() doesn't release the handle, OpenProcess succeeds and returns - // a valid handle to the terminated process. - using SafeProcessHandle handle = SafeProcessHandle.Open(process.Id); - Assert.False(handle.IsInvalid); + SafeProcessHandle? handle = null; + try + { + if (tryOpen) + { + Assert.True(SafeProcessHandle.TryOpen(process.Id, out handle)); + } + else + { + handle = SafeProcessHandle.Open(process.Id); + } + + // On Windows, the kernel process object persists as long as at least one handle is open. + Assert.NotNull(handle); + Assert.False(handle.IsInvalid); + } + finally + { + handle?.Dispose(); + } } else { // On Unix, once the process has been waited for (reaped), it is removed from the process - // table and its PID may be reused. Open throws because the process no longer exists. - Assert.Throws(() => SafeProcessHandle.Open(process.Id)); - } - } - - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void TryOpen_ExitedProcess_BehaviorDependsOnPlatform() - { - using Process process = CreateProcess(static () => RemoteExecutor.SuccessExitCode); - process.Start(); - process.WaitForExit(); - - if (OperatingSystem.IsWindows()) - { - bool result = SafeProcessHandle.TryOpen(process.Id, out SafeProcessHandle? handle); - Assert.True(result); - Assert.NotNull(handle); - Assert.False(handle.IsInvalid); - handle.Dispose(); - } - else - { - bool result = SafeProcessHandle.TryOpen(process.Id, out SafeProcessHandle? handle); - Assert.False(result); - Assert.Null(handle); - } - } - - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void TryGetProcessById_ExitedProcess_BehaviorDependsOnPlatform() - { - using Process process = CreateProcess(static () => RemoteExecutor.SuccessExitCode); - process.Start(); - process.WaitForExit(); - - if (OperatingSystem.IsWindows()) - { - bool result = Process.TryGetProcessById(process.Id, out Process? found); - Assert.True(result); - Assert.NotNull(found); - found.Dispose(); - } - else - { - bool result = Process.TryGetProcessById(process.Id, out Process? found); - Assert.False(result); - Assert.Null(found); + // table and its PID may be reused. + if (tryOpen) + { + bool result = SafeProcessHandle.TryOpen(process.Id, out SafeProcessHandle? handle); + Assert.False(result); + Assert.Null(handle); + } + else + { + Assert.Throws(() => SafeProcessHandle.Open(process.Id)); + } } } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void Open_ThenKill_TerminatesProcess() - { - using Process process = CreateProcess(static () => - { - Thread.Sleep(Timeout.Infinite); - return RemoteExecutor.SuccessExitCode; - }); - process.Start(); - - using SafeProcessHandle handle = SafeProcessHandle.Open(process.Id); - handle.Kill(); - - Assert.True(handle.TryWaitForExit(TimeSpan.FromMilliseconds(WaitInMS), out _)); - } - [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] [InlineData(true)] [InlineData(false)] @@ -226,7 +224,6 @@ public void GetProcessById_KilledProcess_Fails(bool tryGet) }); process.Start(); - SafeProcessHandle handle = process.SafeHandle; int processId = process.Id; process.Kill(); process.WaitForExit(WaitInMS); @@ -241,8 +238,6 @@ public void GetProcessById_KilledProcess_Fails(bool tryGet) { Assert.Throws(() => Process.GetProcessById(processId)); } - - GC.KeepAlive(handle); } } } From 7bb40eb33684f2feb50ab206a8bcf2a7ab2c14fd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 28 May 2026 19:56:29 +0000 Subject: [PATCH 7/8] Merge GetProcessById_KilledProcess_Fails into Open_ExitedProcess_BehaviorDependsOnPlatform with kill parameter Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../tests/ProcessOpenTests.cs | 46 ++++++------------- 1 file changed, 14 insertions(+), 32 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessOpenTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessOpenTests.cs index 7f21de2f89c1dd..ccdf8f65c2b1eb 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessOpenTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessOpenTests.cs @@ -164,12 +164,22 @@ public void GetProcessById_RunningProcess_ReturnsProcess(bool tryGet) } [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - [InlineData(true)] - [InlineData(false)] - public void Open_ExitedProcess_BehaviorDependsOnPlatform(bool tryOpen) + [InlineData(true, true)] + [InlineData(true, false)] + [InlineData(false, true)] + [InlineData(false, false)] + public void Open_ExitedProcess_BehaviorDependsOnPlatform(bool tryOpen, bool kill) { - using Process process = CreateProcess(static () => RemoteExecutor.SuccessExitCode); + using Process process = kill + ? CreateProcess(static () => { Thread.Sleep(Timeout.Infinite); return RemoteExecutor.SuccessExitCode; }) + : CreateProcess(static () => RemoteExecutor.SuccessExitCode); process.Start(); + + if (kill) + { + process.Kill(); + } + process.WaitForExit(); if (OperatingSystem.IsWindows()) @@ -211,33 +221,5 @@ public void Open_ExitedProcess_BehaviorDependsOnPlatform(bool tryOpen) } } } - - [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - [InlineData(true)] - [InlineData(false)] - public void GetProcessById_KilledProcess_Fails(bool tryGet) - { - using Process process = CreateProcess(static () => - { - Thread.Sleep(Timeout.Infinite); - return RemoteExecutor.SuccessExitCode; - }); - process.Start(); - - int processId = process.Id; - process.Kill(); - process.WaitForExit(WaitInMS); - - if (tryGet) - { - bool result = Process.TryGetProcessById(processId, out Process? found); - Assert.False(result); - Assert.Null(found); - } - else - { - Assert.Throws(() => Process.GetProcessById(processId)); - } - } } } From 3600bf12943807d2188e97c596e59ee5f3ea2d10 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 29 May 2026 12:56:09 +0200 Subject: [PATCH 8/8] Apply suggestions from code review Co-authored-by: Adam Sitnik --- .../tests/ProcessOpenTests.cs | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessOpenTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessOpenTests.cs index ccdf8f65c2b1eb..be443367eb2563 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessOpenTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessOpenTests.cs @@ -72,20 +72,6 @@ public void TryOpen_ProtectedProcess_ThrowsUnauthorizedAccessException() Assert.Throws(() => SafeProcessHandle.TryOpen(pid, out _)); } - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsLinux), nameof(PlatformDetection.IsNotPrivilegedProcess))] - public void Open_ProcessOwnedByRoot_ThrowsUnauthorizedAccessException() - { - // PID 1 (init/systemd) is always owned by root - Assert.Throws(() => SafeProcessHandle.Open(1)); - } - - [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsLinux), nameof(PlatformDetection.IsNotPrivilegedProcess))] - public void TryOpen_ProcessOwnedByRoot_ThrowsUnauthorizedAccessException() - { - // PID 1 (init/systemd) is always owned by root - Assert.Throws(() => SafeProcessHandle.TryOpen(1, out _)); - } - [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] [InlineData(true)] [InlineData(false)]