From 6946997e2d500c310f6a80fc8e935daeca3faf18 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 12 Apr 2026 07:31:08 +0000 Subject: [PATCH 01/19] Add Process.ReadAllText and ReadAllBytes with platform-specific multiplexing Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/45ece5ba-166f-4418-887f-3ecaba54615e Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../ref/System.Diagnostics.Process.cs | 2 + .../src/System.Diagnostics.Process.csproj | 7 + .../Diagnostics/Process.Multiplexing.Unix.cs | 131 ++++++++++ .../Process.Multiplexing.Windows.cs | 107 ++++++++ .../Diagnostics/Process.Multiplexing.cs | 234 +++++++++++++++++ .../tests/ProcessMultiplexingTests.cs | 247 ++++++++++++++++++ .../System.Diagnostics.Process.Tests.csproj | 1 + 7 files changed, 729 insertions(+) create mode 100644 src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs create mode 100644 src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs create mode 100644 src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.cs create mode 100644 src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.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 7257744b6de56c..fcd33485d43671 100644 --- a/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs +++ b/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs @@ -185,6 +185,8 @@ public void Refresh() { } [System.Runtime.Versioning.SupportedOSPlatformAttribute("windows")] public static System.Diagnostics.Process? Start(string fileName, string arguments, string userName, System.Security.SecureString password, string domain) { throw null; } public override string ToString() { throw null; } + public (string StandardOutput, string StandardError) ReadAllText(System.TimeSpan? timeout = default(System.TimeSpan?)) { throw null; } + public (byte[] StandardOutput, byte[] StandardError) ReadAllBytes(System.TimeSpan? timeout = default(System.TimeSpan?)) { throw null; } public void WaitForExit() { } public bool WaitForExit(int milliseconds) { throw null; } public bool WaitForExit(System.TimeSpan timeout) { throw null; } diff --git a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj index 2c1c9e0be2eb23..a6681a99050bb3 100644 --- a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -20,6 +20,7 @@ + @@ -224,6 +225,7 @@ + @@ -236,6 +238,7 @@ + @@ -246,6 +249,10 @@ Link="Common\Interop\Unix\Interop.Libraries.cs" /> + + + /// Reads from both standard output and standard error pipes using Unix poll-based multiplexing. + /// + private static partial void ReadBothPipes( + SafeFileHandle outputHandle, + SafeFileHandle errorHandle, + int timeoutMs, + ref byte[] outputBuffer, + ref int outputBytesRead, + ref byte[] errorBuffer, + ref int errorBytesRead) + { + int outputFd = outputHandle.DangerousGetHandle().ToInt32(); + int errorFd = errorHandle.DangerousGetHandle().ToInt32(); + bool outputDone = false; + bool errorDone = false; + + Interop.PollEvent[] pollFds = new Interop.PollEvent[2]; + + long deadline = timeoutMs >= 0 + ? Environment.TickCount64 + timeoutMs + : long.MaxValue; + + while (!outputDone || !errorDone) + { + int numFds = 0; + + int outputIndex = -1; + int errorIndex = -1; + + if (!outputDone) + { + outputIndex = numFds; + pollFds[numFds].FileDescriptor = outputFd; + pollFds[numFds].Events = Interop.PollEvents.POLLIN; + pollFds[numFds].TriggeredEvents = Interop.PollEvents.POLLNONE; + numFds++; + } + + if (!errorDone) + { + errorIndex = numFds; + pollFds[numFds].FileDescriptor = errorFd; + pollFds[numFds].Events = Interop.PollEvents.POLLIN; + pollFds[numFds].TriggeredEvents = Interop.PollEvents.POLLNONE; + numFds++; + } + + int pollTimeout; + if (timeoutMs >= 0) + { + long remaining = deadline - Environment.TickCount64; + if (remaining <= 0) + { + throw new TimeoutException(); + } + + pollTimeout = (int)Math.Min(remaining, int.MaxValue); + } + else + { + pollTimeout = -1; // Infinite + } + + unsafe + { + uint triggered; + fixed (Interop.PollEvent* pPollFds = pollFds) + { + Interop.Error error = Interop.Sys.Poll(pPollFds, (uint)numFds, pollTimeout, &triggered); + if (error != Interop.Error.SUCCESS) + { + if (error == Interop.Error.EINTR) + { + continue; + } + + throw new ComponentModel.Win32Exception(Marshal.GetLastPInvokeError()); + } + + if (triggered == 0) + { + throw new TimeoutException(); + } + } + } + + for (int i = 0; i < numFds; i++) + { + if ((pollFds[i].TriggeredEvents & (Interop.PollEvents.POLLIN | Interop.PollEvents.POLLHUP | Interop.PollEvents.POLLERR)) == Interop.PollEvents.POLLNONE) + { + continue; + } + + bool isError = i == errorIndex; + SafeFileHandle currentHandle = isError ? errorHandle : outputHandle; + ref byte[] currentBuffer = ref (isError ? ref errorBuffer : ref outputBuffer); + ref int currentBytesRead = ref (isError ? ref errorBytesRead : ref outputBytesRead); + ref bool currentDone = ref (isError ? ref errorDone : ref outputDone); + + int bytesRead = RandomAccess.Read(currentHandle, currentBuffer.AsSpan(currentBytesRead), fileOffset: -1); + if (bytesRead > 0) + { + currentBytesRead += bytesRead; + + if (currentBytesRead == currentBuffer.Length) + { + RentLargerBuffer(ref currentBuffer, currentBytesRead); + } + } + else + { + currentDone = true; + } + } + } + } + } +} diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs new file mode 100644 index 00000000000000..d4a738e9982e3a --- /dev/null +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs @@ -0,0 +1,107 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Win32.SafeHandles; + +namespace System.Diagnostics +{ + public partial class Process + { + /// + /// Reads from both standard output and standard error pipes using async IO with tasks. + /// On Windows, the underlying handles are opened for async IO, so async reads can be cancelled. + /// + private static partial void ReadBothPipes( + SafeFileHandle outputHandle, + SafeFileHandle errorHandle, + int timeoutMs, + ref byte[] outputBuffer, + ref int outputBytesRead, + ref byte[] errorBuffer, + ref int errorBytesRead) + { + CancellationTokenSource cts = timeoutMs >= 0 + ? new CancellationTokenSource(timeoutMs) + : new CancellationTokenSource(); + + CancellationToken cancellationToken = cts.Token; + + Task outputTask = RandomAccess.ReadAsync(outputHandle, outputBuffer.AsMemory(outputBytesRead), fileOffset: -1, cancellationToken).AsTask(); + Task errorTask = RandomAccess.ReadAsync(errorHandle, errorBuffer.AsMemory(errorBytesRead), fileOffset: -1, cancellationToken).AsTask(); + + bool outputDone = false; + bool errorDone = false; + + try + { + while (!outputDone || !errorDone) + { + bool isError; + int bytesRead; + + if (outputDone) + { + bytesRead = errorTask.GetAwaiter().GetResult(); + isError = true; + } + else if (errorDone) + { + bytesRead = outputTask.GetAwaiter().GetResult(); + isError = false; + } + else + { +#pragma warning disable CA2025 // Tasks complete or are cancelled before CTS disposal + Task completed = Task.WhenAny(outputTask, errorTask).GetAwaiter().GetResult(); +#pragma warning restore CA2025 + isError = completed == errorTask; + bytesRead = completed.GetAwaiter().GetResult(); + } + + if (bytesRead > 0) + { + if (isError) + { + errorBytesRead += bytesRead; + if (errorBytesRead == errorBuffer.Length) + { + RentLargerBuffer(ref errorBuffer, errorBytesRead); + } + errorTask = RandomAccess.ReadAsync(errorHandle, errorBuffer.AsMemory(errorBytesRead), fileOffset: -1, cancellationToken).AsTask(); + } + else + { + outputBytesRead += bytesRead; + if (outputBytesRead == outputBuffer.Length) + { + RentLargerBuffer(ref outputBuffer, outputBytesRead); + } + outputTask = RandomAccess.ReadAsync(outputHandle, outputBuffer.AsMemory(outputBytesRead), fileOffset: -1, cancellationToken).AsTask(); + } + } + else + { + if (isError) + { + errorDone = true; + } + else + { + outputDone = true; + } + } + } + + cts.Dispose(); + } + catch (OperationCanceledException) + { + cts.Dispose(); + throw new TimeoutException(); + } + } + } +} 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 new file mode 100644 index 00000000000000..ea932a723a9648 --- /dev/null +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.cs @@ -0,0 +1,234 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Buffers; +using System.IO; +using System.Text; +using Microsoft.Win32.SafeHandles; + +namespace System.Diagnostics +{ + public partial class Process + { + /// Initial buffer size for reading process output. + private const int InitialReadAllBufferSize = 4096; + + /// + /// Reads all standard output and standard error of the process as text. + /// + /// + /// The maximum amount of time to wait for the streams to be fully read. + /// When , waits indefinitely. + /// + /// + /// A tuple containing the standard output and standard error text. + /// When a stream was not redirected, the corresponding value is an empty string. + /// + /// + /// Neither standard output nor standard error has been redirected. + /// -or- + /// A redirected stream is already being read asynchronously. + /// + /// + /// The operation did not complete within the specified . + /// + /// + /// The process has been disposed. + /// + public (string StandardOutput, string StandardError) ReadAllText(TimeSpan? timeout = default) + { + (byte[] StandardOutput, byte[] StandardError) bytes = ReadAllBytes(timeout); + + Encoding outputEncoding = _startInfo?.StandardOutputEncoding ?? GetStandardOutputEncoding(); + Encoding errorEncoding = _startInfo?.StandardErrorEncoding ?? GetStandardOutputEncoding(); + + string standardOutput = bytes.StandardOutput.Length > 0 + ? outputEncoding.GetString(bytes.StandardOutput) + : string.Empty; + + string standardError = bytes.StandardError.Length > 0 + ? errorEncoding.GetString(bytes.StandardError) + : string.Empty; + + return (standardOutput, standardError); + } + + /// + /// Reads all standard output and standard error of the process as byte arrays. + /// + /// + /// The maximum amount of time to wait for the streams to be fully read. + /// When , waits indefinitely. + /// + /// + /// A tuple containing the standard output and standard error bytes. + /// When a stream was not redirected, the corresponding value is an empty array. + /// + /// + /// Neither standard output nor standard error has been redirected. + /// -or- + /// A redirected stream is already being read asynchronously. + /// + /// + /// The operation did not complete within the specified . + /// + /// + /// The process has been disposed. + /// + public (byte[] StandardOutput, byte[] StandardError) ReadAllBytes(TimeSpan? timeout = default) + { + CheckDisposed(); + + bool hasOutput = _standardOutput is not null; + bool hasError = _standardError is not null; + + if (!hasOutput && !hasError) + { + throw new InvalidOperationException(SR.CantGetStandardOut); + } + + if (hasOutput && _outputStreamReadMode == StreamReadMode.AsyncMode) + { + throw new InvalidOperationException(SR.CantMixSyncAsyncOperation); + } + + if (hasError && _errorStreamReadMode == StreamReadMode.AsyncMode) + { + throw new InvalidOperationException(SR.CantMixSyncAsyncOperation); + } + + if (hasOutput) + { + _outputStreamReadMode = StreamReadMode.SyncMode; + } + + if (hasError) + { + _errorStreamReadMode = StreamReadMode.SyncMode; + } + + int timeoutMs = timeout.HasValue + ? (int)timeout.Value.TotalMilliseconds + : -1; // Infinite + + byte[] outputBuffer = ArrayPool.Shared.Rent(InitialReadAllBufferSize); + byte[] errorBuffer = ArrayPool.Shared.Rent(InitialReadAllBufferSize); + int outputBytesRead = 0; + int errorBytesRead = 0; + + try + { + SafeFileHandle? outputHandle = hasOutput ? GetSafeFileHandleFromStreamReader(_standardOutput!) : null; + SafeFileHandle? errorHandle = hasError ? GetSafeFileHandleFromStreamReader(_standardError!) : null; + + if (hasOutput && hasError) + { + ReadBothPipes(outputHandle!, errorHandle!, timeoutMs, + ref outputBuffer, ref outputBytesRead, + ref errorBuffer, ref errorBytesRead); + } + else if (hasOutput) + { + ReadSinglePipe(outputHandle!, timeoutMs, ref outputBuffer, ref outputBytesRead); + } + else + { + ReadSinglePipe(errorHandle!, timeoutMs, ref errorBuffer, ref errorBytesRead); + } + + byte[] outputResult = outputBytesRead > 0 + ? outputBuffer.AsSpan(0, outputBytesRead).ToArray() + : Array.Empty(); + + byte[] errorResult = errorBytesRead > 0 + ? errorBuffer.AsSpan(0, errorBytesRead).ToArray() + : Array.Empty(); + + return (outputResult, errorResult); + } + finally + { + ArrayPool.Shared.Return(outputBuffer); + ArrayPool.Shared.Return(errorBuffer); + } + } + + /// + /// Obtains the from the underlying stream of a . + /// On Unix, the stream is an and the handle is obtained via the pipe handle. + /// On Windows, the stream is a opened for async IO. + /// + private static SafeFileHandle GetSafeFileHandleFromStreamReader(StreamReader reader) + { + Stream baseStream = reader.BaseStream; + + if (baseStream is FileStream fileStream) + { + return fileStream.SafeFileHandle; + } + + if (baseStream is System.IO.Pipes.AnonymousPipeClientStream pipeStream) + { + return new SafeFileHandle(pipeStream.SafePipeHandle.DangerousGetHandle(), ownsHandle: false); + } + + throw new InvalidOperationException(); + } + + /// + /// Reads from a single pipe using until EOF or timeout. + /// + private static void ReadSinglePipe(SafeFileHandle handle, int timeoutMs, ref byte[] buffer, ref int bytesRead) + { + long deadline = timeoutMs >= 0 + ? Environment.TickCount64 + timeoutMs + : long.MaxValue; + + while (true) + { + int remaining = buffer.Length - bytesRead; + + int read = RandomAccess.Read(handle, buffer.AsSpan(bytesRead, remaining), fileOffset: -1); + if (read == 0) + { + break; // EOF + } + + bytesRead += read; + + if (bytesRead == buffer.Length) + { + RentLargerBuffer(ref buffer, bytesRead); + } + + if (Environment.TickCount64 >= deadline) + { + throw new TimeoutException(); + } + } + } + + /// + /// Reads from both standard output and standard error pipes using platform-specific multiplexing. + /// + private static partial void ReadBothPipes( + SafeFileHandle outputHandle, + SafeFileHandle errorHandle, + int timeoutMs, + ref byte[] outputBuffer, + ref int outputBytesRead, + ref byte[] errorBuffer, + ref int errorBytesRead); + + /// + /// Rents a larger buffer from the array pool and copies the existing data to it. + /// + internal static void RentLargerBuffer(ref byte[] buffer, int bytesRead) + { + byte[] newBuffer = ArrayPool.Shared.Rent(buffer.Length * 2); + Buffer.BlockCopy(buffer, 0, newBuffer, 0, bytesRead); + ArrayPool.Shared.Return(buffer); + buffer = newBuffer; + } + } +} diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs new file mode 100644 index 00000000000000..7540295289fd2d --- /dev/null +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs @@ -0,0 +1,247 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Text; +using Microsoft.DotNet.RemoteExecutor; +using Xunit; + +namespace System.Diagnostics.Tests +{ + public class ProcessMultiplexingTests : ProcessTestBase + { + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void ReadAllBytes_ThrowsWhenNoStreamsRedirected() + { + Process process = CreateProcess(RemotelyInvokable.Dummy); + process.Start(); + + Assert.Throws(() => process.ReadAllBytes()); + + Assert.True(process.WaitForExit(WaitInMS)); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void ReadAllText_ThrowsWhenNoStreamsRedirected() + { + Process process = CreateProcess(RemotelyInvokable.Dummy); + process.Start(); + + Assert.Throws(() => process.ReadAllText()); + + Assert.True(process.WaitForExit(WaitInMS)); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void ReadAllBytes_ThrowsWhenOutputInAsyncMode() + { + Process process = CreateProcess(RemotelyInvokable.StreamBody); + process.StartInfo.RedirectStandardOutput = true; + process.Start(); + + process.BeginOutputReadLine(); + + Assert.Throws(() => process.ReadAllBytes()); + + process.CancelOutputRead(); + Assert.True(process.WaitForExit(WaitInMS)); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void ReadAllBytes_ThrowsWhenErrorInAsyncMode() + { + Process process = CreateProcess(RemotelyInvokable.ErrorProcessBody); + process.StartInfo.RedirectStandardError = true; + process.Start(); + + process.BeginErrorReadLine(); + + Assert.Throws(() => process.ReadAllBytes()); + + process.CancelErrorRead(); + Assert.True(process.WaitForExit(WaitInMS)); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void ReadAllText_ReadsBothOutputAndError() + { + Process process = CreateProcess(() => + { + Console.Out.Write("stdout_text"); + Console.Error.Write("stderr_text"); + return RemoteExecutor.SuccessExitCode; + }); + process.StartInfo.RedirectStandardOutput = true; + process.StartInfo.RedirectStandardError = true; + process.Start(); + + (string standardOutput, string standardError) = process.ReadAllText(); + + Assert.Equal("stdout_text", standardOutput); + Assert.Equal("stderr_text", standardError); + Assert.True(process.WaitForExit(WaitInMS)); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void ReadAllBytes_ReadsBothOutputAndError() + { + Process process = CreateProcess(() => + { + Console.Out.Write("stdout_bytes"); + Console.Error.Write("stderr_bytes"); + return RemoteExecutor.SuccessExitCode; + }); + process.StartInfo.RedirectStandardOutput = true; + process.StartInfo.RedirectStandardError = true; + process.Start(); + + (byte[] standardOutput, byte[] standardError) = process.ReadAllBytes(); + + Assert.Equal(Encoding.Default.GetBytes("stdout_bytes"), standardOutput); + Assert.Equal(Encoding.Default.GetBytes("stderr_bytes"), standardError); + Assert.True(process.WaitForExit(WaitInMS)); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void ReadAllText_ReadsOnlyStandardOutput() + { + Process process = CreateProcess(() => + { + Console.Out.Write("only_stdout"); + return RemoteExecutor.SuccessExitCode; + }); + process.StartInfo.RedirectStandardOutput = true; + process.Start(); + + (string standardOutput, string standardError) = process.ReadAllText(); + + Assert.Equal("only_stdout", standardOutput); + Assert.Equal(string.Empty, standardError); + Assert.True(process.WaitForExit(WaitInMS)); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void ReadAllText_ReadsOnlyStandardError() + { + Process process = CreateProcess(() => + { + Console.Error.Write("only_stderr"); + return RemoteExecutor.SuccessExitCode; + }); + process.StartInfo.RedirectStandardError = true; + process.Start(); + + (string standardOutput, string standardError) = process.ReadAllText(); + + Assert.Equal(string.Empty, standardOutput); + Assert.Equal("only_stderr", standardError); + Assert.True(process.WaitForExit(WaitInMS)); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void ReadAllText_ReadsLargeOutput() + { + string largeText = new string('A', 100_000); + Process process = CreateProcess(RemotelyInvokable.Echo, largeText); + process.StartInfo.RedirectStandardOutput = true; + process.Start(); + + (string standardOutput, string standardError) = process.ReadAllText(); + + Assert.Equal(largeText + Environment.NewLine, standardOutput); + Assert.Equal(string.Empty, standardError); + Assert.True(process.WaitForExit(WaitInMS)); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void ReadAllText_TimesOutOnBlockingProcess() + { + Process process = CreateProcess(RemotelyInvokable.ReadLine); + process.StartInfo.RedirectStandardOutput = true; + process.StartInfo.RedirectStandardInput = true; + process.Start(); + + Assert.Throws(() => process.ReadAllText(TimeSpan.FromMilliseconds(100))); + + process.Kill(); + Assert.True(process.WaitForExit(WaitInMS)); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void ReadAllBytes_ReadsOnlyStandardOutput() + { + Process process = CreateProcess(() => + { + Console.Out.Write("out_data"); + return RemoteExecutor.SuccessExitCode; + }); + process.StartInfo.RedirectStandardOutput = true; + process.Start(); + + (byte[] standardOutput, byte[] standardError) = process.ReadAllBytes(); + + Assert.Equal(Encoding.Default.GetBytes("out_data"), standardOutput); + Assert.Empty(standardError); + Assert.True(process.WaitForExit(WaitInMS)); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void ReadAllBytes_ReadsOnlyStandardError() + { + Process process = CreateProcess(() => + { + Console.Error.Write("err_data"); + return RemoteExecutor.SuccessExitCode; + }); + process.StartInfo.RedirectStandardError = true; + process.Start(); + + (byte[] standardOutput, byte[] standardError) = process.ReadAllBytes(); + + Assert.Empty(standardOutput); + Assert.Equal(Encoding.Default.GetBytes("err_data"), standardError); + Assert.True(process.WaitForExit(WaitInMS)); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void ReadAllText_EmptyOutput() + { + Process process = CreateProcess(() => RemoteExecutor.SuccessExitCode); + process.StartInfo.RedirectStandardOutput = true; + process.StartInfo.RedirectStandardError = true; + process.Start(); + + (string standardOutput, string standardError) = process.ReadAllText(); + + Assert.Equal(string.Empty, standardOutput); + Assert.Equal(string.Empty, standardError); + Assert.True(process.WaitForExit(WaitInMS)); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void ReadAllBytes_TimesOutOnBlockingBothStreams() + { + Process process = CreateProcess(RemotelyInvokable.ReadLine); + process.StartInfo.RedirectStandardOutput = true; + process.StartInfo.RedirectStandardError = true; + process.StartInfo.RedirectStandardInput = true; + process.Start(); + + Assert.Throws(() => process.ReadAllBytes(TimeSpan.FromMilliseconds(100))); + + process.Kill(); + Assert.True(process.WaitForExit(WaitInMS)); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void ReadAllBytes_ThrowsAfterDispose() + { + Process process = CreateProcess(RemotelyInvokable.Dummy); + process.Start(); + Assert.True(process.WaitForExit(WaitInMS)); + + process.Dispose(); + + Assert.Throws(() => process.ReadAllBytes()); + } + } +} 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 5c345d299a1ac8..dd73a7baa7f533 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 @@ -31,6 +31,7 @@ + From c2dceaf58f8c9ffae1a4c08382f2e38554f85da1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 12 Apr 2026 07:45:54 +0000 Subject: [PATCH 02/19] Fix fileOffset values and refactor to unified ReadPipes (WIP - nullable fixes needed) Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/45ece5ba-166f-4418-887f-3ecaba54615e Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Diagnostics/Process.Multiplexing.Unix.cs | 18 +++--- .../Process.Multiplexing.Windows.cs | 30 +++++----- .../Diagnostics/Process.Multiplexing.cs | 58 +++---------------- 3 files changed, 33 insertions(+), 73 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs index 08e2dc192516fd..9fa69f02e26b3f 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs @@ -11,21 +11,21 @@ namespace System.Diagnostics public partial class Process { /// - /// Reads from both standard output and standard error pipes using Unix poll-based multiplexing. + /// Reads from one or both standard output and standard error pipes using Unix poll-based multiplexing. /// - private static partial void ReadBothPipes( - SafeFileHandle outputHandle, - SafeFileHandle errorHandle, + private static partial void ReadPipes( + SafeFileHandle? outputHandle, + SafeFileHandle? errorHandle, int timeoutMs, ref byte[] outputBuffer, ref int outputBytesRead, ref byte[] errorBuffer, ref int errorBytesRead) { - int outputFd = outputHandle.DangerousGetHandle().ToInt32(); - int errorFd = errorHandle.DangerousGetHandle().ToInt32(); - bool outputDone = false; - bool errorDone = false; + int outputFd = outputHandle is not null ? outputHandle.DangerousGetHandle().ToInt32() : -1; + int errorFd = errorHandle is not null ? errorHandle.DangerousGetHandle().ToInt32() : -1; + bool outputDone = outputHandle is null; + bool errorDone = errorHandle is null; Interop.PollEvent[] pollFds = new Interop.PollEvent[2]; @@ -110,7 +110,7 @@ private static partial void ReadBothPipes( ref int currentBytesRead = ref (isError ? ref errorBytesRead : ref outputBytesRead); ref bool currentDone = ref (isError ? ref errorDone : ref outputDone); - int bytesRead = RandomAccess.Read(currentHandle, currentBuffer.AsSpan(currentBytesRead), fileOffset: -1); + int bytesRead = RandomAccess.Read(currentHandle, currentBuffer.AsSpan(currentBytesRead), fileOffset: 0); if (bytesRead > 0) { currentBytesRead += bytesRead; diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs index d4a738e9982e3a..612005fe91c9cc 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs @@ -11,12 +11,12 @@ namespace System.Diagnostics public partial class Process { /// - /// Reads from both standard output and standard error pipes using async IO with tasks. + /// Reads from one or both standard output and standard error pipes using async IO with tasks. /// On Windows, the underlying handles are opened for async IO, so async reads can be cancelled. /// - private static partial void ReadBothPipes( - SafeFileHandle outputHandle, - SafeFileHandle errorHandle, + private static partial void ReadPipes( + SafeFileHandle? outputHandle, + SafeFileHandle? errorHandle, int timeoutMs, ref byte[] outputBuffer, ref int outputBytesRead, @@ -29,11 +29,15 @@ private static partial void ReadBothPipes( CancellationToken cancellationToken = cts.Token; - Task outputTask = RandomAccess.ReadAsync(outputHandle, outputBuffer.AsMemory(outputBytesRead), fileOffset: -1, cancellationToken).AsTask(); - Task errorTask = RandomAccess.ReadAsync(errorHandle, errorBuffer.AsMemory(errorBytesRead), fileOffset: -1, cancellationToken).AsTask(); + Task? outputTask = outputHandle is not null + ? RandomAccess.ReadAsync(outputHandle, outputBuffer.AsMemory(outputBytesRead), fileOffset: 0, cancellationToken).AsTask() + : null; + Task? errorTask = errorHandle is not null + ? RandomAccess.ReadAsync(errorHandle, errorBuffer.AsMemory(errorBytesRead), fileOffset: 0, cancellationToken).AsTask() + : null; - bool outputDone = false; - bool errorDone = false; + bool outputDone = outputTask is null; + bool errorDone = errorTask is null; try { @@ -44,18 +48,18 @@ private static partial void ReadBothPipes( if (outputDone) { - bytesRead = errorTask.GetAwaiter().GetResult(); + bytesRead = errorTask!.GetAwaiter().GetResult(); isError = true; } else if (errorDone) { - bytesRead = outputTask.GetAwaiter().GetResult(); + bytesRead = outputTask!.GetAwaiter().GetResult(); isError = false; } else { #pragma warning disable CA2025 // Tasks complete or are cancelled before CTS disposal - Task completed = Task.WhenAny(outputTask, errorTask).GetAwaiter().GetResult(); + Task completed = Task.WhenAny(outputTask!, errorTask!).GetAwaiter().GetResult(); #pragma warning restore CA2025 isError = completed == errorTask; bytesRead = completed.GetAwaiter().GetResult(); @@ -70,7 +74,7 @@ private static partial void ReadBothPipes( { RentLargerBuffer(ref errorBuffer, errorBytesRead); } - errorTask = RandomAccess.ReadAsync(errorHandle, errorBuffer.AsMemory(errorBytesRead), fileOffset: -1, cancellationToken).AsTask(); + errorTask = RandomAccess.ReadAsync(errorHandle!, errorBuffer.AsMemory(errorBytesRead), fileOffset: 0, cancellationToken).AsTask(); } else { @@ -79,7 +83,7 @@ private static partial void ReadBothPipes( { RentLargerBuffer(ref outputBuffer, outputBytesRead); } - outputTask = RandomAccess.ReadAsync(outputHandle, outputBuffer.AsMemory(outputBytesRead), fileOffset: -1, cancellationToken).AsTask(); + outputTask = RandomAccess.ReadAsync(outputHandle!, outputBuffer.AsMemory(outputBytesRead), fileOffset: 0, cancellationToken).AsTask(); } } else 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 ea932a723a9648..a14ba7cc35ac7a 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 @@ -121,20 +121,9 @@ public partial class Process SafeFileHandle? outputHandle = hasOutput ? GetSafeFileHandleFromStreamReader(_standardOutput!) : null; SafeFileHandle? errorHandle = hasError ? GetSafeFileHandleFromStreamReader(_standardError!) : null; - if (hasOutput && hasError) - { - ReadBothPipes(outputHandle!, errorHandle!, timeoutMs, - ref outputBuffer, ref outputBytesRead, - ref errorBuffer, ref errorBytesRead); - } - else if (hasOutput) - { - ReadSinglePipe(outputHandle!, timeoutMs, ref outputBuffer, ref outputBytesRead); - } - else - { - ReadSinglePipe(errorHandle!, timeoutMs, ref errorBuffer, ref errorBytesRead); - } + ReadPipes(outputHandle, errorHandle, timeoutMs, + ref outputBuffer, ref outputBytesRead, + ref errorBuffer, ref errorBytesRead); byte[] outputResult = outputBytesRead > 0 ? outputBuffer.AsSpan(0, outputBytesRead).ToArray() @@ -176,44 +165,11 @@ private static SafeFileHandle GetSafeFileHandleFromStreamReader(StreamReader rea } /// - /// Reads from a single pipe using until EOF or timeout. + /// Reads from one or both standard output and standard error pipes using platform-specific multiplexing. /// - private static void ReadSinglePipe(SafeFileHandle handle, int timeoutMs, ref byte[] buffer, ref int bytesRead) - { - long deadline = timeoutMs >= 0 - ? Environment.TickCount64 + timeoutMs - : long.MaxValue; - - while (true) - { - int remaining = buffer.Length - bytesRead; - - int read = RandomAccess.Read(handle, buffer.AsSpan(bytesRead, remaining), fileOffset: -1); - if (read == 0) - { - break; // EOF - } - - bytesRead += read; - - if (bytesRead == buffer.Length) - { - RentLargerBuffer(ref buffer, bytesRead); - } - - if (Environment.TickCount64 >= deadline) - { - throw new TimeoutException(); - } - } - } - - /// - /// Reads from both standard output and standard error pipes using platform-specific multiplexing. - /// - private static partial void ReadBothPipes( - SafeFileHandle outputHandle, - SafeFileHandle errorHandle, + private static partial void ReadPipes( + SafeFileHandle? outputHandle, + SafeFileHandle? errorHandle, int timeoutMs, ref byte[] outputBuffer, ref int outputBytesRead, From d51846184941ef72125ac09c315cab26b204c5cc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 12 Apr 2026 08:57:25 +0000 Subject: [PATCH 03/19] Rewrite Windows ReadPipes to use overlapped IO with wait handles instead of GetAwaiter().GetResult() Replace the async Task-based Windows implementation with proper Win32 overlapped IO: - Allocate NativeOverlapped* with ManualResetEvent for each pipe - Use Interop.Kernel32.ReadFile with overlapped structs - Use WaitHandle.WaitAny for synchronous multiplexing - Use GetOverlappedResult to retrieve bytes read - Use CancelIoEx for timeout cancellation - Add System.Threading.Overlapped project reference - Add ReadFile, GetOverlappedResult, CancelIoEx interop references - Fix nullable warning in Unix implementation Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/8af96aef-e52b-459d-a945-e1159e8be19b Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System.Diagnostics.Process.csproj | 7 + .../Diagnostics/Process.Multiplexing.Unix.cs | 2 +- .../Process.Multiplexing.Windows.cs | 242 ++++++++++++++---- 3 files changed, 195 insertions(+), 56 deletions(-) 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 a6681a99050bb3..e4dd3598698383 100644 --- a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -226,6 +226,12 @@ + + + @@ -413,6 +419,7 @@ + diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs index 9fa69f02e26b3f..9446c75799ba10 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs @@ -105,7 +105,7 @@ private static partial void ReadPipes( } bool isError = i == errorIndex; - SafeFileHandle currentHandle = isError ? errorHandle : outputHandle; + SafeFileHandle currentHandle = (isError ? errorHandle : outputHandle)!; ref byte[] currentBuffer = ref (isError ? ref errorBuffer : ref outputBuffer); ref int currentBytesRead = ref (isError ? ref errorBytesRead : ref outputBytesRead); ref bool currentDone = ref (isError ? ref errorDone : ref outputDone); diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs index 612005fe91c9cc..621c8c5dc56fde 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs @@ -1,9 +1,10 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.IO; +using System.Buffers; +using System.ComponentModel; +using System.Runtime.InteropServices; using System.Threading; -using System.Threading.Tasks; using Microsoft.Win32.SafeHandles; namespace System.Diagnostics @@ -11,8 +12,8 @@ namespace System.Diagnostics public partial class Process { /// - /// Reads from one or both standard output and standard error pipes using async IO with tasks. - /// On Windows, the underlying handles are opened for async IO, so async reads can be cancelled. + /// Reads from one or both standard output and standard error pipes using Windows overlapped IO + /// with wait handles for single-threaded synchronous multiplexing. /// private static partial void ReadPipes( SafeFileHandle? outputHandle, @@ -23,89 +24,220 @@ private static partial void ReadPipes( ref byte[] errorBuffer, ref int errorBytesRead) { - CancellationTokenSource cts = timeoutMs >= 0 - ? new CancellationTokenSource(timeoutMs) - : new CancellationTokenSource(); + bool hasOutput = outputHandle is not null; + bool hasError = errorHandle is not null; - CancellationToken cancellationToken = cts.Token; + MemoryHandle outputPin = hasOutput ? outputBuffer.AsMemory().Pin() : default; + MemoryHandle errorPin = hasError ? errorBuffer.AsMemory().Pin() : default; - Task? outputTask = outputHandle is not null - ? RandomAccess.ReadAsync(outputHandle, outputBuffer.AsMemory(outputBytesRead), fileOffset: 0, cancellationToken).AsTask() - : null; - Task? errorTask = errorHandle is not null - ? RandomAccess.ReadAsync(errorHandle, errorBuffer.AsMemory(errorBytesRead), fileOffset: 0, cancellationToken).AsTask() - : null; - - bool outputDone = outputTask is null; - bool errorDone = errorTask is null; - - try + unsafe { - while (!outputDone || !errorDone) + NativeOverlapped* outputOverlapped = null; + NativeOverlapped* errorOverlapped = null; + EventWaitHandle? outputEvent = null; + EventWaitHandle? errorEvent = null; + + try { - bool isError; - int bytesRead; + if (hasOutput) + { + outputEvent = new EventWaitHandle(initialState: false, EventResetMode.ManualReset); + outputOverlapped = AllocateOverlapped(outputEvent); + } + + if (hasError) + { + errorEvent = new EventWaitHandle(initialState: false, EventResetMode.ManualReset); + errorOverlapped = AllocateOverlapped(errorEvent); + } + + bool outputDone = !hasOutput; + bool errorDone = !hasError; - if (outputDone) + // Build wait handle array: only include handles for pipes we're reading. + WaitHandle[] waitHandles; + int errorWaitIndex; + + if (hasOutput && hasError) { - bytesRead = errorTask!.GetAwaiter().GetResult(); - isError = true; + waitHandles = [outputEvent!, errorEvent!]; + errorWaitIndex = 1; } - else if (errorDone) + else if (hasOutput) { - bytesRead = outputTask!.GetAwaiter().GetResult(); - isError = false; + waitHandles = [outputEvent!]; + errorWaitIndex = -1; } else { -#pragma warning disable CA2025 // Tasks complete or are cancelled before CTS disposal - Task completed = Task.WhenAny(outputTask!, errorTask!).GetAwaiter().GetResult(); -#pragma warning restore CA2025 - isError = completed == errorTask; - bytesRead = completed.GetAwaiter().GetResult(); + waitHandles = [errorEvent!]; + errorWaitIndex = 0; } - if (bytesRead > 0) + // Issue initial reads. + if (hasOutput) { - if (isError) + Interop.Kernel32.ReadFile(outputHandle!, (byte*)outputPin.Pointer + outputBytesRead, + outputBuffer.Length - outputBytesRead, IntPtr.Zero, outputOverlapped); + } + + if (hasError) + { + Interop.Kernel32.ReadFile(errorHandle!, (byte*)errorPin.Pointer + errorBytesRead, + errorBuffer.Length - errorBytesRead, IntPtr.Zero, errorOverlapped); + } + + long deadline = timeoutMs >= 0 + ? Environment.TickCount64 + timeoutMs + : long.MaxValue; + + while (!outputDone || !errorDone) + { + int waitTimeout; + if (timeoutMs >= 0) { - errorBytesRead += bytesRead; - if (errorBytesRead == errorBuffer.Length) + long remaining = deadline - Environment.TickCount64; + if (remaining <= 0) { - RentLargerBuffer(ref errorBuffer, errorBytesRead); + CancelPendingIOIfNeeded(outputHandle, outputDone, outputOverlapped); + CancelPendingIOIfNeeded(errorHandle, errorDone, errorOverlapped); + throw new TimeoutException(); } - errorTask = RandomAccess.ReadAsync(errorHandle!, errorBuffer.AsMemory(errorBytesRead), fileOffset: 0, cancellationToken).AsTask(); + + waitTimeout = (int)Math.Min(remaining, int.MaxValue); } else { - outputBytesRead += bytesRead; - if (outputBytesRead == outputBuffer.Length) - { - RentLargerBuffer(ref outputBuffer, outputBytesRead); - } - outputTask = RandomAccess.ReadAsync(outputHandle!, outputBuffer.AsMemory(outputBytesRead), fileOffset: 0, cancellationToken).AsTask(); + waitTimeout = Timeout.Infinite; } - } - else - { - if (isError) + + int waitResult = WaitHandle.WaitAny(waitHandles, waitTimeout); + + if (waitResult == WaitHandle.WaitTimeout) { - errorDone = true; + CancelPendingIOIfNeeded(outputHandle, outputDone, outputOverlapped); + CancelPendingIOIfNeeded(errorHandle, errorDone, errorOverlapped); + throw new TimeoutException(); + } + + bool isError = waitResult == errorWaitIndex; + NativeOverlapped* currentOverlapped = isError ? errorOverlapped : outputOverlapped; + SafeFileHandle currentHandle = (isError ? errorHandle : outputHandle)!; + ref int totalBytesRead = ref (isError ? ref errorBytesRead : ref outputBytesRead); + ref byte[] currentBuffer = ref (isError ? ref errorBuffer : ref outputBuffer); + EventWaitHandle currentEvent = (isError ? errorEvent : outputEvent)!; + + int bytesRead = GetOverlappedResultForPipe(currentHandle, currentOverlapped); + + if (bytesRead > 0) + { + totalBytesRead += bytesRead; + + if (totalBytesRead == currentBuffer.Length) + { + ref MemoryHandle currentPin = ref (isError ? ref errorPin : ref outputPin); + currentPin.Dispose(); + + RentLargerBuffer(ref currentBuffer, totalBytesRead); + + currentPin = currentBuffer.AsMemory().Pin(); + } + + // Reset the event and overlapped for next read. + ResetOverlapped(currentEvent, currentOverlapped); + + byte* pinPointer = isError ? (byte*)errorPin.Pointer : (byte*)outputPin.Pointer; + Interop.Kernel32.ReadFile(currentHandle, pinPointer + totalBytesRead, + currentBuffer.Length - totalBytesRead, IntPtr.Zero, currentOverlapped); } else { - outputDone = true; + // EOF: pipe write end was closed. + if (isError) + { + errorDone = true; + } + else + { + outputDone = true; + } + + // Reset the event so WaitAny won't trigger on this stale handle. + currentEvent.Reset(); } } } + finally + { + if (outputOverlapped != null) + { + NativeMemory.Free(outputOverlapped); + } + + if (errorOverlapped != null) + { + NativeMemory.Free(errorOverlapped); + } + + outputEvent?.Dispose(); + errorEvent?.Dispose(); + outputPin.Dispose(); + errorPin.Dispose(); + } + } + } - cts.Dispose(); + private static unsafe NativeOverlapped* AllocateOverlapped(EventWaitHandle waitHandle) + { + NativeOverlapped* overlapped = (NativeOverlapped*)NativeMemory.AllocZeroed((nuint)sizeof(NativeOverlapped)); + overlapped->EventHandle = waitHandle.SafeWaitHandle.DangerousGetHandle(); + return overlapped; + } + + private static unsafe void ResetOverlapped(EventWaitHandle waitHandle, NativeOverlapped* overlapped) + { + waitHandle.Reset(); + + overlapped->InternalHigh = IntPtr.Zero; + overlapped->InternalLow = IntPtr.Zero; + overlapped->OffsetHigh = 0; + overlapped->OffsetLow = 0; + overlapped->EventHandle = waitHandle.SafeWaitHandle.DangerousGetHandle(); + } + + private static unsafe int GetOverlappedResultForPipe(SafeFileHandle handle, NativeOverlapped* overlapped) + { + int bytesRead = 0; + if (!Interop.Kernel32.GetOverlappedResult(handle, overlapped, ref bytesRead, bWait: false)) + { + int errorCode = Marshal.GetLastPInvokeError(); + switch (errorCode) + { + case Interop.Errors.ERROR_HANDLE_EOF: + case Interop.Errors.ERROR_BROKEN_PIPE: + case Interop.Errors.ERROR_PIPE_NOT_CONNECTED: + return 0; + default: + throw new Win32Exception(errorCode); + } } - catch (OperationCanceledException) + + return bytesRead; + } + + private static unsafe void CancelPendingIOIfNeeded(SafeFileHandle? handle, bool done, NativeOverlapped* overlapped) + { + if (done || handle is null || overlapped == null) { - cts.Dispose(); - throw new TimeoutException(); + return; } + + // CancelIoEx marks matching outstanding I/O requests for cancellation. + Interop.Kernel32.CancelIoEx(handle, overlapped); + + // We must observe completion before freeing the OVERLAPPED. + int bytesRead = 0; + Interop.Kernel32.GetOverlappedResult(handle, overlapped, ref bytesRead, bWait: true); } } } From 4eb1e88a94d449bf09b5a11a07f7d0c342c10d1a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 12 Apr 2026 09:01:55 +0000 Subject: [PATCH 04/19] Fix code review: make RentLargerBuffer private, fix fully-qualified Win32Exception Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/8af96aef-e52b-459d-a945-e1159e8be19b Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System/Diagnostics/Process.Multiplexing.Unix.cs | 2 +- .../src/System/Diagnostics/Process.Multiplexing.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs index 9446c75799ba10..e1d78d5edb5e3a 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs @@ -87,7 +87,7 @@ private static partial void ReadPipes( continue; } - throw new ComponentModel.Win32Exception(Marshal.GetLastPInvokeError()); + throw new Win32Exception(Marshal.GetLastPInvokeError()); } if (triggered == 0) 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 a14ba7cc35ac7a..5a62dbd5d0c60a 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 @@ -179,7 +179,7 @@ private static partial void ReadPipes( /// /// Rents a larger buffer from the array pool and copies the existing data to it. /// - internal static void RentLargerBuffer(ref byte[] buffer, int bytesRead) + private static void RentLargerBuffer(ref byte[] buffer, int bytesRead) { byte[] newBuffer = ArrayPool.Shared.Rent(buffer.Length * 2); Buffer.BlockCopy(buffer, 0, newBuffer, 0, bytesRead); From 0986690764a64edc4d3af9d99db8a7068d06fb0e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 12 Apr 2026 10:44:36 +0000 Subject: [PATCH 05/19] Address review feedback: require both stdout/stderr redirected, add helper method, DangerousAddRef/Release, remove partial Changes: 1. Require both stdout and stderr to be redirected (throw if either is missing) 2. Create ReadPipesToBuffers helper shared between ReadAllText and ReadAllBytes (ReadAllText avoids extra buffer copy by decoding from rented buffers directly) 3. GetSafeFileHandleFromStreamReader now returns an owner SafeHandle; caller uses DangerousAddRef/DangerousRelease to prevent handle disposal during reads 4. Remove partial method declaration; ReadPipes is a regular static method in each platform file (non-nullable parameters since both handles always provided) 5. Update tests: all tests redirect both stdout and stderr; add two new tests for missing-stream validation Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/cd917b13-f344-47c5-ac1e-cfb2c2da34f6 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Diagnostics/Process.Multiplexing.Unix.cs | 18 +-- .../Process.Multiplexing.Windows.cs | 85 +++------- .../Diagnostics/Process.Multiplexing.cs | 148 +++++++++++------- .../tests/ProcessMultiplexingTests.cs | 40 ++++- 4 files changed, 160 insertions(+), 131 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs index e1d78d5edb5e3a..a0e2bee0f4656d 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs @@ -11,21 +11,21 @@ namespace System.Diagnostics public partial class Process { /// - /// Reads from one or both standard output and standard error pipes using Unix poll-based multiplexing. + /// Reads from both standard output and standard error pipes using Unix poll-based multiplexing. /// - private static partial void ReadPipes( - SafeFileHandle? outputHandle, - SafeFileHandle? errorHandle, + private static void ReadPipes( + SafeFileHandle outputHandle, + SafeFileHandle errorHandle, int timeoutMs, ref byte[] outputBuffer, ref int outputBytesRead, ref byte[] errorBuffer, ref int errorBytesRead) { - int outputFd = outputHandle is not null ? outputHandle.DangerousGetHandle().ToInt32() : -1; - int errorFd = errorHandle is not null ? errorHandle.DangerousGetHandle().ToInt32() : -1; - bool outputDone = outputHandle is null; - bool errorDone = errorHandle is null; + int outputFd = outputHandle.DangerousGetHandle().ToInt32(); + int errorFd = errorHandle.DangerousGetHandle().ToInt32(); + bool outputDone = false; + bool errorDone = false; Interop.PollEvent[] pollFds = new Interop.PollEvent[2]; @@ -105,7 +105,7 @@ private static partial void ReadPipes( } bool isError = i == errorIndex; - SafeFileHandle currentHandle = (isError ? errorHandle : outputHandle)!; + SafeFileHandle currentHandle = isError ? errorHandle : outputHandle; ref byte[] currentBuffer = ref (isError ? ref errorBuffer : ref outputBuffer); ref int currentBytesRead = ref (isError ? ref errorBytesRead : ref outputBytesRead); ref bool currentDone = ref (isError ? ref errorDone : ref outputDone); diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs index 621c8c5dc56fde..1488f9bd9006ba 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs @@ -12,80 +12,45 @@ namespace System.Diagnostics public partial class Process { /// - /// Reads from one or both standard output and standard error pipes using Windows overlapped IO + /// Reads from both standard output and standard error pipes using Windows overlapped IO /// with wait handles for single-threaded synchronous multiplexing. /// - private static partial void ReadPipes( - SafeFileHandle? outputHandle, - SafeFileHandle? errorHandle, + private static void ReadPipes( + SafeFileHandle outputHandle, + SafeFileHandle errorHandle, int timeoutMs, ref byte[] outputBuffer, ref int outputBytesRead, ref byte[] errorBuffer, ref int errorBytesRead) { - bool hasOutput = outputHandle is not null; - bool hasError = errorHandle is not null; - - MemoryHandle outputPin = hasOutput ? outputBuffer.AsMemory().Pin() : default; - MemoryHandle errorPin = hasError ? errorBuffer.AsMemory().Pin() : default; + MemoryHandle outputPin = outputBuffer.AsMemory().Pin(); + MemoryHandle errorPin = errorBuffer.AsMemory().Pin(); unsafe { NativeOverlapped* outputOverlapped = null; NativeOverlapped* errorOverlapped = null; - EventWaitHandle? outputEvent = null; - EventWaitHandle? errorEvent = null; + + EventWaitHandle outputEvent = new(initialState: false, EventResetMode.ManualReset); + EventWaitHandle errorEvent = new(initialState: false, EventResetMode.ManualReset); try { - if (hasOutput) - { - outputEvent = new EventWaitHandle(initialState: false, EventResetMode.ManualReset); - outputOverlapped = AllocateOverlapped(outputEvent); - } + outputOverlapped = AllocateOverlapped(outputEvent); + errorOverlapped = AllocateOverlapped(errorEvent); - if (hasError) - { - errorEvent = new EventWaitHandle(initialState: false, EventResetMode.ManualReset); - errorOverlapped = AllocateOverlapped(errorEvent); - } - - bool outputDone = !hasOutput; - bool errorDone = !hasError; + bool outputDone = false; + bool errorDone = false; - // Build wait handle array: only include handles for pipes we're reading. - WaitHandle[] waitHandles; - int errorWaitIndex; - - if (hasOutput && hasError) - { - waitHandles = [outputEvent!, errorEvent!]; - errorWaitIndex = 1; - } - else if (hasOutput) - { - waitHandles = [outputEvent!]; - errorWaitIndex = -1; - } - else - { - waitHandles = [errorEvent!]; - errorWaitIndex = 0; - } + WaitHandle[] waitHandles = [outputEvent, errorEvent]; // Issue initial reads. - if (hasOutput) - { - Interop.Kernel32.ReadFile(outputHandle!, (byte*)outputPin.Pointer + outputBytesRead, - outputBuffer.Length - outputBytesRead, IntPtr.Zero, outputOverlapped); - } + Interop.Kernel32.ReadFile(outputHandle, (byte*)outputPin.Pointer + outputBytesRead, + outputBuffer.Length - outputBytesRead, IntPtr.Zero, outputOverlapped); - if (hasError) - { - Interop.Kernel32.ReadFile(errorHandle!, (byte*)errorPin.Pointer + errorBytesRead, - errorBuffer.Length - errorBytesRead, IntPtr.Zero, errorOverlapped); - } + Interop.Kernel32.ReadFile(errorHandle, (byte*)errorPin.Pointer + errorBytesRead, + errorBuffer.Length - errorBytesRead, IntPtr.Zero, errorOverlapped); long deadline = timeoutMs >= 0 ? Environment.TickCount64 + timeoutMs @@ -120,12 +85,12 @@ private static partial void ReadPipes( throw new TimeoutException(); } - bool isError = waitResult == errorWaitIndex; + bool isError = waitResult == 1; NativeOverlapped* currentOverlapped = isError ? errorOverlapped : outputOverlapped; - SafeFileHandle currentHandle = (isError ? errorHandle : outputHandle)!; + SafeFileHandle currentHandle = isError ? errorHandle : outputHandle; ref int totalBytesRead = ref (isError ? ref errorBytesRead : ref outputBytesRead); ref byte[] currentBuffer = ref (isError ? ref errorBuffer : ref outputBuffer); - EventWaitHandle currentEvent = (isError ? errorEvent : outputEvent)!; + EventWaitHandle currentEvent = isError ? errorEvent : outputEvent; int bytesRead = GetOverlappedResultForPipe(currentHandle, currentOverlapped); @@ -179,8 +144,8 @@ private static partial void ReadPipes( NativeMemory.Free(errorOverlapped); } - outputEvent?.Dispose(); - errorEvent?.Dispose(); + outputEvent.Dispose(); + errorEvent.Dispose(); outputPin.Dispose(); errorPin.Dispose(); } @@ -225,9 +190,9 @@ private static unsafe int GetOverlappedResultForPipe(SafeFileHandle handle, Nati return bytesRead; } - private static unsafe void CancelPendingIOIfNeeded(SafeFileHandle? handle, bool done, NativeOverlapped* overlapped) + private static unsafe void CancelPendingIOIfNeeded(SafeFileHandle handle, bool done, NativeOverlapped* overlapped) { - if (done || handle is null || overlapped == null) + if (done) { return; } 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 5a62dbd5d0c60a..61630e6e411631 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 @@ -3,6 +3,7 @@ using System.Buffers; using System.IO; +using System.Runtime.InteropServices; using System.Text; using Microsoft.Win32.SafeHandles; @@ -22,10 +23,9 @@ public partial class Process /// /// /// A tuple containing the standard output and standard error text. - /// When a stream was not redirected, the corresponding value is an empty string. /// /// - /// Neither standard output nor standard error has been redirected. + /// Standard output or standard error has not been redirected. /// -or- /// A redirected stream is already being read asynchronously. /// @@ -37,20 +37,33 @@ public partial class Process /// public (string StandardOutput, string StandardError) ReadAllText(TimeSpan? timeout = default) { - (byte[] StandardOutput, byte[] StandardError) bytes = ReadAllBytes(timeout); + byte[] outputBuffer = ArrayPool.Shared.Rent(InitialReadAllBufferSize); + byte[] errorBuffer = ArrayPool.Shared.Rent(InitialReadAllBufferSize); + int outputBytesRead = 0; + int errorBytesRead = 0; - Encoding outputEncoding = _startInfo?.StandardOutputEncoding ?? GetStandardOutputEncoding(); - Encoding errorEncoding = _startInfo?.StandardErrorEncoding ?? GetStandardOutputEncoding(); + try + { + ReadPipesToBuffers(timeout, ref outputBuffer, ref outputBytesRead, ref errorBuffer, ref errorBytesRead); + + Encoding outputEncoding = _startInfo?.StandardOutputEncoding ?? GetStandardOutputEncoding(); + Encoding errorEncoding = _startInfo?.StandardErrorEncoding ?? GetStandardOutputEncoding(); - string standardOutput = bytes.StandardOutput.Length > 0 - ? outputEncoding.GetString(bytes.StandardOutput) - : string.Empty; + string standardOutput = outputBytesRead > 0 + ? outputEncoding.GetString(outputBuffer, 0, outputBytesRead) + : string.Empty; - string standardError = bytes.StandardError.Length > 0 - ? errorEncoding.GetString(bytes.StandardError) - : string.Empty; + string standardError = errorBytesRead > 0 + ? errorEncoding.GetString(errorBuffer, 0, errorBytesRead) + : string.Empty; - return (standardOutput, standardError); + return (standardOutput, standardError); + } + finally + { + ArrayPool.Shared.Return(outputBuffer); + ArrayPool.Shared.Return(errorBuffer); + } } /// @@ -62,10 +75,9 @@ public partial class Process /// /// /// A tuple containing the standard output and standard error bytes. - /// When a stream was not redirected, the corresponding value is an empty array. /// /// - /// Neither standard output nor standard error has been redirected. + /// Standard output or standard error has not been redirected. /// -or- /// A redirected stream is already being read asynchronously. /// @@ -77,68 +89,98 @@ public partial class Process /// public (byte[] StandardOutput, byte[] StandardError) ReadAllBytes(TimeSpan? timeout = default) { - CheckDisposed(); + byte[] outputBuffer = ArrayPool.Shared.Rent(InitialReadAllBufferSize); + byte[] errorBuffer = ArrayPool.Shared.Rent(InitialReadAllBufferSize); + int outputBytesRead = 0; + int errorBytesRead = 0; - bool hasOutput = _standardOutput is not null; - bool hasError = _standardError is not null; + try + { + ReadPipesToBuffers(timeout, ref outputBuffer, ref outputBytesRead, ref errorBuffer, ref errorBytesRead); + + byte[] outputResult = outputBytesRead > 0 + ? outputBuffer.AsSpan(0, outputBytesRead).ToArray() + : Array.Empty(); + + byte[] errorResult = errorBytesRead > 0 + ? errorBuffer.AsSpan(0, errorBytesRead).ToArray() + : Array.Empty(); - if (!hasOutput && !hasError) + return (outputResult, errorResult); + } + finally { - throw new InvalidOperationException(SR.CantGetStandardOut); + ArrayPool.Shared.Return(outputBuffer); + ArrayPool.Shared.Return(errorBuffer); } + } + + /// + /// Validates state, obtains handles, and reads both stdout and stderr pipes into the provided buffers. + /// The caller is responsible for renting and returning the buffers. + /// + private void ReadPipesToBuffers( + TimeSpan? timeout, + ref byte[] outputBuffer, + ref int outputBytesRead, + ref byte[] errorBuffer, + ref int errorBytesRead) + { + CheckDisposed(); - if (hasOutput && _outputStreamReadMode == StreamReadMode.AsyncMode) + if (_standardOutput is null) { - throw new InvalidOperationException(SR.CantMixSyncAsyncOperation); + throw new InvalidOperationException(SR.CantGetStandardOut); } - if (hasError && _errorStreamReadMode == StreamReadMode.AsyncMode) + if (_standardError is null) { - throw new InvalidOperationException(SR.CantMixSyncAsyncOperation); + throw new InvalidOperationException(SR.CantGetStandardError); } - if (hasOutput) + if (_outputStreamReadMode == StreamReadMode.AsyncMode) { - _outputStreamReadMode = StreamReadMode.SyncMode; + throw new InvalidOperationException(SR.CantMixSyncAsyncOperation); } - if (hasError) + if (_errorStreamReadMode == StreamReadMode.AsyncMode) { - _errorStreamReadMode = StreamReadMode.SyncMode; + throw new InvalidOperationException(SR.CantMixSyncAsyncOperation); } + _outputStreamReadMode = StreamReadMode.SyncMode; + _errorStreamReadMode = StreamReadMode.SyncMode; + int timeoutMs = timeout.HasValue ? (int)timeout.Value.TotalMilliseconds : -1; // Infinite - byte[] outputBuffer = ArrayPool.Shared.Rent(InitialReadAllBufferSize); - byte[] errorBuffer = ArrayPool.Shared.Rent(InitialReadAllBufferSize); - int outputBytesRead = 0; - int errorBytesRead = 0; + SafeFileHandle outputHandle = GetSafeFileHandleFromStreamReader(_standardOutput, out SafeHandle outputOwner); + SafeFileHandle errorHandle = GetSafeFileHandleFromStreamReader(_standardError, out SafeHandle errorOwner); + + bool outputRefAdded = false; + bool errorRefAdded = false; try { - SafeFileHandle? outputHandle = hasOutput ? GetSafeFileHandleFromStreamReader(_standardOutput!) : null; - SafeFileHandle? errorHandle = hasError ? GetSafeFileHandleFromStreamReader(_standardError!) : null; + outputOwner.DangerousAddRef(ref outputRefAdded); + errorOwner.DangerousAddRef(ref errorRefAdded); ReadPipes(outputHandle, errorHandle, timeoutMs, ref outputBuffer, ref outputBytesRead, ref errorBuffer, ref errorBytesRead); - - byte[] outputResult = outputBytesRead > 0 - ? outputBuffer.AsSpan(0, outputBytesRead).ToArray() - : Array.Empty(); - - byte[] errorResult = errorBytesRead > 0 - ? errorBuffer.AsSpan(0, errorBytesRead).ToArray() - : Array.Empty(); - - return (outputResult, errorResult); } finally { - ArrayPool.Shared.Return(outputBuffer); - ArrayPool.Shared.Return(errorBuffer); + if (outputRefAdded) + { + outputOwner.DangerousRelease(); + } + + if (errorRefAdded) + { + errorOwner.DangerousRelease(); + } } } @@ -147,35 +189,25 @@ public partial class Process /// On Unix, the stream is an and the handle is obtained via the pipe handle. /// On Windows, the stream is a opened for async IO. /// - private static SafeFileHandle GetSafeFileHandleFromStreamReader(StreamReader reader) + private static SafeFileHandle GetSafeFileHandleFromStreamReader(StreamReader reader, out SafeHandle owner) { Stream baseStream = reader.BaseStream; if (baseStream is FileStream fileStream) { + owner = fileStream.SafeFileHandle; return fileStream.SafeFileHandle; } if (baseStream is System.IO.Pipes.AnonymousPipeClientStream pipeStream) { + owner = pipeStream.SafePipeHandle; return new SafeFileHandle(pipeStream.SafePipeHandle.DangerousGetHandle(), ownsHandle: false); } throw new InvalidOperationException(); } - /// - /// Reads from one or both standard output and standard error pipes using platform-specific multiplexing. - /// - private static partial void ReadPipes( - SafeFileHandle? outputHandle, - SafeFileHandle? errorHandle, - int timeoutMs, - ref byte[] outputBuffer, - ref int outputBytesRead, - ref byte[] errorBuffer, - ref int errorBytesRead); - /// /// Rents a larger buffer from the array pool and copies the existing data to it. /// diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs index 7540295289fd2d..60ba81a8f7c4eb 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs @@ -31,11 +31,36 @@ public void ReadAllText_ThrowsWhenNoStreamsRedirected() Assert.True(process.WaitForExit(WaitInMS)); } + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void ReadAllBytes_ThrowsWhenOnlyOutputRedirected() + { + Process process = CreateProcess(RemotelyInvokable.StreamBody); + process.StartInfo.RedirectStandardOutput = true; + process.Start(); + + Assert.Throws(() => process.ReadAllBytes()); + + Assert.True(process.WaitForExit(WaitInMS)); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void ReadAllBytes_ThrowsWhenOnlyErrorRedirected() + { + Process process = CreateProcess(RemotelyInvokable.ErrorProcessBody); + process.StartInfo.RedirectStandardError = true; + process.Start(); + + Assert.Throws(() => process.ReadAllBytes()); + + Assert.True(process.WaitForExit(WaitInMS)); + } + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] public void ReadAllBytes_ThrowsWhenOutputInAsyncMode() { Process process = CreateProcess(RemotelyInvokable.StreamBody); process.StartInfo.RedirectStandardOutput = true; + process.StartInfo.RedirectStandardError = true; process.Start(); process.BeginOutputReadLine(); @@ -50,6 +75,7 @@ public void ReadAllBytes_ThrowsWhenOutputInAsyncMode() public void ReadAllBytes_ThrowsWhenErrorInAsyncMode() { Process process = CreateProcess(RemotelyInvokable.ErrorProcessBody); + process.StartInfo.RedirectStandardOutput = true; process.StartInfo.RedirectStandardError = true; process.Start(); @@ -102,7 +128,7 @@ public void ReadAllBytes_ReadsBothOutputAndError() } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void ReadAllText_ReadsOnlyStandardOutput() + public void ReadAllText_OutputOnlyOnStdout() { Process process = CreateProcess(() => { @@ -110,6 +136,7 @@ public void ReadAllText_ReadsOnlyStandardOutput() return RemoteExecutor.SuccessExitCode; }); process.StartInfo.RedirectStandardOutput = true; + process.StartInfo.RedirectStandardError = true; process.Start(); (string standardOutput, string standardError) = process.ReadAllText(); @@ -120,13 +147,14 @@ public void ReadAllText_ReadsOnlyStandardOutput() } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void ReadAllText_ReadsOnlyStandardError() + public void ReadAllText_OutputOnlyOnStderr() { Process process = CreateProcess(() => { Console.Error.Write("only_stderr"); return RemoteExecutor.SuccessExitCode; }); + process.StartInfo.RedirectStandardOutput = true; process.StartInfo.RedirectStandardError = true; process.Start(); @@ -143,6 +171,7 @@ public void ReadAllText_ReadsLargeOutput() string largeText = new string('A', 100_000); Process process = CreateProcess(RemotelyInvokable.Echo, largeText); process.StartInfo.RedirectStandardOutput = true; + process.StartInfo.RedirectStandardError = true; process.Start(); (string standardOutput, string standardError) = process.ReadAllText(); @@ -157,6 +186,7 @@ public void ReadAllText_TimesOutOnBlockingProcess() { Process process = CreateProcess(RemotelyInvokable.ReadLine); process.StartInfo.RedirectStandardOutput = true; + process.StartInfo.RedirectStandardError = true; process.StartInfo.RedirectStandardInput = true; process.Start(); @@ -167,7 +197,7 @@ public void ReadAllText_TimesOutOnBlockingProcess() } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void ReadAllBytes_ReadsOnlyStandardOutput() + public void ReadAllBytes_OutputOnlyOnStdout() { Process process = CreateProcess(() => { @@ -175,6 +205,7 @@ public void ReadAllBytes_ReadsOnlyStandardOutput() return RemoteExecutor.SuccessExitCode; }); process.StartInfo.RedirectStandardOutput = true; + process.StartInfo.RedirectStandardError = true; process.Start(); (byte[] standardOutput, byte[] standardError) = process.ReadAllBytes(); @@ -185,13 +216,14 @@ public void ReadAllBytes_ReadsOnlyStandardOutput() } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void ReadAllBytes_ReadsOnlyStandardError() + public void ReadAllBytes_OutputOnlyOnStderr() { Process process = CreateProcess(() => { Console.Error.Write("err_data"); return RemoteExecutor.SuccessExitCode; }); + process.StartInfo.RedirectStandardOutput = true; process.StartInfo.RedirectStandardError = true; process.Start(); From df0e65a94785bbdd5ee401156eaed52f3809e65e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 12 Apr 2026 11:36:40 +0000 Subject: [PATCH 06/19] Address round 2 feedback: extract ValidateReadAllState helper, make Windows ReadPipes fully unsafe with proper cleanup, use is not null Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/451cf437-2de0-409a-937a-0031c3fa69d4 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Process.Multiplexing.Windows.cs | 189 +++++++++--------- .../Diagnostics/Process.Multiplexing.cs | 32 ++- 2 files changed, 117 insertions(+), 104 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs index 1488f9bd9006ba..5ffa672d151df7 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs @@ -15,7 +15,7 @@ public partial class Process /// Reads from both standard output and standard error pipes using Windows overlapped IO /// with wait handles for single-threaded synchronous multiplexing. /// - private static void ReadPipes( + private static unsafe void ReadPipes( SafeFileHandle outputHandle, SafeFileHandle errorHandle, int timeoutMs, @@ -24,131 +24,132 @@ private static void ReadPipes( ref byte[] errorBuffer, ref int errorBytesRead) { - MemoryHandle outputPin = outputBuffer.AsMemory().Pin(); - MemoryHandle errorPin = errorBuffer.AsMemory().Pin(); - - unsafe + MemoryHandle outputPin = default; + MemoryHandle errorPin = default; + NativeOverlapped* outputOverlapped = null; + NativeOverlapped* errorOverlapped = null; + EventWaitHandle? outputEvent = null; + EventWaitHandle? errorEvent = null; + + try { - NativeOverlapped* outputOverlapped = null; - NativeOverlapped* errorOverlapped = null; + outputPin = outputBuffer.AsMemory().Pin(); + errorPin = errorBuffer.AsMemory().Pin(); - EventWaitHandle outputEvent = new(initialState: false, EventResetMode.ManualReset); - EventWaitHandle errorEvent = new(initialState: false, EventResetMode.ManualReset); + outputEvent = new EventWaitHandle(initialState: false, EventResetMode.ManualReset); + errorEvent = new EventWaitHandle(initialState: false, EventResetMode.ManualReset); - try - { - outputOverlapped = AllocateOverlapped(outputEvent); - errorOverlapped = AllocateOverlapped(errorEvent); + outputOverlapped = AllocateOverlapped(outputEvent); + errorOverlapped = AllocateOverlapped(errorEvent); - bool outputDone = false; - bool errorDone = false; + bool outputDone = false; + bool errorDone = false; - WaitHandle[] waitHandles = [outputEvent, errorEvent]; + WaitHandle[] waitHandles = [outputEvent, errorEvent]; - // Issue initial reads. - Interop.Kernel32.ReadFile(outputHandle, (byte*)outputPin.Pointer + outputBytesRead, - outputBuffer.Length - outputBytesRead, IntPtr.Zero, outputOverlapped); + // Issue initial reads. + Interop.Kernel32.ReadFile(outputHandle, (byte*)outputPin.Pointer + outputBytesRead, + outputBuffer.Length - outputBytesRead, IntPtr.Zero, outputOverlapped); - Interop.Kernel32.ReadFile(errorHandle, (byte*)errorPin.Pointer + errorBytesRead, - errorBuffer.Length - errorBytesRead, IntPtr.Zero, errorOverlapped); + Interop.Kernel32.ReadFile(errorHandle, (byte*)errorPin.Pointer + errorBytesRead, + errorBuffer.Length - errorBytesRead, IntPtr.Zero, errorOverlapped); - long deadline = timeoutMs >= 0 - ? Environment.TickCount64 + timeoutMs - : long.MaxValue; + long deadline = timeoutMs >= 0 + ? Environment.TickCount64 + timeoutMs + : long.MaxValue; - while (!outputDone || !errorDone) + while (!outputDone || !errorDone) + { + int waitTimeout; + if (timeoutMs >= 0) { - int waitTimeout; - if (timeoutMs >= 0) - { - long remaining = deadline - Environment.TickCount64; - if (remaining <= 0) - { - CancelPendingIOIfNeeded(outputHandle, outputDone, outputOverlapped); - CancelPendingIOIfNeeded(errorHandle, errorDone, errorOverlapped); - throw new TimeoutException(); - } - - waitTimeout = (int)Math.Min(remaining, int.MaxValue); - } - else - { - waitTimeout = Timeout.Infinite; - } - - int waitResult = WaitHandle.WaitAny(waitHandles, waitTimeout); - - if (waitResult == WaitHandle.WaitTimeout) + long remaining = deadline - Environment.TickCount64; + if (remaining <= 0) { CancelPendingIOIfNeeded(outputHandle, outputDone, outputOverlapped); CancelPendingIOIfNeeded(errorHandle, errorDone, errorOverlapped); throw new TimeoutException(); } - bool isError = waitResult == 1; - NativeOverlapped* currentOverlapped = isError ? errorOverlapped : outputOverlapped; - SafeFileHandle currentHandle = isError ? errorHandle : outputHandle; - ref int totalBytesRead = ref (isError ? ref errorBytesRead : ref outputBytesRead); - ref byte[] currentBuffer = ref (isError ? ref errorBuffer : ref outputBuffer); - EventWaitHandle currentEvent = isError ? errorEvent : outputEvent; + waitTimeout = (int)Math.Min(remaining, int.MaxValue); + } + else + { + waitTimeout = Timeout.Infinite; + } - int bytesRead = GetOverlappedResultForPipe(currentHandle, currentOverlapped); + int waitResult = WaitHandle.WaitAny(waitHandles, waitTimeout); - if (bytesRead > 0) - { - totalBytesRead += bytesRead; + if (waitResult == WaitHandle.WaitTimeout) + { + CancelPendingIOIfNeeded(outputHandle, outputDone, outputOverlapped); + CancelPendingIOIfNeeded(errorHandle, errorDone, errorOverlapped); + throw new TimeoutException(); + } + + bool isError = waitResult == 1; + NativeOverlapped* currentOverlapped = isError ? errorOverlapped : outputOverlapped; + SafeFileHandle currentHandle = isError ? errorHandle : outputHandle; + ref int totalBytesRead = ref (isError ? ref errorBytesRead : ref outputBytesRead); + ref byte[] currentBuffer = ref (isError ? ref errorBuffer : ref outputBuffer); + EventWaitHandle currentEvent = isError ? errorEvent : outputEvent; + + int bytesRead = GetOverlappedResultForPipe(currentHandle, currentOverlapped); - if (totalBytesRead == currentBuffer.Length) - { - ref MemoryHandle currentPin = ref (isError ? ref errorPin : ref outputPin); - currentPin.Dispose(); + if (bytesRead > 0) + { + totalBytesRead += bytesRead; - RentLargerBuffer(ref currentBuffer, totalBytesRead); + if (totalBytesRead == currentBuffer.Length) + { + ref MemoryHandle currentPin = ref (isError ? ref errorPin : ref outputPin); + currentPin.Dispose(); - currentPin = currentBuffer.AsMemory().Pin(); - } + RentLargerBuffer(ref currentBuffer, totalBytesRead); - // Reset the event and overlapped for next read. - ResetOverlapped(currentEvent, currentOverlapped); + currentPin = currentBuffer.AsMemory().Pin(); + } - byte* pinPointer = isError ? (byte*)errorPin.Pointer : (byte*)outputPin.Pointer; - Interop.Kernel32.ReadFile(currentHandle, pinPointer + totalBytesRead, - currentBuffer.Length - totalBytesRead, IntPtr.Zero, currentOverlapped); + // Reset the event and overlapped for next read. + ResetOverlapped(currentEvent, currentOverlapped); + + byte* pinPointer = isError ? (byte*)errorPin.Pointer : (byte*)outputPin.Pointer; + Interop.Kernel32.ReadFile(currentHandle, pinPointer + totalBytesRead, + currentBuffer.Length - totalBytesRead, IntPtr.Zero, currentOverlapped); + } + else + { + // EOF: pipe write end was closed. + if (isError) + { + errorDone = true; } else { - // EOF: pipe write end was closed. - if (isError) - { - errorDone = true; - } - else - { - outputDone = true; - } - - // Reset the event so WaitAny won't trigger on this stale handle. - currentEvent.Reset(); + outputDone = true; } + + // Reset the event so WaitAny won't trigger on this stale handle. + currentEvent.Reset(); } } - finally + } + finally + { + if (outputOverlapped is not null) { - if (outputOverlapped != null) - { - NativeMemory.Free(outputOverlapped); - } - - if (errorOverlapped != null) - { - NativeMemory.Free(errorOverlapped); - } + NativeMemory.Free(outputOverlapped); + } - outputEvent.Dispose(); - errorEvent.Dispose(); - outputPin.Dispose(); - errorPin.Dispose(); + if (errorOverlapped is not null) + { + NativeMemory.Free(errorOverlapped); } + + outputEvent?.Dispose(); + errorEvent?.Dispose(); + outputPin.Dispose(); + errorPin.Dispose(); } } 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 61630e6e411631..7818c24ca2fb5d 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 @@ -37,6 +37,8 @@ public partial class Process /// public (string StandardOutput, string StandardError) ReadAllText(TimeSpan? timeout = default) { + ValidateReadAllState(); + byte[] outputBuffer = ArrayPool.Shared.Rent(InitialReadAllBufferSize); byte[] errorBuffer = ArrayPool.Shared.Rent(InitialReadAllBufferSize); int outputBytesRead = 0; @@ -89,6 +91,8 @@ public partial class Process /// public (byte[] StandardOutput, byte[] StandardError) ReadAllBytes(TimeSpan? timeout = default) { + ValidateReadAllState(); + byte[] outputBuffer = ArrayPool.Shared.Rent(InitialReadAllBufferSize); byte[] errorBuffer = ArrayPool.Shared.Rent(InitialReadAllBufferSize); int outputBytesRead = 0; @@ -116,15 +120,10 @@ public partial class Process } /// - /// Validates state, obtains handles, and reads both stdout and stderr pipes into the provided buffers. - /// The caller is responsible for renting and returning the buffers. + /// Validates that the process is not disposed, both stdout and stderr are redirected, + /// and neither stream is in async mode. Sets both streams to sync mode. /// - private void ReadPipesToBuffers( - TimeSpan? timeout, - ref byte[] outputBuffer, - ref int outputBytesRead, - ref byte[] errorBuffer, - ref int errorBytesRead) + private void ValidateReadAllState() { CheckDisposed(); @@ -150,13 +149,26 @@ private void ReadPipesToBuffers( _outputStreamReadMode = StreamReadMode.SyncMode; _errorStreamReadMode = StreamReadMode.SyncMode; + } + /// + /// Obtains handles and reads both stdout and stderr pipes into the provided buffers. + /// The caller is responsible for calling before renting buffers, + /// and for renting and returning the buffers. + /// + private void ReadPipesToBuffers( + TimeSpan? timeout, + ref byte[] outputBuffer, + ref int outputBytesRead, + ref byte[] errorBuffer, + ref int errorBytesRead) + { int timeoutMs = timeout.HasValue ? (int)timeout.Value.TotalMilliseconds : -1; // Infinite - SafeFileHandle outputHandle = GetSafeFileHandleFromStreamReader(_standardOutput, out SafeHandle outputOwner); - SafeFileHandle errorHandle = GetSafeFileHandleFromStreamReader(_standardError, out SafeHandle errorOwner); + SafeFileHandle outputHandle = GetSafeFileHandleFromStreamReader(_standardOutput!, out SafeHandle outputOwner); + SafeFileHandle errorHandle = GetSafeFileHandleFromStreamReader(_standardError!, out SafeHandle errorOwner); bool outputRefAdded = false; bool errorRefAdded = false; From 5d6e9c0aa33e2653eb01de01ed2ea779211e78a6 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 13 Apr 2026 10:05:55 +0200 Subject: [PATCH 07/19] address my own feedback: - refactor the code - restore deleted comments - fix AV bug by setting the lower bit for EventHandle - fix test bug: process argument can't be that long - refactor the tests to use Theories rather than very similar Fact for bytes and text --- .../Diagnostics/Process.Multiplexing.Unix.cs | 23 +- .../Process.Multiplexing.Windows.cs | 94 ++--- .../Diagnostics/Process.Multiplexing.cs | 31 +- .../tests/ProcessMultiplexingTests.cs | 323 +++++++++--------- 4 files changed, 237 insertions(+), 234 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs index a0e2bee0f4656d..39d7c38ba8258e 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs @@ -24,15 +24,14 @@ private static void ReadPipes( { int outputFd = outputHandle.DangerousGetHandle().ToInt32(); int errorFd = errorHandle.DangerousGetHandle().ToInt32(); - bool outputDone = false; - bool errorDone = false; - Interop.PollEvent[] pollFds = new Interop.PollEvent[2]; + Span pollFds = stackalloc Interop.PollEvent[2]; long deadline = timeoutMs >= 0 ? Environment.TickCount64 + timeoutMs : long.MaxValue; + bool outputDone = false, errorDone = false; while (!outputDone || !errorDone) { int numFds = 0; @@ -59,19 +58,9 @@ private static void ReadPipes( } int pollTimeout; - if (timeoutMs >= 0) + if (!TryGetRemainingTimeout(deadline, timeoutMs, out pollTimeout)) { - long remaining = deadline - Environment.TickCount64; - if (remaining <= 0) - { - throw new TimeoutException(); - } - - pollTimeout = (int)Math.Min(remaining, int.MaxValue); - } - else - { - pollTimeout = -1; // Infinite + throw new TimeoutException(); } unsafe @@ -84,10 +73,12 @@ private static void ReadPipes( { if (error == Interop.Error.EINTR) { + // We don't re-issue the poll immediately because we need to check + // if we've already exceeded the overall timeout. continue; } - throw new Win32Exception(Marshal.GetLastPInvokeError()); + throw new Win32Exception(); } if (triggered == 0) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs index 5ffa672d151df7..2791629bbb4bf7 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs @@ -24,12 +24,9 @@ private static unsafe void ReadPipes( ref byte[] errorBuffer, ref int errorBytesRead) { - MemoryHandle outputPin = default; - MemoryHandle errorPin = default; - NativeOverlapped* outputOverlapped = null; - NativeOverlapped* errorOverlapped = null; - EventWaitHandle? outputEvent = null; - EventWaitHandle? errorEvent = null; + MemoryHandle outputPin = default, errorPin = default; + NativeOverlapped* outputOverlapped = null, errorOverlapped = null; + EventWaitHandle? outputEvent = null, errorEvent = null; try { @@ -42,48 +39,29 @@ private static unsafe void ReadPipes( outputOverlapped = AllocateOverlapped(outputEvent); errorOverlapped = AllocateOverlapped(errorEvent); - bool outputDone = false; - bool errorDone = false; - WaitHandle[] waitHandles = [outputEvent, errorEvent]; // Issue initial reads. - Interop.Kernel32.ReadFile(outputHandle, (byte*)outputPin.Pointer + outputBytesRead, - outputBuffer.Length - outputBytesRead, IntPtr.Zero, outputOverlapped); - - Interop.Kernel32.ReadFile(errorHandle, (byte*)errorPin.Pointer + errorBytesRead, - errorBuffer.Length - errorBytesRead, IntPtr.Zero, errorOverlapped); + Interop.Kernel32.ReadFile(outputHandle, (byte*)outputPin.Pointer, outputBuffer.Length, IntPtr.Zero, outputOverlapped); + Interop.Kernel32.ReadFile(errorHandle, (byte*)errorPin.Pointer, errorBuffer.Length, IntPtr.Zero, errorOverlapped); long deadline = timeoutMs >= 0 ? Environment.TickCount64 + timeoutMs : long.MaxValue; + bool outputDone = false, errorDone = false; while (!outputDone || !errorDone) { - int waitTimeout; - if (timeoutMs >= 0) - { - long remaining = deadline - Environment.TickCount64; - if (remaining <= 0) - { - CancelPendingIOIfNeeded(outputHandle, outputDone, outputOverlapped); - CancelPendingIOIfNeeded(errorHandle, errorDone, errorOverlapped); - throw new TimeoutException(); - } - waitTimeout = (int)Math.Min(remaining, int.MaxValue); - } - else - { - waitTimeout = Timeout.Infinite; - } - - int waitResult = WaitHandle.WaitAny(waitHandles, waitTimeout); + int waitResult = TryGetRemainingTimeout(deadline, timeoutMs, out int remainingMilliseconds) + ? WaitHandle.WaitAny(waitHandles, remainingMilliseconds) + : WaitHandle.WaitTimeout; if (waitResult == WaitHandle.WaitTimeout) { CancelPendingIOIfNeeded(outputHandle, outputDone, outputOverlapped); CancelPendingIOIfNeeded(errorHandle, errorDone, errorOverlapped); + throw new TimeoutException(); } @@ -156,7 +134,13 @@ private static unsafe void ReadPipes( private static unsafe NativeOverlapped* AllocateOverlapped(EventWaitHandle waitHandle) { NativeOverlapped* overlapped = (NativeOverlapped*)NativeMemory.AllocZeroed((nuint)sizeof(NativeOverlapped)); - overlapped->EventHandle = waitHandle.SafeWaitHandle.DangerousGetHandle(); + + overlapped->InternalHigh = IntPtr.Zero; + overlapped->InternalLow = IntPtr.Zero; + overlapped->OffsetHigh = 0; + overlapped->OffsetLow = 0; + overlapped->EventHandle = SetLowOrderBit(waitHandle); + return overlapped; } @@ -168,7 +152,7 @@ private static unsafe void ResetOverlapped(EventWaitHandle waitHandle, NativeOve overlapped->InternalLow = IntPtr.Zero; overlapped->OffsetHigh = 0; overlapped->OffsetLow = 0; - overlapped->EventHandle = waitHandle.SafeWaitHandle.DangerousGetHandle(); + overlapped->EventHandle = SetLowOrderBit(waitHandle); } private static unsafe int GetOverlappedResultForPipe(SafeFileHandle handle, NativeOverlapped* overlapped) @@ -179,10 +163,10 @@ private static unsafe int GetOverlappedResultForPipe(SafeFileHandle handle, Nati int errorCode = Marshal.GetLastPInvokeError(); switch (errorCode) { - case Interop.Errors.ERROR_HANDLE_EOF: - case Interop.Errors.ERROR_BROKEN_PIPE: - case Interop.Errors.ERROR_PIPE_NOT_CONNECTED: - return 0; + case Interop.Errors.ERROR_HANDLE_EOF: // logically success with 0 bytes read (read at end of file) + case Interop.Errors.ERROR_BROKEN_PIPE: // For pipes, ERROR_BROKEN_PIPE is the normal end of the pipe. + case Interop.Errors.ERROR_PIPE_NOT_CONNECTED: // Named pipe server has disconnected, return 0 to match NamedPipeClientStream behaviour + return 0; // EOF! default: throw new Win32Exception(errorCode); } @@ -199,11 +183,39 @@ private static unsafe void CancelPendingIOIfNeeded(SafeFileHandle handle, bool d } // CancelIoEx marks matching outstanding I/O requests for cancellation. - Interop.Kernel32.CancelIoEx(handle, overlapped); + // It does not wait for all canceled operations to complete. + // When CancelIoEx returns true, it means that the cancel request was successfully queued. + if (!Interop.Kernel32.CancelIoEx(handle, overlapped)) + { + // Failure has two common meanings: + // ERROR_NOT_FOUND (extremely common). It means: + // - The I/O already completed. + // - Or it never existed. + // - Or it completed between your decision and the call. + // Other errors indicate real failures (invalid handle, driver limitation, etc.). + int errorCode = Marshal.GetLastPInvokeError(); + Debug.Assert(errorCode == Interop.Errors.ERROR_NOT_FOUND, $"CancelIoEx failed with {errorCode}."); + } - // We must observe completion before freeing the OVERLAPPED. + // We must observe completion before freeing the OVERLAPPED in all the above scenarios. + // Use bWait: true to ensure the I/O operation completes before we free the OVERLAPPED structure. + // Per MSDN: "Do not reuse or free the OVERLAPPED structure until GetOverlappedResult returns." int bytesRead = 0; - Interop.Kernel32.GetOverlappedResult(handle, overlapped, ref bytesRead, bWait: true); + if (!Interop.Kernel32.GetOverlappedResult(handle, overlapped, ref bytesRead, bWait: true)) + { + int errorCode = Marshal.GetLastPInvokeError(); + Debug.Assert(errorCode is Interop.Errors.ERROR_OPERATION_ABORTED or Interop.Errors.ERROR_BROKEN_PIPE, $"GetOverlappedResult failed with {errorCode}."); + } } + + /// + /// Returns the event handle with the low-order bit set. + /// Per https://learn.microsoft.com/windows/win32/api/ioapiset/nf-ioapiset-getqueuedcompletionstatus, + /// setting the low-order bit of hEvent in the OVERLAPPED structure prevents the I/O completion + /// from being queued to a completion port bound to the same file object. The kernel masks off + /// the bit when signaling, so the event still works normally. + /// + private static nint SetLowOrderBit(EventWaitHandle waitHandle) + => waitHandle.SafeWaitHandle.DangerousGetHandle() | 1; } } 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 7818c24ca2fb5d..7a4bfbe903d06f 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 @@ -5,6 +5,7 @@ using System.IO; using System.Runtime.InteropServices; using System.Text; +using System.Threading; using Microsoft.Win32.SafeHandles; namespace System.Diagnostics @@ -131,18 +132,15 @@ private void ValidateReadAllState() { throw new InvalidOperationException(SR.CantGetStandardOut); } - - if (_standardError is null) + else if (_standardError is null) { throw new InvalidOperationException(SR.CantGetStandardError); } - - if (_outputStreamReadMode == StreamReadMode.AsyncMode) + else if (_outputStreamReadMode == StreamReadMode.AsyncMode) { throw new InvalidOperationException(SR.CantMixSyncAsyncOperation); } - - if (_errorStreamReadMode == StreamReadMode.AsyncMode) + else if (_errorStreamReadMode == StreamReadMode.AsyncMode) { throw new InvalidOperationException(SR.CantMixSyncAsyncOperation); } @@ -165,7 +163,7 @@ private void ReadPipesToBuffers( { int timeoutMs = timeout.HasValue ? (int)timeout.Value.TotalMilliseconds - : -1; // Infinite + : Timeout.Infinite; SafeFileHandle outputHandle = GetSafeFileHandleFromStreamReader(_standardOutput!, out SafeHandle outputOwner); SafeFileHandle errorHandle = GetSafeFileHandleFromStreamReader(_standardError!, out SafeHandle errorOwner); @@ -230,5 +228,24 @@ private static void RentLargerBuffer(ref byte[] buffer, int bytesRead) ArrayPool.Shared.Return(buffer); buffer = newBuffer; } + + private static bool TryGetRemainingTimeout(long deadline, int originalTimeout, out int remainingTimeoutMs) + { + if (originalTimeout < 0) + { + remainingTimeoutMs = Timeout.Infinite; + return true; + } + + long remaining = deadline - Environment.TickCount64; + if (remaining <= 0) + { + remainingTimeoutMs = 0; + return false; + } + + remainingTimeoutMs = (int)Math.Min(remaining, int.MaxValue); + return true; + } } } diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs index 60ba81a8f7c4eb..a7b6113dcb4d1e 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs @@ -1,279 +1,262 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.IO; using System.Text; using Microsoft.DotNet.RemoteExecutor; +using Microsoft.Win32.SafeHandles; using Xunit; namespace System.Diagnostics.Tests { public class ProcessMultiplexingTests : ProcessTestBase { - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void ReadAllBytes_ThrowsWhenNoStreamsRedirected() - { - Process process = CreateProcess(RemotelyInvokable.Dummy); - process.Start(); - - Assert.Throws(() => process.ReadAllBytes()); + private const string DontPrintAnything = "DO_NOT_PRINT"; - Assert.True(process.WaitForExit(WaitInMS)); - } - - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void ReadAllText_ThrowsWhenNoStreamsRedirected() + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(true)] + [InlineData(false)] + public void ReadAll_ThrowsAfterDispose(bool bytes) { Process process = CreateProcess(RemotelyInvokable.Dummy); process.Start(); + Assert.True(process.WaitForExit(WaitInMS)); - Assert.Throws(() => process.ReadAllText()); + process.Dispose(); - Assert.True(process.WaitForExit(WaitInMS)); + if (bytes) + { + Assert.Throws(() => process.ReadAllBytes()); + } + else + { + Assert.Throws(() => process.ReadAllText()); + } } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void ReadAllBytes_ThrowsWhenOnlyOutputRedirected() + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(true)] + [InlineData(false)] + public void ReadAll_ThrowsWhenNoStreamsRedirected(bool bytes) { - Process process = CreateProcess(RemotelyInvokable.StreamBody); - process.StartInfo.RedirectStandardOutput = true; + Process process = CreateProcess(RemotelyInvokable.Dummy); process.Start(); - Assert.Throws(() => process.ReadAllBytes()); + if (bytes) + { + Assert.Throws(() => process.ReadAllBytes()); + } + else + { + Assert.Throws(() => process.ReadAllText()); + } Assert.True(process.WaitForExit(WaitInMS)); } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void ReadAllBytes_ThrowsWhenOnlyErrorRedirected() + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(true, true)] + [InlineData(true, false)] + [InlineData(false, true)] + [InlineData(false, false)] + public void ReadAll_ThrowsWhenOnlyOutputOrErrorIsRedirected(bool bytes, bool standardOutput) { - Process process = CreateProcess(RemotelyInvokable.ErrorProcessBody); - process.StartInfo.RedirectStandardError = true; + Process process = CreateProcess(RemotelyInvokable.Dummy); + process.StartInfo.RedirectStandardOutput = standardOutput; + process.StartInfo.RedirectStandardError = !standardOutput; process.Start(); - Assert.Throws(() => process.ReadAllBytes()); + if (bytes) + { + Assert.Throws(() => process.ReadAllBytes()); + } + else + { + Assert.Throws(() => process.ReadAllText()); + } Assert.True(process.WaitForExit(WaitInMS)); } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void ReadAllBytes_ThrowsWhenOutputInAsyncMode() + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(true, true)] + [InlineData(true, false)] + [InlineData(false, true)] + [InlineData(false, false)] + public void ReadAll_ThrowsWhenOutputOrErrorIsInAsyncMode(bool bytes, bool standardOutput) { Process process = CreateProcess(RemotelyInvokable.StreamBody); process.StartInfo.RedirectStandardOutput = true; process.StartInfo.RedirectStandardError = true; process.Start(); - process.BeginOutputReadLine(); - - Assert.Throws(() => process.ReadAllBytes()); - - process.CancelOutputRead(); - Assert.True(process.WaitForExit(WaitInMS)); - } - - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void ReadAllBytes_ThrowsWhenErrorInAsyncMode() - { - Process process = CreateProcess(RemotelyInvokable.ErrorProcessBody); - process.StartInfo.RedirectStandardOutput = true; - process.StartInfo.RedirectStandardError = true; - process.Start(); + if (standardOutput) + { + process.BeginOutputReadLine(); + } + else + { + process.BeginErrorReadLine(); + } - process.BeginErrorReadLine(); + if (bytes) + { + Assert.Throws(() => process.ReadAllBytes()); + } + else + { + Assert.Throws(() => process.ReadAllText()); + } - Assert.Throws(() => process.ReadAllBytes()); + if (standardOutput) + { + process.CancelOutputRead(); + } + else + { + process.CancelErrorRead(); + } - process.CancelErrorRead(); Assert.True(process.WaitForExit(WaitInMS)); } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void ReadAllText_ReadsBothOutputAndError() + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(true)] + [InlineData(false)] + public void ReadAll_ThrowsTimeoutExceptionOnTimeout(bool bytes) { - Process process = CreateProcess(() => - { - Console.Out.Write("stdout_text"); - Console.Error.Write("stderr_text"); - return RemoteExecutor.SuccessExitCode; - }); + Process process = CreateProcess(RemotelyInvokable.ReadLine); process.StartInfo.RedirectStandardOutput = true; process.StartInfo.RedirectStandardError = true; + process.StartInfo.RedirectStandardInput = true; process.Start(); - (string standardOutput, string standardError) = process.ReadAllText(); + try + { + if (bytes) + { + Assert.Throws(() => process.ReadAllBytes(TimeSpan.FromMilliseconds(100))); + } + { + Assert.Throws(() => process.ReadAllText(TimeSpan.FromMilliseconds(100))); + } + } + finally + { + process.Kill(); + } - Assert.Equal("stdout_text", standardOutput); - Assert.Equal("stderr_text", standardError); Assert.True(process.WaitForExit(WaitInMS)); } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void ReadAllBytes_ReadsBothOutputAndError() + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData("hello", "world", true)] + [InlineData("hello", "world", false)] + [InlineData("just output", "", true)] + [InlineData("just output", "", false)] + [InlineData("", "just error", true)] + [InlineData("", "just error", false)] + [InlineData("", "", true)] + [InlineData("", "", false)] + public void ReadAllText_ReadsBothOutputAndError(string? standardOutput, string? standardErrror, bool bytes) { - Process process = CreateProcess(() => - { - Console.Out.Write("stdout_bytes"); - Console.Error.Write("stderr_bytes"); - return RemoteExecutor.SuccessExitCode; - }); - process.StartInfo.RedirectStandardOutput = true; - process.StartInfo.RedirectStandardError = true; - process.Start(); - - (byte[] standardOutput, byte[] standardError) = process.ReadAllBytes(); + using Process process = StartPrintingProcess( + string.IsNullOrEmpty(standardOutput) ? DontPrintAnything : standardOutput, + string.IsNullOrEmpty(standardErrror) ? DontPrintAnything : standardErrror); - Assert.Equal(Encoding.Default.GetBytes("stdout_bytes"), standardOutput); - Assert.Equal(Encoding.Default.GetBytes("stderr_bytes"), standardError); - Assert.True(process.WaitForExit(WaitInMS)); - } + if (bytes) + { + (byte[] capturedOutput, byte[] capturedError) = process.ReadAllBytes(); - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void ReadAllText_OutputOnlyOnStdout() - { - Process process = CreateProcess(() => + Assert.Equal(Encoding.Default.GetBytes(standardOutput), capturedOutput); + Assert.Equal(Encoding.Default.GetBytes(standardErrror), capturedError); + } + else { - Console.Out.Write("only_stdout"); - return RemoteExecutor.SuccessExitCode; - }); - process.StartInfo.RedirectStandardOutput = true; - process.StartInfo.RedirectStandardError = true; - process.Start(); + (string capturedOutput, string capturedError) = process.ReadAllText(); - (string standardOutput, string standardError) = process.ReadAllText(); + Assert.Equal(standardOutput, capturedOutput); + Assert.Equal(standardErrror, capturedError); + } - Assert.Equal("only_stdout", standardOutput); - Assert.Equal(string.Empty, standardError); Assert.True(process.WaitForExit(WaitInMS)); } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void ReadAllText_OutputOnlyOnStderr() + public void ReadAllText_ReadsLargeOutput() { + string testFilePath = GetTestFilePath(); + string largeText = new string('A', 100_000); + File.WriteAllText(testFilePath, largeText); + Process process = CreateProcess(() => { - Console.Error.Write("only_stderr"); + Console.OpenStandardInput().CopyTo(Console.OpenStandardOutput()); return RemoteExecutor.SuccessExitCode; }); - process.StartInfo.RedirectStandardOutput = true; - process.StartInfo.RedirectStandardError = true; - process.Start(); - - (string standardOutput, string standardError) = process.ReadAllText(); - Assert.Equal(string.Empty, standardOutput); - Assert.Equal("only_stderr", standardError); - Assert.True(process.WaitForExit(WaitInMS)); - } - - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void ReadAllText_ReadsLargeOutput() - { - string largeText = new string('A', 100_000); - Process process = CreateProcess(RemotelyInvokable.Echo, largeText); + using SafeFileHandle input = File.OpenHandle(testFilePath, FileMode.Open, FileAccess.Read, FileShare.Read); + process.StartInfo.StandardInputHandle = input; process.StartInfo.RedirectStandardOutput = true; process.StartInfo.RedirectStandardError = true; process.Start(); (string standardOutput, string standardError) = process.ReadAllText(); - Assert.Equal(largeText + Environment.NewLine, standardOutput); + Assert.Equal(largeText, standardOutput); Assert.Equal(string.Empty, standardError); Assert.True(process.WaitForExit(WaitInMS)); } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void ReadAllText_TimesOutOnBlockingProcess() + public void ReadAllBytes_ReadsLargeOutput() { - Process process = CreateProcess(RemotelyInvokable.ReadLine); - process.StartInfo.RedirectStandardOutput = true; - process.StartInfo.RedirectStandardError = true; - process.StartInfo.RedirectStandardInput = true; - process.Start(); - - Assert.Throws(() => process.ReadAllText(TimeSpan.FromMilliseconds(100))); + string testFilePath = GetTestFilePath(); + byte[] largeByteArray = new byte[100_000]; + Random.Shared.NextBytes(largeByteArray); + File.WriteAllBytes(testFilePath, largeByteArray); - process.Kill(); - Assert.True(process.WaitForExit(WaitInMS)); - } - - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void ReadAllBytes_OutputOnlyOnStdout() - { Process process = CreateProcess(() => { - Console.Out.Write("out_data"); + Console.OpenStandardInput().CopyTo(Console.OpenStandardOutput()); return RemoteExecutor.SuccessExitCode; }); + + using SafeFileHandle input = File.OpenHandle(testFilePath, FileMode.Open, FileAccess.Read, FileShare.Read); + process.StartInfo.StandardInputHandle = input; process.StartInfo.RedirectStandardOutput = true; process.StartInfo.RedirectStandardError = true; process.Start(); (byte[] standardOutput, byte[] standardError) = process.ReadAllBytes(); - Assert.Equal(Encoding.Default.GetBytes("out_data"), standardOutput); + Assert.Equal(largeByteArray, standardOutput); Assert.Empty(standardError); Assert.True(process.WaitForExit(WaitInMS)); } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void ReadAllBytes_OutputOnlyOnStderr() + private Process StartPrintingProcess(string stdOutText, string stdErrText) { - Process process = CreateProcess(() => + Process process = CreateProcess((stdOut, stdErr) => { - Console.Error.Write("err_data"); - return RemoteExecutor.SuccessExitCode; - }); - process.StartInfo.RedirectStandardOutput = true; - process.StartInfo.RedirectStandardError = true; - process.Start(); - - (byte[] standardOutput, byte[] standardError) = process.ReadAllBytes(); - - Assert.Empty(standardOutput); - Assert.Equal(Encoding.Default.GetBytes("err_data"), standardError); - Assert.True(process.WaitForExit(WaitInMS)); - } - - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void ReadAllText_EmptyOutput() - { - Process process = CreateProcess(() => RemoteExecutor.SuccessExitCode); - process.StartInfo.RedirectStandardOutput = true; - process.StartInfo.RedirectStandardError = true; - process.Start(); + if (stdOut != DontPrintAnything) + { + Console.Out.Write(stdOut); + } - (string standardOutput, string standardError) = process.ReadAllText(); + if (stdErr != DontPrintAnything) + { + Console.Error.Write(stdErr); + } - Assert.Equal(string.Empty, standardOutput); - Assert.Equal(string.Empty, standardError); - Assert.True(process.WaitForExit(WaitInMS)); - } + return RemoteExecutor.SuccessExitCode; + }, stdOutText, stdErrText); - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void ReadAllBytes_TimesOutOnBlockingBothStreams() - { - Process process = CreateProcess(RemotelyInvokable.ReadLine); process.StartInfo.RedirectStandardOutput = true; process.StartInfo.RedirectStandardError = true; - process.StartInfo.RedirectStandardInput = true; process.Start(); - Assert.Throws(() => process.ReadAllBytes(TimeSpan.FromMilliseconds(100))); - - process.Kill(); - Assert.True(process.WaitForExit(WaitInMS)); - } - - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void ReadAllBytes_ThrowsAfterDispose() - { - Process process = CreateProcess(RemotelyInvokable.Dummy); - process.Start(); - Assert.True(process.WaitForExit(WaitInMS)); - - process.Dispose(); - - Assert.Throws(() => process.ReadAllBytes()); + return process; } } } From 99ef1660d1672cfbd2d76d0d715ebbbfa7da3282 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 13 Apr 2026 14:18:56 +0200 Subject: [PATCH 08/19] Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../Diagnostics/Process.Multiplexing.Unix.cs | 2 +- .../Process.Multiplexing.Windows.cs | 49 +++++++++++++++++-- .../Diagnostics/Process.Multiplexing.cs | 2 +- .../tests/ProcessMultiplexingTests.cs | 11 +++-- 4 files changed, 52 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs index 39d7c38ba8258e..2e4b06ef93fdab 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs @@ -78,7 +78,7 @@ private static void ReadPipes( continue; } - throw new Win32Exception(); + throw Interop.GetExceptionForIoErrno(new Interop.ErrorInfo(error)); } if (triggered == 0) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs index 2791629bbb4bf7..8c26e2202d6f72 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs @@ -41,15 +41,41 @@ private static unsafe void ReadPipes( WaitHandle[] waitHandles = [outputEvent, errorEvent]; + static bool QueueRead( + SafeFileHandle handle, + byte* buffer, + int bufferLength, + NativeOverlapped* overlapped, + EventWaitHandle waitHandle) + { + if (Interop.Kernel32.ReadFile(handle, buffer, bufferLength, IntPtr.Zero, overlapped)) + { + waitHandle.Set(); + return true; + } + + int error = Marshal.GetLastPInvokeError(); + if (error == Interop.Errors.ERROR_IO_PENDING) + { + return true; + } + + if (error == Interop.Errors.ERROR_BROKEN_PIPE || error == Interop.Errors.ERROR_HANDLE_EOF) + { + return false; + } + + throw new Win32Exception(error); + } + // Issue initial reads. - Interop.Kernel32.ReadFile(outputHandle, (byte*)outputPin.Pointer, outputBuffer.Length, IntPtr.Zero, outputOverlapped); - Interop.Kernel32.ReadFile(errorHandle, (byte*)errorPin.Pointer, errorBuffer.Length, IntPtr.Zero, errorOverlapped); + bool outputDone = !QueueRead(outputHandle, (byte*)outputPin.Pointer, outputBuffer.Length, outputOverlapped, outputEvent); + bool errorDone = !QueueRead(errorHandle, (byte*)errorPin.Pointer, errorBuffer.Length, errorOverlapped, errorEvent); long deadline = timeoutMs >= 0 ? Environment.TickCount64 + timeoutMs : long.MaxValue; - bool outputDone = false, errorDone = false; while (!outputDone || !errorDone) { @@ -92,8 +118,21 @@ private static unsafe void ReadPipes( ResetOverlapped(currentEvent, currentOverlapped); byte* pinPointer = isError ? (byte*)errorPin.Pointer : (byte*)outputPin.Pointer; - Interop.Kernel32.ReadFile(currentHandle, pinPointer + totalBytesRead, - currentBuffer.Length - totalBytesRead, IntPtr.Zero, currentOverlapped); + if (!QueueRead(currentHandle, pinPointer + totalBytesRead, + currentBuffer.Length - totalBytesRead, currentOverlapped, currentEvent)) + { + if (isError) + { + errorDone = true; + } + else + { + outputDone = true; + } + + // Ensure WaitAny won't trigger on this stale handle. + currentEvent.Reset(); + } } else { 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 7a4bfbe903d06f..9a685bdc038d42 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 @@ -162,7 +162,7 @@ private void ReadPipesToBuffers( ref int errorBytesRead) { int timeoutMs = timeout.HasValue - ? (int)timeout.Value.TotalMilliseconds + ? ToTimeoutMilliseconds(timeout.Value) : Timeout.Infinite; SafeFileHandle outputHandle = GetSafeFileHandleFromStreamReader(_standardOutput!, out SafeHandle outputOwner); diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs index a7b6113dcb4d1e..4c3b1a50348b1f 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs @@ -137,6 +137,7 @@ public void ReadAll_ThrowsTimeoutExceptionOnTimeout(bool bytes) { Assert.Throws(() => process.ReadAllBytes(TimeSpan.FromMilliseconds(100))); } + else { Assert.Throws(() => process.ReadAllText(TimeSpan.FromMilliseconds(100))); } @@ -158,25 +159,25 @@ public void ReadAll_ThrowsTimeoutExceptionOnTimeout(bool bytes) [InlineData("", "just error", false)] [InlineData("", "", true)] [InlineData("", "", false)] - public void ReadAllText_ReadsBothOutputAndError(string? standardOutput, string? standardErrror, bool bytes) + public void ReadAllText_ReadsBothOutputAndError(string standardOutput, string standardError, bool bytes) { using Process process = StartPrintingProcess( - string.IsNullOrEmpty(standardOutput) ? DontPrintAnything : standardOutput, - string.IsNullOrEmpty(standardErrror) ? DontPrintAnything : standardErrror); + string.IsNullOrEmpty(standardOutput) ? DontPrintAnything : standardOutput, + string.IsNullOrEmpty(standardError) ? DontPrintAnything : standardError); if (bytes) { (byte[] capturedOutput, byte[] capturedError) = process.ReadAllBytes(); Assert.Equal(Encoding.Default.GetBytes(standardOutput), capturedOutput); - Assert.Equal(Encoding.Default.GetBytes(standardErrror), capturedError); + Assert.Equal(Encoding.Default.GetBytes(standardError), capturedError); } else { (string capturedOutput, string capturedError) = process.ReadAllText(); Assert.Equal(standardOutput, capturedOutput); - Assert.Equal(standardErrror, capturedError); + Assert.Equal(standardError, capturedError); } Assert.True(process.WaitForExit(WaitInMS)); From 70df3b18c3abfcd00f8002335f1814913f2a5082 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 13 Apr 2026 14:28:54 +0200 Subject: [PATCH 09/19] fix the build --- .../Diagnostics/Process.Multiplexing.Unix.cs | 2 +- .../Process.Multiplexing.Windows.cs | 54 +++++++++---------- .../Diagnostics/Process.Multiplexing.cs | 2 +- 3 files changed, 29 insertions(+), 29 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs index 2e4b06ef93fdab..c88db707c07a86 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs @@ -78,7 +78,7 @@ private static void ReadPipes( continue; } - throw Interop.GetExceptionForIoErrno(new Interop.ErrorInfo(error)); + throw new Win32Exception((int)error); } if (triggered == 0) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs index 8c26e2202d6f72..0edfdbe34b8f35 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs @@ -41,33 +41,6 @@ private static unsafe void ReadPipes( WaitHandle[] waitHandles = [outputEvent, errorEvent]; - static bool QueueRead( - SafeFileHandle handle, - byte* buffer, - int bufferLength, - NativeOverlapped* overlapped, - EventWaitHandle waitHandle) - { - if (Interop.Kernel32.ReadFile(handle, buffer, bufferLength, IntPtr.Zero, overlapped)) - { - waitHandle.Set(); - return true; - } - - int error = Marshal.GetLastPInvokeError(); - if (error == Interop.Errors.ERROR_IO_PENDING) - { - return true; - } - - if (error == Interop.Errors.ERROR_BROKEN_PIPE || error == Interop.Errors.ERROR_HANDLE_EOF) - { - return false; - } - - throw new Win32Exception(error); - } - // Issue initial reads. bool outputDone = !QueueRead(outputHandle, (byte*)outputPin.Pointer, outputBuffer.Length, outputOverlapped, outputEvent); bool errorDone = !QueueRead(errorHandle, (byte*)errorPin.Pointer, errorBuffer.Length, errorOverlapped, errorEvent); @@ -256,5 +229,32 @@ private static unsafe void CancelPendingIOIfNeeded(SafeFileHandle handle, bool d /// private static nint SetLowOrderBit(EventWaitHandle waitHandle) => waitHandle.SafeWaitHandle.DangerousGetHandle() | 1; + + private static unsafe bool QueueRead( + SafeFileHandle handle, + byte* buffer, + int bufferLength, + NativeOverlapped* overlapped, + EventWaitHandle waitHandle) + { + if (Interop.Kernel32.ReadFile(handle, buffer, bufferLength, IntPtr.Zero, overlapped) != 0) + { + waitHandle.Set(); + return true; + } + + int error = Marshal.GetLastPInvokeError(); + if (error == Interop.Errors.ERROR_IO_PENDING) + { + return true; + } + + if (error == Interop.Errors.ERROR_BROKEN_PIPE || error == Interop.Errors.ERROR_HANDLE_EOF) + { + return false; + } + + throw new Win32Exception(error); + } } } 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 9a685bdc038d42..a1e211738e22cc 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 @@ -215,7 +215,7 @@ private static SafeFileHandle GetSafeFileHandleFromStreamReader(StreamReader rea return new SafeFileHandle(pipeStream.SafePipeHandle.DangerousGetHandle(), ownsHandle: false); } - throw new InvalidOperationException(); + throw new UnreachableException(); } /// From 474a484fe54cf492761088e4a9e99abaab3fc50b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 13 Apr 2026 12:56:07 +0000 Subject: [PATCH 10/19] Fix test missing else, use ToTimeoutMilliseconds for validation, fix RentLargerBuffer overflow Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/34214748-db27-499b-88ef-23a1af15515f Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System/Diagnostics/Process.Multiplexing.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 a1e211738e22cc..4ae578ebf009bb 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 @@ -223,7 +223,8 @@ private static SafeFileHandle GetSafeFileHandleFromStreamReader(StreamReader rea /// private static void RentLargerBuffer(ref byte[] buffer, int bytesRead) { - byte[] newBuffer = ArrayPool.Shared.Rent(buffer.Length * 2); + int newSize = (int)Math.Min((long)buffer.Length * 2, Array.MaxLength); + byte[] newBuffer = ArrayPool.Shared.Rent(newSize); Buffer.BlockCopy(buffer, 0, newBuffer, 0, bytesRead); ArrayPool.Shared.Return(buffer); buffer = newBuffer; From 9775b5d68b6affea0f94959320e13e6cba6bd80a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 13 Apr 2026 13:12:26 +0000 Subject: [PATCH 11/19] Require StreamReadMode.Undefined for ReadAll*, rename test, add sync mode test Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/f6ca8588-63c1-4cd8-aeaf-48c716a4ea11 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Diagnostics/Process.Multiplexing.cs | 10 +++---- .../tests/ProcessMultiplexingTests.cs | 29 ++++++++++++++++++- 2 files changed, 33 insertions(+), 6 deletions(-) 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 4ae578ebf009bb..e1c120ebf3d8b1 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 @@ -28,7 +28,7 @@ public partial class Process /// /// Standard output or standard error has not been redirected. /// -or- - /// A redirected stream is already being read asynchronously. + /// A redirected stream has already been used for synchronous or asynchronous reading. /// /// /// The operation did not complete within the specified . @@ -82,7 +82,7 @@ public partial class Process /// /// Standard output or standard error has not been redirected. /// -or- - /// A redirected stream is already being read asynchronously. + /// A redirected stream has already been used for synchronous or asynchronous reading. /// /// /// The operation did not complete within the specified . @@ -122,7 +122,7 @@ public partial class Process /// /// Validates that the process is not disposed, both stdout and stderr are redirected, - /// and neither stream is in async mode. Sets both streams to sync mode. + /// and neither stream has been used (mode must be Undefined). Sets both streams to sync mode. /// private void ValidateReadAllState() { @@ -136,11 +136,11 @@ private void ValidateReadAllState() { throw new InvalidOperationException(SR.CantGetStandardError); } - else if (_outputStreamReadMode == StreamReadMode.AsyncMode) + else if (_outputStreamReadMode != StreamReadMode.Undefined) { throw new InvalidOperationException(SR.CantMixSyncAsyncOperation); } - else if (_errorStreamReadMode == StreamReadMode.AsyncMode) + else if (_errorStreamReadMode != StreamReadMode.Undefined) { throw new InvalidOperationException(SR.CantMixSyncAsyncOperation); } diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs index 4c3b1a50348b1f..7f8caf6724f673 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs @@ -78,6 +78,33 @@ public void ReadAll_ThrowsWhenOnlyOutputOrErrorIsRedirected(bool bytes, bool sta Assert.True(process.WaitForExit(WaitInMS)); } + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(true, true)] + [InlineData(true, false)] + [InlineData(false, true)] + [InlineData(false, false)] + public void ReadAll_ThrowsWhenOutputOrErrorIsInSyncMode(bool bytes, bool standardOutput) + { + Process process = CreateProcess(RemotelyInvokable.StreamBody); + process.StartInfo.RedirectStandardOutput = true; + process.StartInfo.RedirectStandardError = true; + process.Start(); + + // Access the StreamReader property to set the stream to sync mode + _ = standardOutput ? process.StandardOutput : process.StandardError; + + if (bytes) + { + Assert.Throws(() => process.ReadAllBytes()); + } + else + { + Assert.Throws(() => process.ReadAllText()); + } + + Assert.True(process.WaitForExit(WaitInMS)); + } + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] [InlineData(true, true)] [InlineData(true, false)] @@ -159,7 +186,7 @@ public void ReadAll_ThrowsTimeoutExceptionOnTimeout(bool bytes) [InlineData("", "just error", false)] [InlineData("", "", true)] [InlineData("", "", false)] - public void ReadAllText_ReadsBothOutputAndError(string standardOutput, string standardError, bool bytes) + public void ReadAll_ReadsBothOutputAndError(string standardOutput, string standardError, bool bytes) { using Process process = StartPrintingProcess( string.IsNullOrEmpty(standardOutput) ? DontPrintAnything : standardOutput, From 1d30ed2382d9a7a9e403cf53c1ba006c581e3702 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 13 Apr 2026 14:08:01 +0000 Subject: [PATCH 12/19] Address feedback: Win32Exception fix, ref assembly order, AllocateOverlapped cleanup, new tests, StreamBody->Dummy Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/dac42b05-c591-4e1c-b751-9eadeb376728 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../ref/System.Diagnostics.Process.cs | 4 +- .../Diagnostics/Process.Multiplexing.Unix.cs | 2 +- .../Process.Multiplexing.Windows.cs | 5 -- .../tests/ProcessMultiplexingTests.cs | 67 ++++++++++++++++++- 4 files changed, 69 insertions(+), 9 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 fcd33485d43671..0a6e75ee3cba55 100644 --- a/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs +++ b/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs @@ -157,6 +157,8 @@ public void Kill() { } public void Kill(bool entireProcessTree) { } public static void LeaveDebugMode() { } protected void OnExited() { } + public (byte[] StandardOutput, byte[] StandardError) ReadAllBytes(System.TimeSpan? timeout = default(System.TimeSpan?)) { throw null; } + public (string StandardOutput, string StandardError) ReadAllText(System.TimeSpan? timeout = default(System.TimeSpan?)) { throw null; } public void Refresh() { } [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] @@ -185,8 +187,6 @@ public void Refresh() { } [System.Runtime.Versioning.SupportedOSPlatformAttribute("windows")] public static System.Diagnostics.Process? Start(string fileName, string arguments, string userName, System.Security.SecureString password, string domain) { throw null; } public override string ToString() { throw null; } - public (string StandardOutput, string StandardError) ReadAllText(System.TimeSpan? timeout = default(System.TimeSpan?)) { throw null; } - public (byte[] StandardOutput, byte[] StandardError) ReadAllBytes(System.TimeSpan? timeout = default(System.TimeSpan?)) { throw null; } public void WaitForExit() { } public bool WaitForExit(int milliseconds) { throw null; } public bool WaitForExit(System.TimeSpan timeout) { throw null; } diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs index c88db707c07a86..1fecadb4e26304 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs @@ -78,7 +78,7 @@ private static void ReadPipes( continue; } - throw new Win32Exception((int)error); + throw new Win32Exception(Interop.Sys.ConvertErrorPalToPlatform(error)); } if (triggered == 0) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs index 0edfdbe34b8f35..79aac5356e1aef 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs @@ -146,11 +146,6 @@ private static unsafe void ReadPipes( private static unsafe NativeOverlapped* AllocateOverlapped(EventWaitHandle waitHandle) { NativeOverlapped* overlapped = (NativeOverlapped*)NativeMemory.AllocZeroed((nuint)sizeof(NativeOverlapped)); - - overlapped->InternalHigh = IntPtr.Zero; - overlapped->InternalLow = IntPtr.Zero; - overlapped->OffsetHigh = 0; - overlapped->OffsetLow = 0; overlapped->EventHandle = SetLowOrderBit(waitHandle); return overlapped; diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs index 7f8caf6724f673..8391566ab28df5 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs @@ -85,7 +85,7 @@ public void ReadAll_ThrowsWhenOnlyOutputOrErrorIsRedirected(bool bytes, bool sta [InlineData(false, false)] public void ReadAll_ThrowsWhenOutputOrErrorIsInSyncMode(bool bytes, bool standardOutput) { - Process process = CreateProcess(RemotelyInvokable.StreamBody); + Process process = CreateProcess(RemotelyInvokable.Dummy); process.StartInfo.RedirectStandardOutput = true; process.StartInfo.RedirectStandardError = true; process.Start(); @@ -210,6 +210,71 @@ public void ReadAll_ReadsBothOutputAndError(string standardOutput, string standa Assert.True(process.WaitForExit(WaitInMS)); } + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void ReadAllText_ReadsInterleavedOutput() + { + const int iterations = 100; + using Process process = CreateProcess(() => + { + for (int i = 0; i < iterations; i++) + { + Console.Out.Write($"out{i} "); + Console.Out.Flush(); + Console.Error.Write($"err{i} "); + Console.Error.Flush(); + } + + return RemoteExecutor.SuccessExitCode; + }); + + process.StartInfo.RedirectStandardOutput = true; + process.StartInfo.RedirectStandardError = true; + process.Start(); + + (string standardOutput, string standardError) = process.ReadAllText(); + + for (int i = 0; i < iterations; i++) + { + Assert.Contains($"out{i} ", standardOutput); + Assert.Contains($"err{i} ", standardError); + } + + Assert.True(process.WaitForExit(WaitInMS)); + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void ReadAllBytes_ReadsBinaryDataWithNullBytes() + { + string testFilePath = GetTestFilePath(); + byte[] binaryData = new byte[1024]; + Random.Shared.NextBytes(binaryData); + // Ensure there are null bytes throughout the data. + for (int i = 0; i < binaryData.Length; i += 10) + { + binaryData[i] = 0; + } + + File.WriteAllBytes(testFilePath, binaryData); + + Process process = CreateProcess(() => + { + Console.OpenStandardInput().CopyTo(Console.OpenStandardOutput()); + return RemoteExecutor.SuccessExitCode; + }); + + using SafeFileHandle input = File.OpenHandle(testFilePath, FileMode.Open, FileAccess.Read, FileShare.Read); + process.StartInfo.StandardInputHandle = input; + process.StartInfo.RedirectStandardOutput = true; + process.StartInfo.RedirectStandardError = true; + process.Start(); + + (byte[] standardOutput, byte[] standardError) = process.ReadAllBytes(); + + Assert.Equal(binaryData, standardOutput); + Assert.Empty(standardError); + Assert.True(process.WaitForExit(WaitInMS)); + } + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] public void ReadAllText_ReadsLargeOutput() { From 77dceb09b3933c672806cd83eaf207693bff76e5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 13 Apr 2026 14:14:35 +0000 Subject: [PATCH 13/19] Add using to Process in ReadAllBytes_ReadsBinaryDataWithNullBytes test Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/dac42b05-c591-4e1c-b751-9eadeb376728 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../tests/ProcessMultiplexingTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs index 8391566ab28df5..b9a7913c4b36ff 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs @@ -256,7 +256,7 @@ public void ReadAllBytes_ReadsBinaryDataWithNullBytes() File.WriteAllBytes(testFilePath, binaryData); - Process process = CreateProcess(() => + using Process process = CreateProcess(() => { Console.OpenStandardInput().CopyTo(Console.OpenStandardOutput()); return RemoteExecutor.SuccessExitCode; From 9896de07a71ea59622cf323d4781c49b20bdaadc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 13 Apr 2026 14:27:30 +0000 Subject: [PATCH 14/19] Fix RentLargerBuffer infinite loop at Array.MaxLength, use Assert.Equal in interleaved test Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/560e33ea-a2eb-4f65-a9c9-a2bc3fc32986 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System/Diagnostics/Process.Multiplexing.cs | 5 +++++ .../tests/ProcessMultiplexingTests.cs | 9 +++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) 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 e1c120ebf3d8b1..8c320da866039f 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 @@ -223,6 +223,11 @@ private static SafeFileHandle GetSafeFileHandleFromStreamReader(StreamReader rea /// private static void RentLargerBuffer(ref byte[] buffer, int bytesRead) { + if (buffer.Length >= Array.MaxLength) + { + throw new OutOfMemoryException(); + } + int newSize = (int)Math.Min((long)buffer.Length * 2, Array.MaxLength); byte[] newBuffer = ArrayPool.Shared.Rent(newSize); Buffer.BlockCopy(buffer, 0, newBuffer, 0, bytesRead); diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs index b9a7913c4b36ff..b0fe3b2a78742b 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs @@ -233,12 +233,17 @@ public void ReadAllText_ReadsInterleavedOutput() (string standardOutput, string standardError) = process.ReadAllText(); + var expectedOutput = new StringBuilder(); + var expectedError = new StringBuilder(); for (int i = 0; i < iterations; i++) { - Assert.Contains($"out{i} ", standardOutput); - Assert.Contains($"err{i} ", standardError); + expectedOutput.Append($"out{i} "); + expectedError.Append($"err{i} "); } + Assert.Equal(expectedOutput.ToString(), standardOutput); + Assert.Equal(expectedError.ToString(), standardError); + Assert.True(process.WaitForExit(WaitInMS)); } From c3bd6bcbe5a7b907ad545d6a4204b4ea5eeb9c82 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 13 Apr 2026 16:42:44 +0200 Subject: [PATCH 15/19] address my own feedback --- .../tests/ProcessMultiplexingTests.cs | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs index b0fe3b2a78742b..906cbbe4c47202 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessMultiplexingTests.cs @@ -233,8 +233,8 @@ public void ReadAllText_ReadsInterleavedOutput() (string standardOutput, string standardError) = process.ReadAllText(); - var expectedOutput = new StringBuilder(); - var expectedError = new StringBuilder(); + StringBuilder expectedOutput = new(); + StringBuilder expectedError = new(); for (int i = 0; i < iterations; i++) { expectedOutput.Append($"out{i} "); @@ -261,14 +261,14 @@ public void ReadAllBytes_ReadsBinaryDataWithNullBytes() File.WriteAllBytes(testFilePath, binaryData); - using Process process = CreateProcess(() => + using Process process = CreateProcess(static path => { - Console.OpenStandardInput().CopyTo(Console.OpenStandardOutput()); + using FileStream source = File.OpenRead(path); + source.CopyTo(Console.OpenStandardOutput()); + return RemoteExecutor.SuccessExitCode; - }); + }, testFilePath); - using SafeFileHandle input = File.OpenHandle(testFilePath, FileMode.Open, FileAccess.Read, FileShare.Read); - process.StartInfo.StandardInputHandle = input; process.StartInfo.RedirectStandardOutput = true; process.StartInfo.RedirectStandardError = true; process.Start(); @@ -287,14 +287,14 @@ public void ReadAllText_ReadsLargeOutput() string largeText = new string('A', 100_000); File.WriteAllText(testFilePath, largeText); - Process process = CreateProcess(() => + using Process process = CreateProcess(static path => { - Console.OpenStandardInput().CopyTo(Console.OpenStandardOutput()); + using FileStream source = File.OpenRead(path); + source.CopyTo(Console.OpenStandardOutput()); + return RemoteExecutor.SuccessExitCode; - }); + }, testFilePath); - using SafeFileHandle input = File.OpenHandle(testFilePath, FileMode.Open, FileAccess.Read, FileShare.Read); - process.StartInfo.StandardInputHandle = input; process.StartInfo.RedirectStandardOutput = true; process.StartInfo.RedirectStandardError = true; process.Start(); @@ -314,14 +314,14 @@ public void ReadAllBytes_ReadsLargeOutput() Random.Shared.NextBytes(largeByteArray); File.WriteAllBytes(testFilePath, largeByteArray); - Process process = CreateProcess(() => + using Process process = CreateProcess(static path => { - Console.OpenStandardInput().CopyTo(Console.OpenStandardOutput()); + using FileStream source = File.OpenRead(path); + source.CopyTo(Console.OpenStandardOutput()); + return RemoteExecutor.SuccessExitCode; - }); + }, testFilePath); - using SafeFileHandle input = File.OpenHandle(testFilePath, FileMode.Open, FileAccess.Read, FileShare.Read); - process.StartInfo.StandardInputHandle = input; process.StartInfo.RedirectStandardOutput = true; process.StartInfo.RedirectStandardError = true; process.Start(); From e8cbe82a8b56f4e30e20cd7aa637889f9c40cccc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Apr 2026 16:58:38 +0000 Subject: [PATCH 16/19] Address jkotas and tmds feedback: PinnedGCHandle, RentLargerBuffer improvements, simplified poll check Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/8b364ac1-c21d-4c74-88ca-44d6a5ea4064 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Diagnostics/Process.Multiplexing.Unix.cs | 2 +- .../Process.Multiplexing.Windows.cs | 18 ++++++++---------- .../System/Diagnostics/Process.Multiplexing.cs | 9 +++------ 3 files changed, 12 insertions(+), 17 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs index 1fecadb4e26304..6666667c6471fc 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Unix.cs @@ -90,7 +90,7 @@ private static void ReadPipes( for (int i = 0; i < numFds; i++) { - if ((pollFds[i].TriggeredEvents & (Interop.PollEvents.POLLIN | Interop.PollEvents.POLLHUP | Interop.PollEvents.POLLERR)) == Interop.PollEvents.POLLNONE) + if (pollFds[i].TriggeredEvents == Interop.PollEvents.POLLNONE) { continue; } diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs index 79aac5356e1aef..f8748da60c9a39 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs @@ -1,7 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Buffers; using System.ComponentModel; using System.Runtime.InteropServices; using System.Threading; @@ -24,14 +23,14 @@ private static unsafe void ReadPipes( ref byte[] errorBuffer, ref int errorBytesRead) { - MemoryHandle outputPin = default, errorPin = default; + PinnedGCHandle outputPin = default, errorPin = default; NativeOverlapped* outputOverlapped = null, errorOverlapped = null; EventWaitHandle? outputEvent = null, errorEvent = null; try { - outputPin = outputBuffer.AsMemory().Pin(); - errorPin = errorBuffer.AsMemory().Pin(); + outputPin = new PinnedGCHandle(outputBuffer); + errorPin = new PinnedGCHandle(errorBuffer); outputEvent = new EventWaitHandle(initialState: false, EventResetMode.ManualReset); errorEvent = new EventWaitHandle(initialState: false, EventResetMode.ManualReset); @@ -42,8 +41,8 @@ private static unsafe void ReadPipes( WaitHandle[] waitHandles = [outputEvent, errorEvent]; // Issue initial reads. - bool outputDone = !QueueRead(outputHandle, (byte*)outputPin.Pointer, outputBuffer.Length, outputOverlapped, outputEvent); - bool errorDone = !QueueRead(errorHandle, (byte*)errorPin.Pointer, errorBuffer.Length, errorOverlapped, errorEvent); + bool outputDone = !QueueRead(outputHandle, outputPin.GetAddressOfArrayData(), outputBuffer.Length, outputOverlapped, outputEvent); + bool errorDone = !QueueRead(errorHandle, errorPin.GetAddressOfArrayData(), errorBuffer.Length, errorOverlapped, errorEvent); long deadline = timeoutMs >= 0 ? Environment.TickCount64 + timeoutMs @@ -79,18 +78,17 @@ private static unsafe void ReadPipes( if (totalBytesRead == currentBuffer.Length) { - ref MemoryHandle currentPin = ref (isError ? ref errorPin : ref outputPin); - currentPin.Dispose(); + ref PinnedGCHandle currentPin = ref (isError ? ref errorPin : ref outputPin); RentLargerBuffer(ref currentBuffer, totalBytesRead); - currentPin = currentBuffer.AsMemory().Pin(); + currentPin.Target = currentBuffer; } // Reset the event and overlapped for next read. ResetOverlapped(currentEvent, currentOverlapped); - byte* pinPointer = isError ? (byte*)errorPin.Pointer : (byte*)outputPin.Pointer; + byte* pinPointer = isError ? errorPin.GetAddressOfArrayData() : outputPin.GetAddressOfArrayData(); if (!QueueRead(currentHandle, pinPointer + totalBytesRead, currentBuffer.Length - totalBytesRead, currentOverlapped, currentEvent)) { 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 8c320da866039f..8da1afc4b1a6ad 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 @@ -223,16 +223,13 @@ private static SafeFileHandle GetSafeFileHandleFromStreamReader(StreamReader rea /// private static void RentLargerBuffer(ref byte[] buffer, int bytesRead) { - if (buffer.Length >= Array.MaxLength) - { - throw new OutOfMemoryException(); - } - int newSize = (int)Math.Min((long)buffer.Length * 2, Array.MaxLength); + newSize = Math.Max(buffer.Length + 1, newSize); byte[] newBuffer = ArrayPool.Shared.Rent(newSize); Buffer.BlockCopy(buffer, 0, newBuffer, 0, bytesRead); - ArrayPool.Shared.Return(buffer); + byte[] oldBuffer = buffer; buffer = newBuffer; + ArrayPool.Shared.Return(oldBuffer); } private static bool TryGetRemainingTimeout(long deadline, int originalTimeout, out int remainingTimeoutMs) From 6ecc0b9accd3859cb0dae7a07724ebacb109ca26 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Apr 2026 19:01:59 +0000 Subject: [PATCH 17/19] Flip WaitHandle order to prioritize stderr, add Raymond Chen blog links Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/7af21a01-1482-4765-91af-d73ea6f088cc Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Diagnostics/Process.Multiplexing.Windows.cs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs index f8748da60c9a39..6cbd1fc525e915 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs @@ -38,7 +38,8 @@ private static unsafe void ReadPipes( outputOverlapped = AllocateOverlapped(outputEvent); errorOverlapped = AllocateOverlapped(errorEvent); - WaitHandle[] waitHandles = [outputEvent, errorEvent]; + // Error output gets index 0 so WaitAny services it first when both are signaled. + WaitHandle[] waitHandles = [errorEvent, outputEvent]; // Issue initial reads. bool outputDone = !QueueRead(outputHandle, outputPin.GetAddressOfArrayData(), outputBuffer.Length, outputOverlapped, outputEvent); @@ -63,7 +64,7 @@ private static unsafe void ReadPipes( throw new TimeoutException(); } - bool isError = waitResult == 1; + bool isError = waitResult == 0; NativeOverlapped* currentOverlapped = isError ? errorOverlapped : outputOverlapped; SafeFileHandle currentHandle = isError ? errorHandle : outputHandle; ref int totalBytesRead = ref (isError ? ref errorBytesRead : ref outputBytesRead); @@ -180,6 +181,13 @@ private static unsafe int GetOverlappedResultForPipe(SafeFileHandle handle, Nati return bytesRead; } + /// + /// Cancels a pending overlapped I/O and waits for completion before returning. + /// See Raymond Chen's series on safe cancellation: + /// https://devblogs.microsoft.com/oldnewthing/20110202-00/?p=11613 + /// https://devblogs.microsoft.com/oldnewthing/20110203-00/?p=11603 + /// https://devblogs.microsoft.com/oldnewthing/20110204-00/?p=11583 + /// private static unsafe void CancelPendingIOIfNeeded(SafeFileHandle handle, bool done, NativeOverlapped* overlapped) { if (done) From 8f7c3c8c90f7acbbdaa7d906183b6dd81e583ec1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Apr 2026 19:13:16 +0000 Subject: [PATCH 18/19] =?UTF-8?q?Remove=20Raymond=20Chen=20blog=20links=20?= =?UTF-8?q?from=20CancelPendingIOIfNeeded=20=E2=80=94=20keep=20only=20offi?= =?UTF-8?q?cial=20docs=20references?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/ef4fa489-ab10-4574-9069-ca32ccaac99b Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System/Diagnostics/Process.Multiplexing.Windows.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs index 6cbd1fc525e915..6d28e278a28bac 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Multiplexing.Windows.cs @@ -183,10 +183,6 @@ private static unsafe int GetOverlappedResultForPipe(SafeFileHandle handle, Nati /// /// Cancels a pending overlapped I/O and waits for completion before returning. - /// See Raymond Chen's series on safe cancellation: - /// https://devblogs.microsoft.com/oldnewthing/20110202-00/?p=11613 - /// https://devblogs.microsoft.com/oldnewthing/20110203-00/?p=11603 - /// https://devblogs.microsoft.com/oldnewthing/20110204-00/?p=11583 /// private static unsafe void CancelPendingIOIfNeeded(SafeFileHandle handle, bool done, NativeOverlapped* overlapped) { From c92f673746ef29778e4cb5d5991894720be2483e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Apr 2026 20:36:56 +0000 Subject: [PATCH 19/19] Use non-blocking IO on Unix: DangerousSetIsNonBlocking + Interop.Sys.Read with EAGAIN handling Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/2646b5c6-b0ae-4ae3-99ca-064187661e73 Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System.Diagnostics.Process.csproj | 4 ++ .../Diagnostics/Process.Multiplexing.Unix.cs | 39 +++++++++++++++++-- 2 files changed, 39 insertions(+), 4 deletions(-) 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 e4dd3598698383..6db2a72aeb08fa 100644 --- a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -259,6 +259,10 @@ Link="Common\Interop\Unix\Interop.Poll.Structs.cs" /> + + - /// Reads from both standard output and standard error pipes using Unix poll-based multiplexing. + /// Reads from both standard output and standard error pipes using Unix poll-based multiplexing + /// with non-blocking reads. /// private static void ReadPipes( SafeFileHandle outputHandle, @@ -25,6 +25,11 @@ private static void ReadPipes( int outputFd = outputHandle.DangerousGetHandle().ToInt32(); int errorFd = errorHandle.DangerousGetHandle().ToInt32(); + if (Interop.Sys.Fcntl.DangerousSetIsNonBlocking((IntPtr)outputFd, 1) != 0 || Interop.Sys.Fcntl.DangerousSetIsNonBlocking((IntPtr)errorFd, 1) != 0) + { + throw new Win32Exception(); + } + Span pollFds = stackalloc Interop.PollEvent[2]; long deadline = timeoutMs >= 0 @@ -101,7 +106,7 @@ private static void ReadPipes( ref int currentBytesRead = ref (isError ? ref errorBytesRead : ref outputBytesRead); ref bool currentDone = ref (isError ? ref errorDone : ref outputDone); - int bytesRead = RandomAccess.Read(currentHandle, currentBuffer.AsSpan(currentBytesRead), fileOffset: 0); + int bytesRead = ReadNonBlocking(currentHandle, currentBuffer, currentBytesRead); if (bytesRead > 0) { currentBytesRead += bytesRead; @@ -111,11 +116,37 @@ private static void ReadPipes( RentLargerBuffer(ref currentBuffer, currentBytesRead); } } - else + else if (bytesRead == 0) { + // EOF: pipe write end was closed. currentDone = true; } + // bytesRead < 0 means EAGAIN — nothing available yet, let poll retry. + } + } + } + + /// + /// Performs a non-blocking read from the given handle into the buffer starting at the specified offset. + /// Returns the number of bytes read, 0 for EOF, or -1 for EAGAIN (nothing available yet). + /// + private static unsafe int ReadNonBlocking(SafeFileHandle handle, byte[] buffer, int offset) + { + fixed (byte* pBuffer = buffer) + { + int bytesRead = Interop.Sys.Read(handle, pBuffer + offset, buffer.Length - offset); + if (bytesRead < 0) + { + Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); + if (errorInfo.Error == Interop.Error.EAGAIN) + { + return -1; + } + + throw new Win32Exception(errorInfo.RawErrno); } + + return bytesRead; } } }