From 33ea555c9fa88d9d5a1e8d5c20af6177988eadb5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Apr 2026 09:51:26 +0000 Subject: [PATCH 01/29] Implement SafeProcessHandle.WaitForExit* methods - Add WaitForExit, TryWaitForExit, WaitForExitOrKillOnTimeout, WaitForExitAsync, WaitForExitOrKillOnCancellationAsync to SafeProcessHandle - Windows: uses ProcessWaitHandle + GetExitCodeProcess - Unix: integrates with ProcessWaitState for reaping coordination - Native: WaitPidExitedNoHang now returns terminating signal info - ProcessWaitState: tracks terminating signal for ProcessExitStatus - Ref assembly: updated with new API surface - Tests: added comprehensive WaitForExit* tests Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/3540beba-26c4-4014-b276-d6fa244eda82 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Unix/System.Native/Interop.WaitPid.cs | 2 +- .../ref/System.Diagnostics.Process.cs | 20 ++ .../SafeHandles/SafeProcessHandle.Unix.cs | 163 +++++++++- .../SafeHandles/SafeProcessHandle.Windows.cs | 150 +++++++++ .../Win32/SafeHandles/SafeProcessHandle.cs | 113 +++++++ .../Diagnostics/ProcessWaitState.Unix.cs | 27 +- .../tests/SafeProcessHandleTests.cs | 287 +++++++++++++++++- src/native/libs/System.Native/pal_process.c | 5 +- src/native/libs/System.Native/pal_process.h | 2 +- .../libs/System.Native/pal_process_wasi.c | 2 +- 10 files changed, 745 insertions(+), 26 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.WaitPid.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.WaitPid.cs index 2f4eae4584c0a9..0474a11b0bde9e 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.WaitPid.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.WaitPid.cs @@ -18,6 +18,6 @@ internal static partial class Sys /// 4) on error, -1 is returned. /// [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_WaitPidExitedNoHang", SetLastError = true)] - internal static partial int WaitPidExitedNoHang(int pid, out int exitCode); + internal static partial int WaitPidExitedNoHang(int pid, out int exitCode, out int terminatingSignal); } } 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 7cd0b3015d5f69..e85a16f8fe548e 100644 --- a/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs +++ b/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs @@ -24,6 +24,26 @@ public void Kill() { } [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] public static Microsoft.Win32.SafeHandles.SafeProcessHandle Start(System.Diagnostics.ProcessStartInfo startInfo) { throw null; } + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] + [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] + public bool TryWaitForExit(System.TimeSpan timeout, [System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] out System.Diagnostics.ProcessExitStatus? exitStatus) { throw null; } + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] + [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] + public System.Diagnostics.ProcessExitStatus WaitForExit() { throw null; } + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] + [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] + public System.Threading.Tasks.Task WaitForExitAsync(System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] + [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] + public System.Threading.Tasks.Task WaitForExitOrKillOnCancellationAsync(System.Threading.CancellationToken cancellationToken) { throw null; } + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] + [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] + public System.Diagnostics.ProcessExitStatus WaitForExitOrKillOnTimeout(System.TimeSpan timeout) { throw null; } } } namespace System.Diagnostics 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 2a290ea098724e..7f71498ad191da 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 @@ -8,11 +8,13 @@ using System.Diagnostics.CodeAnalysis; using System.IO; using System.IO.Pipes; +using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Runtime.Versioning; using System.Security; using System.Text; using System.Threading; +using System.Threading.Tasks; using Microsoft.Win32.SafeHandles; namespace Microsoft.Win32.SafeHandles @@ -28,12 +30,14 @@ public sealed partial class SafeProcessHandle : SafeHandleZeroOrMinusOneIsInvali private readonly SafeWaitHandle? _handle; private readonly bool _releaseRef; + private ProcessWaitState? _waitState; private SafeProcessHandle(int processId, ProcessWaitState.Holder waitStateHolder) : base(ownsHandle: true) { ProcessId = processId; + _waitState = waitStateHolder._state; - _handle = waitStateHolder._state.EnsureExitedEvent().GetSafeWaitHandle(); + _handle = _waitState.EnsureExitedEvent().GetSafeWaitHandle(); _handle.DangerousAddRef(ref _releaseRef); SetHandle(_handle.DangerousGetHandle()); } @@ -89,6 +93,163 @@ private bool SignalCore(PosixSignal signal) return true; } + private ProcessExitStatus WaitForExitCore() + { + ProcessWaitState waitState = GetWaitState(); + waitState.WaitForExit(Timeout.Infinite); + + return CreateExitStatus(waitState, canceled: false); + } + + private bool TryWaitForExitCore(int milliseconds, [NotNullWhen(true)] out ProcessExitStatus? exitStatus) + { + ProcessWaitState waitState = GetWaitState(); + if (!waitState.WaitForExit(milliseconds)) + { + exitStatus = null; + return false; + } + + exitStatus = CreateExitStatus(waitState, canceled: false); + return true; + } + + private ProcessExitStatus WaitForExitOrKillOnTimeoutCore(int milliseconds) + { + ProcessWaitState waitState = GetWaitState(); + bool wasKilled = false; + + if (!waitState.WaitForExit(milliseconds)) + { + wasKilled = KillCore(); + waitState.WaitForExit(Timeout.Infinite); + } + + return CreateExitStatus(waitState, canceled: wasKilled); + } + + private async Task WaitForExitAsyncCore(CancellationToken cancellationToken) + { + ProcessWaitState waitState = GetWaitState(); + ManualResetEvent exitedEvent = waitState.EnsureExitedEvent(); + + TaskCompletionSource tcs = new(TaskCreationOptions.RunContinuationsAsynchronously); + RegisteredWaitHandle? registeredWaitHandle = null; + CancellationTokenRegistration ctr = default; + + try + { + registeredWaitHandle = ThreadPool.RegisterWaitForSingleObject( + exitedEvent, + static (state, timedOut) => ((TaskCompletionSource)state!).TrySetResult(true), + tcs, + Timeout.Infinite, + executeOnlyOnce: true); + + if (cancellationToken.CanBeCanceled) + { + ctr = cancellationToken.UnsafeRegister( + static state => + { + var (taskSource, token) = ((TaskCompletionSource taskSource, CancellationToken token))state!; + taskSource.TrySetCanceled(token); + }, + (tcs, cancellationToken)); + } + + await tcs.Task.ConfigureAwait(false); + } + finally + { + ctr.Dispose(); + registeredWaitHandle?.Unregister(null); + } + + return CreateExitStatus(waitState, canceled: false); + } + + private async Task WaitForExitOrKillOnCancellationAsyncCore(CancellationToken cancellationToken) + { + ProcessWaitState waitState = GetWaitState(); + ManualResetEvent exitedEvent = waitState.EnsureExitedEvent(); + + TaskCompletionSource tcs = new(TaskCreationOptions.RunContinuationsAsynchronously); + RegisteredWaitHandle? registeredWaitHandle = null; + CancellationTokenRegistration ctr = default; + StrongBox wasKilledBox = new(false); + + try + { + registeredWaitHandle = ThreadPool.RegisterWaitForSingleObject( + exitedEvent, + static (state, timedOut) => ((TaskCompletionSource)state!).TrySetResult(true), + tcs, + Timeout.Infinite, + executeOnlyOnce: true); + + if (cancellationToken.CanBeCanceled) + { + ctr = cancellationToken.UnsafeRegister( + static state => + { + var (handle, wasCancelled) = ((SafeProcessHandle, StrongBox))state!; + wasCancelled.Value = handle.KillCore(); + }, + (this, wasKilledBox)); + } + + await tcs.Task.ConfigureAwait(false); + } + finally + { + ctr.Dispose(); + registeredWaitHandle?.Unregister(null); + } + + return CreateExitStatus(waitState, canceled: wasKilledBox.Value); + } + + /// + /// Terminates the process by sending SIGKILL. + /// + /// true if the process was terminated; false if it had already exited. + internal bool KillCore() + { + int signalNumber = Interop.Sys.GetPlatformSignalNumber(PosixSignal.SIGKILL); + int killResult = Interop.Sys.Kill(ProcessId, signalNumber); + if (killResult == 0) + { + return true; + } + + Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); + if (errorInfo.Error == Interop.Error.ESRCH) + { + return false; // Process already exited + } + + throw new Win32Exception(errorInfo.RawErrno); + } + + private ProcessWaitState GetWaitState() + { + if (_waitState is null) + { + throw new InvalidOperationException(SR.InvalidProcessHandle); + } + + return _waitState; + } + + private static ProcessExitStatus CreateExitStatus(ProcessWaitState waitState, bool canceled) + { + waitState.GetExited(out int? exitCode, refresh: false); + int? rawSignal = waitState.TerminatingSignal; + PosixSignal? signal = rawSignal.HasValue ? (PosixSignal)rawSignal.Value : null; + + return new ProcessExitStatus(exitCode ?? 0, canceled, signal); + } + private delegate SafeProcessHandle StartWithShellExecuteDelegate(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, SafeFileHandle? stdoutHandle, SafeFileHandle? stderrHandle, out ProcessWaitState.Holder? waitStateHolder); private static StartWithShellExecuteDelegate? s_startWithShellExecute; 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 29e915a0803c0b..b1d99879a2a1af 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 @@ -5,10 +5,13 @@ using System.Collections.Generic; using System.ComponentModel; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Security; using System.Text; using System.Threading; +using System.Threading.Tasks; namespace Microsoft.Win32.SafeHandles { @@ -624,6 +627,153 @@ private static void AssignJobAndResumeThread(IntPtr hThread, SafeProcessHandle p private int GetProcessIdCore() => Interop.Kernel32.GetProcessId(this); + private ProcessExitStatus WaitForExitCore() + { + using Interop.Kernel32.ProcessWaitHandle processWaitHandle = new(this); + processWaitHandle.WaitOne(Timeout.Infinite); + + return new ProcessExitStatus(GetExitCode(), false); + } + + private bool TryWaitForExitCore(int milliseconds, [NotNullWhen(true)] out ProcessExitStatus? exitStatus) + { + using Interop.Kernel32.ProcessWaitHandle processWaitHandle = new(this); + if (!processWaitHandle.WaitOne(milliseconds)) + { + exitStatus = null; + return false; + } + + exitStatus = new ProcessExitStatus(GetExitCode(), false); + return true; + } + + private ProcessExitStatus WaitForExitOrKillOnTimeoutCore(int milliseconds) + { + bool wasKilledOnTimeout = false; + using Interop.Kernel32.ProcessWaitHandle processWaitHandle = new(this); + if (!processWaitHandle.WaitOne(milliseconds)) + { + wasKilledOnTimeout = KillCore(); + processWaitHandle.WaitOne(Timeout.Infinite); + } + + return new ProcessExitStatus(GetExitCode(), wasKilledOnTimeout); + } + + private async Task WaitForExitAsyncCore(CancellationToken cancellationToken) + { + using Interop.Kernel32.ProcessWaitHandle processWaitHandle = new(this); + + TaskCompletionSource tcs = new(TaskCreationOptions.RunContinuationsAsynchronously); + RegisteredWaitHandle? registeredWaitHandle = null; + CancellationTokenRegistration ctr = default; + + try + { + registeredWaitHandle = ThreadPool.RegisterWaitForSingleObject( + processWaitHandle, + static (state, timedOut) => ((TaskCompletionSource)state!).TrySetResult(true), + tcs, + Timeout.Infinite, + executeOnlyOnce: true); + + if (cancellationToken.CanBeCanceled) + { + ctr = cancellationToken.UnsafeRegister( + static state => + { + var (taskSource, token) = ((TaskCompletionSource taskSource, CancellationToken token))state!; + taskSource.TrySetCanceled(token); + }, + (tcs, cancellationToken)); + } + + await tcs.Task.ConfigureAwait(false); + } + finally + { + ctr.Dispose(); + registeredWaitHandle?.Unregister(null); + } + + return new ProcessExitStatus(GetExitCode(), false); + } + + private async Task WaitForExitOrKillOnCancellationAsyncCore(CancellationToken cancellationToken) + { + using Interop.Kernel32.ProcessWaitHandle processWaitHandle = new(this); + + TaskCompletionSource tcs = new(TaskCreationOptions.RunContinuationsAsynchronously); + RegisteredWaitHandle? registeredWaitHandle = null; + CancellationTokenRegistration ctr = default; + StrongBox wasKilledBox = new(false); + + try + { + registeredWaitHandle = ThreadPool.RegisterWaitForSingleObject( + processWaitHandle, + static (state, timedOut) => ((TaskCompletionSource)state!).TrySetResult(true), + tcs, + Timeout.Infinite, + executeOnlyOnce: true); + + if (cancellationToken.CanBeCanceled) + { + ctr = cancellationToken.UnsafeRegister( + static state => + { + var (handle, wasCancelled) = ((SafeProcessHandle, StrongBox))state!; + wasCancelled.Value = handle.KillCore(); + }, + (this, wasKilledBox)); + } + + await tcs.Task.ConfigureAwait(false); + } + finally + { + ctr.Dispose(); + registeredWaitHandle?.Unregister(null); + } + + return new ProcessExitStatus(GetExitCode(), wasKilledBox.Value); + } + + /// + /// Terminates the process. + /// + /// true if the process was terminated; false if it had already exited. + internal bool KillCore() + { + if (Interop.Kernel32.TerminateProcess(this, -1)) + { + return true; + } + + int error = Marshal.GetLastPInvokeError(); + + // If the process has already exited, TerminateProcess fails with ERROR_ACCESS_DENIED. + if (error == Interop.Errors.ERROR_ACCESS_DENIED && + Interop.Kernel32.GetExitCodeProcess(this, out int exitCode) && + exitCode != Interop.Kernel32.HandleOptions.STILL_ACTIVE) + { + return false; + } + + throw new Win32Exception(error); + } + + private int GetExitCode() + { + if (!Interop.Kernel32.GetExitCodeProcess(this, out int exitCode)) + { + throw new Win32Exception(); + } + + return exitCode; + } + private bool SignalCore(PosixSignal signal) { // On Windows, only SIGKILL is supported, mapped to TerminateProcess. 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 67f8e8efa63b8a..7febe64bfebba9 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 @@ -13,10 +13,13 @@ using System; using System.ComponentModel; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.IO; using System.Runtime.Serialization; using System.Runtime.Versioning; using System.Runtime.InteropServices; +using System.Threading; +using System.Threading.Tasks; namespace Microsoft.Win32.SafeHandles { @@ -169,6 +172,106 @@ public bool Signal(PosixSignal signal) return SignalCore(signal); } + /// + /// Waits indefinitely for the process to exit. + /// + /// The exit status of the process. + /// The handle is invalid. + [UnsupportedOSPlatform("ios")] + [UnsupportedOSPlatform("tvos")] + [SupportedOSPlatform("maccatalyst")] + public ProcessExitStatus WaitForExit() + { + Validate(); + + return WaitForExitCore(); + } + + /// + /// Waits for the process to exit within the specified timeout. + /// + /// The maximum time to wait for the process to exit. + /// When this method returns , contains the exit status of the process. + /// if the process exited before the timeout; otherwise, . + /// The handle is invalid. + /// is negative and not equal to , + /// or is greater than milliseconds. + [UnsupportedOSPlatform("ios")] + [UnsupportedOSPlatform("tvos")] + [SupportedOSPlatform("maccatalyst")] + public bool TryWaitForExit(TimeSpan timeout, [NotNullWhen(true)] out ProcessExitStatus? exitStatus) + { + Validate(); + + return TryWaitForExitCore(ToTimeoutInMilliseconds(timeout), out exitStatus); + } + + /// + /// Waits for the process to exit within the specified timeout. + /// If the process does not exit before the timeout, it is killed and then waited for exit. + /// + /// The maximum time to wait for the process to exit before killing it. + /// The exit status of the process. If the process was killed due to timeout, + /// will be . + /// The handle is invalid. + /// is negative and not equal to , + /// or is greater than milliseconds. + [UnsupportedOSPlatform("ios")] + [UnsupportedOSPlatform("tvos")] + [SupportedOSPlatform("maccatalyst")] + public ProcessExitStatus WaitForExitOrKillOnTimeout(TimeSpan timeout) + { + Validate(); + + return WaitForExitOrKillOnTimeoutCore(ToTimeoutInMilliseconds(timeout)); + } + + /// + /// Waits asynchronously for the process to exit. + /// + /// A cancellation token that can be used to cancel the wait operation. + /// A task that represents the asynchronous wait operation. The task result contains the exit status of the process. + /// The handle is invalid. + /// The cancellation token was canceled. + /// + /// When the cancellation token is canceled, this method stops waiting and throws . + /// The process is NOT killed and continues running. If you want to kill the process on cancellation, + /// use instead. + /// + [UnsupportedOSPlatform("ios")] + [UnsupportedOSPlatform("tvos")] + [SupportedOSPlatform("maccatalyst")] + public Task WaitForExitAsync(CancellationToken cancellationToken = default) + { + Validate(); + + return WaitForExitAsyncCore(cancellationToken); + } + + /// + /// Waits asynchronously for the process to exit. + /// When cancelled, kills the process and then waits for it to exit. + /// + /// A cancellation token that can be used to cancel the wait operation and kill the process. + /// A task that represents the asynchronous wait operation. The task result contains the exit status of the process. + /// If the process was killed due to cancellation, will be . + /// The handle is invalid. + /// + /// When the cancellation token is canceled, this method kills the process and waits for it to exit. + /// The returned exit status will have the property set to if the process was killed. + /// If the cancellation token cannot be canceled (e.g., ), this method behaves identically + /// to and will wait indefinitely for the process to exit. + /// + [UnsupportedOSPlatform("ios")] + [UnsupportedOSPlatform("tvos")] + [SupportedOSPlatform("maccatalyst")] + public Task WaitForExitOrKillOnCancellationAsync(CancellationToken cancellationToken) + { + Validate(); + + return WaitForExitOrKillOnCancellationAsyncCore(cancellationToken); + } + private void Validate() { if (IsInvalid) @@ -176,5 +279,15 @@ private void Validate() throw new InvalidOperationException(SR.InvalidProcessHandle); } } + + internal static int ToTimeoutInMilliseconds(TimeSpan timeout) + { + long totalMilliseconds = (long)timeout.TotalMilliseconds; + + ArgumentOutOfRangeException.ThrowIfLessThan(totalMilliseconds, -1, nameof(timeout)); + ArgumentOutOfRangeException.ThrowIfGreaterThan(totalMilliseconds, int.MaxValue, nameof(timeout)); + + return (int)totalMilliseconds; + } } } diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs index ab1dd7e46bf2f6..fb500db2635d41 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs @@ -210,6 +210,8 @@ internal void ReleaseRef() private bool _exited; /// If the process exited, it's exit code, or null if we were unable to determine one. private int? _exitCode; + /// If the process was terminated by a signal, the raw signal number, or null if it was not. + private int? _terminatingSignal; /// /// The approximate time the process exited. We do not have the ability to know exact time a process /// exited, so we approximate it by storing the time that we discovered it exited. @@ -343,6 +345,18 @@ internal bool GetExited(out int? exitCode, bool refresh) } } + /// Gets the terminating signal if the process was killed by a signal. + internal int? TerminatingSignal + { + get + { + lock (_gate) + { + return _terminatingSignal; + } + } + } + private void CheckForNonChildExit() { Debug.Assert(Monitor.IsEntered(_gate)); @@ -539,13 +553,14 @@ private async Task WaitForExitAsync(CancellationToken cancellationToken) } } - private void ChildReaped(int exitCode, bool configureConsole) + private void ChildReaped(int exitCode, int terminatingSignal, bool configureConsole) { lock (_gate) { Debug.Assert(!_exited); _exitCode = exitCode; + _terminatingSignal = terminatingSignal != 0 ? terminatingSignal : null; if (_usesTerminal) { @@ -568,11 +583,12 @@ private bool TryReapChild(bool configureConsole) // Try to get the state of the child process int exitCode; - int waitResult = Interop.Sys.WaitPidExitedNoHang(_processId, out exitCode); + int terminatingSignal; + int waitResult = Interop.Sys.WaitPidExitedNoHang(_processId, out exitCode, out terminatingSignal); if (waitResult == _processId) { - ChildReaped(exitCode, configureConsole); + ChildReaped(exitCode, terminatingSignal, configureConsole); return true; } else if (waitResult == 0) @@ -673,7 +689,8 @@ internal static void CheckChildren(bool reapAll, bool configureConsole) do { int exitCode; - pid = Interop.Sys.WaitPidExitedNoHang(-1, out exitCode); + int terminatingSignal; + pid = Interop.Sys.WaitPidExitedNoHang(-1, out exitCode, out terminatingSignal); if (pid <= 0) { break; @@ -682,7 +699,7 @@ internal static void CheckChildren(bool reapAll, bool configureConsole) // Check if the process is a child that has just terminated. if (s_childProcessWaitStates.TryGetValue(pid, out ProcessWaitState? pws)) { - pws.ChildReaped(exitCode, configureConsole); + pws.ChildReaped(exitCode, terminatingSignal, configureConsole); pws.ReleaseRef(); } } while (true); diff --git a/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs b/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs index 8a0e2394c03a6e..fd636f5daf49c8 100644 --- a/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs @@ -5,6 +5,7 @@ using System.IO; using System.Runtime.InteropServices; using System.Threading; +using System.Threading.Tasks; using Microsoft.DotNet.RemoteExecutor; using Microsoft.Win32.SafeHandles; using Xunit; @@ -44,25 +45,16 @@ public void CanStartProcess() using StreamReader streamReader = new(new FileStream(outputReadPipe, FileAccess.Read, bufferSize: 1, outputReadPipe.IsAsync)); Assert.Equal("ping", streamReader.ReadLine()); - // We can get the process by id only when it's still running, - // so we wait with "pong" until we obtain the process instance. - // When we introduce SafeProcessHandle.WaitForExit* APIs, it's not needed. - using Process fetchedProcess = Process.GetProcessById(processHandle.ProcessId); - using StreamWriter streamWriter = new(new FileStream(inputWritePipe, FileAccess.Write, bufferSize: 1, inputWritePipe.IsAsync)) { AutoFlush = true }; - try - { - streamWriter.WriteLine("pong"); - } - finally - { - fetchedProcess.Kill(); - fetchedProcess.WaitForExit(); - } + streamWriter.WriteLine("pong"); + + ProcessExitStatus exitStatus = processHandle.WaitForExitOrKillOnTimeout(TimeSpan.FromSeconds(30)); + Assert.Equal(RemoteExecutor.SuccessExitCode, exitStatus.ExitCode); + Assert.False(exitStatus.Canceled); } } @@ -119,6 +111,34 @@ public void Signal_InvalidHandle_ThrowsInvalidOperationException() Assert.Throws(() => invalidHandle.Signal(PosixSignal.SIGKILL)); } + [Fact] + public void WaitForExit_InvalidHandle_ThrowsInvalidOperationException() + { + using SafeProcessHandle invalidHandle = new SafeProcessHandle(); + Assert.Throws(() => invalidHandle.WaitForExit()); + } + + [Fact] + public void TryWaitForExit_InvalidHandle_ThrowsInvalidOperationException() + { + using SafeProcessHandle invalidHandle = new SafeProcessHandle(); + Assert.Throws(() => invalidHandle.TryWaitForExit(TimeSpan.Zero, out _)); + } + + [Fact] + public void WaitForExitOrKillOnTimeout_InvalidHandle_ThrowsInvalidOperationException() + { + using SafeProcessHandle invalidHandle = new SafeProcessHandle(); + Assert.Throws(() => invalidHandle.WaitForExitOrKillOnTimeout(TimeSpan.Zero)); + } + + [Fact] + public async Task WaitForExitAsync_InvalidHandle_ThrowsInvalidOperationException() + { + using SafeProcessHandle invalidHandle = new SafeProcessHandle(); + await Assert.ThrowsAsync(() => invalidHandle.WaitForExitAsync()); + } + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] public void Kill_RunningProcess_Terminates() { @@ -129,11 +149,11 @@ public void Kill_RunningProcess_Terminates() }); using SafeProcessHandle processHandle = SafeProcessHandle.Start(process.StartInfo); - using Process fetchedProcess = Process.GetProcessById(processHandle.ProcessId); processHandle.Kill(); - Assert.True(fetchedProcess.WaitForExit(WaitInMS)); + ProcessExitStatus exitStatus = processHandle.WaitForExitOrKillOnTimeout(TimeSpan.FromSeconds(30)); + Assert.NotEqual(0, exitStatus.ExitCode); } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] @@ -249,5 +269,240 @@ public void Kill_HandleWithoutTerminatePermission_ThrowsWin32Exception() process.WaitForExit(); } } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void WaitForExit_ProcessExitsNormally_ReturnsExitCode() + { + Process process = CreateProcess(static () => 42); + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(process.StartInfo); + + ProcessExitStatus exitStatus = processHandle.WaitForExit(); + + Assert.Equal(42, exitStatus.ExitCode); + Assert.False(exitStatus.Canceled); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void WaitForExit_ProcessExitsWithZero_ReturnsZeroExitCode() + { + Process process = CreateProcess(static () => RemoteExecutor.SuccessExitCode); + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(process.StartInfo); + + ProcessExitStatus exitStatus = processHandle.WaitForExit(); + + Assert.Equal(RemoteExecutor.SuccessExitCode, exitStatus.ExitCode); + Assert.False(exitStatus.Canceled); + Assert.Null(exitStatus.Signal); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void TryWaitForExit_ProcessExitsBeforeTimeout_ReturnsTrue() + { + Process process = CreateProcess(static () => RemoteExecutor.SuccessExitCode); + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(process.StartInfo); + + bool exited = processHandle.TryWaitForExit(TimeSpan.FromSeconds(30), out ProcessExitStatus? exitStatus); + + Assert.True(exited); + Assert.NotNull(exitStatus); + Assert.Equal(RemoteExecutor.SuccessExitCode, exitStatus.ExitCode); + Assert.False(exitStatus.Canceled); + Assert.Null(exitStatus.Signal); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void TryWaitForExit_ProcessDoesNotExitBeforeTimeout_ReturnsFalse() + { + Process process = CreateProcess(static () => + { + Thread.Sleep(Timeout.Infinite); + return RemoteExecutor.SuccessExitCode; + }); + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(process.StartInfo); + + try + { + Stopwatch stopwatch = Stopwatch.StartNew(); + bool exited = processHandle.TryWaitForExit(TimeSpan.FromMilliseconds(300), out ProcessExitStatus? exitStatus); + stopwatch.Stop(); + + Assert.False(exited); + Assert.Null(exitStatus); + Assert.InRange(stopwatch.Elapsed, TimeSpan.FromMilliseconds(200), TimeSpan.FromMilliseconds(5000)); + } + finally + { + processHandle.Kill(); + processHandle.WaitForExit(); + } + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void WaitForExitOrKillOnTimeout_ProcessExitsBeforeTimeout_DoesNotKill() + { + Process process = CreateProcess(static () => RemoteExecutor.SuccessExitCode); + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(process.StartInfo); + + ProcessExitStatus exitStatus = processHandle.WaitForExitOrKillOnTimeout(TimeSpan.FromSeconds(30)); + + Assert.Equal(RemoteExecutor.SuccessExitCode, exitStatus.ExitCode); + Assert.False(exitStatus.Canceled); + Assert.Null(exitStatus.Signal); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void WaitForExitOrKillOnTimeout_ProcessDoesNotExitBeforeTimeout_KillsAndReturns() + { + Process process = CreateProcess(static () => + { + Thread.Sleep(Timeout.Infinite); + return RemoteExecutor.SuccessExitCode; + }); + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(process.StartInfo); + + Stopwatch stopwatch = Stopwatch.StartNew(); + ProcessExitStatus exitStatus = processHandle.WaitForExitOrKillOnTimeout(TimeSpan.FromMilliseconds(300)); + stopwatch.Stop(); + + Assert.InRange(stopwatch.Elapsed, TimeSpan.FromMilliseconds(200), TimeSpan.FromSeconds(10)); + Assert.True(exitStatus.Canceled); + Assert.NotEqual(0, exitStatus.ExitCode); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public async Task WaitForExitAsync_ProcessExitsNormally_ReturnsExitCode() + { + Process process = CreateProcess(static () => RemoteExecutor.SuccessExitCode); + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(process.StartInfo); + + using CancellationTokenSource cts = new(TimeSpan.FromSeconds(30)); + ProcessExitStatus exitStatus = await processHandle.WaitForExitAsync(cts.Token); + + Assert.Equal(RemoteExecutor.SuccessExitCode, exitStatus.ExitCode); + Assert.False(exitStatus.Canceled); + Assert.Null(exitStatus.Signal); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public async Task WaitForExitAsync_WithoutCancellationToken_CompletesNormally() + { + Process process = CreateProcess(static () => RemoteExecutor.SuccessExitCode); + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(process.StartInfo); + + ProcessExitStatus exitStatus = await processHandle.WaitForExitAsync(); + + Assert.Equal(RemoteExecutor.SuccessExitCode, exitStatus.ExitCode); + Assert.False(exitStatus.Canceled); + Assert.Null(exitStatus.Signal); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public async Task WaitForExitAsync_CancellationRequested_ThrowsOperationCanceledException() + { + Process process = CreateProcess(static () => + { + Thread.Sleep(Timeout.Infinite); + return RemoteExecutor.SuccessExitCode; + }); + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(process.StartInfo); + + try + { + using CancellationTokenSource cts = new(TimeSpan.FromMilliseconds(300)); + + await Assert.ThrowsAnyAsync( + async () => await processHandle.WaitForExitAsync(cts.Token)); + } + finally + { + processHandle.Kill(); + processHandle.WaitForExit(); + } + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public async Task WaitForExitOrKillOnCancellationAsync_ProcessExitsNormally_ReturnsExitCode() + { + Process process = CreateProcess(static () => RemoteExecutor.SuccessExitCode); + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(process.StartInfo); + + using CancellationTokenSource cts = new(TimeSpan.FromSeconds(30)); + ProcessExitStatus exitStatus = await processHandle.WaitForExitOrKillOnCancellationAsync(cts.Token); + + Assert.Equal(RemoteExecutor.SuccessExitCode, exitStatus.ExitCode); + Assert.False(exitStatus.Canceled); + Assert.Null(exitStatus.Signal); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public async Task WaitForExitOrKillOnCancellationAsync_CancellationRequested_KillsAndReturns() + { + Process process = CreateProcess(static () => + { + Thread.Sleep(Timeout.Infinite); + return RemoteExecutor.SuccessExitCode; + }); + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(process.StartInfo); + + Stopwatch stopwatch = Stopwatch.StartNew(); + using CancellationTokenSource cts = new(TimeSpan.FromMilliseconds(300)); + + ProcessExitStatus exitStatus = await processHandle.WaitForExitOrKillOnCancellationAsync(cts.Token); + stopwatch.Stop(); + + Assert.InRange(stopwatch.Elapsed, TimeSpan.FromMilliseconds(200), TimeSpan.FromSeconds(10)); + Assert.True(exitStatus.Canceled); + Assert.NotEqual(0, exitStatus.ExitCode); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void WaitForExit_CalledAfterKill_ReturnsImmediately() + { + Process process = CreateProcess(static () => + { + Thread.Sleep(Timeout.Infinite); + return RemoteExecutor.SuccessExitCode; + }); + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(process.StartInfo); + processHandle.Kill(); + + Stopwatch stopwatch = Stopwatch.StartNew(); + ProcessExitStatus exitStatus = processHandle.WaitForExitOrKillOnTimeout(TimeSpan.FromSeconds(5)); + stopwatch.Stop(); + + Assert.InRange(stopwatch.Elapsed, TimeSpan.Zero, TimeSpan.FromSeconds(5)); + Assert.NotEqual(RemoteExecutor.SuccessExitCode, exitStatus.ExitCode); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [SkipOnPlatform(TestPlatforms.Windows, "Signal property is Unix-specific")] + public void WaitForExit_ProcessKilledBySignal_ReportsSignal() + { + Process process = CreateProcess(static () => + { + Thread.Sleep(Timeout.Infinite); + return RemoteExecutor.SuccessExitCode; + }); + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(process.StartInfo); + processHandle.Kill(); + + ProcessExitStatus exitStatus = processHandle.WaitForExit(); + + Assert.NotNull(exitStatus.Signal); + Assert.Equal(PosixSignal.SIGKILL, exitStatus.Signal); + } } } diff --git a/src/native/libs/System.Native/pal_process.c b/src/native/libs/System.Native/pal_process.c index 82c00925143113..59a6c24746efa3 100644 --- a/src/native/libs/System.Native/pal_process.c +++ b/src/native/libs/System.Native/pal_process.c @@ -929,9 +929,10 @@ int32_t SystemNative_WaitIdAnyExitedNoHangNoWait(void) return result; } -int32_t SystemNative_WaitPidExitedNoHang(int32_t pid, int32_t* exitCode) +int32_t SystemNative_WaitPidExitedNoHang(int32_t pid, int32_t* exitCode, int32_t* terminatingSignal) { assert(exitCode != NULL); + assert(terminatingSignal != NULL); int32_t result; int status; @@ -942,11 +943,13 @@ int32_t SystemNative_WaitPidExitedNoHang(int32_t pid, int32_t* exitCode) { // the child terminated normally. *exitCode = WEXITSTATUS(status); + *terminatingSignal = 0; } else if (WIFSIGNALED(status)) { // child process was terminated by a signal. *exitCode = 128 + WTERMSIG(status); + *terminatingSignal = WTERMSIG(status); } else { diff --git a/src/native/libs/System.Native/pal_process.h b/src/native/libs/System.Native/pal_process.h index 00bddcd17d78f4..a05994ef9190b2 100644 --- a/src/native/libs/System.Native/pal_process.h +++ b/src/native/libs/System.Native/pal_process.h @@ -187,7 +187,7 @@ PALEXPORT int32_t SystemNative_WaitIdAnyExitedNoHangNoWait(void); * 3) if the child has not yet terminated, 0 is returned * 4) on error, -1 is returned. */ -PALEXPORT int32_t SystemNative_WaitPidExitedNoHang(int32_t pid, int32_t* exitCode); +PALEXPORT int32_t SystemNative_WaitPidExitedNoHang(int32_t pid, int32_t* exitCode, int32_t* terminatingSignal); /** * Gets the configurable limit or variable for system path or file descriptor options. diff --git a/src/native/libs/System.Native/pal_process_wasi.c b/src/native/libs/System.Native/pal_process_wasi.c index ff32ba0a39bba5..1ad9f888bbc04c 100644 --- a/src/native/libs/System.Native/pal_process_wasi.c +++ b/src/native/libs/System.Native/pal_process_wasi.c @@ -76,7 +76,7 @@ int32_t SystemNative_WaitIdAnyExitedNoHangNoWait(void) return -1; } -int32_t SystemNative_WaitPidExitedNoHang(int32_t pid, int32_t* exitCode) +int32_t SystemNative_WaitPidExitedNoHang(int32_t pid, int32_t* exitCode, int32_t* terminatingSignal) { return -1; } From d2f68c91f52357726ace1fc6402770ae6da64a7f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Apr 2026 21:07:42 +0000 Subject: [PATCH 02/29] Address PR feedback: store ProcessExitStatus in ProcessWaitState, move ToTimeoutMilliseconds to ProcessUtils, remove KillCore, fix platform attributes, update native PAL - ProcessWaitState: store ProcessExitStatus? instead of separate _exitCode/_terminatingSignal - Native PAL: use TryConvertSignalCodeToPosixSignal for proper PosixSignal mapping - Move ToTimeoutMilliseconds to ProcessUtils.cs, reuse from Process and SafeProcessHandle - Remove separate KillCore methods on both platforms; use SignalCore on Unix, TerminateProcessCore on Windows - Remove platform attributes from WaitForExit, TryWaitForExit, WaitForExitAsync (non-killing methods) - Fix CreateExitStatus canceled logic: canceled && signal is not null - Update Process.Unix.cs to consume ProcessExitStatus? from GetExited Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/1a601c8b-3c5f-4596-9b8f-5e11188e35ae Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../ref/System.Diagnostics.Process.cs | 9 ----- .../SafeHandles/SafeProcessHandle.Unix.cs | 33 +++---------------- .../SafeHandles/SafeProcessHandle.Windows.cs | 9 +++-- .../Win32/SafeHandles/SafeProcessHandle.cs | 23 ++----------- .../Diagnostics/Process.Multiplexing.cs | 2 +- .../src/System/Diagnostics/Process.Unix.cs | 8 ++--- .../src/System/Diagnostics/Process.cs | 14 ++------ .../src/System/Diagnostics/ProcessUtils.cs | 10 ++++++ .../Diagnostics/ProcessWaitState.Unix.cs | 30 +++++------------ src/native/libs/System.Native/pal_process.c | 8 +++-- src/native/libs/System.Native/pal_process.h | 4 +++ src/native/libs/System.Native/pal_signal.c | 2 +- src/native/libs/System.Native/pal_signal.h | 8 +++++ 13 files changed, 55 insertions(+), 105 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 e85a16f8fe548e..95e7bb1c682e18 100644 --- a/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs +++ b/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs @@ -24,17 +24,8 @@ public void Kill() { } [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] public static Microsoft.Win32.SafeHandles.SafeProcessHandle Start(System.Diagnostics.ProcessStartInfo startInfo) { throw null; } - [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] - [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] - [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] public bool TryWaitForExit(System.TimeSpan timeout, [System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] out System.Diagnostics.ProcessExitStatus? exitStatus) { throw null; } - [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] - [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] - [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] public System.Diagnostics.ProcessExitStatus WaitForExit() { throw null; } - [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] - [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] - [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] public System.Threading.Tasks.Task WaitForExitAsync(System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { 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 7f71498ad191da..415f4d66cbb4fb 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 @@ -121,7 +121,7 @@ private ProcessExitStatus WaitForExitOrKillOnTimeoutCore(int milliseconds) if (!waitState.WaitForExit(milliseconds)) { - wasKilled = KillCore(); + wasKilled = SignalCore(PosixSignal.SIGKILL); waitState.WaitForExit(Timeout.Infinite); } @@ -193,7 +193,7 @@ private async Task WaitForExitOrKillOnCancellationAsyncCore(C static state => { var (handle, wasCancelled) = ((SafeProcessHandle, StrongBox))state!; - wasCancelled.Value = handle.KillCore(); + wasCancelled.Value = handle.SignalCore(PosixSignal.SIGKILL); }, (this, wasKilledBox)); } @@ -209,28 +209,6 @@ private async Task WaitForExitOrKillOnCancellationAsyncCore(C return CreateExitStatus(waitState, canceled: wasKilledBox.Value); } - /// - /// Terminates the process by sending SIGKILL. - /// - /// true if the process was terminated; false if it had already exited. - internal bool KillCore() - { - int signalNumber = Interop.Sys.GetPlatformSignalNumber(PosixSignal.SIGKILL); - int killResult = Interop.Sys.Kill(ProcessId, signalNumber); - if (killResult == 0) - { - return true; - } - - Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); - if (errorInfo.Error == Interop.Error.ESRCH) - { - return false; // Process already exited - } - - throw new Win32Exception(errorInfo.RawErrno); - } - private ProcessWaitState GetWaitState() { if (_waitState is null) @@ -243,11 +221,10 @@ private ProcessWaitState GetWaitState() private static ProcessExitStatus CreateExitStatus(ProcessWaitState waitState, bool canceled) { - waitState.GetExited(out int? exitCode, refresh: false); - int? rawSignal = waitState.TerminatingSignal; - PosixSignal? signal = rawSignal.HasValue ? (PosixSignal)rawSignal.Value : null; + waitState.GetExited(out ProcessExitStatus? exitStatus, refresh: false); + PosixSignal? signal = exitStatus?.Signal; - return new ProcessExitStatus(exitCode ?? 0, canceled, signal); + return new ProcessExitStatus(exitStatus?.ExitCode ?? 0, canceled && signal is not null, signal); } private delegate SafeProcessHandle StartWithShellExecuteDelegate(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, SafeFileHandle? stdoutHandle, SafeFileHandle? stderrHandle, out ProcessWaitState.Holder? waitStateHolder); 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 b1d99879a2a1af..5ed0bbda2e46a4 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 @@ -654,7 +654,7 @@ private ProcessExitStatus WaitForExitOrKillOnTimeoutCore(int milliseconds) using Interop.Kernel32.ProcessWaitHandle processWaitHandle = new(this); if (!processWaitHandle.WaitOne(milliseconds)) { - wasKilledOnTimeout = KillCore(); + wasKilledOnTimeout = TerminateProcessCore(); processWaitHandle.WaitOne(Timeout.Infinite); } @@ -724,7 +724,7 @@ private async Task WaitForExitOrKillOnCancellationAsyncCore(C static state => { var (handle, wasCancelled) = ((SafeProcessHandle, StrongBox))state!; - wasCancelled.Value = handle.KillCore(); + wasCancelled.Value = handle.TerminateProcessCore(); }, (this, wasKilledBox)); } @@ -741,10 +741,10 @@ private async Task WaitForExitOrKillOnCancellationAsyncCore(C } /// - /// Terminates the process. + /// Terminates the process using TerminateProcess. /// /// true if the process was terminated; false if it had already exited. - internal bool KillCore() + private bool TerminateProcessCore() { if (Interop.Kernel32.TerminateProcess(this, -1)) { @@ -753,7 +753,6 @@ internal bool KillCore() int error = Marshal.GetLastPInvokeError(); - // If the process has already exited, TerminateProcess fails with ERROR_ACCESS_DENIED. if (error == Interop.Errors.ERROR_ACCESS_DENIED && Interop.Kernel32.GetExitCodeProcess(this, out int exitCode) && exitCode != Interop.Kernel32.HandleOptions.STILL_ACTIVE) 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 7febe64bfebba9..e1b270aa863f95 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 @@ -177,9 +177,6 @@ public bool Signal(PosixSignal signal) /// /// The exit status of the process. /// The handle is invalid. - [UnsupportedOSPlatform("ios")] - [UnsupportedOSPlatform("tvos")] - [SupportedOSPlatform("maccatalyst")] public ProcessExitStatus WaitForExit() { Validate(); @@ -196,14 +193,11 @@ public ProcessExitStatus WaitForExit() /// The handle is invalid. /// is negative and not equal to , /// or is greater than milliseconds. - [UnsupportedOSPlatform("ios")] - [UnsupportedOSPlatform("tvos")] - [SupportedOSPlatform("maccatalyst")] public bool TryWaitForExit(TimeSpan timeout, [NotNullWhen(true)] out ProcessExitStatus? exitStatus) { Validate(); - return TryWaitForExitCore(ToTimeoutInMilliseconds(timeout), out exitStatus); + return TryWaitForExitCore(ProcessUtils.ToTimeoutMilliseconds(timeout), out exitStatus); } /// @@ -223,7 +217,7 @@ public ProcessExitStatus WaitForExitOrKillOnTimeout(TimeSpan timeout) { Validate(); - return WaitForExitOrKillOnTimeoutCore(ToTimeoutInMilliseconds(timeout)); + return WaitForExitOrKillOnTimeoutCore(ProcessUtils.ToTimeoutMilliseconds(timeout)); } /// @@ -238,9 +232,6 @@ public ProcessExitStatus WaitForExitOrKillOnTimeout(TimeSpan timeout) /// The process is NOT killed and continues running. If you want to kill the process on cancellation, /// use instead. /// - [UnsupportedOSPlatform("ios")] - [UnsupportedOSPlatform("tvos")] - [SupportedOSPlatform("maccatalyst")] public Task WaitForExitAsync(CancellationToken cancellationToken = default) { Validate(); @@ -279,15 +270,5 @@ private void Validate() throw new InvalidOperationException(SR.InvalidProcessHandle); } } - - internal static int ToTimeoutInMilliseconds(TimeSpan timeout) - { - long totalMilliseconds = (long)timeout.TotalMilliseconds; - - ArgumentOutOfRangeException.ThrowIfLessThan(totalMilliseconds, -1, nameof(timeout)); - ArgumentOutOfRangeException.ThrowIfGreaterThan(totalMilliseconds, int.MaxValue, nameof(timeout)); - - return (int)totalMilliseconds; - } } } diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.cs index 71ec5a984a1096..b89ef8a9256765 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.cs @@ -298,7 +298,7 @@ private void ReadPipesToBuffers( ref int errorBytesRead) { int timeoutMs = timeout.HasValue - ? ToTimeoutMilliseconds(timeout.Value) + ? ProcessUtils.ToTimeoutMilliseconds(timeout.Value) : Timeout.Infinite; var outputHandle = GetSafeHandleFromStreamReader(_standardOutput!); diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index 6abe345f996f0c..b385cd6df6f7d1 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -223,11 +223,11 @@ public ProcessModule? MainModule /// Checks whether the process has exited and updates state accordingly. private void UpdateHasExited() { - int? exitCode; - _exited = GetWaitState().GetExited(out exitCode, refresh: true); - if (_exited && exitCode != null) + ProcessExitStatus? exitStatus; + _exited = GetWaitState().GetExited(out exitStatus, refresh: true); + if (_exited && exitStatus is not null) { - _exitCode = exitCode.Value; + _exitCode = exitStatus.ExitCode; } } 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 65b96e440b4061..8966e578389e92 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs @@ -749,7 +749,7 @@ public bool WaitForInputIdle(int milliseconds) /// This state is useful, for example, when your application needs to wait for a starting process /// to finish creating its main window before the application communicates with that window. /// - public bool WaitForInputIdle(TimeSpan timeout) => WaitForInputIdle(ToTimeoutMilliseconds(timeout)); + public bool WaitForInputIdle(TimeSpan timeout) => WaitForInputIdle(ProcessUtils.ToTimeoutMilliseconds(timeout)); public ISynchronizeInvoke? SynchronizingObject { get; set; } @@ -1469,17 +1469,7 @@ public bool WaitForExit(int milliseconds) /// Instructs the Process component to wait the specified number of milliseconds for /// the associated process to exit. /// - public bool WaitForExit(TimeSpan timeout) => WaitForExit(ToTimeoutMilliseconds(timeout)); - - private static int ToTimeoutMilliseconds(TimeSpan timeout) - { - long totalMilliseconds = (long)timeout.TotalMilliseconds; - - ArgumentOutOfRangeException.ThrowIfLessThan(totalMilliseconds, -1, nameof(timeout)); - ArgumentOutOfRangeException.ThrowIfGreaterThan(totalMilliseconds, int.MaxValue, nameof(timeout)); - - return (int)totalMilliseconds; - } + public bool WaitForExit(TimeSpan timeout) => WaitForExit(ProcessUtils.ToTimeoutMilliseconds(timeout)); /// /// Instructs the Process component to wait for the associated process to exit, or diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.cs index 24d53f7fc1391b..a6785d50f93550 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.cs @@ -46,5 +46,15 @@ internal static Win32Exception CreateExceptionForErrorStartingProcess(string err string msg = SR.Format(SR.ErrorStartingProcess, fileName, directoryForException, errorMessage); return new Win32Exception(errorCode, msg); } + + internal static int ToTimeoutMilliseconds(TimeSpan timeout) + { + long totalMilliseconds = (long)timeout.TotalMilliseconds; + + ArgumentOutOfRangeException.ThrowIfLessThan(totalMilliseconds, -1, nameof(timeout)); + ArgumentOutOfRangeException.ThrowIfGreaterThan(totalMilliseconds, int.MaxValue, nameof(timeout)); + + return (int)totalMilliseconds; + } } } diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs index fb500db2635d41..2890f0b421d97d 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs @@ -208,10 +208,8 @@ internal void ReleaseRef() /// Whether the associated process exited. private bool _exited; - /// If the process exited, it's exit code, or null if we were unable to determine one. - private int? _exitCode; - /// If the process was terminated by a signal, the raw signal number, or null if it was not. - private int? _terminatingSignal; + /// If the process exited, its exit status, or null if we were unable to determine one. + private ProcessExitStatus? _exitStatus; /// /// The approximate time the process exited. We do not have the ability to know exact time a process /// exited, so we approximate it by storing the time that we discovered it exited. @@ -312,14 +310,14 @@ internal bool HasExited } } - internal bool GetExited(out int? exitCode, bool refresh) + internal bool GetExited(out ProcessExitStatus? exitStatus, bool refresh) { lock (_gate) { // Have we already exited? If so, return the cached results. if (_exited) { - exitCode = _exitCode; + exitStatus = _exitStatus; return true; } @@ -327,7 +325,7 @@ internal bool GetExited(out int? exitCode, bool refresh) // and that task owns the right to call CheckForNonChildExit. if (!_waitInProgress.IsCompleted) { - exitCode = null; + exitStatus = null; return false; } @@ -340,23 +338,11 @@ internal bool GetExited(out int? exitCode, bool refresh) // We now have an up-to-date snapshot for whether we've exited, // and if we have, what the exit code is (if we were able to find out). - exitCode = _exitCode; + exitStatus = _exitStatus; return _exited; } } - /// Gets the terminating signal if the process was killed by a signal. - internal int? TerminatingSignal - { - get - { - lock (_gate) - { - return _terminatingSignal; - } - } - } - private void CheckForNonChildExit() { Debug.Assert(Monitor.IsEntered(_gate)); @@ -559,8 +545,8 @@ private void ChildReaped(int exitCode, int terminatingSignal, bool configureCons { Debug.Assert(!_exited); - _exitCode = exitCode; - _terminatingSignal = terminatingSignal != 0 ? terminatingSignal : null; + PosixSignal? signal = terminatingSignal != 0 ? (PosixSignal)terminatingSignal : null; + _exitStatus = new ProcessExitStatus(exitCode, canceled: false, signal); if (_usesTerminal) { diff --git a/src/native/libs/System.Native/pal_process.c b/src/native/libs/System.Native/pal_process.c index 59a6c24746efa3..349b02ec7ae665 100644 --- a/src/native/libs/System.Native/pal_process.c +++ b/src/native/libs/System.Native/pal_process.c @@ -3,6 +3,7 @@ #include "pal_config.h" #include "pal_process.h" +#include "pal_signal.h" #include "pal_io.h" #include "pal_utilities.h" @@ -948,8 +949,11 @@ int32_t SystemNative_WaitPidExitedNoHang(int32_t pid, int32_t* exitCode, int32_t else if (WIFSIGNALED(status)) { // child process was terminated by a signal. - *exitCode = 128 + WTERMSIG(status); - *terminatingSignal = WTERMSIG(status); + int sig = WTERMSIG(status); + *exitCode = 128 + sig; + PosixSignal posixSignal = PosixSignalInvalid; + TryConvertSignalCodeToPosixSignal(sig, &posixSignal); + *terminatingSignal = (int32_t)posixSignal; } else { diff --git a/src/native/libs/System.Native/pal_process.h b/src/native/libs/System.Native/pal_process.h index a05994ef9190b2..abcc7d87439cd7 100644 --- a/src/native/libs/System.Native/pal_process.h +++ b/src/native/libs/System.Native/pal_process.h @@ -186,6 +186,10 @@ PALEXPORT int32_t SystemNative_WaitIdAnyExitedNoHangNoWait(void); * 2) if pid is not a child or there are no unwaited-for children, -1 is returned (errno=ECHILD) * 3) if the child has not yet terminated, 0 is returned * 4) on error, -1 is returned. + * + * exitCode: set to WEXITSTATUS on normal exit, or 128 + signal number on signal termination. + * terminatingSignal: set to 0 on normal exit, or the PosixSignal value on signal termination. + * For signals not in the known PosixSignal set, this is the raw signal number. */ PALEXPORT int32_t SystemNative_WaitPidExitedNoHang(int32_t pid, int32_t* exitCode, int32_t* terminatingSignal); diff --git a/src/native/libs/System.Native/pal_signal.c b/src/native/libs/System.Native/pal_signal.c index 18254d590d9fe9..3e02b2466fc65d 100644 --- a/src/native/libs/System.Native/pal_signal.c +++ b/src/native/libs/System.Native/pal_signal.c @@ -83,7 +83,7 @@ static bool IsSigIgn(struct sigaction* action) action->sa_handler == SIG_IGN; } -static bool TryConvertSignalCodeToPosixSignal(int signalCode, PosixSignal* posixSignal) +bool TryConvertSignalCodeToPosixSignal(int signalCode, PosixSignal* posixSignal) { assert(posixSignal != NULL); diff --git a/src/native/libs/System.Native/pal_signal.h b/src/native/libs/System.Native/pal_signal.h index 074c77315f5edc..801251746031c9 100644 --- a/src/native/libs/System.Native/pal_signal.h +++ b/src/native/libs/System.Native/pal_signal.h @@ -83,6 +83,14 @@ PALEXPORT void SystemNative_DisablePosixSignalHandling(int signalCode); */ PALEXPORT void SystemNative_HandleNonCanceledPosixSignal(int32_t signalCode); +/** + * Converts a native signal code to PosixSignal. + * + * Returns true if the signal code was mapped to a known PosixSignal. + * Returns false if the signal is not in the known set (posixSignal is set to the raw signal code). + */ +bool TryConvertSignalCodeToPosixSignal(int signalCode, PosixSignal* posixSignal); + typedef void (*ConsoleSigTtouHandler)(void); /** From 33f89fc4782e1d6390d2f3bcc008caedfc2f10c1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Apr 2026 21:22:42 +0000 Subject: [PATCH 03/29] Add comment explaining ERROR_ACCESS_DENIED handling in TerminateProcessCore Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/1a601c8b-3c5f-4596-9b8f-5e11188e35ae Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Windows.cs | 1 + 1 file changed, 1 insertion(+) 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 5ed0bbda2e46a4..246366892a2307 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 @@ -753,6 +753,7 @@ private bool TerminateProcessCore() int error = Marshal.GetLastPInvokeError(); + // If the process has already exited, TerminateProcess fails with ERROR_ACCESS_DENIED. if (error == Interop.Errors.ERROR_ACCESS_DENIED && Interop.Kernel32.GetExitCodeProcess(this, out int exitCode) && exitCode != Interop.Kernel32.HandleOptions.STILL_ACTIVE) From 7c723d58cdc1dedd48cca8f142b262d122f8941b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Apr 2026 22:09:49 +0000 Subject: [PATCH 04/29] Address PR feedback: remove TerminateProcessCore, use SignalCore with pragma, simplify Unix async via ProcessWaitState.WaitForExitAsync, add PlatformNotSupportedException checks, rename wasKilled to signalWasSent, add Debug.Assert, update ProcessHandlesTests.Windows.cs Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/603f063f-b34a-42f8-ab6b-e1aa2df62675 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../SafeHandles/SafeProcessHandle.Unix.cs | 83 ++++--------------- .../SafeHandles/SafeProcessHandle.Windows.cs | 44 +++------- .../Win32/SafeHandles/SafeProcessHandle.cs | 10 +++ .../Diagnostics/ProcessWaitState.Unix.cs | 48 ++++++++++- .../tests/ProcessHandlesTests.Windows.cs | 14 +--- 5 files changed, 84 insertions(+), 115 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 415f4d66cbb4fb..0d07ec83bf76bf 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 @@ -8,7 +8,6 @@ using System.Diagnostics.CodeAnalysis; using System.IO; using System.IO.Pipes; -using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Runtime.Versioning; using System.Security; @@ -117,53 +116,22 @@ private bool TryWaitForExitCore(int milliseconds, [NotNullWhen(true)] out Proces private ProcessExitStatus WaitForExitOrKillOnTimeoutCore(int milliseconds) { ProcessWaitState waitState = GetWaitState(); - bool wasKilled = false; + bool signalWasSent = false; if (!waitState.WaitForExit(milliseconds)) { - wasKilled = SignalCore(PosixSignal.SIGKILL); + signalWasSent = SignalCore(PosixSignal.SIGKILL); waitState.WaitForExit(Timeout.Infinite); } - return CreateExitStatus(waitState, canceled: wasKilled); + return CreateExitStatus(waitState, canceled: signalWasSent); } private async Task WaitForExitAsyncCore(CancellationToken cancellationToken) { ProcessWaitState waitState = GetWaitState(); - ManualResetEvent exitedEvent = waitState.EnsureExitedEvent(); - TaskCompletionSource tcs = new(TaskCreationOptions.RunContinuationsAsynchronously); - RegisteredWaitHandle? registeredWaitHandle = null; - CancellationTokenRegistration ctr = default; - - try - { - registeredWaitHandle = ThreadPool.RegisterWaitForSingleObject( - exitedEvent, - static (state, timedOut) => ((TaskCompletionSource)state!).TrySetResult(true), - tcs, - Timeout.Infinite, - executeOnlyOnce: true); - - if (cancellationToken.CanBeCanceled) - { - ctr = cancellationToken.UnsafeRegister( - static state => - { - var (taskSource, token) = ((TaskCompletionSource taskSource, CancellationToken token))state!; - taskSource.TrySetCanceled(token); - }, - (tcs, cancellationToken)); - } - - await tcs.Task.ConfigureAwait(false); - } - finally - { - ctr.Dispose(); - registeredWaitHandle?.Unregister(null); - } + await waitState.WaitForExitAsync(cancellationToken).ConfigureAwait(false); return CreateExitStatus(waitState, canceled: false); } @@ -171,42 +139,19 @@ private async Task WaitForExitAsyncCore(CancellationToken can private async Task WaitForExitOrKillOnCancellationAsyncCore(CancellationToken cancellationToken) { ProcessWaitState waitState = GetWaitState(); - ManualResetEvent exitedEvent = waitState.EnsureExitedEvent(); - - TaskCompletionSource tcs = new(TaskCreationOptions.RunContinuationsAsynchronously); - RegisteredWaitHandle? registeredWaitHandle = null; - CancellationTokenRegistration ctr = default; - StrongBox wasKilledBox = new(false); + bool signalWasSent = false; try { - registeredWaitHandle = ThreadPool.RegisterWaitForSingleObject( - exitedEvent, - static (state, timedOut) => ((TaskCompletionSource)state!).TrySetResult(true), - tcs, - Timeout.Infinite, - executeOnlyOnce: true); - - if (cancellationToken.CanBeCanceled) - { - ctr = cancellationToken.UnsafeRegister( - static state => - { - var (handle, wasCancelled) = ((SafeProcessHandle, StrongBox))state!; - wasCancelled.Value = handle.SignalCore(PosixSignal.SIGKILL); - }, - (this, wasKilledBox)); - } - - await tcs.Task.ConfigureAwait(false); + await waitState.WaitForExitAsync(cancellationToken).ConfigureAwait(false); } - finally + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) { - ctr.Dispose(); - registeredWaitHandle?.Unregister(null); + signalWasSent = SignalCore(PosixSignal.SIGKILL); + await waitState.WaitForExitAsync(CancellationToken.None).ConfigureAwait(false); } - return CreateExitStatus(waitState, canceled: wasKilledBox.Value); + return CreateExitStatus(waitState, canceled: signalWasSent); } private ProcessWaitState GetWaitState() @@ -221,10 +166,12 @@ private ProcessWaitState GetWaitState() private static ProcessExitStatus CreateExitStatus(ProcessWaitState waitState, bool canceled) { - waitState.GetExited(out ProcessExitStatus? exitStatus, refresh: false); - PosixSignal? signal = exitStatus?.Signal; + bool exited = waitState.GetExited(out ProcessExitStatus? exitStatus, refresh: false); + Debug.Assert(exited); + Debug.Assert(exitStatus is not null); + PosixSignal? signal = exitStatus.Signal; - return new ProcessExitStatus(exitStatus?.ExitCode ?? 0, canceled && signal is not null, signal); + return new ProcessExitStatus(exitStatus.ExitCode, canceled && signal is not null, signal); } private delegate SafeProcessHandle StartWithShellExecuteDelegate(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, SafeFileHandle? stdoutHandle, SafeFileHandle? stderrHandle, out ProcessWaitState.Holder? waitStateHolder); 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 246366892a2307..edc638f1b27680 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 @@ -650,15 +650,17 @@ private bool TryWaitForExitCore(int milliseconds, [NotNullWhen(true)] out Proces private ProcessExitStatus WaitForExitOrKillOnTimeoutCore(int milliseconds) { - bool wasKilledOnTimeout = false; + bool signalWasSent = false; using Interop.Kernel32.ProcessWaitHandle processWaitHandle = new(this); if (!processWaitHandle.WaitOne(milliseconds)) { - wasKilledOnTimeout = TerminateProcessCore(); +#pragma warning disable CA1416 // PosixSignal.SIGKILL is supported on Windows via SignalCore + signalWasSent = SignalCore(PosixSignal.SIGKILL); +#pragma warning restore CA1416 processWaitHandle.WaitOne(Timeout.Infinite); } - return new ProcessExitStatus(GetExitCode(), wasKilledOnTimeout); + return new ProcessExitStatus(GetExitCode(), signalWasSent); } private async Task WaitForExitAsyncCore(CancellationToken cancellationToken) @@ -707,7 +709,7 @@ private async Task WaitForExitOrKillOnCancellationAsyncCore(C TaskCompletionSource tcs = new(TaskCreationOptions.RunContinuationsAsynchronously); RegisteredWaitHandle? registeredWaitHandle = null; CancellationTokenRegistration ctr = default; - StrongBox wasKilledBox = new(false); + StrongBox signalWasSentBox = new(false); try { @@ -723,10 +725,12 @@ private async Task WaitForExitOrKillOnCancellationAsyncCore(C ctr = cancellationToken.UnsafeRegister( static state => { - var (handle, wasCancelled) = ((SafeProcessHandle, StrongBox))state!; - wasCancelled.Value = handle.TerminateProcessCore(); + var (handle, signalWasSent) = ((SafeProcessHandle, StrongBox))state!; +#pragma warning disable CA1416 // PosixSignal.SIGKILL is supported on Windows via SignalCore + signalWasSent.Value = handle.SignalCore(PosixSignal.SIGKILL); +#pragma warning restore CA1416 }, - (this, wasKilledBox)); + (this, signalWasSentBox)); } await tcs.Task.ConfigureAwait(false); @@ -737,31 +741,7 @@ private async Task WaitForExitOrKillOnCancellationAsyncCore(C registeredWaitHandle?.Unregister(null); } - return new ProcessExitStatus(GetExitCode(), wasKilledBox.Value); - } - - /// - /// Terminates the process using TerminateProcess. - /// - /// true if the process was terminated; false if it had already exited. - private bool TerminateProcessCore() - { - if (Interop.Kernel32.TerminateProcess(this, -1)) - { - return true; - } - - int error = Marshal.GetLastPInvokeError(); - - // If the process has already exited, TerminateProcess fails with ERROR_ACCESS_DENIED. - if (error == Interop.Errors.ERROR_ACCESS_DENIED && - Interop.Kernel32.GetExitCodeProcess(this, out int exitCode) && - exitCode != Interop.Kernel32.HandleOptions.STILL_ACTIVE) - { - return false; - } - - throw new Win32Exception(error); + return new ProcessExitStatus(GetExitCode(), signalWasSentBox.Value); } private int GetExitCode() 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 e1b270aa863f95..19aef202a66547 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 @@ -217,6 +217,11 @@ public ProcessExitStatus WaitForExitOrKillOnTimeout(TimeSpan timeout) { Validate(); + if (!ProcessUtils.PlatformSupportsProcessStartAndKill) + { + throw new PlatformNotSupportedException(); + } + return WaitForExitOrKillOnTimeoutCore(ProcessUtils.ToTimeoutMilliseconds(timeout)); } @@ -260,6 +265,11 @@ public Task WaitForExitOrKillOnCancellationAsync(Cancellation { Validate(); + if (!ProcessUtils.PlatformSupportsProcessStartAndKill) + { + throw new PlatformNotSupportedException(); + } + return WaitForExitOrKillOnCancellationAsyncCore(cancellationToken); } diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs index 2890f0b421d97d..29c991214cffe1 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs @@ -279,10 +279,10 @@ internal ManualResetEvent EnsureExitedEvent() { // If we haven't exited, we need to spin up an asynchronous operation that // will complete the _exitedEvent when the other process exits. If there's already - // another operation underway, then WaitForExitAsync will just tack ours onto the + // another operation underway, then PollForNonChildExitAsync will just tack ours onto the // end of it; we can't be sure it'll actually monitor the process until it exits, // as it may have been created with a cancelable token. - _waitInProgress = WaitForExitAsync(CancellationToken.None); + _waitInProgress = PollForNonChildExitAsync(CancellationToken.None); } } } @@ -464,7 +464,7 @@ internal bool WaitForExit(int millisecondsTimeout) CancellationToken token = remainingTimeout == Timeout.Infinite ? CancellationToken.None : (cts = new CancellationTokenSource(remainingTimeout)).Token; - _waitInProgress = waitTask = WaitForExitAsync(token); + _waitInProgress = waitTask = PollForNonChildExitAsync(token); } } // lock(_gate) @@ -492,6 +492,46 @@ internal bool WaitForExit(int millisecondsTimeout) } } + /// Asynchronously waits for the process to exit. + /// A token to cancel the wait. + /// A task that completes when the process exits or the token is canceled. + internal async Task WaitForExitAsync(CancellationToken cancellationToken) + { + ManualResetEvent exitEvent = EnsureExitedEvent(); + + TaskCompletionSource tcs = new(TaskCreationOptions.RunContinuationsAsynchronously); + RegisteredWaitHandle? registeredWaitHandle = null; + CancellationTokenRegistration ctr = default; + + try + { + registeredWaitHandle = ThreadPool.RegisterWaitForSingleObject( + exitEvent, + static (state, timedOut) => ((TaskCompletionSource)state!).TrySetResult(), + tcs, + Timeout.Infinite, + executeOnlyOnce: true); + + if (cancellationToken.CanBeCanceled) + { + ctr = cancellationToken.UnsafeRegister( + static state => + { + var (taskSource, token) = ((TaskCompletionSource, CancellationToken))state!; + taskSource.TrySetCanceled(token); + }, + (tcs, cancellationToken)); + } + + await tcs.Task.ConfigureAwait(false); + } + finally + { + ctr.Dispose(); + registeredWaitHandle?.Unregister(null); + } + } + /// Spawns an asynchronous polling loop for process completion. /// A token to monitor to exit the polling loop. /// The task representing the loop. @@ -501,7 +541,7 @@ internal bool WaitForExit(int millisecondsTimeout) /// token, so if the caller is providing a token and a previous task, it should wait on the /// returned task with the token in order to avoid delayed wake-ups. /// - private async Task WaitForExitAsync(CancellationToken cancellationToken) + private async Task PollForNonChildExitAsync(CancellationToken cancellationToken) { Debug.Assert(Monitor.IsEntered(_gate)); Debug.Assert(!_isChild); diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.Windows.cs index 92d2c7bfde2fbe..0d98ab3e0fbb7e 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.Windows.cs @@ -152,10 +152,6 @@ private unsafe int RunWithInvalidHandles(ProcessStartInfo startInfo) { using SafeProcessHandle safeProcessHandle = new(processInfo.hProcess, ownsHandle: true); - // We have started a suspended process, so we can get process by Id before it exits. - // As soon as SafeProcessHandle.WaitForExit* are implemented (#126293), we can use them instead. - using Process process = Process.GetProcessById(processInfo.dwProcessId); - if (Interop.Kernel32.ResumeThread(processInfo.hThread) == 0xFFFFFFFF) { throw new Win32Exception(); @@ -163,17 +159,13 @@ private unsafe int RunWithInvalidHandles(ProcessStartInfo startInfo) try { - process.WaitForExit(WaitInMS); - - // To avoid "Process was not started by this object, so requested information cannot be determined." - // we fetch the exit code directly here. - Assert.True(Interop.Kernel32.GetExitCodeProcess(safeProcessHandle, out int exitCode)); + ProcessExitStatus exitStatus = safeProcessHandle.WaitForExitOrKillOnTimeout(TimeSpan.FromMilliseconds(WaitInMS)); - return exitCode; + return exitStatus.ExitCode; } finally { - process.Kill(); + safeProcessHandle.Kill(); } } finally From df26b6bdba6adea93cf1eed816ff1e3d5b01d404 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 17 Apr 2026 08:17:46 +0200 Subject: [PATCH 05/29] address my own feedback: - there is no need to start suspended process anymore, as we can wait on the handle to exit directly - revert changes I did not like --- .../SafeHandles/SafeProcessHandle.Unix.cs | 69 +++++++++++++++++-- .../Diagnostics/ProcessWaitState.Unix.cs | 48 ++----------- .../tests/ProcessHandlesTests.Windows.cs | 9 +-- .../System.Diagnostics.Process.Tests.csproj | 2 - 4 files changed, 67 insertions(+), 61 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 0d07ec83bf76bf..c640e1113814c5 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 @@ -8,6 +8,7 @@ using System.Diagnostics.CodeAnalysis; using System.IO; using System.IO.Pipes; +using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Runtime.Versioning; using System.Security; @@ -130,8 +131,39 @@ private ProcessExitStatus WaitForExitOrKillOnTimeoutCore(int milliseconds) private async Task WaitForExitAsyncCore(CancellationToken cancellationToken) { ProcessWaitState waitState = GetWaitState(); + ManualResetEvent exitedEvent = waitState.EnsureExitedEvent(); - await waitState.WaitForExitAsync(cancellationToken).ConfigureAwait(false); + TaskCompletionSource tcs = new(TaskCreationOptions.RunContinuationsAsynchronously); + RegisteredWaitHandle? registeredWaitHandle = null; + CancellationTokenRegistration ctr = default; + + try + { + registeredWaitHandle = ThreadPool.RegisterWaitForSingleObject( + exitedEvent, + static (state, timedOut) => ((TaskCompletionSource)state!).TrySetResult(true), + tcs, + Timeout.Infinite, + executeOnlyOnce: true); + + if (cancellationToken.CanBeCanceled) + { + ctr = cancellationToken.UnsafeRegister( + static state => + { + var (taskSource, token) = ((TaskCompletionSource taskSource, CancellationToken token))state!; + taskSource.TrySetCanceled(token); + }, + (tcs, cancellationToken)); + } + + await tcs.Task.ConfigureAwait(false); + } + finally + { + ctr.Dispose(); + registeredWaitHandle?.Unregister(null); + } return CreateExitStatus(waitState, canceled: false); } @@ -139,19 +171,42 @@ private async Task WaitForExitAsyncCore(CancellationToken can private async Task WaitForExitOrKillOnCancellationAsyncCore(CancellationToken cancellationToken) { ProcessWaitState waitState = GetWaitState(); - bool signalWasSent = false; + ManualResetEvent exitedEvent = waitState.EnsureExitedEvent(); + + TaskCompletionSource tcs = new(TaskCreationOptions.RunContinuationsAsynchronously); + RegisteredWaitHandle? registeredWaitHandle = null; + CancellationTokenRegistration ctr = default; + StrongBox signalWasSentBox = new(false); try { - await waitState.WaitForExitAsync(cancellationToken).ConfigureAwait(false); + registeredWaitHandle = ThreadPool.RegisterWaitForSingleObject( + exitedEvent, + static (state, timedOut) => ((TaskCompletionSource)state!).TrySetResult(true), + tcs, + Timeout.Infinite, + executeOnlyOnce: true); + + if (cancellationToken.CanBeCanceled) + { + ctr = cancellationToken.UnsafeRegister( + static state => + { + var (handle, signalWasSent) = ((SafeProcessHandle, StrongBox))state!; + signalWasSent.Value = handle.SignalCore(PosixSignal.SIGKILL); + }, + (this, signalWasSentBox)); + } + + await tcs.Task.ConfigureAwait(false); } - catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + finally { - signalWasSent = SignalCore(PosixSignal.SIGKILL); - await waitState.WaitForExitAsync(CancellationToken.None).ConfigureAwait(false); + ctr.Dispose(); + registeredWaitHandle?.Unregister(null); } - return CreateExitStatus(waitState, canceled: signalWasSent); + return CreateExitStatus(waitState, canceled: signalWasSentBox.Value); } private ProcessWaitState GetWaitState() diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs index 29c991214cffe1..2890f0b421d97d 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs @@ -279,10 +279,10 @@ internal ManualResetEvent EnsureExitedEvent() { // If we haven't exited, we need to spin up an asynchronous operation that // will complete the _exitedEvent when the other process exits. If there's already - // another operation underway, then PollForNonChildExitAsync will just tack ours onto the + // another operation underway, then WaitForExitAsync will just tack ours onto the // end of it; we can't be sure it'll actually monitor the process until it exits, // as it may have been created with a cancelable token. - _waitInProgress = PollForNonChildExitAsync(CancellationToken.None); + _waitInProgress = WaitForExitAsync(CancellationToken.None); } } } @@ -464,7 +464,7 @@ internal bool WaitForExit(int millisecondsTimeout) CancellationToken token = remainingTimeout == Timeout.Infinite ? CancellationToken.None : (cts = new CancellationTokenSource(remainingTimeout)).Token; - _waitInProgress = waitTask = PollForNonChildExitAsync(token); + _waitInProgress = waitTask = WaitForExitAsync(token); } } // lock(_gate) @@ -492,46 +492,6 @@ internal bool WaitForExit(int millisecondsTimeout) } } - /// Asynchronously waits for the process to exit. - /// A token to cancel the wait. - /// A task that completes when the process exits or the token is canceled. - internal async Task WaitForExitAsync(CancellationToken cancellationToken) - { - ManualResetEvent exitEvent = EnsureExitedEvent(); - - TaskCompletionSource tcs = new(TaskCreationOptions.RunContinuationsAsynchronously); - RegisteredWaitHandle? registeredWaitHandle = null; - CancellationTokenRegistration ctr = default; - - try - { - registeredWaitHandle = ThreadPool.RegisterWaitForSingleObject( - exitEvent, - static (state, timedOut) => ((TaskCompletionSource)state!).TrySetResult(), - tcs, - Timeout.Infinite, - executeOnlyOnce: true); - - if (cancellationToken.CanBeCanceled) - { - ctr = cancellationToken.UnsafeRegister( - static state => - { - var (taskSource, token) = ((TaskCompletionSource, CancellationToken))state!; - taskSource.TrySetCanceled(token); - }, - (tcs, cancellationToken)); - } - - await tcs.Task.ConfigureAwait(false); - } - finally - { - ctr.Dispose(); - registeredWaitHandle?.Unregister(null); - } - } - /// Spawns an asynchronous polling loop for process completion. /// A token to monitor to exit the polling loop. /// The task representing the loop. @@ -541,7 +501,7 @@ internal async Task WaitForExitAsync(CancellationToken cancellationToken) /// token, so if the caller is providing a token and a previous task, it should wait on the /// returned task with the token in order to avoid delayed wake-ups. /// - private async Task PollForNonChildExitAsync(CancellationToken cancellationToken) + private async Task WaitForExitAsync(CancellationToken cancellationToken) { Debug.Assert(Monitor.IsEntered(_gate)); Debug.Assert(!_isChild); diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.Windows.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.Windows.cs index 0d98ab3e0fbb7e..c49b71627e0a0e 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.Windows.cs @@ -108,7 +108,6 @@ public void ProcessStartedWithInvalidHandles_CanRedirectOutput(bool restrictHand private unsafe int RunWithInvalidHandles(ProcessStartInfo startInfo) { const nint INVALID_HANDLE_VALUE = -1; - const int CREATE_SUSPENDED = 4; // RemoteExector has provided us with the right path and arguments, // we just need to add the terminating null character. @@ -135,7 +134,7 @@ private unsafe int RunWithInvalidHandles(ProcessStartInfo startInfo) ref unused_SecAttrs, ref unused_SecAttrs, bInheritHandles: false, - CREATE_SUSPENDED | Interop.Kernel32.EXTENDED_STARTUPINFO_PRESENT, + Interop.Kernel32.EXTENDED_STARTUPINFO_PRESENT, null, null, &startupInfoEx, @@ -152,15 +151,9 @@ private unsafe int RunWithInvalidHandles(ProcessStartInfo startInfo) { using SafeProcessHandle safeProcessHandle = new(processInfo.hProcess, ownsHandle: true); - if (Interop.Kernel32.ResumeThread(processInfo.hThread) == 0xFFFFFFFF) - { - throw new Win32Exception(); - } - try { ProcessExitStatus exitStatus = safeProcessHandle.WaitForExitOrKillOnTimeout(TimeSpan.FromMilliseconds(WaitInMS)); - return exitStatus.ExitCode; } finally 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 dd73a7baa7f533..21f314fd25e7d8 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 @@ -72,8 +72,6 @@ Link="Common\Interop\Windows\Kernel32\Interop.SetConsoleCtrlHandler.cs" /> - From 1fd57850740a2f96093934f8d1b95c537a8b9e96 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 17 Apr 2026 09:03:41 +0200 Subject: [PATCH 06/29] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../Win32/SafeHandles/SafeProcessHandle.Unix.cs | 13 ++++++++++--- .../Win32/SafeHandles/SafeProcessHandle.Windows.cs | 13 ++++++++++--- .../src/System/Diagnostics/ProcessWaitState.Unix.cs | 2 +- 3 files changed, 21 insertions(+), 7 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 c640e1113814c5..28c2105e2927af 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 @@ -192,10 +192,17 @@ private async Task WaitForExitOrKillOnCancellationAsyncCore(C ctr = cancellationToken.UnsafeRegister( static state => { - var (handle, signalWasSent) = ((SafeProcessHandle, StrongBox))state!; - signalWasSent.Value = handle.SignalCore(PosixSignal.SIGKILL); + var (handle, signalWasSent, tcs) = ((SafeProcessHandle, StrongBox, TaskCompletionSource))state!; + try + { + signalWasSent.Value = handle.SignalCore(PosixSignal.SIGKILL); + } + catch (Exception ex) + { + tcs.TrySetException(ex); + } }, - (this, signalWasSentBox)); + (this, signalWasSentBox, tcs)); } await tcs.Task.ConfigureAwait(false); 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 edc638f1b27680..2cb6728b64d7d1 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 @@ -725,12 +725,19 @@ private async Task WaitForExitOrKillOnCancellationAsyncCore(C ctr = cancellationToken.UnsafeRegister( static state => { - var (handle, signalWasSent) = ((SafeProcessHandle, StrongBox))state!; + var (handle, signalWasSent, tcs) = ((SafeProcessHandle, StrongBox, TaskCompletionSource))state!; + try + { #pragma warning disable CA1416 // PosixSignal.SIGKILL is supported on Windows via SignalCore - signalWasSent.Value = handle.SignalCore(PosixSignal.SIGKILL); + signalWasSent.Value = handle.SignalCore(PosixSignal.SIGKILL); #pragma warning restore CA1416 + } + catch (Exception ex) + { + tcs.TrySetException(ex); + } }, - (this, signalWasSentBox)); + (this, signalWasSentBox, tcs)); } await tcs.Task.ConfigureAwait(false); diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs index 2890f0b421d97d..f4b665b4bad1d4 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs @@ -337,7 +337,7 @@ internal bool GetExited(out ProcessExitStatus? exitStatus, bool refresh) } // We now have an up-to-date snapshot for whether we've exited, - // and if we have, what the exit code is (if we were able to find out). + // and if we have, what the exit status is (if we were able to find out). exitStatus = _exitStatus; return _exited; } From fedd0fb25d68da398e5bb0c8c9941347774c99f0 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 17 Apr 2026 13:35:04 +0200 Subject: [PATCH 07/29] reduce code duplication --- .../SafeHandles/SafeProcessHandle.Unix.cs | 88 +---------------- .../SafeHandles/SafeProcessHandle.Windows.cs | 88 +---------------- .../Win32/SafeHandles/SafeProcessHandle.cs | 97 ++++++++++++++++++- 3 files changed, 97 insertions(+), 176 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 28c2105e2927af..887404119a73af 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 @@ -128,93 +128,9 @@ private ProcessExitStatus WaitForExitOrKillOnTimeoutCore(int milliseconds) return CreateExitStatus(waitState, canceled: signalWasSent); } - private async Task WaitForExitAsyncCore(CancellationToken cancellationToken) - { - ProcessWaitState waitState = GetWaitState(); - ManualResetEvent exitedEvent = waitState.EnsureExitedEvent(); - - TaskCompletionSource tcs = new(TaskCreationOptions.RunContinuationsAsynchronously); - RegisteredWaitHandle? registeredWaitHandle = null; - CancellationTokenRegistration ctr = default; - - try - { - registeredWaitHandle = ThreadPool.RegisterWaitForSingleObject( - exitedEvent, - static (state, timedOut) => ((TaskCompletionSource)state!).TrySetResult(true), - tcs, - Timeout.Infinite, - executeOnlyOnce: true); - - if (cancellationToken.CanBeCanceled) - { - ctr = cancellationToken.UnsafeRegister( - static state => - { - var (taskSource, token) = ((TaskCompletionSource taskSource, CancellationToken token))state!; - taskSource.TrySetCanceled(token); - }, - (tcs, cancellationToken)); - } - - await tcs.Task.ConfigureAwait(false); - } - finally - { - ctr.Dispose(); - registeredWaitHandle?.Unregister(null); - } - - return CreateExitStatus(waitState, canceled: false); - } + private ManualResetEvent GetWaitHandle() => GetWaitState().EnsureExitedEvent(); - private async Task WaitForExitOrKillOnCancellationAsyncCore(CancellationToken cancellationToken) - { - ProcessWaitState waitState = GetWaitState(); - ManualResetEvent exitedEvent = waitState.EnsureExitedEvent(); - - TaskCompletionSource tcs = new(TaskCreationOptions.RunContinuationsAsynchronously); - RegisteredWaitHandle? registeredWaitHandle = null; - CancellationTokenRegistration ctr = default; - StrongBox signalWasSentBox = new(false); - - try - { - registeredWaitHandle = ThreadPool.RegisterWaitForSingleObject( - exitedEvent, - static (state, timedOut) => ((TaskCompletionSource)state!).TrySetResult(true), - tcs, - Timeout.Infinite, - executeOnlyOnce: true); - - if (cancellationToken.CanBeCanceled) - { - ctr = cancellationToken.UnsafeRegister( - static state => - { - var (handle, signalWasSent, tcs) = ((SafeProcessHandle, StrongBox, TaskCompletionSource))state!; - try - { - signalWasSent.Value = handle.SignalCore(PosixSignal.SIGKILL); - } - catch (Exception ex) - { - tcs.TrySetException(ex); - } - }, - (this, signalWasSentBox, tcs)); - } - - await tcs.Task.ConfigureAwait(false); - } - finally - { - ctr.Dispose(); - registeredWaitHandle?.Unregister(null); - } - - return CreateExitStatus(waitState, canceled: signalWasSentBox.Value); - } + private ProcessExitStatus GetExitStatus(bool canceled = false) => CreateExitStatus(GetWaitState(), canceled); private ProcessWaitState GetWaitState() { 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 2cb6728b64d7d1..9b7b6f441b3e53 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 @@ -663,93 +663,9 @@ private ProcessExitStatus WaitForExitOrKillOnTimeoutCore(int milliseconds) return new ProcessExitStatus(GetExitCode(), signalWasSent); } - private async Task WaitForExitAsyncCore(CancellationToken cancellationToken) - { - using Interop.Kernel32.ProcessWaitHandle processWaitHandle = new(this); - - TaskCompletionSource tcs = new(TaskCreationOptions.RunContinuationsAsynchronously); - RegisteredWaitHandle? registeredWaitHandle = null; - CancellationTokenRegistration ctr = default; - - try - { - registeredWaitHandle = ThreadPool.RegisterWaitForSingleObject( - processWaitHandle, - static (state, timedOut) => ((TaskCompletionSource)state!).TrySetResult(true), - tcs, - Timeout.Infinite, - executeOnlyOnce: true); - - if (cancellationToken.CanBeCanceled) - { - ctr = cancellationToken.UnsafeRegister( - static state => - { - var (taskSource, token) = ((TaskCompletionSource taskSource, CancellationToken token))state!; - taskSource.TrySetCanceled(token); - }, - (tcs, cancellationToken)); - } + private Interop.Kernel32.ProcessWaitHandle GetWaitHandle() => new(this); - await tcs.Task.ConfigureAwait(false); - } - finally - { - ctr.Dispose(); - registeredWaitHandle?.Unregister(null); - } - - return new ProcessExitStatus(GetExitCode(), false); - } - - private async Task WaitForExitOrKillOnCancellationAsyncCore(CancellationToken cancellationToken) - { - using Interop.Kernel32.ProcessWaitHandle processWaitHandle = new(this); - - TaskCompletionSource tcs = new(TaskCreationOptions.RunContinuationsAsynchronously); - RegisteredWaitHandle? registeredWaitHandle = null; - CancellationTokenRegistration ctr = default; - StrongBox signalWasSentBox = new(false); - - try - { - registeredWaitHandle = ThreadPool.RegisterWaitForSingleObject( - processWaitHandle, - static (state, timedOut) => ((TaskCompletionSource)state!).TrySetResult(true), - tcs, - Timeout.Infinite, - executeOnlyOnce: true); - - if (cancellationToken.CanBeCanceled) - { - ctr = cancellationToken.UnsafeRegister( - static state => - { - var (handle, signalWasSent, tcs) = ((SafeProcessHandle, StrongBox, TaskCompletionSource))state!; - try - { -#pragma warning disable CA1416 // PosixSignal.SIGKILL is supported on Windows via SignalCore - signalWasSent.Value = handle.SignalCore(PosixSignal.SIGKILL); -#pragma warning restore CA1416 - } - catch (Exception ex) - { - tcs.TrySetException(ex); - } - }, - (this, signalWasSentBox, tcs)); - } - - await tcs.Task.ConfigureAwait(false); - } - finally - { - ctr.Dispose(); - registeredWaitHandle?.Unregister(null); - } - - return new ProcessExitStatus(GetExitCode(), signalWasSentBox.Value); - } + private ProcessExitStatus GetExitStatus(bool canceled = false) => new(GetExitCode(), canceled); private int GetExitCode() { 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 af6795ef7cc1f6..0d868b026f3162 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 @@ -20,6 +20,7 @@ using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; +using System.Runtime.CompilerServices; namespace Microsoft.Win32.SafeHandles { @@ -246,11 +247,51 @@ public ProcessExitStatus WaitForExitOrKillOnTimeout(TimeSpan timeout) /// The process is NOT killed and continues running. If you want to kill the process on cancellation, /// use instead. /// - public Task WaitForExitAsync(CancellationToken cancellationToken = default) + public async Task WaitForExitAsync(CancellationToken cancellationToken = default) { Validate(); - return WaitForExitAsyncCore(cancellationToken); + TaskCompletionSource tcs = new(TaskCreationOptions.RunContinuationsAsynchronously); + RegisteredWaitHandle? registeredWaitHandle = null; + CancellationTokenRegistration ctr = default; + + var exitedEvent = GetWaitHandle(); + + try + { + registeredWaitHandle = ThreadPool.RegisterWaitForSingleObject( + exitedEvent, + static (state, timedOut) => ((TaskCompletionSource)state!).TrySetResult(true), + tcs, + Timeout.Infinite, + executeOnlyOnce: true); + + if (cancellationToken.CanBeCanceled) + { + ctr = cancellationToken.UnsafeRegister( + static state => + { + var (taskSource, token) = ((TaskCompletionSource taskSource, CancellationToken token))state!; + taskSource.TrySetCanceled(token); + }, + (tcs, cancellationToken)); + } + + await tcs.Task.ConfigureAwait(false); + } + finally + { + ctr.Dispose(); + registeredWaitHandle?.Unregister(null); + + // On Unix, we don't own the ManualResetEvent. + if (OperatingSystem.IsWindows()) + { + exitedEvent.Dispose(); + } + } + + return GetExitStatus(); } /// @@ -270,7 +311,7 @@ public Task WaitForExitAsync(CancellationToken cancellationTo [UnsupportedOSPlatform("ios")] [UnsupportedOSPlatform("tvos")] [SupportedOSPlatform("maccatalyst")] - public Task WaitForExitOrKillOnCancellationAsync(CancellationToken cancellationToken) + public async Task WaitForExitOrKillOnCancellationAsync(CancellationToken cancellationToken) { Validate(); @@ -279,7 +320,55 @@ public Task WaitForExitOrKillOnCancellationAsync(Cancellation throw new PlatformNotSupportedException(); } - return WaitForExitOrKillOnCancellationAsyncCore(cancellationToken); + TaskCompletionSource tcs = new(TaskCreationOptions.RunContinuationsAsynchronously); + RegisteredWaitHandle? registeredWaitHandle = null; + CancellationTokenRegistration ctr = default; + StrongBox signalWasSentBox = new(false); + + var exitedEvent = GetWaitHandle(); + + try + { + registeredWaitHandle = ThreadPool.RegisterWaitForSingleObject( + exitedEvent, + static (state, timedOut) => ((TaskCompletionSource)state!).TrySetResult(true), + tcs, + Timeout.Infinite, + executeOnlyOnce: true); + + if (cancellationToken.CanBeCanceled) + { + ctr = cancellationToken.UnsafeRegister( + static state => + { + var (handle, signalWasSent, tcs) = ((SafeProcessHandle, StrongBox, TaskCompletionSource))state!; + try + { + signalWasSent.Value = handle.SignalCore(PosixSignal.SIGKILL); + } + catch (Exception ex) + { + tcs.TrySetException(ex); + } + }, + (this, signalWasSentBox, tcs)); + } + + await tcs.Task.ConfigureAwait(false); + } + finally + { + ctr.Dispose(); + registeredWaitHandle?.Unregister(null); + + // On Unix, we don't own the ManualResetEvent. + if (OperatingSystem.IsWindows()) + { + exitedEvent.Dispose(); + } + } + + return GetExitStatus(canceled: signalWasSentBox.Value); } private void Validate() From ef41f822e43a22a998e2c0f7977676f6749c4c50 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 17 Apr 2026 12:22:07 +0000 Subject: [PATCH 08/29] Fix _waitState lazy initialization and add signal/Process.SafeHandle tests - Fix GetWaitState() on Unix to lazily initialize _waitState from ProcessWaitState table so that SafeProcessHandle from Process.SafeHandle works for WaitForExit* - Add _lazyWaitStateHolder field to properly manage ref count lifecycle - Convert signal test from Fact to Theory with SIGKILL and SIGTERM - Add Process.SafeHandle WaitForExit* tests to ProcessWaitingTests Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/52916352-c074-4483-8dca-72cf3696462e Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../SafeHandles/SafeProcessHandle.Unix.cs | 40 ++++++++++++- .../tests/ProcessWaitingTests.cs | 59 +++++++++++++++++++ .../tests/SafeProcessHandleTests.cs | 10 ++-- 3 files changed, 103 insertions(+), 6 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 887404119a73af..1a9df8b32bec6a 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 @@ -31,6 +31,7 @@ public sealed partial class SafeProcessHandle : SafeHandleZeroOrMinusOneIsInvali private readonly SafeWaitHandle? _handle; private readonly bool _releaseRef; private ProcessWaitState? _waitState; + private ProcessWaitState.Holder? _lazyWaitStateHolder; private SafeProcessHandle(int processId, ProcessWaitState.Holder waitStateHolder) : base(ownsHandle: true) { @@ -57,6 +58,7 @@ protected override bool ReleaseHandle() Debug.Assert(_handle != null); _handle.DangerousRelease(); } + _lazyWaitStateHolder?.Dispose(); return true; } @@ -134,12 +136,46 @@ private ProcessExitStatus WaitForExitOrKillOnTimeoutCore(int milliseconds) private ProcessWaitState GetWaitState() { - if (_waitState is null) + ProcessWaitState? waitState = _waitState; + if (waitState is not null) + { + return waitState; + } + + return EnsureWaitStateInitialized(); + } + + private ProcessWaitState EnsureWaitStateInitialized() + { + // Double-check: another thread may have initialized it. + ProcessWaitState? waitState = _waitState; + if (waitState is not null) + { + return waitState; + } + + int processId = ProcessId; + if (processId == 0) { throw new InvalidOperationException(SR.InvalidProcessHandle); } - return _waitState; + ProcessWaitState.Holder holder = new ProcessWaitState.Holder(processId, isNewChild: false, usesTerminal: false); + waitState = holder._state; + + if (Interlocked.CompareExchange(ref _waitState, waitState, null) is null) + { + // We won the race — store the holder so its ref count is released in ReleaseHandle. + _lazyWaitStateHolder = holder; + } + else + { + // Another thread initialized first — release our holder. + holder.Dispose(); + waitState = _waitState; + } + + return waitState; } private static ProcessExitStatus CreateExitStatus(ProcessWaitState waitState, bool canceled) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessWaitingTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessWaitingTests.cs index 22db2706974389..82cf5f94509876 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessWaitingTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessWaitingTests.cs @@ -7,6 +7,7 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.DotNet.RemoteExecutor; +using Microsoft.Win32.SafeHandles; using Xunit; namespace System.Diagnostics.Tests @@ -671,5 +672,63 @@ public async Task WaitForExitAsync_NotDirected_ThrowsInvalidOperationException() var process = new Process(); await Assert.ThrowsAsync(() => process.WaitForExitAsync()); } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void ProcessSafeHandle_WaitForExit_ReturnsExitCode() + { + Process process = CreateProcess(static () => RemoteExecutor.SuccessExitCode); + process.Start(); + + ProcessExitStatus exitStatus = process.SafeHandle.WaitForExit(); + + Assert.Equal(RemoteExecutor.SuccessExitCode, exitStatus.ExitCode); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void ProcessSafeHandle_TryWaitForExit_ReturnsExitCode() + { + Process process = CreateProcess(static () => RemoteExecutor.SuccessExitCode); + process.Start(); + + bool exited = process.SafeHandle.TryWaitForExit(TimeSpan.FromMilliseconds(WaitInMS), out ProcessExitStatus? exitStatus); + + Assert.True(exited); + Assert.NotNull(exitStatus); + Assert.Equal(RemoteExecutor.SuccessExitCode, exitStatus.ExitCode); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public async Task ProcessSafeHandle_WaitForExitAsync_ReturnsExitCode() + { + Process process = CreateProcess(static () => RemoteExecutor.SuccessExitCode); + process.Start(); + + ProcessExitStatus exitStatus = await process.SafeHandle.WaitForExitAsync(); + + Assert.Equal(RemoteExecutor.SuccessExitCode, exitStatus.ExitCode); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void ProcessSafeHandle_WaitForExitOrKillOnTimeout_ReturnsExitCode() + { + Process process = CreateProcessLong(); + process.Start(); + + ProcessExitStatus exitStatus = process.SafeHandle.WaitForExitOrKillOnTimeout(TimeSpan.FromMilliseconds(0)); + + Assert.True(exitStatus.Canceled); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public async Task ProcessSafeHandle_WaitForExitOrKillOnCancellationAsync_ReturnsExitCode() + { + Process process = CreateProcessLong(); + process.Start(); + + using CancellationTokenSource cts = new CancellationTokenSource(0); + ProcessExitStatus exitStatus = await process.SafeHandle.WaitForExitOrKillOnCancellationAsync(cts.Token); + + Assert.True(exitStatus.Canceled); + } } } diff --git a/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs b/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs index fd636f5daf49c8..45be5c39f35cce 100644 --- a/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs @@ -486,9 +486,11 @@ public void WaitForExit_CalledAfterKill_ReturnsImmediately() Assert.NotEqual(RemoteExecutor.SuccessExitCode, exitStatus.ExitCode); } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] [SkipOnPlatform(TestPlatforms.Windows, "Signal property is Unix-specific")] - public void WaitForExit_ProcessKilledBySignal_ReportsSignal() + [InlineData(PosixSignal.SIGKILL)] + [InlineData(PosixSignal.SIGTERM)] + public void WaitForExit_ProcessKilledBySignal_ReportsSignal(PosixSignal signal) { Process process = CreateProcess(static () => { @@ -497,12 +499,12 @@ public void WaitForExit_ProcessKilledBySignal_ReportsSignal() }); using SafeProcessHandle processHandle = SafeProcessHandle.Start(process.StartInfo); - processHandle.Kill(); + processHandle.Signal(signal); ProcessExitStatus exitStatus = processHandle.WaitForExit(); Assert.NotNull(exitStatus.Signal); - Assert.Equal(PosixSignal.SIGKILL, exitStatus.Signal); + Assert.Equal(signal, exitStatus.Signal); } } } From d1c541c650a1abaecf5b223a7f29a2c9db9c170b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 17 Apr 2026 12:33:17 +0000 Subject: [PATCH 09/29] Address code review: fix race condition in EnsureWaitStateInitialized, rename test methods Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/52916352-c074-4483-8dca-72cf3696462e Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Win32/SafeHandles/SafeProcessHandle.Unix.cs | 14 ++++++-------- .../tests/ProcessWaitingTests.cs | 4 ++-- 2 files changed, 8 insertions(+), 10 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 1a9df8b32bec6a..1842ff9f9eea31 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 @@ -163,19 +163,17 @@ private ProcessWaitState EnsureWaitStateInitialized() ProcessWaitState.Holder holder = new ProcessWaitState.Holder(processId, isNewChild: false, usesTerminal: false); waitState = holder._state; - if (Interlocked.CompareExchange(ref _waitState, waitState, null) is null) + ProcessWaitState? existing = Interlocked.CompareExchange(ref _waitState, waitState, null); + if (existing is null) { // We won the race — store the holder so its ref count is released in ReleaseHandle. _lazyWaitStateHolder = holder; - } - else - { - // Another thread initialized first — release our holder. - holder.Dispose(); - waitState = _waitState; + return waitState; } - return waitState; + // Another thread initialized first — release our holder and use theirs. + holder.Dispose(); + return existing; } private static ProcessExitStatus CreateExitStatus(ProcessWaitState waitState, bool canceled) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessWaitingTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessWaitingTests.cs index 82cf5f94509876..a0a8ba46fb95c8 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessWaitingTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessWaitingTests.cs @@ -709,7 +709,7 @@ public async Task ProcessSafeHandle_WaitForExitAsync_ReturnsExitCode() } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void ProcessSafeHandle_WaitForExitOrKillOnTimeout_ReturnsExitCode() + public void ProcessSafeHandle_WaitForExitOrKillOnTimeout_KillsOnTimeout() { Process process = CreateProcessLong(); process.Start(); @@ -720,7 +720,7 @@ public void ProcessSafeHandle_WaitForExitOrKillOnTimeout_ReturnsExitCode() } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public async Task ProcessSafeHandle_WaitForExitOrKillOnCancellationAsync_ReturnsExitCode() + public async Task ProcessSafeHandle_WaitForExitOrKillOnCancellationAsync_KillsOnCancellation() { Process process = CreateProcessLong(); process.Start(); From 77e51435ea2909c2c2af7a105196eb77d82c7ac6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 18 Apr 2026 22:09:35 +0000 Subject: [PATCH 10/29] Merge duplicate tests into theories with bool useAsync parameter - Use WaitInMS timeout at line 55 per reviewer suggestion - Merge two WaitForExit_ProcessExitsNormally tests into one - Merge sync/async WaitForExit into theory with bool useAsync - Merge sync/async WaitForExitOrKill normal-exit into theory - Merge sync/async WaitForExitOrKill timeout/cancel into theory Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/3a1553e0-3850-4588-b95f-5452296a0275 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../tests/SafeProcessHandleTests.cs | 101 +++++------------- 1 file changed, 26 insertions(+), 75 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs b/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs index 45be5c39f35cce..5d0c593aa366be 100644 --- a/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs @@ -52,7 +52,7 @@ public void CanStartProcess() streamWriter.WriteLine("pong"); - ProcessExitStatus exitStatus = processHandle.WaitForExitOrKillOnTimeout(TimeSpan.FromSeconds(30)); + ProcessExitStatus exitStatus = processHandle.WaitForExitOrKillOnTimeout(TimeSpan.FromMilliseconds(WaitInMS)); Assert.Equal(RemoteExecutor.SuccessExitCode, exitStatus.ExitCode); Assert.False(exitStatus.Canceled); } @@ -270,27 +270,19 @@ public void Kill_HandleWithoutTerminatePermission_ThrowsWin32Exception() } } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void WaitForExit_ProcessExitsNormally_ReturnsExitCode() - { - Process process = CreateProcess(static () => 42); - - using SafeProcessHandle processHandle = SafeProcessHandle.Start(process.StartInfo); - - ProcessExitStatus exitStatus = processHandle.WaitForExit(); - - Assert.Equal(42, exitStatus.ExitCode); - Assert.False(exitStatus.Canceled); - } - - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void WaitForExit_ProcessExitsWithZero_ReturnsZeroExitCode() + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(true)] + [InlineData(false)] + public async Task WaitForExit_ProcessExitsNormally_ReturnsExitCode(bool useAsync) { Process process = CreateProcess(static () => RemoteExecutor.SuccessExitCode); using SafeProcessHandle processHandle = SafeProcessHandle.Start(process.StartInfo); + using CancellationTokenSource cts = new(TimeSpan.FromSeconds(30)); - ProcessExitStatus exitStatus = processHandle.WaitForExit(); + ProcessExitStatus exitStatus = useAsync + ? await processHandle.WaitForExitAsync(cts.Token) + : processHandle.WaitForExit(); Assert.Equal(RemoteExecutor.SuccessExitCode, exitStatus.ExitCode); Assert.False(exitStatus.Canceled); @@ -341,22 +333,29 @@ public void TryWaitForExit_ProcessDoesNotExitBeforeTimeout_ReturnsFalse() } } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void WaitForExitOrKillOnTimeout_ProcessExitsBeforeTimeout_DoesNotKill() + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(true)] + [InlineData(false)] + public async Task WaitForExitOrKill_ProcessExitsBeforeTimeout_DoesNotKill(bool useAsync) { Process process = CreateProcess(static () => RemoteExecutor.SuccessExitCode); using SafeProcessHandle processHandle = SafeProcessHandle.Start(process.StartInfo); + using CancellationTokenSource cts = new(TimeSpan.FromSeconds(30)); - ProcessExitStatus exitStatus = processHandle.WaitForExitOrKillOnTimeout(TimeSpan.FromSeconds(30)); + ProcessExitStatus exitStatus = useAsync + ? await processHandle.WaitForExitOrKillOnCancellationAsync(cts.Token) + : processHandle.WaitForExitOrKillOnTimeout(TimeSpan.FromSeconds(30)); Assert.Equal(RemoteExecutor.SuccessExitCode, exitStatus.ExitCode); Assert.False(exitStatus.Canceled); Assert.Null(exitStatus.Signal); } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void WaitForExitOrKillOnTimeout_ProcessDoesNotExitBeforeTimeout_KillsAndReturns() + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(true)] + [InlineData(false)] + public async Task WaitForExitOrKill_ProcessDoesNotExit_KillsAndReturns(bool useAsync) { Process process = CreateProcess(static () => { @@ -367,7 +366,11 @@ public void WaitForExitOrKillOnTimeout_ProcessDoesNotExitBeforeTimeout_KillsAndR using SafeProcessHandle processHandle = SafeProcessHandle.Start(process.StartInfo); Stopwatch stopwatch = Stopwatch.StartNew(); - ProcessExitStatus exitStatus = processHandle.WaitForExitOrKillOnTimeout(TimeSpan.FromMilliseconds(300)); + using CancellationTokenSource cts = new(TimeSpan.FromMilliseconds(300)); + + ProcessExitStatus exitStatus = useAsync + ? await processHandle.WaitForExitOrKillOnCancellationAsync(cts.Token) + : processHandle.WaitForExitOrKillOnTimeout(TimeSpan.FromMilliseconds(300)); stopwatch.Stop(); Assert.InRange(stopwatch.Elapsed, TimeSpan.FromMilliseconds(200), TimeSpan.FromSeconds(10)); @@ -375,21 +378,6 @@ public void WaitForExitOrKillOnTimeout_ProcessDoesNotExitBeforeTimeout_KillsAndR Assert.NotEqual(0, exitStatus.ExitCode); } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public async Task WaitForExitAsync_ProcessExitsNormally_ReturnsExitCode() - { - Process process = CreateProcess(static () => RemoteExecutor.SuccessExitCode); - - using SafeProcessHandle processHandle = SafeProcessHandle.Start(process.StartInfo); - - using CancellationTokenSource cts = new(TimeSpan.FromSeconds(30)); - ProcessExitStatus exitStatus = await processHandle.WaitForExitAsync(cts.Token); - - Assert.Equal(RemoteExecutor.SuccessExitCode, exitStatus.ExitCode); - Assert.False(exitStatus.Canceled); - Assert.Null(exitStatus.Signal); - } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] public async Task WaitForExitAsync_WithoutCancellationToken_CompletesNormally() { @@ -429,43 +417,6 @@ await Assert.ThrowsAnyAsync( } } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public async Task WaitForExitOrKillOnCancellationAsync_ProcessExitsNormally_ReturnsExitCode() - { - Process process = CreateProcess(static () => RemoteExecutor.SuccessExitCode); - - using SafeProcessHandle processHandle = SafeProcessHandle.Start(process.StartInfo); - - using CancellationTokenSource cts = new(TimeSpan.FromSeconds(30)); - ProcessExitStatus exitStatus = await processHandle.WaitForExitOrKillOnCancellationAsync(cts.Token); - - Assert.Equal(RemoteExecutor.SuccessExitCode, exitStatus.ExitCode); - Assert.False(exitStatus.Canceled); - Assert.Null(exitStatus.Signal); - } - - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public async Task WaitForExitOrKillOnCancellationAsync_CancellationRequested_KillsAndReturns() - { - Process process = CreateProcess(static () => - { - Thread.Sleep(Timeout.Infinite); - return RemoteExecutor.SuccessExitCode; - }); - - using SafeProcessHandle processHandle = SafeProcessHandle.Start(process.StartInfo); - - Stopwatch stopwatch = Stopwatch.StartNew(); - using CancellationTokenSource cts = new(TimeSpan.FromMilliseconds(300)); - - ProcessExitStatus exitStatus = await processHandle.WaitForExitOrKillOnCancellationAsync(cts.Token); - stopwatch.Stop(); - - Assert.InRange(stopwatch.Elapsed, TimeSpan.FromMilliseconds(200), TimeSpan.FromSeconds(10)); - Assert.True(exitStatus.Canceled); - Assert.NotEqual(0, exitStatus.ExitCode); - } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] public void WaitForExit_CalledAfterKill_ReturnsImmediately() { From 77a6493bc43a25ab61ac25254c0b655122b960a4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 19 Apr 2026 07:38:49 +0000 Subject: [PATCH 11/29] Simplify EnsureWaitStateInitialized to use ProcessWaitState.Holder(processId) matching Process pattern Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/3f4f9d58-e237-49c2-9220-e41731a20fde Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 1842ff9f9eea31..15286cf2b91af9 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 @@ -160,7 +160,7 @@ private ProcessWaitState EnsureWaitStateInitialized() throw new InvalidOperationException(SR.InvalidProcessHandle); } - ProcessWaitState.Holder holder = new ProcessWaitState.Holder(processId, isNewChild: false, usesTerminal: false); + ProcessWaitState.Holder holder = new ProcessWaitState.Holder(processId); waitState = holder._state; ProcessWaitState? existing = Interlocked.CompareExchange(ref _waitState, waitState, null); From d09130b5ec5008e7177d32870fafc9d3cf6887bf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 20 Apr 2026 06:45:42 +0000 Subject: [PATCH 12/29] Simplify wait state holder ownership: make _waitState readonly, remove lazy init All SafeProcessHandle instances are now created with a ProcessWaitState: - Added Holder(Holder) copy constructor to ProcessWaitState.Holder - Added IncrementRefCount() to ProcessWaitState for ref counting - SafeProcessHandle now owns its own Holder copy (disposed in ReleaseHandle) - Made _waitState readonly, removed _lazyWaitStateHolder and EnsureWaitStateInitialized - Updated GetProcessHandle() in Process.Unix.cs to pass Holder when creating SafeProcessHandle Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/a866727c-2478-4cdc-a3ec-6ce0e20f929e Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../SafeHandles/SafeProcessHandle.Unix.cs | 52 ++++--------------- .../src/System/Diagnostics/Process.Unix.cs | 5 +- .../Diagnostics/ProcessWaitState.Unix.cs | 17 ++++++ 3 files changed, 32 insertions(+), 42 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 15286cf2b91af9..674ce5e2ccad2d 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 @@ -30,23 +30,26 @@ public sealed partial class SafeProcessHandle : SafeHandleZeroOrMinusOneIsInvali private readonly SafeWaitHandle? _handle; private readonly bool _releaseRef; - private ProcessWaitState? _waitState; - private ProcessWaitState.Holder? _lazyWaitStateHolder; + private readonly ProcessWaitState? _waitState; + private readonly ProcessWaitState.Holder? _waitStateHolder; private SafeProcessHandle(int processId, ProcessWaitState.Holder waitStateHolder) : base(ownsHandle: true) { ProcessId = processId; - _waitState = waitStateHolder._state; + _waitStateHolder = new ProcessWaitState.Holder(waitStateHolder); + _waitState = _waitStateHolder._state; _handle = _waitState.EnsureExitedEvent().GetSafeWaitHandle(); _handle.DangerousAddRef(ref _releaseRef); SetHandle(_handle.DangerousGetHandle()); } - internal SafeProcessHandle(int processId, SafeWaitHandle handle) : + internal SafeProcessHandle(int processId, ProcessWaitState.Holder waitStateHolder, SafeWaitHandle handle) : this(handle.DangerousGetHandle(), ownsHandle: true) { ProcessId = processId; + _waitStateHolder = new ProcessWaitState.Holder(waitStateHolder); + _waitState = _waitStateHolder._state; _handle = handle; handle.DangerousAddRef(ref _releaseRef); } @@ -58,7 +61,7 @@ protected override bool ReleaseHandle() Debug.Assert(_handle != null); _handle.DangerousRelease(); } - _lazyWaitStateHolder?.Dispose(); + _waitStateHolder?.Dispose(); return true; } @@ -136,44 +139,12 @@ private ProcessExitStatus WaitForExitOrKillOnTimeoutCore(int milliseconds) private ProcessWaitState GetWaitState() { - ProcessWaitState? waitState = _waitState; - if (waitState is not null) - { - return waitState; - } - - return EnsureWaitStateInitialized(); - } - - private ProcessWaitState EnsureWaitStateInitialized() - { - // Double-check: another thread may have initialized it. - ProcessWaitState? waitState = _waitState; - if (waitState is not null) - { - return waitState; - } - - int processId = ProcessId; - if (processId == 0) + if (_waitState is null) { throw new InvalidOperationException(SR.InvalidProcessHandle); } - ProcessWaitState.Holder holder = new ProcessWaitState.Holder(processId); - waitState = holder._state; - - ProcessWaitState? existing = Interlocked.CompareExchange(ref _waitState, waitState, null); - if (existing is null) - { - // We won the race — store the holder so its ref count is released in ReleaseHandle. - _lazyWaitStateHolder = holder; - return waitState; - } - - // Another thread initialized first — release our holder and use theirs. - holder.Dispose(); - return existing; + return _waitState; } private static ProcessExitStatus CreateExitStatus(ProcessWaitState waitState, bool canceled) @@ -193,8 +164,7 @@ private static SafeProcessHandle StartCore(ProcessStartInfo startInfo, SafeFileH { SafeProcessHandle startedProcess = StartCore(startInfo, stdinHandle, stdoutHandle, stderrHandle, inheritedHandlesSnapshot, out ProcessWaitState.Holder? waitStateHolder); - // For standalone SafeProcessHandle.Start, we dispose the wait state holder immediately. - // The DangerousAddRef on the SafeWaitHandle (Unix) keeps the handle alive. + // The SafeProcessHandle constructor created its own Holder copy, so we can dispose the original. waitStateHolder?.Dispose(); return startedProcess; diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index b385cd6df6f7d1..2625b732a62ca3 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -352,7 +352,10 @@ private SafeProcessHandle GetProcessHandle() } EnsureState(State.HaveNonExitedId | State.IsLocal); - return new SafeProcessHandle(_processId, GetSafeWaitHandle()); + // GetWaitState() ensures _waitStateHolder is initialized. + // The SafeProcessHandle constructor creates its own Holder copy (incrementing the ref count). + GetWaitState(); + return new SafeProcessHandle(_processId, _waitStateHolder!, GetSafeWaitHandle()); } private bool StartCore(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, SafeFileHandle? stdoutHandle, SafeFileHandle? stderrHandle, SafeHandle[]? inheritedHandles) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs index f4b665b4bad1d4..1773af40357893 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs @@ -59,6 +59,13 @@ internal Holder(int processId, bool isNewChild = false, bool usesTerminal = fals _state = ProcessWaitState.AddRef(processId, isNewChild, usesTerminal); } + /// Creates an additional holder for the same wait state, incrementing the ref count. + internal Holder(Holder existing) + { + _state = existing._state; + _state.IncrementRefCount(); + } + ~Holder() { // Don't try to Dispose resources (like ManualResetEvents) if @@ -154,6 +161,16 @@ internal static ProcessWaitState AddRef(int processId, bool isNewChild, bool use } } + /// Increments the ref count for this wait state object. + internal void IncrementRefCount() + { + Dictionary waitStates = _isChild ? s_childProcessWaitStates : s_processWaitStates; + lock (waitStates) + { + _outstandingRefCount++; + } + } + /// /// Decrements the ref count on the wait state object, and if it's the last one, /// removes it from the table. From d4815a09d17be846e45fac78910ee988596a751d Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 20 Apr 2026 13:13:34 +0200 Subject: [PATCH 13/29] address code review feedback and fix the implementation --- .../SafeHandles/SafeProcessHandle.Unix.cs | 37 +++---------------- .../src/System/Diagnostics/Process.Unix.cs | 6 +-- .../Diagnostics/ProcessWaitState.Unix.cs | 14 +++---- 3 files changed, 13 insertions(+), 44 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 674ce5e2ccad2d..1defa2dd4d8fe9 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 @@ -30,30 +30,18 @@ public sealed partial class SafeProcessHandle : SafeHandleZeroOrMinusOneIsInvali private readonly SafeWaitHandle? _handle; private readonly bool _releaseRef; - private readonly ProcessWaitState? _waitState; private readonly ProcessWaitState.Holder? _waitStateHolder; - private SafeProcessHandle(int processId, ProcessWaitState.Holder waitStateHolder) : base(ownsHandle: true) + internal SafeProcessHandle(int processId, ProcessWaitState.Holder waitStateHolder) : base(ownsHandle: true) { ProcessId = processId; - _waitStateHolder = new ProcessWaitState.Holder(waitStateHolder); - _waitState = _waitStateHolder._state; + _waitStateHolder = waitStateHolder; - _handle = _waitState.EnsureExitedEvent().GetSafeWaitHandle(); + _handle = _waitStateHolder._state.EnsureExitedEvent().GetSafeWaitHandle(); _handle.DangerousAddRef(ref _releaseRef); SetHandle(_handle.DangerousGetHandle()); } - internal SafeProcessHandle(int processId, ProcessWaitState.Holder waitStateHolder, SafeWaitHandle handle) : - this(handle.DangerousGetHandle(), ownsHandle: true) - { - ProcessId = processId; - _waitStateHolder = new ProcessWaitState.Holder(waitStateHolder); - _waitState = _waitStateHolder._state; - _handle = handle; - handle.DangerousAddRef(ref _releaseRef); - } - protected override bool ReleaseHandle() { if (_releaseRef) @@ -137,15 +125,7 @@ private ProcessExitStatus WaitForExitOrKillOnTimeoutCore(int milliseconds) private ProcessExitStatus GetExitStatus(bool canceled = false) => CreateExitStatus(GetWaitState(), canceled); - private ProcessWaitState GetWaitState() - { - if (_waitState is null) - { - throw new InvalidOperationException(SR.InvalidProcessHandle); - } - - return _waitState; - } + private ProcessWaitState GetWaitState() => _waitStateHolder is null ? throw new InvalidOperationException(SR.InvalidProcessHandle) : _waitStateHolder._state; private static ProcessExitStatus CreateExitStatus(ProcessWaitState waitState, bool canceled) { @@ -161,14 +141,7 @@ private static ProcessExitStatus CreateExitStatus(ProcessWaitState waitState, bo private static StartWithShellExecuteDelegate? s_startWithShellExecute; private static SafeProcessHandle StartCore(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, SafeFileHandle? stdoutHandle, SafeFileHandle? stderrHandle, SafeHandle[]? inheritedHandlesSnapshot = null) - { - SafeProcessHandle startedProcess = StartCore(startInfo, stdinHandle, stdoutHandle, stderrHandle, inheritedHandlesSnapshot, out ProcessWaitState.Holder? waitStateHolder); - - // The SafeProcessHandle constructor created its own Holder copy, so we can dispose the original. - waitStateHolder?.Dispose(); - - return startedProcess; - } + => StartCore(startInfo, stdinHandle, stdoutHandle, stderrHandle, inheritedHandlesSnapshot, out _); internal static SafeProcessHandle StartCore(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, SafeFileHandle? stdoutHandle, SafeFileHandle? stderrHandle, SafeHandle[]? inheritedHandles, out ProcessWaitState.Holder? waitStateHolder) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index 2625b732a62ca3..c18d3790b7aa5d 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -355,7 +355,7 @@ private SafeProcessHandle GetProcessHandle() // GetWaitState() ensures _waitStateHolder is initialized. // The SafeProcessHandle constructor creates its own Holder copy (incrementing the ref count). GetWaitState(); - return new SafeProcessHandle(_processId, _waitStateHolder!, GetSafeWaitHandle()); + return new SafeProcessHandle(_processId, _waitStateHolder!.IncrementRefCount()); } private bool StartCore(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, SafeFileHandle? stdoutHandle, SafeFileHandle? stderrHandle, SafeHandle[]? inheritedHandles) @@ -363,13 +363,13 @@ private bool StartCore(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, SafeProcessHandle startedProcess = SafeProcessHandle.StartCore(startInfo, stdinHandle, stdoutHandle, stderrHandle, inheritedHandles, out ProcessWaitState.Holder? waitStateHolder); Debug.Assert(!startedProcess.IsInvalid); - _waitStateHolder = waitStateHolder; + // SafeProcessHandle has its own copy of the wait state holder, so we need to increment the ref count for our copy. + _waitStateHolder = waitStateHolder!.IncrementRefCount(); SetProcessHandle(startedProcess); SetProcessId(startedProcess.ProcessId); return true; } - /// Finalizable holder for the underlying shared wait state object. private ProcessWaitState.Holder? _waitStateHolder; diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs index 1773af40357893..b67f54eb2fe08a 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs @@ -59,21 +59,18 @@ internal Holder(int processId, bool isNewChild = false, bool usesTerminal = fals _state = ProcessWaitState.AddRef(processId, isNewChild, usesTerminal); } + private Holder(ProcessWaitState source) => _state = source; + /// Creates an additional holder for the same wait state, incrementing the ref count. - internal Holder(Holder existing) + internal Holder IncrementRefCount() { - _state = existing._state; _state.IncrementRefCount(); + return new(_state); } ~Holder() { - // Don't try to Dispose resources (like ManualResetEvents) if - // the process is shutting down. - if (_state != null && !Environment.HasShutdownStarted) - { - _state.ReleaseRef(); - } + _state?.ReleaseRef(); } public void Dispose() @@ -182,7 +179,6 @@ internal void ReleaseRef() lock (waitStates) { bool foundState = waitStates.TryGetValue(_processId, out pws); - Debug.Assert(foundState); if (foundState) { --_outstandingRefCount; From 7add4f074a7ec50d3fdddba5fb91ac9886cdbf77 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 20 Apr 2026 18:20:40 +0200 Subject: [PATCH 14/29] address code review feedback: don't use an extra field to store PID on Unix --- .../SafeHandles/SafeProcessHandle.Unix.cs | 28 ++++++++++++++----- .../SafeHandles/SafeProcessHandle.Windows.cs | 21 ++++++++++++-- .../Win32/SafeHandles/SafeProcessHandle.cs | 19 ------------- .../src/System/Diagnostics/Process.Unix.cs | 2 +- .../Diagnostics/ProcessWaitState.Unix.cs | 14 +++++----- 5 files changed, 48 insertions(+), 36 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 1defa2dd4d8fe9..8ee381dd9dc3e8 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 @@ -32,16 +32,33 @@ public sealed partial class SafeProcessHandle : SafeHandleZeroOrMinusOneIsInvali private readonly bool _releaseRef; private readonly ProcessWaitState.Holder? _waitStateHolder; - internal SafeProcessHandle(int processId, ProcessWaitState.Holder waitStateHolder) : base(ownsHandle: true) + internal SafeProcessHandle(ProcessWaitState.Holder waitStateHolder) : base(ownsHandle: true) { - ProcessId = processId; _waitStateHolder = waitStateHolder; - _handle = _waitStateHolder._state.EnsureExitedEvent().GetSafeWaitHandle(); _handle.DangerousAddRef(ref _releaseRef); SetHandle(_handle.DangerousGetHandle()); } + /// + /// Gets the process ID. + /// + public int ProcessId + { + get + { + Validate(); + + if (_waitStateHolder is null) + { + // On Unix, we don't use process descriptors yet, so we can't get PID. + throw new PlatformNotSupportedException(); + } + + return _waitStateHolder._state.ProcessId; + } + } + protected override bool ReleaseHandle() { if (_releaseRef) @@ -53,9 +70,6 @@ protected override bool ReleaseHandle() return true; } - // On Unix, we don't use process descriptors yet, so we can't get PID. - private static int GetProcessIdCore() => throw new PlatformNotSupportedException(); - private bool SignalCore(PosixSignal signal) { if (!ProcessUtils.PlatformSupportsProcessStartAndKill) @@ -332,7 +346,7 @@ private static SafeProcessHandle ForkAndExecProcess( throw ProcessUtils.CreateExceptionForErrorStartingProcess(new Interop.ErrorInfo(errno).GetErrorMessage(), errno, resolvedFilename, cwd); } - return new SafeProcessHandle(childPid, waitStateHolder!); + return new SafeProcessHandle(waitStateHolder!); } } } 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 9b7b6f441b3e53..1466c4fe5aef26 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 @@ -24,6 +24,25 @@ public sealed partial class SafeProcessHandle : SafeHandleZeroOrMinusOneIsInvali // by the OS, which terminates all child processes in the job. private static readonly Lazy s_killOnParentExitJob = new(CreateKillOnParentExitJob); + /// + /// Gets the process ID. + /// + public int ProcessId + { + get + { + Validate(); + + if (field == -1) + { + field = Interop.Kernel32.GetProcessId(this); + } + + return field; + } + private set; + } = -1; + protected override bool ReleaseHandle() { return Interop.Kernel32.CloseHandle(handle); @@ -625,8 +644,6 @@ private static void AssignJobAndResumeThread(IntPtr hThread, SafeProcessHandle p } } - private int GetProcessIdCore() => Interop.Kernel32.GetProcessId(this); - private ProcessExitStatus WaitForExitCore() { using Interop.Kernel32.ProcessWaitHandle processWaitHandle = new(this); 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 0d868b026f3162..2373108e15e352 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 @@ -33,25 +33,6 @@ public sealed partial class SafeProcessHandle : SafeHandleZeroOrMinusOneIsInvali internal static void EnsureShellExecuteFunc() => s_startWithShellExecute ??= StartWithShellExecute; - /// - /// Gets the process ID. - /// - public int ProcessId - { - get - { - Validate(); - - if (field == -1) - { - field = GetProcessIdCore(); - } - - return field; - } - private set; - } = -1; - /// /// Creates a . /// diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index c18d3790b7aa5d..5a872620094743 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -355,7 +355,7 @@ private SafeProcessHandle GetProcessHandle() // GetWaitState() ensures _waitStateHolder is initialized. // The SafeProcessHandle constructor creates its own Holder copy (incrementing the ref count). GetWaitState(); - return new SafeProcessHandle(_processId, _waitStateHolder!.IncrementRefCount()); + return new SafeProcessHandle(_waitStateHolder!.IncrementRefCount()); } private bool StartCore(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, SafeFileHandle? stdoutHandle, SafeFileHandle? stderrHandle, SafeHandle[]? inheritedHandles) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs index b67f54eb2fe08a..31b26fb4ca7d4f 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs @@ -178,7 +178,7 @@ internal void ReleaseRef() Dictionary waitStates = _isChild ? s_childProcessWaitStates : s_processWaitStates; lock (waitStates) { - bool foundState = waitStates.TryGetValue(_processId, out pws); + bool foundState = waitStates.TryGetValue(ProcessId, out pws); if (foundState) { --_outstandingRefCount; @@ -187,7 +187,7 @@ internal void ReleaseRef() // The dictionary may contain a different ProcessWaitState if the pid was recycled. if (pws == this) { - waitStates.Remove(_processId); + waitStates.Remove(ProcessId); } pws = this; } @@ -207,7 +207,7 @@ internal void ReleaseRef() /// private readonly object _gate = new object(); /// ID of the associated process. - private readonly int _processId; + internal readonly int ProcessId; /// Associated process is a child process. private readonly bool _isChild; /// Associated process is a child that can use the terminal. @@ -239,7 +239,7 @@ internal void ReleaseRef() private ProcessWaitState(int processId, bool isChild, bool usesTerminal, DateTime exitTime = default) { Debug.Assert(processId >= 0); - _processId = processId; + ProcessId = processId; _isChild = isChild; _usesTerminal = usesTerminal; _exitTime = exitTime; @@ -364,7 +364,7 @@ private void CheckForNonChildExit() bool exited; // We won't be able to get an exit code, but we'll at least be able to determine if the process is // still running. - int killResult = Interop.Sys.Kill(_processId, 0); // 0 means don't send a signal, used to check if process is still alive + int killResult = Interop.Sys.Kill(ProcessId, 0); // 0 means don't send a signal, used to check if process is still alive if (killResult == 0) { // Process is still running. This could also be a defunct process that has completed @@ -583,9 +583,9 @@ private bool TryReapChild(bool configureConsole) // Try to get the state of the child process int exitCode; int terminatingSignal; - int waitResult = Interop.Sys.WaitPidExitedNoHang(_processId, out exitCode, out terminatingSignal); + int waitResult = Interop.Sys.WaitPidExitedNoHang(ProcessId, out exitCode, out terminatingSignal); - if (waitResult == _processId) + if (waitResult == ProcessId) { ChildReaped(exitCode, terminatingSignal, configureConsole); return true; From c0cd053856379726b80576c81e89cd6cb2937f77 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 20 Apr 2026 18:29:36 +0200 Subject: [PATCH 15/29] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/native/libs/System.Native/pal_process.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/native/libs/System.Native/pal_process.c b/src/native/libs/System.Native/pal_process.c index 349b02ec7ae665..bc2a713849c1b7 100644 --- a/src/native/libs/System.Native/pal_process.c +++ b/src/native/libs/System.Native/pal_process.c @@ -935,6 +935,8 @@ int32_t SystemNative_WaitPidExitedNoHang(int32_t pid, int32_t* exitCode, int32_t assert(exitCode != NULL); assert(terminatingSignal != NULL); + *exitCode = 0; + *terminatingSignal = 0; int32_t result; int status; while (CheckInterrupted(result = waitpid(pid, &status, WNOHANG))); From 5cf7e5832b0cd45987da0ba9aaa9e6da49609a5b Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 20 Apr 2026 20:54:18 +0200 Subject: [PATCH 16/29] address code review feedback: don't use an extra field to store PID on Unix --- .../SafeHandles/SafeProcessHandle.Unix.cs | 16 +++++- .../Win32/SafeHandles/SafeProcessHandle.cs | 15 +++++ .../src/Resources/Strings.resx | 3 + .../Diagnostics/ProcessWaitState.Unix.cs | 16 +++--- .../tests/SafeProcessHandleTests.cs | 55 +++++++++++++++++++ 5 files changed, 96 insertions(+), 9 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 8ee381dd9dc3e8..0e31297c6e66e8 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 @@ -139,10 +139,24 @@ private ProcessExitStatus WaitForExitOrKillOnTimeoutCore(int milliseconds) private ProcessExitStatus GetExitStatus(bool canceled = false) => CreateExitStatus(GetWaitState(), canceled); - private ProcessWaitState GetWaitState() => _waitStateHolder is null ? throw new InvalidOperationException(SR.InvalidProcessHandle) : _waitStateHolder._state; + private ProcessWaitState GetWaitState() + { + if (_waitStateHolder is null) + { + throw new InvalidOperationException(SR.InvalidProcessHandle); + } + + if (!_waitStateHolder._state.IsChild) + { + throw new NotSupportedException(SR.NotSupportedForNonChildProcess); + } + + return _waitStateHolder._state; + } private static ProcessExitStatus CreateExitStatus(ProcessWaitState waitState, bool canceled) { + // GetWaitState ensures the process is not a child process, so obtaining the exit status should never fail. bool exited = waitState.GetExited(out ProcessExitStatus? exitStatus, refresh: false); Debug.Assert(exited); Debug.Assert(exitStatus is not 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 2373108e15e352..29345f6ff1db79 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 @@ -167,6 +167,9 @@ public bool Signal(PosixSignal signal) /// Waits indefinitely for the process to exit. /// /// The exit status of the process. + /// + /// On Unix it's impossible to obtain exit status of a non-child process. + /// /// The handle is invalid. public ProcessExitStatus WaitForExit() { @@ -181,6 +184,9 @@ public ProcessExitStatus WaitForExit() /// The maximum time to wait for the process to exit. /// When this method returns , contains the exit status of the process. /// if the process exited before the timeout; otherwise, . + /// + /// On Unix it's impossible to obtain exit status of a non-child process. + /// /// The handle is invalid. /// is negative and not equal to , /// or is greater than milliseconds. @@ -198,6 +204,9 @@ public bool TryWaitForExit(TimeSpan timeout, [NotNullWhen(true)] out ProcessExit /// The maximum time to wait for the process to exit before killing it. /// The exit status of the process. If the process was killed due to timeout, /// will be . + /// + /// On Unix it's impossible to obtain exit status of a non-child process. + /// /// The handle is invalid. /// is negative and not equal to , /// or is greater than milliseconds. @@ -224,9 +233,12 @@ public ProcessExitStatus WaitForExitOrKillOnTimeout(TimeSpan timeout) /// The handle is invalid. /// The cancellation token was canceled. /// + /// /// When the cancellation token is canceled, this method stops waiting and throws . /// The process is NOT killed and continues running. If you want to kill the process on cancellation, /// use instead. + /// + /// On Unix it's impossible to obtain exit status of a non-child process. /// public async Task WaitForExitAsync(CancellationToken cancellationToken = default) { @@ -284,10 +296,13 @@ public async Task WaitForExitAsync(CancellationToken cancella /// If the process was killed due to cancellation, will be . /// The handle is invalid. /// + /// /// When the cancellation token is canceled, this method kills the process and waits for it to exit. /// The returned exit status will have the property set to if the process was killed. /// If the cancellation token cannot be canceled (e.g., ), this method behaves identically /// to and will wait indefinitely for the process to exit. + /// + /// On Unix it's impossible to obtain exit status of a non-child process. /// [UnsupportedOSPlatform("ios")] [UnsupportedOSPlatform("tvos")] diff --git a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx index 8e504d19fcf803..f71222c8313bae 100644 --- a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx +++ b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx @@ -366,4 +366,7 @@ UseShellExecute is not supported by StartAndForget. On Windows, shell execution may not create a new process, which would make it impossible to return a valid process ID. + + On Unix it's impossible to obtain exit status of a non-child process. + diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs index 31b26fb4ca7d4f..2e4583f85e384b 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessWaitState.Unix.cs @@ -161,7 +161,7 @@ internal static ProcessWaitState AddRef(int processId, bool isNewChild, bool use /// Increments the ref count for this wait state object. internal void IncrementRefCount() { - Dictionary waitStates = _isChild ? s_childProcessWaitStates : s_processWaitStates; + Dictionary waitStates = IsChild ? s_childProcessWaitStates : s_processWaitStates; lock (waitStates) { _outstandingRefCount++; @@ -175,7 +175,7 @@ internal void IncrementRefCount() internal void ReleaseRef() { ProcessWaitState? pws; - Dictionary waitStates = _isChild ? s_childProcessWaitStates : s_processWaitStates; + Dictionary waitStates = IsChild ? s_childProcessWaitStates : s_processWaitStates; lock (waitStates) { bool foundState = waitStates.TryGetValue(ProcessId, out pws); @@ -209,7 +209,7 @@ internal void ReleaseRef() /// ID of the associated process. internal readonly int ProcessId; /// Associated process is a child process. - private readonly bool _isChild; + internal readonly bool IsChild; /// Associated process is a child that can use the terminal. private readonly bool _usesTerminal; @@ -240,7 +240,7 @@ private ProcessWaitState(int processId, bool isChild, bool usesTerminal, DateTim { Debug.Assert(processId >= 0); ProcessId = processId; - _isChild = isChild; + this.IsChild = isChild; _usesTerminal = usesTerminal; _exitTime = exitTime; } @@ -288,7 +288,7 @@ internal ManualResetEvent EnsureExitedEvent() _exitedEvent = new ManualResetEvent(initialState: _exited); if (!_exited) { - if (!_isChild) + if (!IsChild) { // If we haven't exited, we need to spin up an asynchronous operation that // will complete the _exitedEvent when the other process exits. If there's already @@ -359,7 +359,7 @@ internal bool GetExited(out ProcessExitStatus? exitStatus, bool refresh) private void CheckForNonChildExit() { Debug.Assert(Monitor.IsEntered(_gate)); - if (!_isChild) + if (!IsChild) { bool exited; // We won't be able to get an exit code, but we'll at least be able to determine if the process is @@ -409,7 +409,7 @@ internal bool WaitForExit(int millisecondsTimeout) { Debug.Assert(!Monitor.IsEntered(_gate)); - if (_isChild) + if (IsChild) { lock (_gate) { @@ -517,7 +517,7 @@ internal bool WaitForExit(int millisecondsTimeout) private async Task WaitForExitAsync(CancellationToken cancellationToken) { Debug.Assert(Monitor.IsEntered(_gate)); - Debug.Assert(!_isChild); + Debug.Assert(!IsChild); // Wait for the previous waiting task to complete. We need to ensure that this call completes asynchronously, // in order to escape the caller's lock and avoid blocking the caller by any work in the below loop, so diff --git a/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs b/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs index 5d0c593aa366be..c8f1d5b8254091 100644 --- a/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs @@ -457,5 +457,60 @@ public void WaitForExit_ProcessKilledBySignal_ReportsSignal(PosixSignal signal) Assert.NotNull(exitStatus.Signal); Assert.Equal(signal, exitStatus.Signal); } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public async Task WaitForExit_NonChildProcess_NotSupportedOnUnix() + { + RemoteInvokeOptions remoteInvokeOptions = new() { CheckExitCode = false }; + remoteInvokeOptions.StartInfo.RedirectStandardOutput = true; + remoteInvokeOptions.StartInfo.RedirectStandardInput = true; + + using RemoteInvokeHandle childHandle = RemoteExecutor.Invoke( + () => + { + using Process grandChild = CreateProcessLong(); + grandChild.Start(); + + Console.WriteLine(grandChild.Id); + + // Keep it alive to avoid any kind of re-parenting on Unix + _ = Console.ReadLine(); + + return RemoteExecutor.SuccessExitCode; + }, remoteInvokeOptions); + + int grandChildPid = int.Parse(childHandle.Process.StandardOutput.ReadLine()); + + // Obtain a Process instance before the child exits to avoid PID reuse issues. + using Process grandchild = Process.GetProcessById(grandChildPid); + + try + { + await Verify(grandchild.SafeHandle); + } + finally + { + grandchild.Kill(); + childHandle.Process.Kill(); + } + + static async Task Verify(SafeProcessHandle handle) + { + if (OperatingSystem.IsWindows()) + { + Assert.False(handle.TryWaitForExit(TimeSpan.Zero, out _)); + + handle.Kill(); + ProcessExitStatus processExitStatus = await handle.WaitForExitAsync(); + Assert.Equal(-1, processExitStatus.ExitCode); + } + else + { + Assert.Throws(() => handle.WaitForExit()); + Assert.Throws(() => handle.TryWaitForExit(TimeSpan.FromSeconds(1), out _)); + await Assert.ThrowsAsync(async () => await handle.WaitForExitAsync()); + } + } + } } } From ee3eeec9b222fb2979c1ca1bc6bb931608b796f7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 20 Apr 2026 20:44:53 +0000 Subject: [PATCH 17/29] Change NotSupportedException to PlatformNotSupportedException for non-child process on Unix Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/fb9ffb85-16b8-436b-b588-0f5527b85606 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs | 2 +- .../tests/SafeProcessHandleTests.cs | 6 +++--- 2 files changed, 4 insertions(+), 4 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 0e31297c6e66e8..f0b1f2ed3485c0 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 @@ -148,7 +148,7 @@ private ProcessWaitState GetWaitState() if (!_waitStateHolder._state.IsChild) { - throw new NotSupportedException(SR.NotSupportedForNonChildProcess); + throw new PlatformNotSupportedException(SR.NotSupportedForNonChildProcess); } return _waitStateHolder._state; diff --git a/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs b/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs index c8f1d5b8254091..a9dacea578c48f 100644 --- a/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/SafeProcessHandleTests.cs @@ -506,9 +506,9 @@ static async Task Verify(SafeProcessHandle handle) } else { - Assert.Throws(() => handle.WaitForExit()); - Assert.Throws(() => handle.TryWaitForExit(TimeSpan.FromSeconds(1), out _)); - await Assert.ThrowsAsync(async () => await handle.WaitForExitAsync()); + Assert.Throws(() => handle.WaitForExit()); + Assert.Throws(() => handle.TryWaitForExit(TimeSpan.FromSeconds(1), out _)); + await Assert.ThrowsAsync(async () => await handle.WaitForExitAsync()); } } } From 890797262299bcde4102c1a4e11b5b045ae3d05c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 20 Apr 2026 21:09:21 +0000 Subject: [PATCH 18/29] Fix resource string grammar, update XML docs with PNSE, fix comments Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/233caaed-255c-439e-88f8-2add4a7073d5 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Win32/SafeHandles/SafeProcessHandle.Unix.cs | 2 +- .../Win32/SafeHandles/SafeProcessHandle.cs | 15 ++++++++++----- .../src/Resources/Strings.resx | 2 +- .../src/System/Diagnostics/Process.Unix.cs | 2 +- 4 files changed, 13 insertions(+), 8 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 f0b1f2ed3485c0..1190103005d8a0 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 @@ -156,7 +156,7 @@ private ProcessWaitState GetWaitState() private static ProcessExitStatus CreateExitStatus(ProcessWaitState waitState, bool canceled) { - // GetWaitState ensures the process is not a child process, so obtaining the exit status should never fail. + // GetWaitState ensures the process is a child process, so obtaining the exit status should never fail. bool exited = waitState.GetExited(out ProcessExitStatus? exitStatus, refresh: false); Debug.Assert(exited); Debug.Assert(exitStatus is not 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 29345f6ff1db79..52cc4a558aeb31 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 @@ -168,9 +168,10 @@ public bool Signal(PosixSignal signal) /// /// The exit status of the process. /// - /// On Unix it's impossible to obtain exit status of a non-child process. + /// On Unix, it's impossible to obtain the exit status of a non-child process. /// /// The handle is invalid. + /// On Unix, the process is not a child process. public ProcessExitStatus WaitForExit() { Validate(); @@ -185,9 +186,10 @@ public ProcessExitStatus WaitForExit() /// When this method returns , contains the exit status of the process. /// if the process exited before the timeout; otherwise, . /// - /// On Unix it's impossible to obtain exit status of a non-child process. + /// On Unix, it's impossible to obtain the exit status of a non-child process. /// /// The handle is invalid. + /// On Unix, the process is not a child process. /// is negative and not equal to , /// or is greater than milliseconds. public bool TryWaitForExit(TimeSpan timeout, [NotNullWhen(true)] out ProcessExitStatus? exitStatus) @@ -205,9 +207,10 @@ public bool TryWaitForExit(TimeSpan timeout, [NotNullWhen(true)] out ProcessExit /// The exit status of the process. If the process was killed due to timeout, /// will be . /// - /// On Unix it's impossible to obtain exit status of a non-child process. + /// On Unix, it's impossible to obtain the exit status of a non-child process. /// /// The handle is invalid. + /// On Unix, the process is not a child process. /// is negative and not equal to , /// or is greater than milliseconds. [UnsupportedOSPlatform("ios")] @@ -238,8 +241,9 @@ public ProcessExitStatus WaitForExitOrKillOnTimeout(TimeSpan timeout) /// The process is NOT killed and continues running. If you want to kill the process on cancellation, /// use instead. /// - /// On Unix it's impossible to obtain exit status of a non-child process. + /// On Unix, it's impossible to obtain the exit status of a non-child process. /// + /// On Unix, the process is not a child process. public async Task WaitForExitAsync(CancellationToken cancellationToken = default) { Validate(); @@ -302,8 +306,9 @@ public async Task WaitForExitAsync(CancellationToken cancella /// If the cancellation token cannot be canceled (e.g., ), this method behaves identically /// to and will wait indefinitely for the process to exit. /// - /// On Unix it's impossible to obtain exit status of a non-child process. + /// On Unix, it's impossible to obtain the exit status of a non-child process. /// + /// On Unix, the process is not a child process. [UnsupportedOSPlatform("ios")] [UnsupportedOSPlatform("tvos")] [SupportedOSPlatform("maccatalyst")] diff --git a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx index f71222c8313bae..da63478da67736 100644 --- a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx +++ b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx @@ -367,6 +367,6 @@ UseShellExecute is not supported by StartAndForget. On Windows, shell execution may not create a new process, which would make it impossible to return a valid process ID. - On Unix it's impossible to obtain exit status of a non-child process. + On Unix, it's impossible to obtain the exit status of a non-child process. diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index 5a872620094743..bd7a9df16cb809 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -353,7 +353,7 @@ private SafeProcessHandle GetProcessHandle() EnsureState(State.HaveNonExitedId | State.IsLocal); // GetWaitState() ensures _waitStateHolder is initialized. - // The SafeProcessHandle constructor creates its own Holder copy (incrementing the ref count). + // IncrementRefCount() creates the Holder reference that is passed to SafeProcessHandle. GetWaitState(); return new SafeProcessHandle(_waitStateHolder!.IncrementRefCount()); } From 2d458e87cbdf321ea46f0281b21905d7dc61107e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 20 Apr 2026 21:43:02 +0000 Subject: [PATCH 19/29] Implement Process.Run, RunAsync, RunAndCaptureText, RunAndCaptureTextAsync APIs Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/6d89c552-8d80-40db-bf17-c2f9effc55ca Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../ref/System.Diagnostics.Process.cs | 40 +++ .../src/Resources/Strings.resx | 4 +- .../src/System.Diagnostics.Process.csproj | 1 + .../System/Diagnostics/Process.Scenarios.cs | 291 +++++++++++++++- .../System/Diagnostics/ProcessTextOutput.cs | 53 +++ .../tests/RunTests.cs | 310 ++++++++++++++++++ .../System.Diagnostics.Process.Tests.csproj | 1 + 7 files changed, 694 insertions(+), 6 deletions(-) create mode 100644 src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessTextOutput.cs create mode 100644 src/libraries/System.Diagnostics.Process/tests/RunTests.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 9116d196197614..0907c2dc7b69f1 100644 --- a/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs +++ b/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs @@ -176,6 +176,38 @@ protected void OnExited() { } public void Refresh() { } [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] + [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] + public static System.Diagnostics.ProcessExitStatus Run(System.Diagnostics.ProcessStartInfo startInfo, System.TimeSpan? timeout = default(System.TimeSpan?)) { throw null; } + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] + [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] + public static System.Diagnostics.ProcessExitStatus Run(string fileName, System.Collections.Generic.IList? arguments = null, System.TimeSpan? timeout = default(System.TimeSpan?)) { throw null; } + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] + [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] + public static System.Diagnostics.ProcessTextOutput RunAndCaptureText(System.Diagnostics.ProcessStartInfo startInfo, System.TimeSpan? timeout = default(System.TimeSpan?)) { throw null; } + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] + [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] + public static System.Diagnostics.ProcessTextOutput RunAndCaptureText(string fileName, System.Collections.Generic.IList? arguments = null, System.TimeSpan? timeout = default(System.TimeSpan?)) { throw null; } + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] + [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] + public static System.Threading.Tasks.Task RunAndCaptureTextAsync(System.Diagnostics.ProcessStartInfo startInfo, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] + [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] + public static System.Threading.Tasks.Task RunAndCaptureTextAsync(string fileName, System.Collections.Generic.IList? arguments = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] + [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] + public static System.Threading.Tasks.Task RunAsync(System.Diagnostics.ProcessStartInfo startInfo, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] + [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] + public static System.Threading.Tasks.Task RunAsync(string fileName, System.Collections.Generic.IList? arguments = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { 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 public bool Start() { throw null; } [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] @@ -320,6 +352,14 @@ public ProcessStartInfo(string fileName, System.Collections.Generic.IEnumerable< [System.Diagnostics.CodeAnalysis.AllowNullAttribute] public string WorkingDirectory { get { throw null; } set { } } } + public sealed partial class ProcessTextOutput + { + public ProcessTextOutput(System.Diagnostics.ProcessExitStatus exitStatus, string standardOutput, string standardError, int processId) { throw null; } + public System.Diagnostics.ProcessExitStatus ExitStatus { get { throw null; } } + public int ProcessId { get { throw null; } } + public string StandardError { get { throw null; } } + public string StandardOutput { get { throw null; } } + } [System.ComponentModel.DesignerAttribute("System.Diagnostics.Design.ProcessThreadDesigner, System.Design, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a")] public partial class ProcessThread : System.ComponentModel.Component { diff --git a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx index da63478da67736..c6f8b61d1ab230 100644 --- a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx +++ b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx @@ -363,8 +363,8 @@ InheritedHandles must not contain duplicates. - - UseShellExecute is not supported by StartAndForget. On Windows, shell execution may not create a new process, which would make it impossible to return a valid process ID. + + UseShellExecute is not supported by {0}. On Windows, shell execution may not create a new process, which would make it impossible to return a valid process ID. On Unix, it's impossible to obtain the exit status of a non-child process. diff --git a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj index 9e820236325805..bc1e3b89bb10de 100644 --- a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -24,6 +24,7 @@ + diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Scenarios.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Scenarios.cs index 9df4a1f51e50dc..15d2fa3cc227fa 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Scenarios.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Scenarios.cs @@ -3,6 +3,8 @@ using System.Collections.Generic; using System.Runtime.Versioning; +using System.Threading; +using System.Threading.Tasks; using Microsoft.Win32.SafeHandles; namespace System.Diagnostics @@ -46,10 +48,7 @@ public static int StartAndForget(ProcessStartInfo startInfo) { ArgumentNullException.ThrowIfNull(startInfo); - if (startInfo.UseShellExecute) - { - throw new InvalidOperationException(SR.StartAndForget_UseShellExecuteNotSupported); - } + ThrowIfUseShellExecute(startInfo, nameof(StartAndForget)); using SafeProcessHandle processHandle = SafeProcessHandle.Start(startInfo, fallbackToNull: true); return processHandle.ProcessId; @@ -95,5 +94,289 @@ public static int StartAndForget(string fileName, IList? arguments = nul return StartAndForget(startInfo); } + + /// + /// Starts the process described by , waits for it to exit, and returns its exit status. + /// + /// The that contains the information used to start the process. + /// + /// The maximum amount of time to wait for the process to exit. + /// When , waits indefinitely. + /// If the process does not exit within the specified timeout, it is killed. + /// + /// The exit status of the process. + /// is . + /// + /// is set to . + /// + [UnsupportedOSPlatform("ios")] + [UnsupportedOSPlatform("tvos")] + [SupportedOSPlatform("maccatalyst")] + public static ProcessExitStatus Run(ProcessStartInfo startInfo, TimeSpan? timeout = default) + { + ArgumentNullException.ThrowIfNull(startInfo); + + ThrowIfUseShellExecute(startInfo, nameof(Run)); + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(startInfo); + + return timeout.HasValue + ? processHandle.WaitForExitOrKillOnTimeout(timeout.Value) + : processHandle.WaitForExit(); + } + + /// + /// Starts a process with the specified file name and optional arguments, waits for it to exit, and returns its exit status. + /// + /// The name of the application or document to start. + /// + /// The command-line arguments to pass to the process. Pass or an empty list + /// to start the process without additional arguments. + /// + /// + /// The maximum amount of time to wait for the process to exit. + /// When , waits indefinitely. + /// If the process does not exit within the specified timeout, it is killed. + /// + /// The exit status of the process. + /// is . + /// is empty. + [UnsupportedOSPlatform("ios")] + [UnsupportedOSPlatform("tvos")] + [SupportedOSPlatform("maccatalyst")] + public static ProcessExitStatus Run(string fileName, IList? arguments = null, TimeSpan? timeout = default) + { + ProcessStartInfo startInfo = CreateStartInfo(fileName, arguments); + + return Run(startInfo, timeout); + } + + /// + /// Asynchronously starts the process described by , waits for it to exit, and returns its exit status. + /// + /// The that contains the information used to start the process. + /// + /// A token to cancel the asynchronous operation. + /// If the token is canceled, the process is killed. + /// + /// A task that represents the asynchronous operation. The value of the task contains the exit status of the process. + /// is . + /// + /// is set to . + /// + [UnsupportedOSPlatform("ios")] + [UnsupportedOSPlatform("tvos")] + [SupportedOSPlatform("maccatalyst")] + public static Task RunAsync(ProcessStartInfo startInfo, CancellationToken cancellationToken = default) + { + ArgumentNullException.ThrowIfNull(startInfo); + + ThrowIfUseShellExecute(startInfo, nameof(RunAsync)); + + SafeProcessHandle processHandle = SafeProcessHandle.Start(startInfo); + + return RunAsyncCore(processHandle, cancellationToken); + } + + /// + /// Asynchronously starts a process with the specified file name and optional arguments, waits for it to exit, and returns its exit status. + /// + /// The name of the application or document to start. + /// + /// The command-line arguments to pass to the process. Pass or an empty list + /// to start the process without additional arguments. + /// + /// + /// A token to cancel the asynchronous operation. + /// If the token is canceled, the process is killed. + /// + /// A task that represents the asynchronous operation. The value of the task contains the exit status of the process. + /// is . + /// is empty. + [UnsupportedOSPlatform("ios")] + [UnsupportedOSPlatform("tvos")] + [SupportedOSPlatform("maccatalyst")] + public static Task RunAsync(string fileName, IList? arguments = null, CancellationToken cancellationToken = default) + { + ProcessStartInfo startInfo = CreateStartInfo(fileName, arguments); + + return RunAsync(startInfo, cancellationToken); + } + + /// + /// Starts the process described by , captures its standard output and error, + /// waits for it to exit, and returns the captured text and exit status. + /// + /// The that contains the information used to start the process. + /// + /// The maximum amount of time to wait for the process to exit. + /// When , waits indefinitely. + /// If the process does not exit within the specified timeout, it is killed. + /// + /// The captured text output and exit status of the process. + /// is . + /// + /// is set to . + /// + [UnsupportedOSPlatform("ios")] + [UnsupportedOSPlatform("tvos")] + [SupportedOSPlatform("maccatalyst")] + public static ProcessTextOutput RunAndCaptureText(ProcessStartInfo startInfo, TimeSpan? timeout = default) + { + ArgumentNullException.ThrowIfNull(startInfo); + + ThrowIfUseShellExecute(startInfo, nameof(RunAndCaptureText)); + + startInfo.RedirectStandardOutput = true; + startInfo.RedirectStandardError = true; + + using Process process = Start(startInfo)!; + + (string standardOutput, string standardError) = process.ReadAllText(timeout); + + ProcessExitStatus exitStatus = timeout.HasValue + ? process.SafeHandle.WaitForExitOrKillOnTimeout(timeout.Value) + : process.SafeHandle.WaitForExit(); + + return new ProcessTextOutput(exitStatus, standardOutput, standardError, process.Id); + } + + /// + /// Starts a process with the specified file name and optional arguments, captures its standard output and error, + /// waits for it to exit, and returns the captured text and exit status. + /// + /// The name of the application or document to start. + /// + /// The command-line arguments to pass to the process. Pass or an empty list + /// to start the process without additional arguments. + /// + /// + /// The maximum amount of time to wait for the process to exit. + /// When , waits indefinitely. + /// If the process does not exit within the specified timeout, it is killed. + /// + /// The captured text output and exit status of the process. + /// is . + /// is empty. + [UnsupportedOSPlatform("ios")] + [UnsupportedOSPlatform("tvos")] + [SupportedOSPlatform("maccatalyst")] + public static ProcessTextOutput RunAndCaptureText(string fileName, IList? arguments = null, TimeSpan? timeout = default) + { + ProcessStartInfo startInfo = CreateStartInfo(fileName, arguments); + + return RunAndCaptureText(startInfo, timeout); + } + + /// + /// Asynchronously starts the process described by , captures its standard output and error, + /// waits for it to exit, and returns the captured text and exit status. + /// + /// The that contains the information used to start the process. + /// + /// A token to cancel the asynchronous operation. + /// If the token is canceled, the process is killed. + /// + /// A task that represents the asynchronous operation. The value of the task contains the captured text output and exit status of the process. + /// is . + /// + /// is set to . + /// + [UnsupportedOSPlatform("ios")] + [UnsupportedOSPlatform("tvos")] + [SupportedOSPlatform("maccatalyst")] + public static Task RunAndCaptureTextAsync(ProcessStartInfo startInfo, CancellationToken cancellationToken = default) + { + ArgumentNullException.ThrowIfNull(startInfo); + + ThrowIfUseShellExecute(startInfo, nameof(RunAndCaptureTextAsync)); + + startInfo.RedirectStandardOutput = true; + startInfo.RedirectStandardError = true; + + Process process = Start(startInfo)!; + + return RunAndCaptureTextAsyncCore(process, cancellationToken); + } + + /// + /// Asynchronously starts a process with the specified file name and optional arguments, captures its standard output and error, + /// waits for it to exit, and returns the captured text and exit status. + /// + /// The name of the application or document to start. + /// + /// The command-line arguments to pass to the process. Pass or an empty list + /// to start the process without additional arguments. + /// + /// + /// A token to cancel the asynchronous operation. + /// If the token is canceled, the process is killed. + /// + /// A task that represents the asynchronous operation. The value of the task contains the captured text output and exit status of the process. + /// is . + /// is empty. + [UnsupportedOSPlatform("ios")] + [UnsupportedOSPlatform("tvos")] + [SupportedOSPlatform("maccatalyst")] + public static Task RunAndCaptureTextAsync(string fileName, IList? arguments = null, CancellationToken cancellationToken = default) + { + ProcessStartInfo startInfo = CreateStartInfo(fileName, arguments); + + return RunAndCaptureTextAsync(startInfo, cancellationToken); + } + + [UnsupportedOSPlatform("ios")] + [UnsupportedOSPlatform("tvos")] + [SupportedOSPlatform("maccatalyst")] + private static async Task RunAsyncCore(SafeProcessHandle processHandle, CancellationToken cancellationToken) + { + using (processHandle) + { + return cancellationToken.CanBeCanceled + ? await processHandle.WaitForExitOrKillOnCancellationAsync(cancellationToken).ConfigureAwait(false) + : await processHandle.WaitForExitAsync(cancellationToken).ConfigureAwait(false); + } + } + + [UnsupportedOSPlatform("ios")] + [UnsupportedOSPlatform("tvos")] + [SupportedOSPlatform("maccatalyst")] + private static async Task RunAndCaptureTextAsyncCore(Process process, CancellationToken cancellationToken) + { + using (process) + { + (string standardOutput, string standardError) = await process.ReadAllTextAsync(cancellationToken).ConfigureAwait(false); + + ProcessExitStatus exitStatus = cancellationToken.CanBeCanceled + ? await process.SafeHandle.WaitForExitOrKillOnCancellationAsync(cancellationToken).ConfigureAwait(false) + : await process.SafeHandle.WaitForExitAsync(cancellationToken).ConfigureAwait(false); + + return new ProcessTextOutput(exitStatus, standardOutput, standardError, process.Id); + } + } + + private static ProcessStartInfo CreateStartInfo(string fileName, IList? arguments) + { + ArgumentException.ThrowIfNullOrEmpty(fileName); + + ProcessStartInfo startInfo = new(fileName); + if (arguments is not null) + { + foreach (string argument in arguments) + { + startInfo.ArgumentList.Add(argument); + } + } + + return startInfo; + } + + private static void ThrowIfUseShellExecute(ProcessStartInfo startInfo, string methodName) + { + if (startInfo.UseShellExecute) + { + throw new InvalidOperationException(SR.Format(SR.UseShellExecuteNotSupportedForScenario, methodName)); + } + } } } diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessTextOutput.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessTextOutput.cs new file mode 100644 index 00000000000000..d094cfb852c00b --- /dev/null +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessTextOutput.cs @@ -0,0 +1,53 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Diagnostics +{ + /// + /// Represents the captured text output and exit status of a completed process. + /// + public sealed class ProcessTextOutput + { + /// + /// Initializes a new instance of the class. + /// + /// The exit status of the process. + /// The captured standard output text. + /// The captured standard error text. + /// The process ID of the completed process. + /// + /// , , or is . + /// + public ProcessTextOutput(ProcessExitStatus exitStatus, string standardOutput, string standardError, int processId) + { + ArgumentNullException.ThrowIfNull(exitStatus); + ArgumentNullException.ThrowIfNull(standardOutput); + ArgumentNullException.ThrowIfNull(standardError); + + ExitStatus = exitStatus; + StandardOutput = standardOutput; + StandardError = standardError; + ProcessId = processId; + } + + /// + /// Gets the exit status of the process. + /// + public ProcessExitStatus ExitStatus { get; } + + /// + /// Gets the captured standard output text of the process. + /// + public string StandardOutput { get; } + + /// + /// Gets the captured standard error text of the process. + /// + public string StandardError { get; } + + /// + /// Gets the process ID of the completed process. + /// + public int ProcessId { get; } + } +} diff --git a/src/libraries/System.Diagnostics.Process/tests/RunTests.cs b/src/libraries/System.Diagnostics.Process/tests/RunTests.cs new file mode 100644 index 00000000000000..05e9149c4959f6 --- /dev/null +++ b/src/libraries/System.Diagnostics.Process/tests/RunTests.cs @@ -0,0 +1,310 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.DotNet.RemoteExecutor; +using Xunit; + +namespace System.Diagnostics.Tests +{ + public class RunTests : ProcessTestBase + { + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(false)] + [InlineData(true)] + public async Task Run_ExitCodeIsReturned(bool useAsync) + { + using Process template = CreateProcess(RemotelyInvokable.Dummy); + + ProcessExitStatus exitStatus = useAsync + ? await Process.RunAsync(template.StartInfo) + : Process.Run(template.StartInfo); + + Assert.Equal(RemotelyInvokable.SuccessExitCode, exitStatus.ExitCode); + Assert.False(exitStatus.Canceled); + } + + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(false)] + [InlineData(true)] + public async Task Run_WithFileName_ExitCodeIsReturned(bool useAsync) + { + using Process template = CreateProcess(RemotelyInvokable.Dummy); + List? arguments = MapToArgumentList(template.StartInfo); + + ProcessExitStatus exitStatus = useAsync + ? await Process.RunAsync(template.StartInfo.FileName, arguments) + : Process.Run(template.StartInfo.FileName, arguments); + + Assert.Equal(RemotelyInvokable.SuccessExitCode, exitStatus.ExitCode); + Assert.False(exitStatus.Canceled); + } + + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(false)] + [InlineData(true)] + public async Task Run_WithTimeout_ExitCodeIsReturned(bool useAsync) + { + using Process template = CreateProcess(RemotelyInvokable.Dummy); + + ProcessExitStatus exitStatus = useAsync + ? await Process.RunAsync(template.StartInfo) + : Process.Run(template.StartInfo, TimeSpan.FromMinutes(1)); + + Assert.Equal(RemotelyInvokable.SuccessExitCode, exitStatus.ExitCode); + Assert.False(exitStatus.Canceled); + } + + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(false)] + [InlineData(true)] + public async Task Run_WithTimeoutOrCancellation_KillsLongRunningProcess(bool useAsync) + { + using Process template = CreateSleepProcess((int)TimeSpan.FromHours(1).TotalMilliseconds); + + ProcessExitStatus exitStatus = useAsync + ? await Process.RunAsync(template.StartInfo, new CancellationTokenSource(TimeSpan.FromMilliseconds(100)).Token) + : Process.Run(template.StartInfo, TimeSpan.FromMilliseconds(100)); + + Assert.True(exitStatus.Canceled); + } + + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(false)] + [InlineData(true)] + public async Task RunAndCaptureText_CapturesOutput(bool useAsync) + { + using Process template = CreateProcess(static () => + { + Console.Write("hello"); + Console.Error.Write("world"); + return RemoteExecutor.SuccessExitCode; + }); + + ProcessTextOutput result = useAsync + ? await Process.RunAndCaptureTextAsync(template.StartInfo) + : Process.RunAndCaptureText(template.StartInfo); + + Assert.Equal(RemoteExecutor.SuccessExitCode, result.ExitStatus.ExitCode); + Assert.False(result.ExitStatus.Canceled); + Assert.Equal("hello", result.StandardOutput); + Assert.Equal("world", result.StandardError); + Assert.True(result.ProcessId > 0); + } + + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(false)] + [InlineData(true)] + public async Task RunAndCaptureText_WithFileName_CapturesOutput(bool useAsync) + { + using Process template = CreateProcess(static () => + { + Console.Write("output"); + Console.Error.Write("error"); + return RemoteExecutor.SuccessExitCode; + }); + + List? arguments = MapToArgumentList(template.StartInfo); + + ProcessTextOutput result = useAsync + ? await Process.RunAndCaptureTextAsync(template.StartInfo.FileName, arguments) + : Process.RunAndCaptureText(template.StartInfo.FileName, arguments); + + Assert.Equal(RemoteExecutor.SuccessExitCode, result.ExitStatus.ExitCode); + Assert.Equal("output", result.StandardOutput); + Assert.Equal("error", result.StandardError); + } + + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(false)] + [InlineData(true)] + public async Task RunAndCaptureText_EmptyOutput(bool useAsync) + { + using Process template = CreateProcess(RemotelyInvokable.Dummy); + + ProcessTextOutput result = useAsync + ? await Process.RunAndCaptureTextAsync(template.StartInfo) + : Process.RunAndCaptureText(template.StartInfo); + + Assert.Equal(RemotelyInvokable.SuccessExitCode, result.ExitStatus.ExitCode); + Assert.Equal(string.Empty, result.StandardOutput); + Assert.Equal(string.Empty, result.StandardError); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Run_NullStartInfo_ThrowsArgumentNullException(bool useAsync) + { + if (useAsync) + { + await Assert.ThrowsAsync("startInfo", () => Process.RunAsync((ProcessStartInfo)null!)); + } + else + { + AssertExtensions.Throws("startInfo", () => Process.Run((ProcessStartInfo)null!)); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Run_NullFileName_ThrowsArgumentNullException(bool useAsync) + { + if (useAsync) + { + await Assert.ThrowsAsync("fileName", () => Process.RunAsync((string)null!)); + } + else + { + AssertExtensions.Throws("fileName", () => Process.Run((string)null!)); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Run_EmptyFileName_ThrowsArgumentException(bool useAsync) + { + if (useAsync) + { + await Assert.ThrowsAsync("fileName", () => Process.RunAsync(string.Empty)); + } + else + { + AssertExtensions.Throws("fileName", () => Process.Run(string.Empty)); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task RunAndCaptureText_NullStartInfo_ThrowsArgumentNullException(bool useAsync) + { + if (useAsync) + { + await Assert.ThrowsAsync("startInfo", () => Process.RunAndCaptureTextAsync((ProcessStartInfo)null!)); + } + else + { + AssertExtensions.Throws("startInfo", () => Process.RunAndCaptureText((ProcessStartInfo)null!)); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task RunAndCaptureText_NullFileName_ThrowsArgumentNullException(bool useAsync) + { + if (useAsync) + { + await Assert.ThrowsAsync("fileName", () => Process.RunAndCaptureTextAsync((string)null!)); + } + else + { + AssertExtensions.Throws("fileName", () => Process.RunAndCaptureText((string)null!)); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task RunAndCaptureText_EmptyFileName_ThrowsArgumentException(bool useAsync) + { + if (useAsync) + { + await Assert.ThrowsAsync("fileName", () => Process.RunAndCaptureTextAsync(string.Empty)); + } + else + { + AssertExtensions.Throws("fileName", () => Process.RunAndCaptureText(string.Empty)); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void Run_UseShellExecute_ThrowsInvalidOperationException(bool useAsync) + { + ProcessStartInfo startInfo = new("someprocess") { UseShellExecute = true }; + + Assert.Throws(() => + { + if (useAsync) + { + Process.RunAsync(startInfo); + } + else + { + Process.Run(startInfo); + } + }); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void RunAndCaptureText_UseShellExecute_ThrowsInvalidOperationException(bool useAsync) + { + ProcessStartInfo startInfo = new("someprocess") { UseShellExecute = true }; + + Assert.Throws(() => + { + if (useAsync) + { + Process.RunAndCaptureTextAsync(startInfo); + } + else + { + Process.RunAndCaptureText(startInfo); + } + }); + } + + // RemoteExecutor populates ProcessStartInfo.Arguments, but the filename overloads + // take an argument list, so this helper maps the serialized argument string. + private static List? MapToArgumentList(ProcessStartInfo startInfo) + { + string arguments = startInfo.Arguments; + if (string.IsNullOrEmpty(arguments)) + { + return null; + } + + List list = new(); + System.Text.StringBuilder builder = new(); + bool isQuoted = false; + + foreach (char c in arguments) + { + switch (c) + { + case '"' when !isQuoted: + isQuoted = true; + break; + case ' ' when !isQuoted: + case '"' when isQuoted: + if (builder.Length > 0) + { + list.Add(builder.ToString()); + builder.Clear(); + } + isQuoted = false; + break; + default: + builder.Append(c); + break; + } + } + + if (builder.Length > 0) + { + list.Add(builder.ToString()); + } + + return list; + } + } +} 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 a9332ad98c999f..f1ed6055933db3 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 @@ -39,6 +39,7 @@ + From ef3169ed02ccb9226ed4a20801904761cf140482 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 09:59:48 +0000 Subject: [PATCH 20/29] Address PR feedback: use CreateStartInfo, expression-bodies, inline helpers, fix timeout tracking, reuse MapToArgumentList Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/840381dc-b4f5-46ce-a7ed-bdde61b0ca90 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../System/Diagnostics/Process.Scenarios.cs | 108 ++++++------------ .../tests/RunTests.cs | 80 +++---------- .../tests/StartAndForget.cs | 2 +- 3 files changed, 51 insertions(+), 139 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Scenarios.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Scenarios.cs index 15d2fa3cc227fa..3051122a0d5234 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Scenarios.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Scenarios.cs @@ -80,20 +80,7 @@ public static int StartAndForget(ProcessStartInfo startInfo) [UnsupportedOSPlatform("tvos")] [SupportedOSPlatform("maccatalyst")] public static int StartAndForget(string fileName, IList? arguments = null) - { - ArgumentNullException.ThrowIfNull(fileName); - - ProcessStartInfo startInfo = new(fileName); - if (arguments is not null) - { - foreach (string argument in arguments) - { - startInfo.ArgumentList.Add(argument); - } - } - - return StartAndForget(startInfo); - } + => StartAndForget(CreateStartInfo(fileName, arguments)); /// /// Starts the process described by , waits for it to exit, and returns its exit status. @@ -145,11 +132,7 @@ public static ProcessExitStatus Run(ProcessStartInfo startInfo, TimeSpan? timeou [UnsupportedOSPlatform("tvos")] [SupportedOSPlatform("maccatalyst")] public static ProcessExitStatus Run(string fileName, IList? arguments = null, TimeSpan? timeout = default) - { - ProcessStartInfo startInfo = CreateStartInfo(fileName, arguments); - - return Run(startInfo, timeout); - } + => Run(CreateStartInfo(fileName, arguments), timeout); /// /// Asynchronously starts the process described by , waits for it to exit, and returns its exit status. @@ -167,15 +150,17 @@ public static ProcessExitStatus Run(string fileName, IList? arguments = [UnsupportedOSPlatform("ios")] [UnsupportedOSPlatform("tvos")] [SupportedOSPlatform("maccatalyst")] - public static Task RunAsync(ProcessStartInfo startInfo, CancellationToken cancellationToken = default) + public static async Task RunAsync(ProcessStartInfo startInfo, CancellationToken cancellationToken = default) { ArgumentNullException.ThrowIfNull(startInfo); ThrowIfUseShellExecute(startInfo, nameof(RunAsync)); - SafeProcessHandle processHandle = SafeProcessHandle.Start(startInfo); + using SafeProcessHandle processHandle = SafeProcessHandle.Start(startInfo); - return RunAsyncCore(processHandle, cancellationToken); + return cancellationToken.CanBeCanceled + ? await processHandle.WaitForExitOrKillOnCancellationAsync(cancellationToken).ConfigureAwait(false) + : await processHandle.WaitForExitAsync(cancellationToken).ConfigureAwait(false); } /// @@ -197,11 +182,7 @@ public static Task RunAsync(ProcessStartInfo startInfo, Cance [UnsupportedOSPlatform("tvos")] [SupportedOSPlatform("maccatalyst")] public static Task RunAsync(string fileName, IList? arguments = null, CancellationToken cancellationToken = default) - { - ProcessStartInfo startInfo = CreateStartInfo(fileName, arguments); - - return RunAsync(startInfo, cancellationToken); - } + => RunAsync(CreateStartInfo(fileName, arguments), cancellationToken); /// /// Starts the process described by , captures its standard output and error, @@ -230,13 +211,26 @@ public static ProcessTextOutput RunAndCaptureText(ProcessStartInfo startInfo, Ti startInfo.RedirectStandardOutput = true; startInfo.RedirectStandardError = true; + long startTimestamp = Stopwatch.GetTimestamp(); + using Process process = Start(startInfo)!; (string standardOutput, string standardError) = process.ReadAllText(timeout); - ProcessExitStatus exitStatus = timeout.HasValue - ? process.SafeHandle.WaitForExitOrKillOnTimeout(timeout.Value) - : process.SafeHandle.WaitForExit(); + ProcessExitStatus exitStatus; + if (timeout.HasValue) + { + TimeSpan elapsed = Stopwatch.GetElapsedTime(startTimestamp); + TimeSpan remaining = timeout.Value - elapsed; + + exitStatus = remaining > TimeSpan.Zero + ? process.SafeHandle.WaitForExitOrKillOnTimeout(remaining) + : process.SafeHandle.WaitForExitOrKillOnTimeout(TimeSpan.Zero); + } + else + { + exitStatus = process.SafeHandle.WaitForExit(); + } return new ProcessTextOutput(exitStatus, standardOutput, standardError, process.Id); } @@ -262,11 +256,7 @@ public static ProcessTextOutput RunAndCaptureText(ProcessStartInfo startInfo, Ti [UnsupportedOSPlatform("tvos")] [SupportedOSPlatform("maccatalyst")] public static ProcessTextOutput RunAndCaptureText(string fileName, IList? arguments = null, TimeSpan? timeout = default) - { - ProcessStartInfo startInfo = CreateStartInfo(fileName, arguments); - - return RunAndCaptureText(startInfo, timeout); - } + => RunAndCaptureText(CreateStartInfo(fileName, arguments), timeout); /// /// Asynchronously starts the process described by , captures its standard output and error, @@ -285,7 +275,7 @@ public static ProcessTextOutput RunAndCaptureText(string fileName, IList [UnsupportedOSPlatform("ios")] [UnsupportedOSPlatform("tvos")] [SupportedOSPlatform("maccatalyst")] - public static Task RunAndCaptureTextAsync(ProcessStartInfo startInfo, CancellationToken cancellationToken = default) + public static async Task RunAndCaptureTextAsync(ProcessStartInfo startInfo, CancellationToken cancellationToken = default) { ArgumentNullException.ThrowIfNull(startInfo); @@ -294,9 +284,15 @@ public static Task RunAndCaptureTextAsync(ProcessStartInfo st startInfo.RedirectStandardOutput = true; startInfo.RedirectStandardError = true; - Process process = Start(startInfo)!; + using Process process = Start(startInfo)!; + + (string standardOutput, string standardError) = await process.ReadAllTextAsync(cancellationToken).ConfigureAwait(false); + + ProcessExitStatus exitStatus = cancellationToken.CanBeCanceled + ? await process.SafeHandle.WaitForExitOrKillOnCancellationAsync(cancellationToken).ConfigureAwait(false) + : await process.SafeHandle.WaitForExitAsync(cancellationToken).ConfigureAwait(false); - return RunAndCaptureTextAsyncCore(process, cancellationToken); + return new ProcessTextOutput(exitStatus, standardOutput, standardError, process.Id); } /// @@ -319,41 +315,7 @@ public static Task RunAndCaptureTextAsync(ProcessStartInfo st [UnsupportedOSPlatform("tvos")] [SupportedOSPlatform("maccatalyst")] public static Task RunAndCaptureTextAsync(string fileName, IList? arguments = null, CancellationToken cancellationToken = default) - { - ProcessStartInfo startInfo = CreateStartInfo(fileName, arguments); - - return RunAndCaptureTextAsync(startInfo, cancellationToken); - } - - [UnsupportedOSPlatform("ios")] - [UnsupportedOSPlatform("tvos")] - [SupportedOSPlatform("maccatalyst")] - private static async Task RunAsyncCore(SafeProcessHandle processHandle, CancellationToken cancellationToken) - { - using (processHandle) - { - return cancellationToken.CanBeCanceled - ? await processHandle.WaitForExitOrKillOnCancellationAsync(cancellationToken).ConfigureAwait(false) - : await processHandle.WaitForExitAsync(cancellationToken).ConfigureAwait(false); - } - } - - [UnsupportedOSPlatform("ios")] - [UnsupportedOSPlatform("tvos")] - [SupportedOSPlatform("maccatalyst")] - private static async Task RunAndCaptureTextAsyncCore(Process process, CancellationToken cancellationToken) - { - using (process) - { - (string standardOutput, string standardError) = await process.ReadAllTextAsync(cancellationToken).ConfigureAwait(false); - - ProcessExitStatus exitStatus = cancellationToken.CanBeCanceled - ? await process.SafeHandle.WaitForExitOrKillOnCancellationAsync(cancellationToken).ConfigureAwait(false) - : await process.SafeHandle.WaitForExitAsync(cancellationToken).ConfigureAwait(false); - - return new ProcessTextOutput(exitStatus, standardOutput, standardError, process.Id); - } - } + => RunAndCaptureTextAsync(CreateStartInfo(fileName, arguments), cancellationToken); private static ProcessStartInfo CreateStartInfo(string fileName, IList? arguments) { diff --git a/src/libraries/System.Diagnostics.Process/tests/RunTests.cs b/src/libraries/System.Diagnostics.Process/tests/RunTests.cs index 05e9149c4959f6..f38dc5b412b6cb 100644 --- a/src/libraries/System.Diagnostics.Process/tests/RunTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/RunTests.cs @@ -32,7 +32,7 @@ public async Task Run_ExitCodeIsReturned(bool useAsync) public async Task Run_WithFileName_ExitCodeIsReturned(bool useAsync) { using Process template = CreateProcess(RemotelyInvokable.Dummy); - List? arguments = MapToArgumentList(template.StartInfo); + List? arguments = StartAndForgetTests.MapToArgumentList(template.StartInfo); ProcessExitStatus exitStatus = useAsync ? await Process.RunAsync(template.StartInfo.FileName, arguments) @@ -106,7 +106,7 @@ public async Task RunAndCaptureText_WithFileName_CapturesOutput(bool useAsync) return RemoteExecutor.SuccessExitCode; }); - List? arguments = MapToArgumentList(template.StartInfo); + List? arguments = StartAndForgetTests.MapToArgumentList(template.StartInfo); ProcessTextOutput result = useAsync ? await Process.RunAndCaptureTextAsync(template.StartInfo.FileName, arguments) @@ -226,85 +226,35 @@ public async Task RunAndCaptureText_EmptyFileName_ThrowsArgumentException(bool u [Theory] [InlineData(false)] [InlineData(true)] - public void Run_UseShellExecute_ThrowsInvalidOperationException(bool useAsync) + public async Task Run_UseShellExecute_ThrowsInvalidOperationException(bool useAsync) { ProcessStartInfo startInfo = new("someprocess") { UseShellExecute = true }; - Assert.Throws(() => + if (useAsync) { - if (useAsync) - { - Process.RunAsync(startInfo); - } - else - { - Process.Run(startInfo); - } - }); + await Assert.ThrowsAsync(() => Process.RunAsync(startInfo)); + } + else + { + Assert.Throws(() => Process.Run(startInfo)); + } } [Theory] [InlineData(false)] [InlineData(true)] - public void RunAndCaptureText_UseShellExecute_ThrowsInvalidOperationException(bool useAsync) + public async Task RunAndCaptureText_UseShellExecute_ThrowsInvalidOperationException(bool useAsync) { ProcessStartInfo startInfo = new("someprocess") { UseShellExecute = true }; - Assert.Throws(() => - { - if (useAsync) - { - Process.RunAndCaptureTextAsync(startInfo); - } - else - { - Process.RunAndCaptureText(startInfo); - } - }); - } - - // RemoteExecutor populates ProcessStartInfo.Arguments, but the filename overloads - // take an argument list, so this helper maps the serialized argument string. - private static List? MapToArgumentList(ProcessStartInfo startInfo) - { - string arguments = startInfo.Arguments; - if (string.IsNullOrEmpty(arguments)) - { - return null; - } - - List list = new(); - System.Text.StringBuilder builder = new(); - bool isQuoted = false; - - foreach (char c in arguments) + if (useAsync) { - switch (c) - { - case '"' when !isQuoted: - isQuoted = true; - break; - case ' ' when !isQuoted: - case '"' when isQuoted: - if (builder.Length > 0) - { - list.Add(builder.ToString()); - builder.Clear(); - } - isQuoted = false; - break; - default: - builder.Append(c); - break; - } + await Assert.ThrowsAsync(() => Process.RunAndCaptureTextAsync(startInfo)); } - - if (builder.Length > 0) + else { - list.Add(builder.ToString()); + Assert.Throws(() => Process.RunAndCaptureText(startInfo)); } - - return list; } } } diff --git a/src/libraries/System.Diagnostics.Process/tests/StartAndForget.cs b/src/libraries/System.Diagnostics.Process/tests/StartAndForget.cs index dc20e3fd9a8a45..93a4b5645aac83 100644 --- a/src/libraries/System.Diagnostics.Process/tests/StartAndForget.cs +++ b/src/libraries/System.Diagnostics.Process/tests/StartAndForget.cs @@ -115,7 +115,7 @@ public void StartAndForget_WithUseShellExecute_ThrowsInvalidOperationException() // RemoteExecutor populates ProcessStartInfo.Arguments, but StartAndForget(fileName, arguments) // takes an argument list, so this helper maps the serialized argument string for this test. - private static List? MapToArgumentList(ProcessStartInfo startInfo) + internal static List? MapToArgumentList(ProcessStartInfo startInfo) { string arguments = startInfo.Arguments; if (string.IsNullOrEmpty(arguments)) From 7c7651fc14c132da056c9f51a6ce9819579b5a14 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 13:22:17 +0000 Subject: [PATCH 21/29] Move MapToArgumentList from StartAndForgetTests to Helpers class Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/307d0010-7f67-4270-8c69-bbc3f601c3e1 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../tests/Helpers.cs | 46 +++++++++++++++++++ .../tests/RunTests.cs | 4 +- .../tests/StartAndForget.cs | 45 +----------------- 3 files changed, 49 insertions(+), 46 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/Helpers.cs b/src/libraries/System.Diagnostics.Process/tests/Helpers.cs index cf5507a5ee26fc..5b3e0d2e0e0292 100644 --- a/src/libraries/System.Diagnostics.Process/tests/Helpers.cs +++ b/src/libraries/System.Diagnostics.Process/tests/Helpers.cs @@ -2,8 +2,10 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections.Generic; using System.ComponentModel; using System.Security.Principal; +using System.Text; using System.Threading.Tasks; using Xunit.Sdk; @@ -11,6 +13,50 @@ namespace System.Diagnostics.Tests { internal static class Helpers { + // RemoteExecutor populates ProcessStartInfo.Arguments, but the fileName overloads + // take an argument list, so this helper maps the serialized argument string. + public static List? MapToArgumentList(ProcessStartInfo startInfo) + { + string arguments = startInfo.Arguments; + if (string.IsNullOrEmpty(arguments)) + { + return null; + } + + List list = new(); + StringBuilder builder = new(); + bool isQuoted = false; + + foreach (char c in arguments) + { + switch (c) + { + case '"' when !isQuoted: + isQuoted = true; + break; + case ' ' when !isQuoted: + case '"' when isQuoted: + if (builder.Length > 0) + { + list.Add(builder.ToString()); + builder.Clear(); + } + isQuoted = false; + break; + default: + builder.Append(c); + break; + } + } + + if (builder.Length > 0) + { + list.Add(builder.ToString()); + } + + return list; + } + public const int PassingTestTimeoutMilliseconds = 60_000; public static async Task RetryWithBackoff(Action action, int delayInMilliseconds = 10, int times = 10) diff --git a/src/libraries/System.Diagnostics.Process/tests/RunTests.cs b/src/libraries/System.Diagnostics.Process/tests/RunTests.cs index f38dc5b412b6cb..24c1af6eeccb73 100644 --- a/src/libraries/System.Diagnostics.Process/tests/RunTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/RunTests.cs @@ -32,7 +32,7 @@ public async Task Run_ExitCodeIsReturned(bool useAsync) public async Task Run_WithFileName_ExitCodeIsReturned(bool useAsync) { using Process template = CreateProcess(RemotelyInvokable.Dummy); - List? arguments = StartAndForgetTests.MapToArgumentList(template.StartInfo); + List? arguments = Helpers.MapToArgumentList(template.StartInfo); ProcessExitStatus exitStatus = useAsync ? await Process.RunAsync(template.StartInfo.FileName, arguments) @@ -106,7 +106,7 @@ public async Task RunAndCaptureText_WithFileName_CapturesOutput(bool useAsync) return RemoteExecutor.SuccessExitCode; }); - List? arguments = StartAndForgetTests.MapToArgumentList(template.StartInfo); + List? arguments = Helpers.MapToArgumentList(template.StartInfo); ProcessTextOutput result = useAsync ? await Process.RunAndCaptureTextAsync(template.StartInfo.FileName, arguments) diff --git a/src/libraries/System.Diagnostics.Process/tests/StartAndForget.cs b/src/libraries/System.Diagnostics.Process/tests/StartAndForget.cs index 93a4b5645aac83..ebdd85e9833275 100644 --- a/src/libraries/System.Diagnostics.Process/tests/StartAndForget.cs +++ b/src/libraries/System.Diagnostics.Process/tests/StartAndForget.cs @@ -3,7 +3,6 @@ using System.Collections.Generic; using System.IO; -using System.Text; using Microsoft.DotNet.RemoteExecutor; using Microsoft.Win32.SafeHandles; using Xunit; @@ -20,7 +19,7 @@ public void StartAndForget_StartsProcessAndReturnsValidPid(bool useProcessStartI using Process template = CreateSleepProcess((int)TimeSpan.FromHours(1).TotalMilliseconds); int pid = useProcessStartInfo ? Process.StartAndForget(template.StartInfo) - : Process.StartAndForget(template.StartInfo.FileName, MapToArgumentList(template.StartInfo)); + : Process.StartAndForget(template.StartInfo.FileName, Helpers.MapToArgumentList(template.StartInfo)); Assert.True(pid > 0); @@ -113,48 +112,6 @@ public void StartAndForget_WithUseShellExecute_ThrowsInvalidOperationException() Assert.Throws(() => Process.StartAndForget(startInfo)); } - // RemoteExecutor populates ProcessStartInfo.Arguments, but StartAndForget(fileName, arguments) - // takes an argument list, so this helper maps the serialized argument string for this test. - internal static List? MapToArgumentList(ProcessStartInfo startInfo) - { - string arguments = startInfo.Arguments; - if (string.IsNullOrEmpty(arguments)) - { - return null; - } - List list = new(); - StringBuilder builder = new(); - bool isQuoted = false; - - foreach (char c in arguments) - { - switch (c) - { - case '"' when !isQuoted: - isQuoted = true; - break; - case ' ' when !isQuoted: - case '"' when isQuoted: - if (builder.Length > 0) - { - list.Add(builder.ToString()); - builder.Clear(); - } - isQuoted = false; - break; - default: - builder.Append(c); - break; - } - } - - if (builder.Length > 0) - { - list.Add(builder.ToString()); - } - - return list; - } } } From 55c1b1815bf9d354435ee553d760b079d0487164 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 18:26:05 +0000 Subject: [PATCH 22/29] Kill process when ReadAll* throws; replace RemotelyInvokable.Dummy with explicit lambdas in tests Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/77e483ad-6804-4ecb-b688-5e7fc4de3638 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../System/Diagnostics/Process.Scenarios.cs | 22 +++++++++++++++++-- .../tests/RunTests.cs | 16 +++++++------- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Scenarios.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Scenarios.cs index 3051122a0d5234..c86977b664880d 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Scenarios.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Scenarios.cs @@ -215,7 +215,16 @@ public static ProcessTextOutput RunAndCaptureText(ProcessStartInfo startInfo, Ti using Process process = Start(startInfo)!; - (string standardOutput, string standardError) = process.ReadAllText(timeout); + string standardOutput, standardError; + try + { + (standardOutput, standardError) = process.ReadAllText(timeout); + } + catch + { + process.Kill(); + throw; + } ProcessExitStatus exitStatus; if (timeout.HasValue) @@ -286,7 +295,16 @@ public static async Task RunAndCaptureTextAsync(ProcessStartI using Process process = Start(startInfo)!; - (string standardOutput, string standardError) = await process.ReadAllTextAsync(cancellationToken).ConfigureAwait(false); + string standardOutput, standardError; + try + { + (standardOutput, standardError) = await process.ReadAllTextAsync(cancellationToken).ConfigureAwait(false); + } + catch + { + process.Kill(); + throw; + } ProcessExitStatus exitStatus = cancellationToken.CanBeCanceled ? await process.SafeHandle.WaitForExitOrKillOnCancellationAsync(cancellationToken).ConfigureAwait(false) diff --git a/src/libraries/System.Diagnostics.Process/tests/RunTests.cs b/src/libraries/System.Diagnostics.Process/tests/RunTests.cs index 24c1af6eeccb73..b4c1ceeba44f40 100644 --- a/src/libraries/System.Diagnostics.Process/tests/RunTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/RunTests.cs @@ -16,13 +16,13 @@ public class RunTests : ProcessTestBase [InlineData(true)] public async Task Run_ExitCodeIsReturned(bool useAsync) { - using Process template = CreateProcess(RemotelyInvokable.Dummy); + using Process template = CreateProcess(static () => RemoteExecutor.SuccessExitCode); ProcessExitStatus exitStatus = useAsync ? await Process.RunAsync(template.StartInfo) : Process.Run(template.StartInfo); - Assert.Equal(RemotelyInvokable.SuccessExitCode, exitStatus.ExitCode); + Assert.Equal(RemoteExecutor.SuccessExitCode, exitStatus.ExitCode); Assert.False(exitStatus.Canceled); } @@ -31,14 +31,14 @@ public async Task Run_ExitCodeIsReturned(bool useAsync) [InlineData(true)] public async Task Run_WithFileName_ExitCodeIsReturned(bool useAsync) { - using Process template = CreateProcess(RemotelyInvokable.Dummy); + using Process template = CreateProcess(static () => RemoteExecutor.SuccessExitCode); List? arguments = Helpers.MapToArgumentList(template.StartInfo); ProcessExitStatus exitStatus = useAsync ? await Process.RunAsync(template.StartInfo.FileName, arguments) : Process.Run(template.StartInfo.FileName, arguments); - Assert.Equal(RemotelyInvokable.SuccessExitCode, exitStatus.ExitCode); + Assert.Equal(RemoteExecutor.SuccessExitCode, exitStatus.ExitCode); Assert.False(exitStatus.Canceled); } @@ -47,13 +47,13 @@ public async Task Run_WithFileName_ExitCodeIsReturned(bool useAsync) [InlineData(true)] public async Task Run_WithTimeout_ExitCodeIsReturned(bool useAsync) { - using Process template = CreateProcess(RemotelyInvokable.Dummy); + using Process template = CreateProcess(static () => RemoteExecutor.SuccessExitCode); ProcessExitStatus exitStatus = useAsync ? await Process.RunAsync(template.StartInfo) : Process.Run(template.StartInfo, TimeSpan.FromMinutes(1)); - Assert.Equal(RemotelyInvokable.SuccessExitCode, exitStatus.ExitCode); + Assert.Equal(RemoteExecutor.SuccessExitCode, exitStatus.ExitCode); Assert.False(exitStatus.Canceled); } @@ -122,13 +122,13 @@ public async Task RunAndCaptureText_WithFileName_CapturesOutput(bool useAsync) [InlineData(true)] public async Task RunAndCaptureText_EmptyOutput(bool useAsync) { - using Process template = CreateProcess(RemotelyInvokable.Dummy); + using Process template = CreateProcess(static () => RemoteExecutor.SuccessExitCode); ProcessTextOutput result = useAsync ? await Process.RunAndCaptureTextAsync(template.StartInfo) : Process.RunAndCaptureText(template.StartInfo); - Assert.Equal(RemotelyInvokable.SuccessExitCode, result.ExitStatus.ExitCode); + Assert.Equal(RemoteExecutor.SuccessExitCode, result.ExitStatus.ExitCode); Assert.Equal(string.Empty, result.StandardOutput); Assert.Equal(string.Empty, result.StandardError); } From bbb0c70ff31985afa4d7b019b748a33c6224fdc5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 21:26:06 +0000 Subject: [PATCH 23/29] Address review feedback: Process.{0} in error message, simplify timeout logic, fix Kill() exception masking, fix async test path Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/c47fa8e9-5961-4cf0-aed3-da97948d0e36 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/Resources/Strings.resx | 2 +- .../src/System/Diagnostics/Process.Scenarios.cs | 10 ++++------ .../System.Diagnostics.Process/tests/RunTests.cs | 2 +- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx index c6f8b61d1ab230..a558c9f7156c59 100644 --- a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx +++ b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx @@ -364,7 +364,7 @@ InheritedHandles must not contain duplicates. - UseShellExecute is not supported by {0}. On Windows, shell execution may not create a new process, which would make it impossible to return a valid process ID. + UseShellExecute is not supported by Process.{0}. On Windows, shell execution may not create a new process, which would make it impossible to return a valid process ID. On Unix, it's impossible to obtain the exit status of a non-child process. diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Scenarios.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Scenarios.cs index c86977b664880d..a5d3ad4cc0747a 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Scenarios.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Scenarios.cs @@ -222,7 +222,7 @@ public static ProcessTextOutput RunAndCaptureText(ProcessStartInfo startInfo, Ti } catch { - process.Kill(); + try { process.Kill(); } catch { } throw; } @@ -231,10 +231,8 @@ public static ProcessTextOutput RunAndCaptureText(ProcessStartInfo startInfo, Ti { TimeSpan elapsed = Stopwatch.GetElapsedTime(startTimestamp); TimeSpan remaining = timeout.Value - elapsed; - - exitStatus = remaining > TimeSpan.Zero - ? process.SafeHandle.WaitForExitOrKillOnTimeout(remaining) - : process.SafeHandle.WaitForExitOrKillOnTimeout(TimeSpan.Zero); + remaining = remaining >= TimeSpan.Zero ? remaining : TimeSpan.Zero; + exitStatus = process.SafeHandle.WaitForExitOrKillOnTimeout(remaining); } else { @@ -302,7 +300,7 @@ public static async Task RunAndCaptureTextAsync(ProcessStartI } catch { - process.Kill(); + try { process.Kill(); } catch { } throw; } diff --git a/src/libraries/System.Diagnostics.Process/tests/RunTests.cs b/src/libraries/System.Diagnostics.Process/tests/RunTests.cs index b4c1ceeba44f40..0db8ecf667163d 100644 --- a/src/libraries/System.Diagnostics.Process/tests/RunTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/RunTests.cs @@ -50,7 +50,7 @@ public async Task Run_WithTimeout_ExitCodeIsReturned(bool useAsync) using Process template = CreateProcess(static () => RemoteExecutor.SuccessExitCode); ProcessExitStatus exitStatus = useAsync - ? await Process.RunAsync(template.StartInfo) + ? await Process.RunAsync(template.StartInfo, new CancellationTokenSource(TimeSpan.FromMinutes(1)).Token) : Process.Run(template.StartInfo, TimeSpan.FromMinutes(1)); Assert.Equal(RemoteExecutor.SuccessExitCode, exitStatus.ExitCode); From 9ff3e6256ca3e2eee3f6899779d17cb9cc16dba3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 21:27:18 +0000 Subject: [PATCH 24/29] Fix CancellationTokenSource leak in Run_WithTimeout test Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/c47fa8e9-5961-4cf0-aed3-da97948d0e36 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../System.Diagnostics.Process/tests/RunTests.cs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/RunTests.cs b/src/libraries/System.Diagnostics.Process/tests/RunTests.cs index 0db8ecf667163d..d5164e3577ee78 100644 --- a/src/libraries/System.Diagnostics.Process/tests/RunTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/RunTests.cs @@ -49,9 +49,16 @@ public async Task Run_WithTimeout_ExitCodeIsReturned(bool useAsync) { using Process template = CreateProcess(static () => RemoteExecutor.SuccessExitCode); - ProcessExitStatus exitStatus = useAsync - ? await Process.RunAsync(template.StartInfo, new CancellationTokenSource(TimeSpan.FromMinutes(1)).Token) - : Process.Run(template.StartInfo, TimeSpan.FromMinutes(1)); + ProcessExitStatus exitStatus; + if (useAsync) + { + using var cts = new CancellationTokenSource(TimeSpan.FromMinutes(1)); + exitStatus = await Process.RunAsync(template.StartInfo, cts.Token); + } + else + { + exitStatus = Process.Run(template.StartInfo, TimeSpan.FromMinutes(1)); + } Assert.Equal(RemoteExecutor.SuccessExitCode, exitStatus.ExitCode); Assert.False(exitStatus.Canceled); From fc2f007004e225aedabfda1324ed56499dba1ef3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 21:56:50 +0000 Subject: [PATCH 25/29] Require RedirectStandardOutput/Error on PSI overloads instead of mutating Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/73a14058-c419-4277-b5e2-885c9c6c8607 Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com> --- .../src/Resources/Strings.resx | 3 ++ .../System/Diagnostics/Process.Scenarios.cs | 37 ++++++++++++++----- .../tests/RunTests.cs | 6 +++ 3 files changed, 36 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx index a558c9f7156c59..92a6dd8e10675e 100644 --- a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx +++ b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx @@ -366,6 +366,9 @@ UseShellExecute is not supported by Process.{0}. On Windows, shell execution may not create a new process, which would make it impossible to return a valid process ID. + + RedirectStandardOutput and RedirectStandardError must both be set to true when calling Process.{0} with a ProcessStartInfo. + On Unix, it's impossible to obtain the exit status of a non-child process. diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Scenarios.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Scenarios.cs index a5d3ad4cc0747a..9591de06153850 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Scenarios.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Scenarios.cs @@ -197,7 +197,9 @@ public static Task RunAsync(string fileName, IList? a /// The captured text output and exit status of the process. /// is . /// - /// is set to . + /// is set to . + /// -or- + /// or is not set to . /// [UnsupportedOSPlatform("ios")] [UnsupportedOSPlatform("tvos")] @@ -207,9 +209,7 @@ public static ProcessTextOutput RunAndCaptureText(ProcessStartInfo startInfo, Ti ArgumentNullException.ThrowIfNull(startInfo); ThrowIfUseShellExecute(startInfo, nameof(RunAndCaptureText)); - - startInfo.RedirectStandardOutput = true; - startInfo.RedirectStandardError = true; + ThrowIfNotRedirected(startInfo, nameof(RunAndCaptureText)); long startTimestamp = Stopwatch.GetTimestamp(); @@ -263,7 +263,7 @@ public static ProcessTextOutput RunAndCaptureText(ProcessStartInfo startInfo, Ti [UnsupportedOSPlatform("tvos")] [SupportedOSPlatform("maccatalyst")] public static ProcessTextOutput RunAndCaptureText(string fileName, IList? arguments = null, TimeSpan? timeout = default) - => RunAndCaptureText(CreateStartInfo(fileName, arguments), timeout); + => RunAndCaptureText(CreateStartInfoForCapture(fileName, arguments), timeout); /// /// Asynchronously starts the process described by , captures its standard output and error, @@ -277,7 +277,9 @@ public static ProcessTextOutput RunAndCaptureText(string fileName, IList /// A task that represents the asynchronous operation. The value of the task contains the captured text output and exit status of the process. /// is . /// - /// is set to . + /// is set to . + /// -or- + /// or is not set to . /// [UnsupportedOSPlatform("ios")] [UnsupportedOSPlatform("tvos")] @@ -287,9 +289,7 @@ public static async Task RunAndCaptureTextAsync(ProcessStartI ArgumentNullException.ThrowIfNull(startInfo); ThrowIfUseShellExecute(startInfo, nameof(RunAndCaptureTextAsync)); - - startInfo.RedirectStandardOutput = true; - startInfo.RedirectStandardError = true; + ThrowIfNotRedirected(startInfo, nameof(RunAndCaptureTextAsync)); using Process process = Start(startInfo)!; @@ -331,7 +331,7 @@ public static async Task RunAndCaptureTextAsync(ProcessStartI [UnsupportedOSPlatform("tvos")] [SupportedOSPlatform("maccatalyst")] public static Task RunAndCaptureTextAsync(string fileName, IList? arguments = null, CancellationToken cancellationToken = default) - => RunAndCaptureTextAsync(CreateStartInfo(fileName, arguments), cancellationToken); + => RunAndCaptureTextAsync(CreateStartInfoForCapture(fileName, arguments), cancellationToken); private static ProcessStartInfo CreateStartInfo(string fileName, IList? arguments) { @@ -349,6 +349,15 @@ private static ProcessStartInfo CreateStartInfo(string fileName, IList? return startInfo; } + private static ProcessStartInfo CreateStartInfoForCapture(string fileName, IList? arguments) + { + ProcessStartInfo startInfo = CreateStartInfo(fileName, arguments); + startInfo.RedirectStandardOutput = true; + startInfo.RedirectStandardError = true; + + return startInfo; + } + private static void ThrowIfUseShellExecute(ProcessStartInfo startInfo, string methodName) { if (startInfo.UseShellExecute) @@ -356,5 +365,13 @@ private static void ThrowIfUseShellExecute(ProcessStartInfo startInfo, string me throw new InvalidOperationException(SR.Format(SR.UseShellExecuteNotSupportedForScenario, methodName)); } } + + private static void ThrowIfNotRedirected(ProcessStartInfo startInfo, string methodName) + { + if (!startInfo.RedirectStandardOutput || !startInfo.RedirectStandardError) + { + throw new InvalidOperationException(SR.Format(SR.RedirectStandardOutputAndErrorRequired, methodName)); + } + } } } diff --git a/src/libraries/System.Diagnostics.Process/tests/RunTests.cs b/src/libraries/System.Diagnostics.Process/tests/RunTests.cs index d5164e3577ee78..f7dda81f2b4a4e 100644 --- a/src/libraries/System.Diagnostics.Process/tests/RunTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/RunTests.cs @@ -90,6 +90,9 @@ public async Task RunAndCaptureText_CapturesOutput(bool useAsync) return RemoteExecutor.SuccessExitCode; }); + template.StartInfo.RedirectStandardOutput = true; + template.StartInfo.RedirectStandardError = true; + ProcessTextOutput result = useAsync ? await Process.RunAndCaptureTextAsync(template.StartInfo) : Process.RunAndCaptureText(template.StartInfo); @@ -131,6 +134,9 @@ public async Task RunAndCaptureText_EmptyOutput(bool useAsync) { using Process template = CreateProcess(static () => RemoteExecutor.SuccessExitCode); + template.StartInfo.RedirectStandardOutput = true; + template.StartInfo.RedirectStandardError = true; + ProcessTextOutput result = useAsync ? await Process.RunAndCaptureTextAsync(template.StartInfo) : Process.RunAndCaptureText(template.StartInfo); From 391c3c4ceaa56e04af28413ae04d94d3a90e86ba Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 21:57:57 +0000 Subject: [PATCH 26/29] Simplify RunAsync/RunAndCaptureTextAsync: always use WaitForExitOrKillOnCancellationAsync Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/73a14058-c419-4277-b5e2-885c9c6c8607 Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com> --- .../src/System/Diagnostics/Process.Scenarios.cs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Scenarios.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Scenarios.cs index 9591de06153850..e2a251d4373416 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Scenarios.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Scenarios.cs @@ -158,9 +158,7 @@ public static async Task RunAsync(ProcessStartInfo startInfo, using SafeProcessHandle processHandle = SafeProcessHandle.Start(startInfo); - return cancellationToken.CanBeCanceled - ? await processHandle.WaitForExitOrKillOnCancellationAsync(cancellationToken).ConfigureAwait(false) - : await processHandle.WaitForExitAsync(cancellationToken).ConfigureAwait(false); + return await processHandle.WaitForExitOrKillOnCancellationAsync(cancellationToken).ConfigureAwait(false); } /// @@ -304,9 +302,7 @@ public static async Task RunAndCaptureTextAsync(ProcessStartI throw; } - ProcessExitStatus exitStatus = cancellationToken.CanBeCanceled - ? await process.SafeHandle.WaitForExitOrKillOnCancellationAsync(cancellationToken).ConfigureAwait(false) - : await process.SafeHandle.WaitForExitAsync(cancellationToken).ConfigureAwait(false); + ProcessExitStatus exitStatus = await process.SafeHandle.WaitForExitOrKillOnCancellationAsync(cancellationToken).ConfigureAwait(false); return new ProcessTextOutput(exitStatus, standardOutput, standardError, process.Id); } From f78f6dcbe80ae581a5a5dfe2e129d781afb765f4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 21:59:55 +0000 Subject: [PATCH 27/29] Audit and improve test coverage for all API parameter combinations Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/73a14058-c419-4277-b5e2-885c9c6c8607 Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com> --- .../tests/RunTests.cs | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/src/libraries/System.Diagnostics.Process/tests/RunTests.cs b/src/libraries/System.Diagnostics.Process/tests/RunTests.cs index f7dda81f2b4a4e..e925bc1a7caf32 100644 --- a/src/libraries/System.Diagnostics.Process/tests/RunTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/RunTests.cs @@ -146,6 +146,85 @@ public async Task RunAndCaptureText_EmptyOutput(bool useAsync) Assert.Equal(string.Empty, result.StandardError); } + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(false)] + [InlineData(true)] + public async Task RunAndCaptureText_WithTimeoutOrCancellation_CapturesOutput(bool useAsync) + { + using Process template = CreateProcess(static () => + { + Console.Write("captured"); + Console.Error.Write("errors"); + return RemoteExecutor.SuccessExitCode; + }); + + template.StartInfo.RedirectStandardOutput = true; + template.StartInfo.RedirectStandardError = true; + + ProcessTextOutput result; + if (useAsync) + { + using var cts = new CancellationTokenSource(TimeSpan.FromMinutes(1)); + result = await Process.RunAndCaptureTextAsync(template.StartInfo, cts.Token); + } + else + { + result = Process.RunAndCaptureText(template.StartInfo, TimeSpan.FromMinutes(1)); + } + + Assert.Equal(RemoteExecutor.SuccessExitCode, result.ExitStatus.ExitCode); + Assert.False(result.ExitStatus.Canceled); + Assert.Equal("captured", result.StandardOutput); + Assert.Equal("errors", result.StandardError); + } + + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(false)] + [InlineData(true)] + public async Task RunAndCaptureText_WithTimeoutOrCancellation_KillsLongRunningProcess(bool useAsync) + { + using Process template = CreateSleepProcess((int)TimeSpan.FromHours(1).TotalMilliseconds); + + template.StartInfo.RedirectStandardOutput = true; + template.StartInfo.RedirectStandardError = true; + + if (useAsync) + { + await Assert.ThrowsAnyAsync(async () => + await Process.RunAndCaptureTextAsync(template.StartInfo, new CancellationTokenSource(TimeSpan.FromMilliseconds(100)).Token)); + } + else + { + Assert.ThrowsAny(() => + Process.RunAndCaptureText(template.StartInfo, TimeSpan.FromMilliseconds(100))); + } + } + + [Theory] + [InlineData(false, false, true)] + [InlineData(false, true, false)] + [InlineData(false, false, false)] + [InlineData(true, false, true)] + [InlineData(true, true, false)] + [InlineData(true, false, false)] + public async Task RunAndCaptureText_NotRedirected_ThrowsInvalidOperationException(bool useAsync, bool redirectOutput, bool redirectError) + { + ProcessStartInfo startInfo = new("someprocess") + { + RedirectStandardOutput = redirectOutput, + RedirectStandardError = redirectError + }; + + if (useAsync) + { + await Assert.ThrowsAsync(() => Process.RunAndCaptureTextAsync(startInfo)); + } + else + { + Assert.Throws(() => Process.RunAndCaptureText(startInfo)); + } + } + [Theory] [InlineData(false)] [InlineData(true)] From c15a6ef389dc53bbb01723d885764ba0f35fab13 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 21 Apr 2026 22:01:11 +0000 Subject: [PATCH 28/29] Fix CancellationTokenSource leaks in tests by using 'using' statements Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/73a14058-c419-4277-b5e2-885c9c6c8607 Co-authored-by: danmoseley <6385855+danmoseley@users.noreply.github.com> --- .../System.Diagnostics.Process/tests/RunTests.cs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/RunTests.cs b/src/libraries/System.Diagnostics.Process/tests/RunTests.cs index e925bc1a7caf32..3bb8366d0df9d8 100644 --- a/src/libraries/System.Diagnostics.Process/tests/RunTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/RunTests.cs @@ -71,9 +71,16 @@ public async Task Run_WithTimeoutOrCancellation_KillsLongRunningProcess(bool use { using Process template = CreateSleepProcess((int)TimeSpan.FromHours(1).TotalMilliseconds); - ProcessExitStatus exitStatus = useAsync - ? await Process.RunAsync(template.StartInfo, new CancellationTokenSource(TimeSpan.FromMilliseconds(100)).Token) - : Process.Run(template.StartInfo, TimeSpan.FromMilliseconds(100)); + ProcessExitStatus exitStatus; + if (useAsync) + { + using var cts = new CancellationTokenSource(TimeSpan.FromMilliseconds(100)); + exitStatus = await Process.RunAsync(template.StartInfo, cts.Token); + } + else + { + exitStatus = Process.Run(template.StartInfo, TimeSpan.FromMilliseconds(100)); + } Assert.True(exitStatus.Canceled); } @@ -190,8 +197,9 @@ public async Task RunAndCaptureText_WithTimeoutOrCancellation_KillsLongRunningPr if (useAsync) { + using var cts = new CancellationTokenSource(TimeSpan.FromMilliseconds(100)); await Assert.ThrowsAnyAsync(async () => - await Process.RunAndCaptureTextAsync(template.StartInfo, new CancellationTokenSource(TimeSpan.FromMilliseconds(100)).Token)); + await Process.RunAndCaptureTextAsync(template.StartInfo, cts.Token)); } else { From a68f2f23106304b9543eb21289cf1a6275a96cc5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 22 Apr 2026 04:13:41 +0000 Subject: [PATCH 29/29] Implement Process.Run, RunAsync, RunAndCaptureText, RunAndCaptureTextAsync APIs - Run/RunAsync: synchronously/asynchronously starts a process, waits for exit, returns ProcessExitStatus - RunAndCaptureText/RunAndCaptureTextAsync: starts a process, captures stdout/stderr, returns ProcessTextOutput - PSI overloads for RunAndCapture* require RedirectStandardOutput and RedirectStandardError to be enabled - String overloads use CreateStartInfoForCapture helper to set redirect flags - All async methods always use WaitForExitOrKillOnCancellationAsync - Comprehensive tests for all API parameters and permutations Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../ref/System.Diagnostics.Process.cs | 40 ++ .../src/Resources/Strings.resx | 7 +- .../src/System.Diagnostics.Process.csproj | 1 + .../System/Diagnostics/Process.Scenarios.cs | 286 +++++++++++++- .../System/Diagnostics/ProcessTextOutput.cs | 53 +++ .../tests/Helpers.cs | 46 +++ .../tests/RunTests.cs | 360 ++++++++++++++++++ .../tests/StartAndForget.cs | 45 +-- .../System.Diagnostics.Process.Tests.csproj | 1 + 9 files changed, 787 insertions(+), 52 deletions(-) create mode 100644 src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessTextOutput.cs create mode 100644 src/libraries/System.Diagnostics.Process/tests/RunTests.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 9116d196197614..0907c2dc7b69f1 100644 --- a/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs +++ b/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs @@ -176,6 +176,38 @@ protected void OnExited() { } public void Refresh() { } [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] + [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] + public static System.Diagnostics.ProcessExitStatus Run(System.Diagnostics.ProcessStartInfo startInfo, System.TimeSpan? timeout = default(System.TimeSpan?)) { throw null; } + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] + [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] + public static System.Diagnostics.ProcessExitStatus Run(string fileName, System.Collections.Generic.IList? arguments = null, System.TimeSpan? timeout = default(System.TimeSpan?)) { throw null; } + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] + [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] + public static System.Diagnostics.ProcessTextOutput RunAndCaptureText(System.Diagnostics.ProcessStartInfo startInfo, System.TimeSpan? timeout = default(System.TimeSpan?)) { throw null; } + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] + [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] + public static System.Diagnostics.ProcessTextOutput RunAndCaptureText(string fileName, System.Collections.Generic.IList? arguments = null, System.TimeSpan? timeout = default(System.TimeSpan?)) { throw null; } + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] + [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] + public static System.Threading.Tasks.Task RunAndCaptureTextAsync(System.Diagnostics.ProcessStartInfo startInfo, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] + [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] + public static System.Threading.Tasks.Task RunAndCaptureTextAsync(string fileName, System.Collections.Generic.IList? arguments = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] + [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] + public static System.Threading.Tasks.Task RunAsync(System.Diagnostics.ProcessStartInfo startInfo, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; } + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] + [System.Runtime.Versioning.SupportedOSPlatformAttribute("maccatalyst")] + public static System.Threading.Tasks.Task RunAsync(string fileName, System.Collections.Generic.IList? arguments = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { 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 public bool Start() { throw null; } [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] @@ -320,6 +352,14 @@ public ProcessStartInfo(string fileName, System.Collections.Generic.IEnumerable< [System.Diagnostics.CodeAnalysis.AllowNullAttribute] public string WorkingDirectory { get { throw null; } set { } } } + public sealed partial class ProcessTextOutput + { + public ProcessTextOutput(System.Diagnostics.ProcessExitStatus exitStatus, string standardOutput, string standardError, int processId) { throw null; } + public System.Diagnostics.ProcessExitStatus ExitStatus { get { throw null; } } + public int ProcessId { get { throw null; } } + public string StandardError { get { throw null; } } + public string StandardOutput { get { throw null; } } + } [System.ComponentModel.DesignerAttribute("System.Diagnostics.Design.ProcessThreadDesigner, System.Design, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a")] public partial class ProcessThread : System.ComponentModel.Component { diff --git a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx index da63478da67736..92a6dd8e10675e 100644 --- a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx +++ b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx @@ -363,8 +363,11 @@ InheritedHandles must not contain duplicates. - - UseShellExecute is not supported by StartAndForget. On Windows, shell execution may not create a new process, which would make it impossible to return a valid process ID. + + UseShellExecute is not supported by Process.{0}. On Windows, shell execution may not create a new process, which would make it impossible to return a valid process ID. + + + RedirectStandardOutput and RedirectStandardError must both be set to true when calling Process.{0} with a ProcessStartInfo. On Unix, it's impossible to obtain the exit status of a non-child process. diff --git a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj index 9e820236325805..bc1e3b89bb10de 100644 --- a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -24,6 +24,7 @@ + diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Scenarios.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Scenarios.cs index 9df4a1f51e50dc..e2a251d4373416 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Scenarios.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Scenarios.cs @@ -3,6 +3,8 @@ using System.Collections.Generic; using System.Runtime.Versioning; +using System.Threading; +using System.Threading.Tasks; using Microsoft.Win32.SafeHandles; namespace System.Diagnostics @@ -46,10 +48,7 @@ public static int StartAndForget(ProcessStartInfo startInfo) { ArgumentNullException.ThrowIfNull(startInfo); - if (startInfo.UseShellExecute) - { - throw new InvalidOperationException(SR.StartAndForget_UseShellExecuteNotSupported); - } + ThrowIfUseShellExecute(startInfo, nameof(StartAndForget)); using SafeProcessHandle processHandle = SafeProcessHandle.Start(startInfo, fallbackToNull: true); return processHandle.ProcessId; @@ -81,8 +80,258 @@ public static int StartAndForget(ProcessStartInfo startInfo) [UnsupportedOSPlatform("tvos")] [SupportedOSPlatform("maccatalyst")] public static int StartAndForget(string fileName, IList? arguments = null) + => StartAndForget(CreateStartInfo(fileName, arguments)); + + /// + /// Starts the process described by , waits for it to exit, and returns its exit status. + /// + /// The that contains the information used to start the process. + /// + /// The maximum amount of time to wait for the process to exit. + /// When , waits indefinitely. + /// If the process does not exit within the specified timeout, it is killed. + /// + /// The exit status of the process. + /// is . + /// + /// is set to . + /// + [UnsupportedOSPlatform("ios")] + [UnsupportedOSPlatform("tvos")] + [SupportedOSPlatform("maccatalyst")] + public static ProcessExitStatus Run(ProcessStartInfo startInfo, TimeSpan? timeout = default) + { + ArgumentNullException.ThrowIfNull(startInfo); + + ThrowIfUseShellExecute(startInfo, nameof(Run)); + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(startInfo); + + return timeout.HasValue + ? processHandle.WaitForExitOrKillOnTimeout(timeout.Value) + : processHandle.WaitForExit(); + } + + /// + /// Starts a process with the specified file name and optional arguments, waits for it to exit, and returns its exit status. + /// + /// The name of the application or document to start. + /// + /// The command-line arguments to pass to the process. Pass or an empty list + /// to start the process without additional arguments. + /// + /// + /// The maximum amount of time to wait for the process to exit. + /// When , waits indefinitely. + /// If the process does not exit within the specified timeout, it is killed. + /// + /// The exit status of the process. + /// is . + /// is empty. + [UnsupportedOSPlatform("ios")] + [UnsupportedOSPlatform("tvos")] + [SupportedOSPlatform("maccatalyst")] + public static ProcessExitStatus Run(string fileName, IList? arguments = null, TimeSpan? timeout = default) + => Run(CreateStartInfo(fileName, arguments), timeout); + + /// + /// Asynchronously starts the process described by , waits for it to exit, and returns its exit status. + /// + /// The that contains the information used to start the process. + /// + /// A token to cancel the asynchronous operation. + /// If the token is canceled, the process is killed. + /// + /// A task that represents the asynchronous operation. The value of the task contains the exit status of the process. + /// is . + /// + /// is set to . + /// + [UnsupportedOSPlatform("ios")] + [UnsupportedOSPlatform("tvos")] + [SupportedOSPlatform("maccatalyst")] + public static async Task RunAsync(ProcessStartInfo startInfo, CancellationToken cancellationToken = default) + { + ArgumentNullException.ThrowIfNull(startInfo); + + ThrowIfUseShellExecute(startInfo, nameof(RunAsync)); + + using SafeProcessHandle processHandle = SafeProcessHandle.Start(startInfo); + + return await processHandle.WaitForExitOrKillOnCancellationAsync(cancellationToken).ConfigureAwait(false); + } + + /// + /// Asynchronously starts a process with the specified file name and optional arguments, waits for it to exit, and returns its exit status. + /// + /// The name of the application or document to start. + /// + /// The command-line arguments to pass to the process. Pass or an empty list + /// to start the process without additional arguments. + /// + /// + /// A token to cancel the asynchronous operation. + /// If the token is canceled, the process is killed. + /// + /// A task that represents the asynchronous operation. The value of the task contains the exit status of the process. + /// is . + /// is empty. + [UnsupportedOSPlatform("ios")] + [UnsupportedOSPlatform("tvos")] + [SupportedOSPlatform("maccatalyst")] + public static Task RunAsync(string fileName, IList? arguments = null, CancellationToken cancellationToken = default) + => RunAsync(CreateStartInfo(fileName, arguments), cancellationToken); + + /// + /// Starts the process described by , captures its standard output and error, + /// waits for it to exit, and returns the captured text and exit status. + /// + /// The that contains the information used to start the process. + /// + /// The maximum amount of time to wait for the process to exit. + /// When , waits indefinitely. + /// If the process does not exit within the specified timeout, it is killed. + /// + /// The captured text output and exit status of the process. + /// is . + /// + /// is set to . + /// -or- + /// or is not set to . + /// + [UnsupportedOSPlatform("ios")] + [UnsupportedOSPlatform("tvos")] + [SupportedOSPlatform("maccatalyst")] + public static ProcessTextOutput RunAndCaptureText(ProcessStartInfo startInfo, TimeSpan? timeout = default) + { + ArgumentNullException.ThrowIfNull(startInfo); + + ThrowIfUseShellExecute(startInfo, nameof(RunAndCaptureText)); + ThrowIfNotRedirected(startInfo, nameof(RunAndCaptureText)); + + long startTimestamp = Stopwatch.GetTimestamp(); + + using Process process = Start(startInfo)!; + + string standardOutput, standardError; + try + { + (standardOutput, standardError) = process.ReadAllText(timeout); + } + catch + { + try { process.Kill(); } catch { } + throw; + } + + ProcessExitStatus exitStatus; + if (timeout.HasValue) + { + TimeSpan elapsed = Stopwatch.GetElapsedTime(startTimestamp); + TimeSpan remaining = timeout.Value - elapsed; + remaining = remaining >= TimeSpan.Zero ? remaining : TimeSpan.Zero; + exitStatus = process.SafeHandle.WaitForExitOrKillOnTimeout(remaining); + } + else + { + exitStatus = process.SafeHandle.WaitForExit(); + } + + return new ProcessTextOutput(exitStatus, standardOutput, standardError, process.Id); + } + + /// + /// Starts a process with the specified file name and optional arguments, captures its standard output and error, + /// waits for it to exit, and returns the captured text and exit status. + /// + /// The name of the application or document to start. + /// + /// The command-line arguments to pass to the process. Pass or an empty list + /// to start the process without additional arguments. + /// + /// + /// The maximum amount of time to wait for the process to exit. + /// When , waits indefinitely. + /// If the process does not exit within the specified timeout, it is killed. + /// + /// The captured text output and exit status of the process. + /// is . + /// is empty. + [UnsupportedOSPlatform("ios")] + [UnsupportedOSPlatform("tvos")] + [SupportedOSPlatform("maccatalyst")] + public static ProcessTextOutput RunAndCaptureText(string fileName, IList? arguments = null, TimeSpan? timeout = default) + => RunAndCaptureText(CreateStartInfoForCapture(fileName, arguments), timeout); + + /// + /// Asynchronously starts the process described by , captures its standard output and error, + /// waits for it to exit, and returns the captured text and exit status. + /// + /// The that contains the information used to start the process. + /// + /// A token to cancel the asynchronous operation. + /// If the token is canceled, the process is killed. + /// + /// A task that represents the asynchronous operation. The value of the task contains the captured text output and exit status of the process. + /// is . + /// + /// is set to . + /// -or- + /// or is not set to . + /// + [UnsupportedOSPlatform("ios")] + [UnsupportedOSPlatform("tvos")] + [SupportedOSPlatform("maccatalyst")] + public static async Task RunAndCaptureTextAsync(ProcessStartInfo startInfo, CancellationToken cancellationToken = default) { - ArgumentNullException.ThrowIfNull(fileName); + ArgumentNullException.ThrowIfNull(startInfo); + + ThrowIfUseShellExecute(startInfo, nameof(RunAndCaptureTextAsync)); + ThrowIfNotRedirected(startInfo, nameof(RunAndCaptureTextAsync)); + + using Process process = Start(startInfo)!; + + string standardOutput, standardError; + try + { + (standardOutput, standardError) = await process.ReadAllTextAsync(cancellationToken).ConfigureAwait(false); + } + catch + { + try { process.Kill(); } catch { } + throw; + } + + ProcessExitStatus exitStatus = await process.SafeHandle.WaitForExitOrKillOnCancellationAsync(cancellationToken).ConfigureAwait(false); + + return new ProcessTextOutput(exitStatus, standardOutput, standardError, process.Id); + } + + /// + /// Asynchronously starts a process with the specified file name and optional arguments, captures its standard output and error, + /// waits for it to exit, and returns the captured text and exit status. + /// + /// The name of the application or document to start. + /// + /// The command-line arguments to pass to the process. Pass or an empty list + /// to start the process without additional arguments. + /// + /// + /// A token to cancel the asynchronous operation. + /// If the token is canceled, the process is killed. + /// + /// A task that represents the asynchronous operation. The value of the task contains the captured text output and exit status of the process. + /// is . + /// is empty. + [UnsupportedOSPlatform("ios")] + [UnsupportedOSPlatform("tvos")] + [SupportedOSPlatform("maccatalyst")] + public static Task RunAndCaptureTextAsync(string fileName, IList? arguments = null, CancellationToken cancellationToken = default) + => RunAndCaptureTextAsync(CreateStartInfoForCapture(fileName, arguments), cancellationToken); + + private static ProcessStartInfo CreateStartInfo(string fileName, IList? arguments) + { + ArgumentException.ThrowIfNullOrEmpty(fileName); ProcessStartInfo startInfo = new(fileName); if (arguments is not null) @@ -93,7 +342,32 @@ public static int StartAndForget(string fileName, IList? arguments = nul } } - return StartAndForget(startInfo); + return startInfo; + } + + private static ProcessStartInfo CreateStartInfoForCapture(string fileName, IList? arguments) + { + ProcessStartInfo startInfo = CreateStartInfo(fileName, arguments); + startInfo.RedirectStandardOutput = true; + startInfo.RedirectStandardError = true; + + return startInfo; + } + + private static void ThrowIfUseShellExecute(ProcessStartInfo startInfo, string methodName) + { + if (startInfo.UseShellExecute) + { + throw new InvalidOperationException(SR.Format(SR.UseShellExecuteNotSupportedForScenario, methodName)); + } + } + + private static void ThrowIfNotRedirected(ProcessStartInfo startInfo, string methodName) + { + if (!startInfo.RedirectStandardOutput || !startInfo.RedirectStandardError) + { + throw new InvalidOperationException(SR.Format(SR.RedirectStandardOutputAndErrorRequired, methodName)); + } } } } diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessTextOutput.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessTextOutput.cs new file mode 100644 index 00000000000000..d094cfb852c00b --- /dev/null +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessTextOutput.cs @@ -0,0 +1,53 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Diagnostics +{ + /// + /// Represents the captured text output and exit status of a completed process. + /// + public sealed class ProcessTextOutput + { + /// + /// Initializes a new instance of the class. + /// + /// The exit status of the process. + /// The captured standard output text. + /// The captured standard error text. + /// The process ID of the completed process. + /// + /// , , or is . + /// + public ProcessTextOutput(ProcessExitStatus exitStatus, string standardOutput, string standardError, int processId) + { + ArgumentNullException.ThrowIfNull(exitStatus); + ArgumentNullException.ThrowIfNull(standardOutput); + ArgumentNullException.ThrowIfNull(standardError); + + ExitStatus = exitStatus; + StandardOutput = standardOutput; + StandardError = standardError; + ProcessId = processId; + } + + /// + /// Gets the exit status of the process. + /// + public ProcessExitStatus ExitStatus { get; } + + /// + /// Gets the captured standard output text of the process. + /// + public string StandardOutput { get; } + + /// + /// Gets the captured standard error text of the process. + /// + public string StandardError { get; } + + /// + /// Gets the process ID of the completed process. + /// + public int ProcessId { get; } + } +} diff --git a/src/libraries/System.Diagnostics.Process/tests/Helpers.cs b/src/libraries/System.Diagnostics.Process/tests/Helpers.cs index cf5507a5ee26fc..5b3e0d2e0e0292 100644 --- a/src/libraries/System.Diagnostics.Process/tests/Helpers.cs +++ b/src/libraries/System.Diagnostics.Process/tests/Helpers.cs @@ -2,8 +2,10 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections.Generic; using System.ComponentModel; using System.Security.Principal; +using System.Text; using System.Threading.Tasks; using Xunit.Sdk; @@ -11,6 +13,50 @@ namespace System.Diagnostics.Tests { internal static class Helpers { + // RemoteExecutor populates ProcessStartInfo.Arguments, but the fileName overloads + // take an argument list, so this helper maps the serialized argument string. + public static List? MapToArgumentList(ProcessStartInfo startInfo) + { + string arguments = startInfo.Arguments; + if (string.IsNullOrEmpty(arguments)) + { + return null; + } + + List list = new(); + StringBuilder builder = new(); + bool isQuoted = false; + + foreach (char c in arguments) + { + switch (c) + { + case '"' when !isQuoted: + isQuoted = true; + break; + case ' ' when !isQuoted: + case '"' when isQuoted: + if (builder.Length > 0) + { + list.Add(builder.ToString()); + builder.Clear(); + } + isQuoted = false; + break; + default: + builder.Append(c); + break; + } + } + + if (builder.Length > 0) + { + list.Add(builder.ToString()); + } + + return list; + } + public const int PassingTestTimeoutMilliseconds = 60_000; public static async Task RetryWithBackoff(Action action, int delayInMilliseconds = 10, int times = 10) diff --git a/src/libraries/System.Diagnostics.Process/tests/RunTests.cs b/src/libraries/System.Diagnostics.Process/tests/RunTests.cs new file mode 100644 index 00000000000000..3bb8366d0df9d8 --- /dev/null +++ b/src/libraries/System.Diagnostics.Process/tests/RunTests.cs @@ -0,0 +1,360 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.DotNet.RemoteExecutor; +using Xunit; + +namespace System.Diagnostics.Tests +{ + public class RunTests : ProcessTestBase + { + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(false)] + [InlineData(true)] + public async Task Run_ExitCodeIsReturned(bool useAsync) + { + using Process template = CreateProcess(static () => RemoteExecutor.SuccessExitCode); + + ProcessExitStatus exitStatus = useAsync + ? await Process.RunAsync(template.StartInfo) + : Process.Run(template.StartInfo); + + Assert.Equal(RemoteExecutor.SuccessExitCode, exitStatus.ExitCode); + Assert.False(exitStatus.Canceled); + } + + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(false)] + [InlineData(true)] + public async Task Run_WithFileName_ExitCodeIsReturned(bool useAsync) + { + using Process template = CreateProcess(static () => RemoteExecutor.SuccessExitCode); + List? arguments = Helpers.MapToArgumentList(template.StartInfo); + + ProcessExitStatus exitStatus = useAsync + ? await Process.RunAsync(template.StartInfo.FileName, arguments) + : Process.Run(template.StartInfo.FileName, arguments); + + Assert.Equal(RemoteExecutor.SuccessExitCode, exitStatus.ExitCode); + Assert.False(exitStatus.Canceled); + } + + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(false)] + [InlineData(true)] + public async Task Run_WithTimeout_ExitCodeIsReturned(bool useAsync) + { + using Process template = CreateProcess(static () => RemoteExecutor.SuccessExitCode); + + ProcessExitStatus exitStatus; + if (useAsync) + { + using var cts = new CancellationTokenSource(TimeSpan.FromMinutes(1)); + exitStatus = await Process.RunAsync(template.StartInfo, cts.Token); + } + else + { + exitStatus = Process.Run(template.StartInfo, TimeSpan.FromMinutes(1)); + } + + Assert.Equal(RemoteExecutor.SuccessExitCode, exitStatus.ExitCode); + Assert.False(exitStatus.Canceled); + } + + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(false)] + [InlineData(true)] + public async Task Run_WithTimeoutOrCancellation_KillsLongRunningProcess(bool useAsync) + { + using Process template = CreateSleepProcess((int)TimeSpan.FromHours(1).TotalMilliseconds); + + ProcessExitStatus exitStatus; + if (useAsync) + { + using var cts = new CancellationTokenSource(TimeSpan.FromMilliseconds(100)); + exitStatus = await Process.RunAsync(template.StartInfo, cts.Token); + } + else + { + exitStatus = Process.Run(template.StartInfo, TimeSpan.FromMilliseconds(100)); + } + + Assert.True(exitStatus.Canceled); + } + + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(false)] + [InlineData(true)] + public async Task RunAndCaptureText_CapturesOutput(bool useAsync) + { + using Process template = CreateProcess(static () => + { + Console.Write("hello"); + Console.Error.Write("world"); + return RemoteExecutor.SuccessExitCode; + }); + + template.StartInfo.RedirectStandardOutput = true; + template.StartInfo.RedirectStandardError = true; + + ProcessTextOutput result = useAsync + ? await Process.RunAndCaptureTextAsync(template.StartInfo) + : Process.RunAndCaptureText(template.StartInfo); + + Assert.Equal(RemoteExecutor.SuccessExitCode, result.ExitStatus.ExitCode); + Assert.False(result.ExitStatus.Canceled); + Assert.Equal("hello", result.StandardOutput); + Assert.Equal("world", result.StandardError); + Assert.True(result.ProcessId > 0); + } + + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(false)] + [InlineData(true)] + public async Task RunAndCaptureText_WithFileName_CapturesOutput(bool useAsync) + { + using Process template = CreateProcess(static () => + { + Console.Write("output"); + Console.Error.Write("error"); + return RemoteExecutor.SuccessExitCode; + }); + + List? arguments = Helpers.MapToArgumentList(template.StartInfo); + + ProcessTextOutput result = useAsync + ? await Process.RunAndCaptureTextAsync(template.StartInfo.FileName, arguments) + : Process.RunAndCaptureText(template.StartInfo.FileName, arguments); + + Assert.Equal(RemoteExecutor.SuccessExitCode, result.ExitStatus.ExitCode); + Assert.Equal("output", result.StandardOutput); + Assert.Equal("error", result.StandardError); + } + + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(false)] + [InlineData(true)] + public async Task RunAndCaptureText_EmptyOutput(bool useAsync) + { + using Process template = CreateProcess(static () => RemoteExecutor.SuccessExitCode); + + template.StartInfo.RedirectStandardOutput = true; + template.StartInfo.RedirectStandardError = true; + + ProcessTextOutput result = useAsync + ? await Process.RunAndCaptureTextAsync(template.StartInfo) + : Process.RunAndCaptureText(template.StartInfo); + + Assert.Equal(RemoteExecutor.SuccessExitCode, result.ExitStatus.ExitCode); + Assert.Equal(string.Empty, result.StandardOutput); + Assert.Equal(string.Empty, result.StandardError); + } + + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(false)] + [InlineData(true)] + public async Task RunAndCaptureText_WithTimeoutOrCancellation_CapturesOutput(bool useAsync) + { + using Process template = CreateProcess(static () => + { + Console.Write("captured"); + Console.Error.Write("errors"); + return RemoteExecutor.SuccessExitCode; + }); + + template.StartInfo.RedirectStandardOutput = true; + template.StartInfo.RedirectStandardError = true; + + ProcessTextOutput result; + if (useAsync) + { + using var cts = new CancellationTokenSource(TimeSpan.FromMinutes(1)); + result = await Process.RunAndCaptureTextAsync(template.StartInfo, cts.Token); + } + else + { + result = Process.RunAndCaptureText(template.StartInfo, TimeSpan.FromMinutes(1)); + } + + Assert.Equal(RemoteExecutor.SuccessExitCode, result.ExitStatus.ExitCode); + Assert.False(result.ExitStatus.Canceled); + Assert.Equal("captured", result.StandardOutput); + Assert.Equal("errors", result.StandardError); + } + + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(false)] + [InlineData(true)] + public async Task RunAndCaptureText_WithTimeoutOrCancellation_KillsLongRunningProcess(bool useAsync) + { + using Process template = CreateSleepProcess((int)TimeSpan.FromHours(1).TotalMilliseconds); + + template.StartInfo.RedirectStandardOutput = true; + template.StartInfo.RedirectStandardError = true; + + if (useAsync) + { + using var cts = new CancellationTokenSource(TimeSpan.FromMilliseconds(100)); + await Assert.ThrowsAnyAsync(async () => + await Process.RunAndCaptureTextAsync(template.StartInfo, cts.Token)); + } + else + { + Assert.ThrowsAny(() => + Process.RunAndCaptureText(template.StartInfo, TimeSpan.FromMilliseconds(100))); + } + } + + [Theory] + [InlineData(false, false, true)] + [InlineData(false, true, false)] + [InlineData(false, false, false)] + [InlineData(true, false, true)] + [InlineData(true, true, false)] + [InlineData(true, false, false)] + public async Task RunAndCaptureText_NotRedirected_ThrowsInvalidOperationException(bool useAsync, bool redirectOutput, bool redirectError) + { + ProcessStartInfo startInfo = new("someprocess") + { + RedirectStandardOutput = redirectOutput, + RedirectStandardError = redirectError + }; + + if (useAsync) + { + await Assert.ThrowsAsync(() => Process.RunAndCaptureTextAsync(startInfo)); + } + else + { + Assert.Throws(() => Process.RunAndCaptureText(startInfo)); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Run_NullStartInfo_ThrowsArgumentNullException(bool useAsync) + { + if (useAsync) + { + await Assert.ThrowsAsync("startInfo", () => Process.RunAsync((ProcessStartInfo)null!)); + } + else + { + AssertExtensions.Throws("startInfo", () => Process.Run((ProcessStartInfo)null!)); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Run_NullFileName_ThrowsArgumentNullException(bool useAsync) + { + if (useAsync) + { + await Assert.ThrowsAsync("fileName", () => Process.RunAsync((string)null!)); + } + else + { + AssertExtensions.Throws("fileName", () => Process.Run((string)null!)); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Run_EmptyFileName_ThrowsArgumentException(bool useAsync) + { + if (useAsync) + { + await Assert.ThrowsAsync("fileName", () => Process.RunAsync(string.Empty)); + } + else + { + AssertExtensions.Throws("fileName", () => Process.Run(string.Empty)); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task RunAndCaptureText_NullStartInfo_ThrowsArgumentNullException(bool useAsync) + { + if (useAsync) + { + await Assert.ThrowsAsync("startInfo", () => Process.RunAndCaptureTextAsync((ProcessStartInfo)null!)); + } + else + { + AssertExtensions.Throws("startInfo", () => Process.RunAndCaptureText((ProcessStartInfo)null!)); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task RunAndCaptureText_NullFileName_ThrowsArgumentNullException(bool useAsync) + { + if (useAsync) + { + await Assert.ThrowsAsync("fileName", () => Process.RunAndCaptureTextAsync((string)null!)); + } + else + { + AssertExtensions.Throws("fileName", () => Process.RunAndCaptureText((string)null!)); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task RunAndCaptureText_EmptyFileName_ThrowsArgumentException(bool useAsync) + { + if (useAsync) + { + await Assert.ThrowsAsync("fileName", () => Process.RunAndCaptureTextAsync(string.Empty)); + } + else + { + AssertExtensions.Throws("fileName", () => Process.RunAndCaptureText(string.Empty)); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Run_UseShellExecute_ThrowsInvalidOperationException(bool useAsync) + { + ProcessStartInfo startInfo = new("someprocess") { UseShellExecute = true }; + + if (useAsync) + { + await Assert.ThrowsAsync(() => Process.RunAsync(startInfo)); + } + else + { + Assert.Throws(() => Process.Run(startInfo)); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task RunAndCaptureText_UseShellExecute_ThrowsInvalidOperationException(bool useAsync) + { + ProcessStartInfo startInfo = new("someprocess") { UseShellExecute = true }; + + if (useAsync) + { + await Assert.ThrowsAsync(() => Process.RunAndCaptureTextAsync(startInfo)); + } + else + { + Assert.Throws(() => Process.RunAndCaptureText(startInfo)); + } + } + } +} diff --git a/src/libraries/System.Diagnostics.Process/tests/StartAndForget.cs b/src/libraries/System.Diagnostics.Process/tests/StartAndForget.cs index dc20e3fd9a8a45..ebdd85e9833275 100644 --- a/src/libraries/System.Diagnostics.Process/tests/StartAndForget.cs +++ b/src/libraries/System.Diagnostics.Process/tests/StartAndForget.cs @@ -3,7 +3,6 @@ using System.Collections.Generic; using System.IO; -using System.Text; using Microsoft.DotNet.RemoteExecutor; using Microsoft.Win32.SafeHandles; using Xunit; @@ -20,7 +19,7 @@ public void StartAndForget_StartsProcessAndReturnsValidPid(bool useProcessStartI using Process template = CreateSleepProcess((int)TimeSpan.FromHours(1).TotalMilliseconds); int pid = useProcessStartInfo ? Process.StartAndForget(template.StartInfo) - : Process.StartAndForget(template.StartInfo.FileName, MapToArgumentList(template.StartInfo)); + : Process.StartAndForget(template.StartInfo.FileName, Helpers.MapToArgumentList(template.StartInfo)); Assert.True(pid > 0); @@ -113,48 +112,6 @@ public void StartAndForget_WithUseShellExecute_ThrowsInvalidOperationException() Assert.Throws(() => Process.StartAndForget(startInfo)); } - // RemoteExecutor populates ProcessStartInfo.Arguments, but StartAndForget(fileName, arguments) - // takes an argument list, so this helper maps the serialized argument string for this test. - private static List? MapToArgumentList(ProcessStartInfo startInfo) - { - string arguments = startInfo.Arguments; - if (string.IsNullOrEmpty(arguments)) - { - return null; - } - List list = new(); - StringBuilder builder = new(); - bool isQuoted = false; - - foreach (char c in arguments) - { - switch (c) - { - case '"' when !isQuoted: - isQuoted = true; - break; - case ' ' when !isQuoted: - case '"' when isQuoted: - if (builder.Length > 0) - { - list.Add(builder.ToString()); - builder.Clear(); - } - isQuoted = false; - break; - default: - builder.Append(c); - break; - } - } - - if (builder.Length > 0) - { - list.Add(builder.ToString()); - } - - return list; - } } } 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 a9332ad98c999f..f1ed6055933db3 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 @@ -39,6 +39,7 @@ +