diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs index 8a7e2b914c4850..f073bd1e8d54f5 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs @@ -640,12 +640,12 @@ ref processInfo // pointer to PROCESS_INFORMATION if (startInfo.RedirectStandardOutput) { Encoding enc = startInfo.StandardOutputEncoding ?? GetEncoding((int)Interop.Kernel32.GetConsoleOutputCP()); - _standardOutput = new StreamReader(new FileStream(parentOutputPipeHandle!, FileAccess.Read, 4096, false), enc, true, 4096); + _standardOutput = new StreamReader(new FileStream(parentOutputPipeHandle!, FileAccess.Read, 4096, parentOutputPipeHandle!.IsAsync), enc, true, 4096); } if (startInfo.RedirectStandardError) { Encoding enc = startInfo.StandardErrorEncoding ?? GetEncoding((int)Interop.Kernel32.GetConsoleOutputCP()); - _standardError = new StreamReader(new FileStream(parentErrorPipeHandle!, FileAccess.Read, 4096, false), enc, true, 4096); + _standardError = new StreamReader(new FileStream(parentErrorPipeHandle!, FileAccess.Read, 4096, parentErrorPipeHandle!.IsAsync), enc, true, 4096); } commandLine.Dispose(); @@ -807,9 +807,13 @@ private SafeProcessHandle GetProcessHandle(int access, bool throwIfExited = true // methods such as WriteLine as well as native CRT functions like printf) which are making an // assumption that the console standard handles (obtained via GetStdHandle()) are opened // for synchronous I/O and hence they can work fine with ReadFile/WriteFile synchronously! + // We therefore only open the parent's end of the pipe for async I/O (overlapped), while the + // child's end is always opened for synchronous I/O so the child process can use it normally. private static void CreatePipe(out SafeFileHandle parentHandle, out SafeFileHandle childHandle, bool parentInputs) { - SafeFileHandle.CreateAnonymousPipe(out SafeFileHandle readHandle, out SafeFileHandle writeHandle); + // Only the parent's read end benefits from async I/O; stdin is always sync. + // asyncRead applies to the read handle; asyncWrite to the write handle. + SafeFileHandle.CreateAnonymousPipe(out SafeFileHandle readHandle, out SafeFileHandle writeHandle, asyncRead: !parentInputs, asyncWrite: false); // parentInputs=true: parent writes to pipe, child reads (stdin redirect). // parentInputs=false: parent reads from pipe, child writes (stdout/stderr redirect). diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStandardConsoleTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStandardConsoleTests.cs index b55384a1606527..d89f4909505e35 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStandardConsoleTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStandardConsoleTests.cs @@ -1,9 +1,12 @@ // 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.IO.Pipes; using System.Text; using Microsoft.DotNet.RemoteExecutor; using Microsoft.Win32; +using Microsoft.Win32.SafeHandles; using Xunit; namespace System.Diagnostics.Tests @@ -55,5 +58,46 @@ void RunWithExpectedCodePage(int expectedCodePage) Interop.SetConsoleOutputCP(outputEncoding); } } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void Start_Redirect_StandardHandles_UseRightAsyncMode() + { + using (Process process = CreateProcess(() => + { + Assert.False(Console.OpenStandardInputHandle().IsAsync); + Assert.False(Console.OpenStandardOutputHandle().IsAsync); + Assert.False(Console.OpenStandardErrorHandle().IsAsync); + + return RemoteExecutor.SuccessExitCode; + })) + { + process.StartInfo.RedirectStandardInput = true; + process.StartInfo.RedirectStandardOutput = true; + process.StartInfo.RedirectStandardError = true; + + Assert.True(process.Start()); + + Assert.False(GetSafeFileHandle(process.StandardInput.BaseStream).IsAsync); + Assert.Equal(OperatingSystem.IsWindows(), GetSafeFileHandle(process.StandardOutput.BaseStream).IsAsync); + Assert.Equal(OperatingSystem.IsWindows(), GetSafeFileHandle(process.StandardError.BaseStream).IsAsync); + + process.WaitForExit(); // ensure event handlers have completed + Assert.Equal(RemoteExecutor.SuccessExitCode, process.ExitCode); + } + + static SafeFileHandle GetSafeFileHandle(Stream baseStream) + { + switch (baseStream) + { + case FileStream fileStream: + return fileStream.SafeFileHandle; + case AnonymousPipeClientStream anonymousPipeStream: + SafePipeHandle safePipeHandle = anonymousPipeStream.SafePipeHandle; + return new SafeFileHandle(safePipeHandle.DangerousGetHandle(), ownsHandle: false); + default: + throw new NotSupportedException(); + } + } + } } } diff --git a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs index 3d5f8d22db02ba..dad993147493cb 100644 --- a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs @@ -46,7 +46,14 @@ public static partial void CreateAnonymousPipe(out SafeFileHandle readHandle, ou else { // When one or both ends are async, use named pipes to support async I/O. - string pipeName = $@"\\.\pipe\dotnet_{Guid.NewGuid():N}"; + + // From https://learn.microsoft.com/windows/win32/api/winbase/nf-winbase-createnamedpipea#remarks: + // "Windows 10, version 1709: Pipes are only supported within an app-container; ie, + // from one UWP process to another UWP process that's part of the same app. + // Also, named pipes must use the syntax \\.\pipe\LOCAL\ for the pipe name." + // That is why we use "LOCAL" namespace for the pipe name, + // so that it works in AppContainer and outside of it. + string pipeName = $@"\\.\pipe\LOCAL\dotnet_{Guid.NewGuid():N}"; // Security: we don't need to specify a security descriptor, because // we allow only for 1 instance of the pipe and immediately open the write end, @@ -64,8 +71,15 @@ public static partial void CreateAnonymousPipe(out SafeFileHandle readHandle, ou const int pipeMode = (int)(Interop.Kernel32.PipeOptions.PIPE_TYPE_BYTE | Interop.Kernel32.PipeOptions.PIPE_READMODE_BYTE); // Data is read from the pipe as a stream of bytes - // We could consider specifying a larger buffer size. - tempReadHandle = Interop.Kernel32.CreateNamedPipeFileHandle(pipeName, openMode, pipeMode, 1, 0, 0, 0, ref securityAttributes); + tempReadHandle = Interop.Kernel32.CreateNamedPipeFileHandle( + pipeName, + openMode, + pipeMode, + maxInstances: 1, // we don't want anybody else to open this pipe + outBufferSize: 0, // unused (we use it only for reading) + inBufferSize: 4 * 4096, // CreatePipe uses a 4096 buffer by default, we use bigger buffer for better performance + defaultTimeout: (int)TimeSpan.FromSeconds(120).TotalMilliseconds, // same as the default for CreatePipe + ref securityAttributes); try {