From f47b2fd066243e26ee086d35bd6974b94fd35708 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 18 Mar 2026 22:14:22 +0000 Subject: [PATCH 01/22] Refactor StartCore to accept SafeFileHandle parameters; move pipe creation and stream setup to Process.cs Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Interop.ForkAndExecProcess.cs | 7 +- .../src/System/Diagnostics/Process.Unix.cs | 83 +++++++------ .../src/System/Diagnostics/Process.Win32.cs | 4 +- .../src/System/Diagnostics/Process.Windows.cs | 110 ++++++++---------- .../src/System/Diagnostics/Process.cs | 84 ++++++++++++- src/native/libs/System.Native/pal_process.c | 86 ++++---------- src/native/libs/System.Native/pal_process.h | 6 +- .../libs/System.Native/pal_process_wasi.c | 6 +- 8 files changed, 206 insertions(+), 180 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs index bc947d0ef4ef49..24e82a12dc2727 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs @@ -6,6 +6,7 @@ using System.Diagnostics; using System.Runtime.InteropServices; using System.Text; +using Microsoft.Win32.SafeHandles; internal static partial class Interop { @@ -15,7 +16,7 @@ internal static unsafe int ForkAndExecProcess( string filename, string[] argv, string[] envp, string? cwd, bool redirectStdin, bool redirectStdout, bool redirectStderr, bool setUser, uint userId, uint groupId, uint[]? groups, - out int lpChildPid, out int stdinFd, out int stdoutFd, out int stderrFd, bool shouldThrow = true) + out int lpChildPid, SafeFileHandle stdinFd, SafeFileHandle stdoutFd, SafeFileHandle stderrFd, bool shouldThrow = true) { byte** argvPtr = null, envpPtr = null; int result = -1; @@ -29,7 +30,7 @@ internal static unsafe int ForkAndExecProcess( filename, argvPtr, envpPtr, cwd, redirectStdin ? 1 : 0, redirectStdout ? 1 : 0, redirectStderr ? 1 : 0, setUser ? 1 : 0, userId, groupId, pGroups, groups?.Length ?? 0, - out lpChildPid, out stdinFd, out stdoutFd, out stderrFd); + out lpChildPid, stdinFd, stdoutFd, stderrFd); } return result == 0 ? 0 : Marshal.GetLastPInvokeError(); } @@ -45,7 +46,7 @@ private static unsafe partial int ForkAndExecProcess( string filename, byte** argv, byte** envp, string? cwd, int redirectStdin, int redirectStdout, int redirectStderr, int setUser, uint userId, uint groupId, uint* groups, int groupsLength, - out int lpChildPid, out int stdinFd, out int stdoutFd, out int stderrFd); + out int lpChildPid, SafeFileHandle stdinFd, SafeFileHandle stdoutFd, SafeFileHandle stderrFd); private static unsafe void AllocNullTerminatedArray(string[] arr, ref byte** arrPtr) { diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index f26a8f70ffba74..fd8163751ea2e8 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -364,7 +364,7 @@ private SafeProcessHandle GetProcessHandle() /// With UseShellExecute option, we'll try the shell tools to launch it(e.g. "open fileName") /// /// The start info with which to start the process. - private bool StartCore(ProcessStartInfo startInfo) + private bool StartCore(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, SafeFileHandle? stdoutHandle, SafeFileHandle? stderrHandle) { if (PlatformDoesNotSupportProcessStartAndKill) { @@ -384,7 +384,6 @@ private bool StartCore(ProcessStartInfo startInfo) } } - int stdinFd = -1, stdoutFd = -1, stderrFd = -1; string[] envp = CreateEnvp(startInfo); string? cwd = !string.IsNullOrWhiteSpace(startInfo.WorkingDirectory) ? startInfo.WorkingDirectory : null; @@ -428,7 +427,7 @@ private bool StartCore(ProcessStartInfo startInfo) isExecuting = ForkAndExecProcess( startInfo, filename, argv, envp, cwd, setCredentials, userId, groupId, groups, - out stdinFd, out stdoutFd, out stderrFd, usesTerminal, + stdinHandle, stdoutHandle, stderrHandle, usesTerminal, throwOnNoExec: false); // return false instead of throwing on ENOEXEC } @@ -441,7 +440,7 @@ private bool StartCore(ProcessStartInfo startInfo) ForkAndExecProcess( startInfo, filename, argv, envp, cwd, setCredentials, userId, groupId, groups, - out stdinFd, out stdoutFd, out stderrFd, usesTerminal); + stdinHandle, stdoutHandle, stderrHandle, usesTerminal); } } else @@ -456,31 +455,7 @@ private bool StartCore(ProcessStartInfo startInfo) ForkAndExecProcess( startInfo, filename, argv, envp, cwd, setCredentials, userId, groupId, groups, - out stdinFd, out stdoutFd, out stderrFd, usesTerminal); - } - - // Configure the parent's ends of the redirection streams. - // We use UTF8 encoding without BOM by-default(instead of Console encoding as on Windows) - // as there is no good way to get this information from the native layer - // and we do not want to take dependency on Console contract. - if (startInfo.RedirectStandardInput) - { - Debug.Assert(stdinFd >= 0); - _standardInput = new StreamWriter(OpenStream(stdinFd, PipeDirection.Out), - startInfo.StandardInputEncoding ?? Encoding.Default, StreamBufferSize) - { AutoFlush = true }; - } - if (startInfo.RedirectStandardOutput) - { - Debug.Assert(stdoutFd >= 0); - _standardOutput = new StreamReader(OpenStream(stdoutFd, PipeDirection.In), - startInfo.StandardOutputEncoding ?? Encoding.Default, true, StreamBufferSize); - } - if (startInfo.RedirectStandardError) - { - Debug.Assert(stderrFd >= 0); - _standardError = new StreamReader(OpenStream(stderrFd, PipeDirection.In), - startInfo.StandardErrorEncoding ?? Encoding.Default, true, StreamBufferSize); + stdinHandle, stdoutHandle, stderrHandle, usesTerminal); } return true; @@ -490,7 +465,7 @@ private bool ForkAndExecProcess( ProcessStartInfo startInfo, string? resolvedFilename, string[] argv, string[] envp, string? cwd, bool setCredentials, uint userId, uint groupId, uint[]? groups, - out int stdinFd, out int stdoutFd, out int stderrFd, + SafeFileHandle? stdinHandle, SafeFileHandle? stdoutHandle, SafeFileHandle? stderrHandle, bool usesTerminal, bool throwOnNoExec = true) { if (string.IsNullOrEmpty(resolvedFilename)) @@ -511,8 +486,8 @@ private bool ForkAndExecProcess( int childPid; - // Invoke the shim fork/execve routine. It will create pipes for all requested - // redirects, fork a child process, map the pipe ends onto the appropriate stdin/stdout/stderr + // Invoke the shim fork/execve routine. It will fork a child process, + // map the provided file handles onto the appropriate stdin/stdout/stderr // descriptors, and execve to execute the requested process. The shim implementation // is used to fork/execve as executing managed code in a forked process is not safe (only // the calling thread will transfer, thread IDs aren't stable across the fork, etc.) @@ -520,7 +495,10 @@ private bool ForkAndExecProcess( resolvedFilename, argv, envp, cwd, startInfo.RedirectStandardInput, startInfo.RedirectStandardOutput, startInfo.RedirectStandardError, setCredentials, userId, groupId, groups, - out childPid, out stdinFd, out stdoutFd, out stderrFd); + out childPid, + stdinHandle ?? s_invalidSafeFileHandle, + stdoutHandle ?? s_invalidSafeFileHandle, + stderrHandle ?? s_invalidSafeFileHandle); if (errno == 0) { @@ -563,8 +541,7 @@ private bool ForkAndExecProcess( /// Finalizable holder for the underlying shared wait state object. private ProcessWaitState.Holder? _waitStateHolder; - /// Size to use for redirect streams and stream readers/writers. - private const int StreamBufferSize = 4096; + private static readonly SafeFileHandle s_invalidSafeFileHandle = new SafeFileHandle(new IntPtr(-1), ownsHandle: false); /// Converts the filename and arguments information from a ProcessStartInfo into an argv array. /// The ProcessStartInfo. @@ -753,16 +730,38 @@ internal static TimeSpan TicksToTimeSpan(double ticks) return TimeSpan.FromSeconds(ticks / (double)ticksPerSecond); } - /// Opens a stream around the specified file descriptor and with the specified access. - /// The file descriptor. - /// The pipe direction. - /// The opened stream. - private static AnonymousPipeClientStream OpenStream(int fd, PipeDirection direction) + /// Opens a stream around the specified file handle. + private static partial Stream OpenStream(SafeFileHandle handle, FileAccess access) + { + PipeDirection direction = access == FileAccess.Write ? PipeDirection.Out : PipeDirection.In; + return new AnonymousPipeClientStream(direction, new SafePipeHandle(handle.DangerousGetHandle(), ownsHandle: false)); + } + + /// Creates an anonymous pipe, returning handles for the parent and child ends. + private static unsafe partial void CreatePipe(out SafeFileHandle parentHandle, out SafeFileHandle childHandle, bool parentInputs) { - Debug.Assert(fd >= 0); - return new AnonymousPipeClientStream(direction, new SafePipeHandle((IntPtr)fd, ownsHandle: true)); + int* fds = stackalloc int[2]; + int result = Interop.Sys.Pipe(fds, Interop.Sys.PipeFlags.O_CLOEXEC); + if (result != 0) + { + throw new Win32Exception(); + } + + SafeFileHandle readHandle = new SafeFileHandle((IntPtr)fds[Interop.Sys.ReadEndOfPipe], ownsHandle: true); + SafeFileHandle writeHandle = new SafeFileHandle((IntPtr)fds[Interop.Sys.WriteEndOfPipe], ownsHandle: true); + + // parentInputs=true: parent writes to pipe, child reads (stdin redirect). + // parentInputs=false: parent reads from pipe, child writes (stdout/stderr redirect). + parentHandle = parentInputs ? writeHandle : readHandle; + childHandle = parentInputs ? readHandle : writeHandle; } + /// Gets the default encoding for standard input. + private static partial Encoding GetStandardInputEncoding() => Encoding.Default; + + /// Gets the default encoding for standard output/error. + private static partial Encoding GetStandardOutputEncoding() => Encoding.Default; + /// Parses a command-line argument string into a list of arguments. /// The argument string. /// The list into which the component arguments should be stored. diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Win32.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Win32.cs index bd551a53f8d311..759cc64b394a02 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Win32.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Win32.cs @@ -20,11 +20,11 @@ public partial class Process : IDisposable private bool _haveResponding; private bool _responding; - private bool StartCore(ProcessStartInfo startInfo) + private bool StartCore(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, SafeFileHandle? stdoutHandle, SafeFileHandle? stderrHandle) { return startInfo.UseShellExecute ? StartWithShellExecuteEx(startInfo) - : StartWithCreateProcess(startInfo); + : StartWithCreateProcess(startInfo, stdinHandle, stdoutHandle, stderrHandle); } private unsafe bool StartWithShellExecuteEx(ProcessStartInfo startInfo) 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 f073bd1e8d54f5..ebcbdae9979fce 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 @@ -425,7 +425,10 @@ private void SetWorkingSetLimitsCore(IntPtr? newMin, IntPtr? newMax, out IntPtr /// Starts the process using the supplied start info. /// The start info with which to start the process. - private unsafe bool StartWithCreateProcess(ProcessStartInfo startInfo) + /// The child's stdin handle, or null if not redirecting. + /// The child's stdout handle, or null if not redirecting. + /// The child's stderr handle, or null if not redirecting. + private unsafe bool StartWithCreateProcess(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, SafeFileHandle? stdoutHandle, SafeFileHandle? stderrHandle) { // See knowledge base article Q190351 for an explanation of the following code. Noteworthy tricky points: // * The handles are duplicated as inheritable before they are passed to CreateProcess so @@ -441,13 +444,10 @@ private unsafe bool StartWithCreateProcess(ProcessStartInfo startInfo) Interop.Kernel32.SECURITY_ATTRIBUTES unused_SecAttrs = default; SafeProcessHandle procSH = new SafeProcessHandle(); - // handles used in parent process - SafeFileHandle? parentInputPipeHandle = null; - SafeFileHandle? childInputPipeHandle = null; - SafeFileHandle? parentOutputPipeHandle = null; - SafeFileHandle? childOutputPipeHandle = null; - SafeFileHandle? parentErrorPipeHandle = null; - SafeFileHandle? childErrorPipeHandle = null; + // Inheritable copies of the child handles for CreateProcess + SafeFileHandle? inheritableStdinHandle = null; + SafeFileHandle? inheritableStdoutHandle = null; + SafeFileHandle? inheritableStderrHandle = null; // Take a global lock to synchronize all redirect pipe handle creations and CreateProcess // calls. We do not want one process to inherit the handles created concurrently for another @@ -464,34 +464,34 @@ private unsafe bool StartWithCreateProcess(ProcessStartInfo startInfo) { if (startInfo.RedirectStandardInput) { - CreatePipe(out parentInputPipeHandle, out childInputPipeHandle, true); + inheritableStdinHandle = DuplicateAsInheritable(stdinHandle!); } else { - childInputPipeHandle = Console.OpenStandardInputHandle(); + inheritableStdinHandle = Console.OpenStandardInputHandle(); } if (startInfo.RedirectStandardOutput) { - CreatePipe(out parentOutputPipeHandle, out childOutputPipeHandle, false); + inheritableStdoutHandle = DuplicateAsInheritable(stdoutHandle!); } else { - childOutputPipeHandle = Console.OpenStandardOutputHandle(); + inheritableStdoutHandle = Console.OpenStandardOutputHandle(); } if (startInfo.RedirectStandardError) { - CreatePipe(out parentErrorPipeHandle, out childErrorPipeHandle, false); + inheritableStderrHandle = DuplicateAsInheritable(stderrHandle!); } else { - childErrorPipeHandle = Console.OpenStandardErrorHandle(); + inheritableStderrHandle = Console.OpenStandardErrorHandle(); } - startupInfo.hStdInput = childInputPipeHandle.DangerousGetHandle(); - startupInfo.hStdOutput = childOutputPipeHandle.DangerousGetHandle(); - startupInfo.hStdError = childErrorPipeHandle.DangerousGetHandle(); + startupInfo.hStdInput = inheritableStdinHandle.DangerousGetHandle(); + startupInfo.hStdOutput = inheritableStdoutHandle.DangerousGetHandle(); + startupInfo.hStdError = inheritableStderrHandle.DangerousGetHandle(); startupInfo.dwFlags = Interop.Advapi32.StartupInfoOptions.STARTF_USESTDHANDLES; } @@ -617,37 +617,17 @@ ref processInfo // pointer to PROCESS_INFORMATION } catch { - parentInputPipeHandle?.Dispose(); - parentOutputPipeHandle?.Dispose(); - parentErrorPipeHandle?.Dispose(); procSH.Dispose(); throw; } finally { - childInputPipeHandle?.Dispose(); - childOutputPipeHandle?.Dispose(); - childErrorPipeHandle?.Dispose(); + inheritableStdinHandle?.Dispose(); + inheritableStdoutHandle?.Dispose(); + inheritableStderrHandle?.Dispose(); } } - if (startInfo.RedirectStandardInput) - { - Encoding enc = startInfo.StandardInputEncoding ?? GetEncoding((int)Interop.Kernel32.GetConsoleCP()); - _standardInput = new StreamWriter(new FileStream(parentInputPipeHandle!, FileAccess.Write, 4096, false), enc, 4096); - _standardInput.AutoFlush = true; - } - if (startInfo.RedirectStandardOutput) - { - Encoding enc = startInfo.StandardOutputEncoding ?? GetEncoding((int)Interop.Kernel32.GetConsoleOutputCP()); - _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, parentErrorPipeHandle!.IsAsync), enc, true, 4096); - } - commandLine.Dispose(); if (procSH.IsInvalid) @@ -661,6 +641,23 @@ ref processInfo // pointer to PROCESS_INFORMATION return true; } + /// Duplicates a handle as inheritable so the child process can use it. + private static SafeFileHandle DuplicateAsInheritable(SafeFileHandle handle) + { + IntPtr currentProcHandle = Interop.Kernel32.GetCurrentProcess(); + if (!Interop.Kernel32.DuplicateHandle(currentProcHandle, + handle, + currentProcHandle, + out SafeFileHandle duplicatedHandle, + 0, + bInheritHandle: true, + Interop.Kernel32.HandleOptions.DUPLICATE_SAME_ACCESS)) + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + return duplicatedHandle; + } + private static ConsoleEncoding GetEncoding(int codePage) { Encoding enc = EncodingHelper.GetSupportedConsoleEncoding(codePage); @@ -809,7 +806,7 @@ private SafeProcessHandle GetProcessHandle(int access, bool throwIfExited = true // 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) + private static partial void CreatePipe(out SafeFileHandle parentHandle, out SafeFileHandle childHandle, bool parentInputs) { // 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. @@ -818,28 +815,21 @@ private static void CreatePipe(out SafeFileHandle parentHandle, out SafeFileHand // parentInputs=true: parent writes to pipe, child reads (stdin redirect). // parentInputs=false: parent reads from pipe, child writes (stdout/stderr redirect). parentHandle = parentInputs ? writeHandle : readHandle; - SafeFileHandle hTmpChild = parentInputs ? readHandle : writeHandle; - - // Duplicate the child handle to be inheritable so that the child process - // has access. The original non-inheritable handle is closed afterwards. - IntPtr currentProcHandle = Interop.Kernel32.GetCurrentProcess(); - if (!Interop.Kernel32.DuplicateHandle(currentProcHandle, - hTmpChild, - currentProcHandle, - out childHandle, - 0, - bInheritHandle: true, - Interop.Kernel32.HandleOptions.DUPLICATE_SAME_ACCESS)) - { - int lastError = Marshal.GetLastWin32Error(); - parentHandle.Dispose(); - hTmpChild.Dispose(); - throw new Win32Exception(lastError); - } + childHandle = parentInputs ? readHandle : writeHandle; + } - hTmpChild.Dispose(); + /// Opens a stream around the specified file handle. + private static partial Stream OpenStream(SafeFileHandle handle, FileAccess access) + { + return new FileStream(handle, access, StreamBufferSize, handle.IsAsync); } + /// Gets the default encoding for standard input. + private static partial Encoding GetStandardInputEncoding() => GetEncoding((int)Interop.Kernel32.GetConsoleCP()); + + /// Gets the default encoding for standard output/error. + private static partial Encoding GetStandardOutputEncoding() => GetEncoding((int)Interop.Kernel32.GetConsoleOutputCP()); + private static string GetEnvironmentVariablesBlock(DictionaryWrapper sd) { // https://learn.microsoft.com/windows/win32/procthread/changing-environment-variables diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs index 4218596c129644..c622454a6140f9 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs @@ -1227,6 +1227,24 @@ private void SetProcessId(int processId) /// Additional optional configuration hook after a process ID is set. partial void ConfigureAfterProcessIdSet(); + /// Creates an anonymous pipe, returning handles for the parent and child ends. + /// The parent's end of the pipe. + /// The child's end of the pipe. + /// true if the parent writes (stdin redirect); false if the parent reads (stdout/stderr redirect). + private static partial void CreatePipe(out SafeFileHandle parentHandle, out SafeFileHandle childHandle, bool parentInputs); + + /// Opens a stream around the specified file handle. + private static partial Stream OpenStream(SafeFileHandle handle, FileAccess access); + + /// Gets the default encoding for standard input. + private static partial Encoding GetStandardInputEncoding(); + + /// Gets the default encoding for standard output/error. + private static partial Encoding GetStandardOutputEncoding(); + + /// Size to use for redirect streams and stream readers/writers. + private const int StreamBufferSize = 4096; + /// /// /// Starts a process specified by the property of this @@ -1281,7 +1299,71 @@ public bool Start() SerializationGuard.ThrowIfDeserializationInProgress("AllowProcessCreation", ref s_cachedSerializationSwitch); - return StartCore(startInfo); + SafeFileHandle? parentInputPipeHandle = null; + SafeFileHandle? parentOutputPipeHandle = null; + SafeFileHandle? parentErrorPipeHandle = null; + + SafeFileHandle? childInputPipeHandle = null; + SafeFileHandle? childOutputPipeHandle = null; + SafeFileHandle? childErrorPipeHandle = null; + + try + { + if (startInfo.RedirectStandardInput || startInfo.RedirectStandardOutput || startInfo.RedirectStandardError) + { + if (startInfo.RedirectStandardInput) + { + CreatePipe(out parentInputPipeHandle, out childInputPipeHandle, true); + } + + if (startInfo.RedirectStandardOutput) + { + CreatePipe(out parentOutputPipeHandle, out childOutputPipeHandle, false); + } + + if (startInfo.RedirectStandardError) + { + CreatePipe(out parentErrorPipeHandle, out childErrorPipeHandle, false); + } + } + + if (!StartCore(startInfo, childInputPipeHandle, childOutputPipeHandle, childErrorPipeHandle)) + { + return false; + } + } + catch + { + parentInputPipeHandle?.Dispose(); + parentOutputPipeHandle?.Dispose(); + parentErrorPipeHandle?.Dispose(); + throw; + } + finally + { + childInputPipeHandle?.Dispose(); + childOutputPipeHandle?.Dispose(); + childErrorPipeHandle?.Dispose(); + } + + if (startInfo.RedirectStandardInput) + { + _standardInput = new StreamWriter(OpenStream(parentInputPipeHandle!, FileAccess.Write), + startInfo.StandardInputEncoding ?? GetStandardInputEncoding(), StreamBufferSize) + { AutoFlush = true }; + } + if (startInfo.RedirectStandardOutput) + { + _standardOutput = new StreamReader(OpenStream(parentOutputPipeHandle!, FileAccess.Read), + startInfo.StandardOutputEncoding ?? GetStandardOutputEncoding(), true, StreamBufferSize); + } + if (startInfo.RedirectStandardError) + { + _standardError = new StreamReader(OpenStream(parentErrorPipeHandle!, FileAccess.Read), + startInfo.StandardErrorEncoding ?? GetStandardOutputEncoding(), true, StreamBufferSize); + } + + return true; } /// diff --git a/src/native/libs/System.Native/pal_process.c b/src/native/libs/System.Native/pal_process.c index 069cb9410492e8..858827ed58cad2 100644 --- a/src/native/libs/System.Native/pal_process.c +++ b/src/native/libs/System.Native/pal_process.c @@ -56,10 +56,6 @@ c_static_assert(PAL_PRIO_PROCESS == (int)PRIO_PROCESS); c_static_assert(PAL_PRIO_PGRP == (int)PRIO_PGRP); c_static_assert(PAL_PRIO_USER == (int)PRIO_USER); -#if !HAVE_PIPE2 -static pthread_mutex_t ProcessCreateLock = PTHREAD_MUTEX_INITIALIZER; -#endif - enum { READ_END_OF_PIPE = 0, @@ -74,10 +70,18 @@ static void CloseIfOpen(int fd) } } -static int Dup2WithInterruptedRetry(int oldfd, int newfd) +static int Dup2WithInterruptedRetry(int oldfd, int newfd, int removeCloExec) { int result; while (CheckInterrupted(result = dup2(oldfd, newfd))); + if (result != -1 && removeCloExec) + { + int flags = fcntl(newfd, F_GETFD); + if (flags != -1 && (flags & FD_CLOEXEC)) + { + fcntl(newfd, F_SETFD, flags & ~FD_CLOEXEC); + } + } return result; } @@ -222,16 +226,13 @@ int32_t SystemNative_ForkAndExecProcess(const char* filename, uint32_t* groups, int32_t groupsLength, int32_t* childPid, - int32_t* stdinFd, - int32_t* stdoutFd, - int32_t* stderrFd) + int32_t stdinFd, + int32_t stdoutFd, + int32_t stderrFd) { #if HAVE_FORK -#if !HAVE_PIPE2 - bool haveProcessCreateLock = false; -#endif bool success = true; - int stdinFds[2] = {-1, -1}, stdoutFds[2] = {-1, -1}, stderrFds[2] = {-1, -1}, waitForChildToExecPipe[2] = {-1, -1}; + int waitForChildToExecPipe[2] = {-1, -1}; pid_t processId = -1; uint32_t* getGroupsBuffer = NULL; sigset_t signal_set; @@ -244,8 +245,7 @@ int32_t SystemNative_ForkAndExecProcess(const char* filename, pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &thread_cancel_state); #endif - assert(NULL != filename && NULL != argv && NULL != envp && NULL != stdinFd && - NULL != stdoutFd && NULL != stderrFd && NULL != childPid && + assert(NULL != filename && NULL != argv && NULL != envp && NULL != childPid && (groupsLength == 0 || groups != NULL) && "null argument."); assert((redirectStdin & ~1) == 0 && (redirectStdout & ~1) == 0 && @@ -273,30 +273,6 @@ int32_t SystemNative_ForkAndExecProcess(const char* filename, goto done; } -#if !HAVE_PIPE2 - // We do not have pipe2(); take the lock to emulate it race free. - // If another process were to be launched between the pipe creation and the fcntl call to set CLOEXEC on it, that - // file descriptor will be inherited into the other child process, eventually causing a deadlock either in the loop - // below that waits for that pipe to be closed or in StreamReader.ReadToEnd() in the calling code. - if (pthread_mutex_lock(&ProcessCreateLock) != 0) - { - // This check is pretty much just checking for trashed memory. - success = false; - goto done; - } - haveProcessCreateLock = true; -#endif - - // Open pipes for any requests to redirect stdin/stdout/stderr and set the - // close-on-exec flag to the pipe file descriptors. - if ((redirectStdin && SystemNative_Pipe(stdinFds, PAL_O_CLOEXEC) != 0) || - (redirectStdout && SystemNative_Pipe(stdoutFds, PAL_O_CLOEXEC) != 0) || - (redirectStderr && SystemNative_Pipe(stderrFds, PAL_O_CLOEXEC) != 0)) - { - success = false; - goto done; - } - // We create a pipe purely for the benefit of knowing when the child process has called exec. // We can use that to block waiting on the pipe to be closed, which lets us block the parent // from returning until the child process is actually transitioned to the target program. This @@ -394,11 +370,11 @@ int32_t SystemNative_ForkAndExecProcess(const char* filename, } pthread_sigmask(SIG_SETMASK, &old_signal_set, &junk_signal_set); // Not all architectures allow NULL here - // For any redirections that should happen, dup the pipe descriptors onto stdin/out/err. - // We don't need to explicitly close out the old pipe descriptors as they will be closed on the 'execve' call. - if ((redirectStdin && Dup2WithInterruptedRetry(stdinFds[READ_END_OF_PIPE], STDIN_FILENO) == -1) || - (redirectStdout && Dup2WithInterruptedRetry(stdoutFds[WRITE_END_OF_PIPE], STDOUT_FILENO) == -1) || - (redirectStderr && Dup2WithInterruptedRetry(stderrFds[WRITE_END_OF_PIPE], STDERR_FILENO) == -1)) + // For any redirections that should happen, dup the provided file descriptors onto stdin/out/err. + // The provided file handles have CLOEXEC enabled by default, so we clear it after dup2. + if ((redirectStdin && Dup2WithInterruptedRetry(stdinFd, STDIN_FILENO, 1) == -1) || + (redirectStdout && Dup2WithInterruptedRetry(stdoutFd, STDOUT_FILENO, 1) == -1) || + (redirectStderr && Dup2WithInterruptedRetry(stderrFd, STDERR_FILENO, 1) == -1)) { ExitChild(waitForChildToExecPipe[WRITE_END_OF_PIPE], errno); } @@ -441,26 +417,11 @@ int32_t SystemNative_ForkAndExecProcess(const char* filename, // This is the parent process. processId == pid of the child *childPid = processId; - *stdinFd = stdinFds[WRITE_END_OF_PIPE]; - *stdoutFd = stdoutFds[READ_END_OF_PIPE]; - *stderrFd = stderrFds[READ_END_OF_PIPE]; done:; -#if !HAVE_PIPE2 - if (haveProcessCreateLock) - { - pthread_mutex_unlock(&ProcessCreateLock); - } -#endif int priorErrno = errno; - // Regardless of success or failure, close the parent's copy of the child's end of - // any opened pipes. The parent doesn't need them anymore. - CloseIfOpen(stdinFds[READ_END_OF_PIPE]); - CloseIfOpen(stdoutFds[WRITE_END_OF_PIPE]); - CloseIfOpen(stderrFds[WRITE_END_OF_PIPE]); - // Also close the write end of the exec waiting pipe, and wait for the pipe to be closed // by trying to read from it (the read will wake up when the pipe is closed and broken). // Ignore any errors... this is a best-effort attempt. @@ -480,13 +441,9 @@ done:; CloseIfOpen(waitForChildToExecPipe[READ_END_OF_PIPE]); } - // If we failed, close everything else and give back error values in all out arguments. + // If we failed, give back error values in all out arguments. if (!success) { - CloseIfOpen(stdinFds[WRITE_END_OF_PIPE]); - CloseIfOpen(stdoutFds[READ_END_OF_PIPE]); - CloseIfOpen(stderrFds[READ_END_OF_PIPE]); - // Reap child if (processId > 0) { @@ -494,9 +451,6 @@ done:; waitpid(processId, &status, 0); } - *stdinFd = -1; - *stdoutFd = -1; - *stderrFd = -1; *childPid = -1; errno = priorErrno; diff --git a/src/native/libs/System.Native/pal_process.h b/src/native/libs/System.Native/pal_process.h index 6ef66da7acf84f..81a850c7362ca1 100644 --- a/src/native/libs/System.Native/pal_process.h +++ b/src/native/libs/System.Native/pal_process.h @@ -32,9 +32,9 @@ PALEXPORT int32_t SystemNative_ForkAndExecProcess( uint32_t* groups, // the groups under which the child process should run int32_t groupsLength, // the length of groups int32_t* childPid, // [out] the child process' id - int32_t* stdinFd, // [out] if redirectStdin, the parent's fd for the child's stdin - int32_t* stdoutFd, // [out] if redirectStdout, the parent's fd for the child's stdout - int32_t* stderrFd); // [out] if redirectStderr, the parent's fd for the child's stderr + int32_t stdinFd, // if redirectStdin, the fd for the child's stdin + int32_t stdoutFd, // if redirectStdout, the fd for the child's stdout + int32_t stderrFd); // if redirectStderr, the fd for the child's stderr /************ * The values below in the header are fixed and correct for managed callers to use forever. diff --git a/src/native/libs/System.Native/pal_process_wasi.c b/src/native/libs/System.Native/pal_process_wasi.c index 569a0066eec69e..061240920ee8c1 100644 --- a/src/native/libs/System.Native/pal_process_wasi.c +++ b/src/native/libs/System.Native/pal_process_wasi.c @@ -30,9 +30,9 @@ int32_t SystemNative_ForkAndExecProcess(const char* filename, uint32_t* groups, int32_t groupsLength, int32_t* childPid, - int32_t* stdinFd, - int32_t* stdoutFd, - int32_t* stderrFd) + int32_t stdinFd, + int32_t stdoutFd, + int32_t stderrFd) { return -1; } From fea2a806b6e4f3eb4cf3a2855d4748af6c870a74 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 18 Mar 2026 22:36:57 +0000 Subject: [PATCH 02/22] Fix build errors: XML doc comments and unsafe partial mismatch; add Interop.Pipe.cs to csproj Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System.Diagnostics.Process.csproj | 2 ++ .../src/System/Diagnostics/Process.Unix.cs | 16 +++++++++++++--- 2 files changed, 15 insertions(+), 3 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 1a4cb17a51ccb4..7e6d3454cff9e5 100644 --- a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -248,6 +248,8 @@ Link="Common\Interop\Unix\Interop.GetHostName.cs" /> + /// The start info with which to start the process. + /// The child's stdin handle, or null if not redirecting. + /// The child's stdout handle, or null if not redirecting. + /// The child's stderr handle, or null if not redirecting. private bool StartCore(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, SafeFileHandle? stdoutHandle, SafeFileHandle? stderrHandle) { if (PlatformDoesNotSupportProcessStartAndKill) @@ -738,10 +741,17 @@ private static partial Stream OpenStream(SafeFileHandle handle, FileAccess acces } /// Creates an anonymous pipe, returning handles for the parent and child ends. - private static unsafe partial void CreatePipe(out SafeFileHandle parentHandle, out SafeFileHandle childHandle, bool parentInputs) + private static partial void CreatePipe(out SafeFileHandle parentHandle, out SafeFileHandle childHandle, bool parentInputs) { - int* fds = stackalloc int[2]; - int result = Interop.Sys.Pipe(fds, Interop.Sys.PipeFlags.O_CLOEXEC); + Span fds = stackalloc int[2]; + int result; + unsafe + { + fixed (int* fdsPtr = fds) + { + result = Interop.Sys.Pipe(fdsPtr, Interop.Sys.PipeFlags.O_CLOEXEC); + } + } if (result != 0) { throw new Win32Exception(); From 07d9369bffd38809d9fd12049e90ac7126215d3c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 19 Mar 2026 08:30:05 +0000 Subject: [PATCH 03/22] Address PR feedback: remove partial methods, use SafeFileHandle.CreateAnonymousPipe directly, simplify Windows StartWithCreateProcess Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/System.Diagnostics.Process.csproj | 2 - .../src/System/Diagnostics/Process.Unix.cs | 50 +++------------- .../src/System/Diagnostics/Process.Windows.cs | 60 ++----------------- .../src/System/Diagnostics/Process.cs | 35 +++++------ 4 files changed, 29 insertions(+), 118 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 7e6d3454cff9e5..1a4cb17a51ccb4 100644 --- a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -248,8 +248,6 @@ Link="Common\Interop\Unix\Interop.GetHostName.cs" /> - - /// Starts the process using the supplied start info. - /// With UseShellExecute option, we'll try the shell tools to launch it(e.g. "open fileName") - /// - /// The start info with which to start the process. - /// The child's stdin handle, or null if not redirecting. - /// The child's stdout handle, or null if not redirecting. - /// The child's stderr handle, or null if not redirecting. private bool StartCore(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, SafeFileHandle? stdoutHandle, SafeFileHandle? stderrHandle) { if (PlatformDoesNotSupportProcessStartAndKill) @@ -499,9 +491,9 @@ private bool ForkAndExecProcess( startInfo.RedirectStandardInput, startInfo.RedirectStandardOutput, startInfo.RedirectStandardError, setCredentials, userId, groupId, groups, out childPid, - stdinHandle ?? s_invalidSafeFileHandle, - stdoutHandle ?? s_invalidSafeFileHandle, - stderrHandle ?? s_invalidSafeFileHandle); + stdinHandle!, + stdoutHandle!, + stderrHandle!); if (errno == 0) { @@ -544,8 +536,6 @@ private bool ForkAndExecProcess( /// Finalizable holder for the underlying shared wait state object. private ProcessWaitState.Holder? _waitStateHolder; - private static readonly SafeFileHandle s_invalidSafeFileHandle = new SafeFileHandle(new IntPtr(-1), ownsHandle: false); - /// Converts the filename and arguments information from a ProcessStartInfo into an argv array. /// The ProcessStartInfo. /// Resolved executable to open ProcessStartInfo.FileName @@ -734,43 +724,17 @@ internal static TimeSpan TicksToTimeSpan(double ticks) } /// Opens a stream around the specified file handle. - private static partial Stream OpenStream(SafeFileHandle handle, FileAccess access) + private static AnonymousPipeClientStream OpenStream(SafeFileHandle handle, FileAccess access) { PipeDirection direction = access == FileAccess.Write ? PipeDirection.Out : PipeDirection.In; - return new AnonymousPipeClientStream(direction, new SafePipeHandle(handle.DangerousGetHandle(), ownsHandle: false)); - } - - /// Creates an anonymous pipe, returning handles for the parent and child ends. - private static partial void CreatePipe(out SafeFileHandle parentHandle, out SafeFileHandle childHandle, bool parentInputs) - { - Span fds = stackalloc int[2]; - int result; - unsafe - { - fixed (int* fdsPtr = fds) - { - result = Interop.Sys.Pipe(fdsPtr, Interop.Sys.PipeFlags.O_CLOEXEC); - } - } - if (result != 0) - { - throw new Win32Exception(); - } - - SafeFileHandle readHandle = new SafeFileHandle((IntPtr)fds[Interop.Sys.ReadEndOfPipe], ownsHandle: true); - SafeFileHandle writeHandle = new SafeFileHandle((IntPtr)fds[Interop.Sys.WriteEndOfPipe], ownsHandle: true); - - // parentInputs=true: parent writes to pipe, child reads (stdin redirect). - // parentInputs=false: parent reads from pipe, child writes (stdout/stderr redirect). - parentHandle = parentInputs ? writeHandle : readHandle; - childHandle = parentInputs ? readHandle : writeHandle; + return new AnonymousPipeClientStream(direction, new SafePipeHandle(handle.DangerousGetHandle(), ownsHandle: true)); } /// Gets the default encoding for standard input. - private static partial Encoding GetStandardInputEncoding() => Encoding.Default; + private static Encoding GetStandardInputEncoding() => Encoding.Default; /// Gets the default encoding for standard output/error. - private static partial Encoding GetStandardOutputEncoding() => Encoding.Default; + private static Encoding GetStandardOutputEncoding() => Encoding.Default; /// Parses a command-line argument string into a list of arguments. /// The argument string. 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 ebcbdae9979fce..373a0a01951748 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 @@ -424,10 +424,6 @@ private void SetWorkingSetLimitsCore(IntPtr? newMin, IntPtr? newMax, out IntPtr } /// Starts the process using the supplied start info. - /// The start info with which to start the process. - /// The child's stdin handle, or null if not redirecting. - /// The child's stdout handle, or null if not redirecting. - /// The child's stderr handle, or null if not redirecting. private unsafe bool StartWithCreateProcess(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, SafeFileHandle? stdoutHandle, SafeFileHandle? stderrHandle) { // See knowledge base article Q190351 for an explanation of the following code. Noteworthy tricky points: @@ -462,32 +458,9 @@ private unsafe bool StartWithCreateProcess(ProcessStartInfo startInfo, SafeFileH // set up the streams if (startInfo.RedirectStandardInput || startInfo.RedirectStandardOutput || startInfo.RedirectStandardError) { - if (startInfo.RedirectStandardInput) - { - inheritableStdinHandle = DuplicateAsInheritable(stdinHandle!); - } - else - { - inheritableStdinHandle = Console.OpenStandardInputHandle(); - } - - if (startInfo.RedirectStandardOutput) - { - inheritableStdoutHandle = DuplicateAsInheritable(stdoutHandle!); - } - else - { - inheritableStdoutHandle = Console.OpenStandardOutputHandle(); - } - - if (startInfo.RedirectStandardError) - { - inheritableStderrHandle = DuplicateAsInheritable(stderrHandle!); - } - else - { - inheritableStderrHandle = Console.OpenStandardErrorHandle(); - } + inheritableStdinHandle = DuplicateAsInheritable(stdinHandle!); + inheritableStdoutHandle = DuplicateAsInheritable(stdoutHandle!); + inheritableStderrHandle = DuplicateAsInheritable(stderrHandle!); startupInfo.hStdInput = inheritableStdinHandle.DangerousGetHandle(); startupInfo.hStdOutput = inheritableStdoutHandle.DangerousGetHandle(); @@ -797,38 +770,17 @@ private SafeProcessHandle GetProcessHandle(int access, bool throwIfExited = true } } - // Using synchronous Anonymous pipes for process input/output redirection means we would end up - // wasting a worker threadpool thread per pipe instance. Overlapped pipe IO is desirable, since - // it will take advantage of the NT IO completion port infrastructure. But we can't really use - // Overlapped I/O for process input/output as it would break Console apps (managed Console class - // 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 partial void CreatePipe(out SafeFileHandle parentHandle, out SafeFileHandle childHandle, bool parentInputs) - { - // 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). - parentHandle = parentInputs ? writeHandle : readHandle; - childHandle = parentInputs ? readHandle : writeHandle; - } - /// Opens a stream around the specified file handle. - private static partial Stream OpenStream(SafeFileHandle handle, FileAccess access) + private static FileStream OpenStream(SafeFileHandle handle, FileAccess access) { return new FileStream(handle, access, StreamBufferSize, handle.IsAsync); } /// Gets the default encoding for standard input. - private static partial Encoding GetStandardInputEncoding() => GetEncoding((int)Interop.Kernel32.GetConsoleCP()); + private static ConsoleEncoding GetStandardInputEncoding() => GetEncoding((int)Interop.Kernel32.GetConsoleCP()); /// Gets the default encoding for standard output/error. - private static partial Encoding GetStandardOutputEncoding() => GetEncoding((int)Interop.Kernel32.GetConsoleOutputCP()); + private static ConsoleEncoding GetStandardOutputEncoding() => GetEncoding((int)Interop.Kernel32.GetConsoleOutputCP()); private static string GetEnvironmentVariablesBlock(DictionaryWrapper sd) { diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs index c622454a6140f9..7f2ba1cbe37606 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs @@ -1227,21 +1227,6 @@ private void SetProcessId(int processId) /// Additional optional configuration hook after a process ID is set. partial void ConfigureAfterProcessIdSet(); - /// Creates an anonymous pipe, returning handles for the parent and child ends. - /// The parent's end of the pipe. - /// The child's end of the pipe. - /// true if the parent writes (stdin redirect); false if the parent reads (stdout/stderr redirect). - private static partial void CreatePipe(out SafeFileHandle parentHandle, out SafeFileHandle childHandle, bool parentInputs); - - /// Opens a stream around the specified file handle. - private static partial Stream OpenStream(SafeFileHandle handle, FileAccess access); - - /// Gets the default encoding for standard input. - private static partial Encoding GetStandardInputEncoding(); - - /// Gets the default encoding for standard output/error. - private static partial Encoding GetStandardOutputEncoding(); - /// Size to use for redirect streams and stream readers/writers. private const int StreamBufferSize = 4096; @@ -1309,21 +1294,33 @@ public bool Start() try { - if (startInfo.RedirectStandardInput || startInfo.RedirectStandardOutput || startInfo.RedirectStandardError) + if (!startInfo.UseShellExecute || !OperatingSystem.IsWindows()) { if (startInfo.RedirectStandardInput) { - CreatePipe(out parentInputPipeHandle, out childInputPipeHandle, true); + SafeFileHandle.CreateAnonymousPipe(out childInputPipeHandle, out parentInputPipeHandle); + } + else + { + childInputPipeHandle = Console.OpenStandardInputHandle(); } if (startInfo.RedirectStandardOutput) { - CreatePipe(out parentOutputPipeHandle, out childOutputPipeHandle, false); + SafeFileHandle.CreateAnonymousPipe(out parentOutputPipeHandle, out childOutputPipeHandle, asyncRead: OperatingSystem.IsWindows()); + } + else + { + childOutputPipeHandle = Console.OpenStandardOutputHandle(); } if (startInfo.RedirectStandardError) { - CreatePipe(out parentErrorPipeHandle, out childErrorPipeHandle, false); + SafeFileHandle.CreateAnonymousPipe(out parentErrorPipeHandle, out childErrorPipeHandle, asyncRead: OperatingSystem.IsWindows()); + } + else + { + childErrorPipeHandle = Console.OpenStandardErrorHandle(); } } From b65842b21ca3fbb364570a6df4cc884bd03ae6fa Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 19 Mar 2026 12:33:34 +0100 Subject: [PATCH 04/22] address my own feedback --- .../Unix/System.Native/Interop.IsATty.cs | 5 + ...omicNonInheritablePipeCreationSupported.cs | 30 ++ .../src/System.Diagnostics.Process.csproj | 8 +- ...ConfigureTerminalForChildProcesses.Unix.cs | 8 +- .../src/System/Diagnostics/Process.Unix.cs | 48 ++-- .../src/System/Diagnostics/Process.Win32.cs | 9 +- .../src/System/Diagnostics/Process.Windows.cs | 263 +++++++++--------- .../src/System/Diagnostics/Process.cs | 73 +++-- .../src/System/Diagnostics/ProcessUtils.cs | 3 + src/native/libs/System.Native/entrypoints.c | 1 + src/native/libs/System.Native/pal_io.c | 9 + src/native/libs/System.Native/pal_io.h | 7 + 12 files changed, 277 insertions(+), 187 deletions(-) create mode 100644 src/libraries/Common/src/Interop/Unix/System.Native/Interop.IsAtomicNonInheritablePipeCreationSupported.cs diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.IsATty.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.IsATty.cs index beb79d77d5165b..ae905f656dc549 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.IsATty.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.IsATty.cs @@ -3,6 +3,7 @@ using System; using System.Runtime.InteropServices; +using Microsoft.Win32.SafeHandles; internal static partial class Interop { @@ -11,5 +12,9 @@ internal static partial class Sys [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_IsATty")] [return: MarshalAs(UnmanagedType.Bool)] internal static partial bool IsATty(IntPtr fd); + + [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_IsATty")] + [return: MarshalAs(UnmanagedType.Bool)] + internal static partial bool IsATty(SafeFileHandle fd); } } diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.IsAtomicNonInheritablePipeCreationSupported.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.IsAtomicNonInheritablePipeCreationSupported.cs new file mode 100644 index 00000000000000..1c62417d692329 --- /dev/null +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.IsAtomicNonInheritablePipeCreationSupported.cs @@ -0,0 +1,30 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.InteropServices; + +internal static partial class Interop +{ + internal static partial class Sys + { + [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_IsAtomicNonInheritablePipeCreationSupported", SetLastError = false)] + [return: MarshalAs(UnmanagedType.Bool)] + private static partial bool IsAtomicNonInheritablePipeCreationSupportedImpl(); + + private static NullableBool s_atomicNonInheritablePipeCreationSupported; + + internal static bool IsAtomicNonInheritablePipeCreationSupported + { + get + { + NullableBool isSupported = s_atomicNonInheritablePipeCreationSupported; + if (isSupported == NullableBool.Undefined) + { + s_atomicNonInheritablePipeCreationSupported = isSupported = IsAtomicNonInheritablePipeCreationSupportedImpl() ? NullableBool.True : NullableBool.False; + } + return isSupported == NullableBool.True; + } + } + } +} 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 1a4cb17a51ccb4..3499b5c439110a 100644 --- a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -286,6 +286,12 @@ Link="Common\Interop\Unix\Interop.GetEUid.cs" /> + + + @@ -302,8 +308,6 @@ - 0) { - Debug.Assert(s_processStartLock.IsReadLockHeld); + Debug.Assert(ProcessUtils.s_processStartLock.IsReadLockHeld); Debug.Assert(configureConsole); // At least one child is using the terminal. @@ -25,7 +25,7 @@ internal static void ConfigureTerminalForChildProcesses(int increment, bool conf } else { - Debug.Assert(s_processStartLock.IsWriteLockHeld); + Debug.Assert(ProcessUtils.s_processStartLock.IsWriteLockHeld); if (childrenUsingTerminalRemaining == 0 && configureConsole) { @@ -44,7 +44,7 @@ private static unsafe void SetDelayedSigChildConsoleConfigurationHandler() private static void DelayedSigChildConsoleConfiguration() { // Lock to avoid races with Process.Start - s_processStartLock.EnterWriteLock(); + ProcessUtils.s_processStartLock.EnterWriteLock(); try { if (s_childrenUsingTerminalCount == 0) @@ -55,7 +55,7 @@ private static void DelayedSigChildConsoleConfiguration() } finally { - s_processStartLock.ExitWriteLock(); + ProcessUtils.s_processStartLock.ExitWriteLock(); } } diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index 54c5b189349b2e..ec99a5ee96cfae 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -18,7 +18,6 @@ public partial class Process : IDisposable { private static volatile bool s_initialized; private static readonly object s_initializedGate = new object(); - private static readonly ReaderWriterLockSlim s_processStartLock = new ReaderWriterLockSlim(); /// /// Puts a Process component in state to interact with operating system processes that run in a @@ -361,6 +360,8 @@ private SafeProcessHandle GetProcessHandle() private bool StartCore(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, SafeFileHandle? stdoutHandle, SafeFileHandle? stderrHandle) { + Debug.Assert(stdinHandle is not null && stdoutHandle is not null && stderrHandle is not null); + if (PlatformDoesNotSupportProcessStartAndKill) { throw new PlatformNotSupportedException(); @@ -371,14 +372,6 @@ private bool StartCore(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, string? filename; string[] argv; - if (startInfo.UseShellExecute) - { - if (startInfo.RedirectStandardInput || startInfo.RedirectStandardOutput || startInfo.RedirectStandardError) - { - throw new InvalidOperationException(SR.CantRedirectStreams); - } - } - string[] envp = CreateEnvp(startInfo); string? cwd = !string.IsNullOrWhiteSpace(startInfo.WorkingDirectory) ? startInfo.WorkingDirectory : null; @@ -395,9 +388,7 @@ private bool StartCore(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, // Unix applications expect the terminal to be in an echoing state by default. // To support processes that interact with the terminal (e.g. 'vi'), we need to configure the // terminal to echo. We keep this configuration as long as there are children possibly using the terminal. - bool usesTerminal = !(startInfo.RedirectStandardInput && - startInfo.RedirectStandardOutput && - startInfo.RedirectStandardError); + bool usesTerminal = Interop.Sys.IsATty(stdinHandle) || Interop.Sys.IsATty(stdoutHandle) || Interop.Sys.IsATty(stderrHandle); if (startInfo.UseShellExecute) { @@ -456,11 +447,13 @@ private bool StartCore(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, return true; } + private static bool SupportsAtomicNonInheritablePipeCreation => Interop.Sys.IsAtomicNonInheritablePipeCreationSupported; + private bool ForkAndExecProcess( ProcessStartInfo startInfo, string? resolvedFilename, string[] argv, string[] envp, string? cwd, bool setCredentials, uint userId, uint groupId, uint[]? groups, - SafeFileHandle? stdinHandle, SafeFileHandle? stdoutHandle, SafeFileHandle? stderrHandle, + SafeFileHandle stdinHandle, SafeFileHandle stdoutHandle, SafeFileHandle stderrHandle, bool usesTerminal, bool throwOnNoExec = true) { if (string.IsNullOrEmpty(resolvedFilename)) @@ -471,7 +464,7 @@ private bool ForkAndExecProcess( // Lock to avoid races with OnSigChild // By using a ReaderWriterLock we allow multiple processes to start concurrently. - s_processStartLock.EnterReadLock(); + ProcessUtils.s_processStartLock.EnterReadLock(); try { if (usesTerminal) @@ -491,9 +484,9 @@ private bool ForkAndExecProcess( startInfo.RedirectStandardInput, startInfo.RedirectStandardOutput, startInfo.RedirectStandardError, setCredentials, userId, groupId, groups, out childPid, - stdinHandle!, - stdoutHandle!, - stderrHandle!); + stdinHandle, + stdoutHandle, + stderrHandle); if (errno == 0) { @@ -521,14 +514,14 @@ private bool ForkAndExecProcess( } finally { - s_processStartLock.ExitReadLock(); + ProcessUtils.s_processStartLock.ExitReadLock(); if (_waitStateHolder == null && usesTerminal) { // We failed to launch a child that could use the terminal. - s_processStartLock.EnterWriteLock(); + ProcessUtils.s_processStartLock.EnterWriteLock(); ConfigureTerminalForChildProcesses(-1); - s_processStartLock.ExitWriteLock(); + ProcessUtils.s_processStartLock.ExitWriteLock(); } } } @@ -723,11 +716,18 @@ internal static TimeSpan TicksToTimeSpan(double ticks) return TimeSpan.FromSeconds(ticks / (double)ticksPerSecond); } - /// Opens a stream around the specified file handle. private static AnonymousPipeClientStream OpenStream(SafeFileHandle handle, FileAccess access) { PipeDirection direction = access == FileAccess.Write ? PipeDirection.Out : PipeDirection.In; - return new AnonymousPipeClientStream(direction, new SafePipeHandle(handle.DangerousGetHandle(), ownsHandle: true)); + + // Transfer the ownership to SafePipeHandle, so that it can be properly released when the AnonymousPipeClientStream is disposed. + SafePipeHandle safePipeHandle = new(handle.DangerousGetHandle(), ownsHandle: true); + handle.SetHandleAsInvalid(); + handle.Dispose(); + + // In contrary to Windows, we use AnonymousPipeClientStream which is internally backed by a Socket on Unix. + // This provides best performance and cancellation support. + return new AnonymousPipeClientStream(direction, safePipeHandle); } /// Gets the default encoding for standard input. @@ -990,7 +990,7 @@ private static int OnSigChild(int reapAll, int configureConsole) // DelayedSigChildConsoleConfiguration will be called. // Lock to avoid races with Process.Start - s_processStartLock.EnterWriteLock(); + ProcessUtils.s_processStartLock.EnterWriteLock(); try { bool childrenUsingTerminalPre = AreChildrenUsingTerminal; @@ -1002,7 +1002,7 @@ private static int OnSigChild(int reapAll, int configureConsole) } finally { - s_processStartLock.ExitWriteLock(); + ProcessUtils.s_processStartLock.ExitWriteLock(); } } diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Win32.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Win32.cs index 759cc64b394a02..40dd848d1df136 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Win32.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Win32.cs @@ -20,11 +20,15 @@ public partial class Process : IDisposable private bool _haveResponding; private bool _responding; + private static bool SupportsAtomicNonInheritablePipeCreation => true; + private bool StartCore(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, SafeFileHandle? stdoutHandle, SafeFileHandle? stderrHandle) { + Debug.Assert(startInfo.UseShellExecute == (stdinHandle is null && stdoutHandle is null && stderrHandle is null)); + return startInfo.UseShellExecute ? StartWithShellExecuteEx(startInfo) - : StartWithCreateProcess(startInfo, stdinHandle, stdoutHandle, stderrHandle); + : StartWithCreateProcess(startInfo, stdinHandle!, stdoutHandle!, stderrHandle!); } private unsafe bool StartWithShellExecuteEx(ProcessStartInfo startInfo) @@ -32,9 +36,6 @@ private unsafe bool StartWithShellExecuteEx(ProcessStartInfo startInfo) if (!string.IsNullOrEmpty(startInfo.UserName) || startInfo.Password != null) throw new InvalidOperationException(SR.CantStartAsUser); - if (startInfo.RedirectStandardInput || startInfo.RedirectStandardOutput || startInfo.RedirectStandardError) - throw new InvalidOperationException(SR.CantRedirectStreams); - if (startInfo.StandardInputEncoding != null) throw new InvalidOperationException(SR.StandardInputEncodingNotAllowed); 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 373a0a01951748..bce23b6b6f81d8 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 @@ -16,8 +16,6 @@ namespace System.Diagnostics { public partial class Process : IDisposable { - private static readonly object s_createProcessLock = new object(); - private string? _processName; /// @@ -424,7 +422,7 @@ private void SetWorkingSetLimitsCore(IntPtr? newMin, IntPtr? newMax, out IntPtr } /// Starts the process using the supplied start info. - private unsafe bool StartWithCreateProcess(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, SafeFileHandle? stdoutHandle, SafeFileHandle? stderrHandle) + private unsafe bool StartWithCreateProcess(ProcessStartInfo startInfo, SafeFileHandle stdinHandle, SafeFileHandle stdoutHandle, SafeFileHandle stderrHandle) { // See knowledge base article Q190351 for an explanation of the following code. Noteworthy tricky points: // * The handles are duplicated as inheritable before they are passed to CreateProcess so @@ -449,159 +447,156 @@ private unsafe bool StartWithCreateProcess(ProcessStartInfo startInfo, SafeFileH // calls. We do not want one process to inherit the handles created concurrently for another // process, as that will impact the ownership and lifetimes of those handles now inherited // into multiple child processes. - lock (s_createProcessLock) + + ProcessUtils.s_processStartLock.EnterWriteLock(); + try { - try + startupInfo.cb = sizeof(Interop.Kernel32.STARTUPINFO); + + inheritableStdinHandle = DuplicateAsInheritable(stdinHandle); + inheritableStdoutHandle = DuplicateAsInheritable(stdoutHandle); + inheritableStderrHandle = DuplicateAsInheritable(stderrHandle); + + startupInfo.hStdInput = inheritableStdinHandle.DangerousGetHandle(); + startupInfo.hStdOutput = inheritableStdoutHandle.DangerousGetHandle(); + startupInfo.hStdError = inheritableStderrHandle.DangerousGetHandle(); + + startupInfo.dwFlags = Interop.Advapi32.StartupInfoOptions.STARTF_USESTDHANDLES; + + if (startInfo.WindowStyle != ProcessWindowStyle.Normal) { - startupInfo.cb = sizeof(Interop.Kernel32.STARTUPINFO); + startupInfo.wShowWindow = (short)GetShowWindowFromWindowStyle(startInfo.WindowStyle); + startupInfo.dwFlags |= Interop.Advapi32.StartupInfoOptions.STARTF_USESHOWWINDOW; + } - // set up the streams - if (startInfo.RedirectStandardInput || startInfo.RedirectStandardOutput || startInfo.RedirectStandardError) - { - inheritableStdinHandle = DuplicateAsInheritable(stdinHandle!); - inheritableStdoutHandle = DuplicateAsInheritable(stdoutHandle!); - inheritableStderrHandle = DuplicateAsInheritable(stderrHandle!); + // set up the creation flags parameter + int creationFlags = 0; + if (startInfo.CreateNoWindow) creationFlags |= Interop.Advapi32.StartupInfoOptions.CREATE_NO_WINDOW; + if (startInfo.CreateNewProcessGroup) creationFlags |= Interop.Advapi32.StartupInfoOptions.CREATE_NEW_PROCESS_GROUP; - startupInfo.hStdInput = inheritableStdinHandle.DangerousGetHandle(); - startupInfo.hStdOutput = inheritableStdoutHandle.DangerousGetHandle(); - startupInfo.hStdError = inheritableStderrHandle.DangerousGetHandle(); + // set up the environment block parameter + string? environmentBlock = null; + if (startInfo._environmentVariables != null) + { + creationFlags |= Interop.Advapi32.StartupInfoOptions.CREATE_UNICODE_ENVIRONMENT; + environmentBlock = GetEnvironmentVariablesBlock(startInfo._environmentVariables!); + } - startupInfo.dwFlags = Interop.Advapi32.StartupInfoOptions.STARTF_USESTDHANDLES; - } + string? workingDirectory = startInfo.WorkingDirectory; + if (workingDirectory.Length == 0) + { + workingDirectory = null; + } - if (startInfo.WindowStyle != ProcessWindowStyle.Normal) + bool retVal; + int errorCode = 0; + + if (startInfo.UserName.Length != 0) + { + if (startInfo.Password != null && startInfo.PasswordInClearText != null) { - startupInfo.wShowWindow = (short)GetShowWindowFromWindowStyle(startInfo.WindowStyle); - startupInfo.dwFlags |= Interop.Advapi32.StartupInfoOptions.STARTF_USESHOWWINDOW; + throw new ArgumentException(SR.CantSetDuplicatePassword); } - // set up the creation flags parameter - int creationFlags = 0; - if (startInfo.CreateNoWindow) creationFlags |= Interop.Advapi32.StartupInfoOptions.CREATE_NO_WINDOW; - if (startInfo.CreateNewProcessGroup) creationFlags |= Interop.Advapi32.StartupInfoOptions.CREATE_NEW_PROCESS_GROUP; - - // set up the environment block parameter - string? environmentBlock = null; - if (startInfo._environmentVariables != null) + Interop.Advapi32.LogonFlags logonFlags = (Interop.Advapi32.LogonFlags)0; + if (startInfo.LoadUserProfile && startInfo.UseCredentialsForNetworkingOnly) { - creationFlags |= Interop.Advapi32.StartupInfoOptions.CREATE_UNICODE_ENVIRONMENT; - environmentBlock = GetEnvironmentVariablesBlock(startInfo._environmentVariables!); + throw new ArgumentException(SR.CantEnableConflictingLogonFlags, nameof(startInfo)); } - - string? workingDirectory = startInfo.WorkingDirectory; - if (workingDirectory.Length == 0) + else if (startInfo.LoadUserProfile) { - workingDirectory = null; + logonFlags = Interop.Advapi32.LogonFlags.LOGON_WITH_PROFILE; } - - bool retVal; - int errorCode = 0; - - if (startInfo.UserName.Length != 0) + else if (startInfo.UseCredentialsForNetworkingOnly) { - if (startInfo.Password != null && startInfo.PasswordInClearText != null) - { - throw new ArgumentException(SR.CantSetDuplicatePassword); - } - - Interop.Advapi32.LogonFlags logonFlags = (Interop.Advapi32.LogonFlags)0; - if (startInfo.LoadUserProfile && startInfo.UseCredentialsForNetworkingOnly) - { - throw new ArgumentException(SR.CantEnableConflictingLogonFlags, nameof(startInfo)); - } - else if (startInfo.LoadUserProfile) - { - logonFlags = Interop.Advapi32.LogonFlags.LOGON_WITH_PROFILE; - } - else if (startInfo.UseCredentialsForNetworkingOnly) - { - logonFlags = Interop.Advapi32.LogonFlags.LOGON_NETCREDENTIALS_ONLY; - } - - commandLine.NullTerminate(); - fixed (char* passwordInClearTextPtr = startInfo.PasswordInClearText ?? string.Empty) - fixed (char* environmentBlockPtr = environmentBlock) - fixed (char* commandLinePtr = &commandLine.GetPinnableReference()) - { - IntPtr passwordPtr = (startInfo.Password != null) ? - Marshal.SecureStringToGlobalAllocUnicode(startInfo.Password) : IntPtr.Zero; - - try - { - retVal = Interop.Advapi32.CreateProcessWithLogonW( - startInfo.UserName, - startInfo.Domain, - (passwordPtr != IntPtr.Zero) ? passwordPtr : (IntPtr)passwordInClearTextPtr, - logonFlags, - null, // we don't need this since all the info is in commandLine - commandLinePtr, - creationFlags, - (IntPtr)environmentBlockPtr, - workingDirectory, - ref startupInfo, // pointer to STARTUPINFO - ref processInfo // pointer to PROCESS_INFORMATION - ); - if (!retVal) - errorCode = Marshal.GetLastWin32Error(); - } - finally - { - if (passwordPtr != IntPtr.Zero) - Marshal.ZeroFreeGlobalAllocUnicode(passwordPtr); - } - } + logonFlags = Interop.Advapi32.LogonFlags.LOGON_NETCREDENTIALS_ONLY; } - else + + commandLine.NullTerminate(); + fixed (char* passwordInClearTextPtr = startInfo.PasswordInClearText ?? string.Empty) + fixed (char* environmentBlockPtr = environmentBlock) + fixed (char* commandLinePtr = &commandLine.GetPinnableReference()) { - commandLine.NullTerminate(); - fixed (char* environmentBlockPtr = environmentBlock) - fixed (char* commandLinePtr = &commandLine.GetPinnableReference()) + IntPtr passwordPtr = (startInfo.Password != null) ? + Marshal.SecureStringToGlobalAllocUnicode(startInfo.Password) : IntPtr.Zero; + + try { - retVal = Interop.Kernel32.CreateProcess( - null, // we don't need this since all the info is in commandLine - commandLinePtr, // pointer to the command line string - ref unused_SecAttrs, // address to process security attributes, we don't need to inherit the handle - ref unused_SecAttrs, // address to thread security attributes. - true, // handle inheritance flag - creationFlags, // creation flags - (IntPtr)environmentBlockPtr, // pointer to new environment block - workingDirectory, // pointer to current directory name - ref startupInfo, // pointer to STARTUPINFO - ref processInfo // pointer to PROCESS_INFORMATION + retVal = Interop.Advapi32.CreateProcessWithLogonW( + startInfo.UserName, + startInfo.Domain, + (passwordPtr != IntPtr.Zero) ? passwordPtr : (IntPtr)passwordInClearTextPtr, + logonFlags, + null, // we don't need this since all the info is in commandLine + commandLinePtr, + creationFlags, + (IntPtr)environmentBlockPtr, + workingDirectory, + ref startupInfo, // pointer to STARTUPINFO + ref processInfo // pointer to PROCESS_INFORMATION ); if (!retVal) errorCode = Marshal.GetLastWin32Error(); } - } - - if (processInfo.hProcess != IntPtr.Zero && processInfo.hProcess != new IntPtr(-1)) - Marshal.InitHandle(procSH, processInfo.hProcess); - if (processInfo.hThread != IntPtr.Zero && processInfo.hThread != new IntPtr(-1)) - Interop.Kernel32.CloseHandle(processInfo.hThread); - - if (!retVal) - { - string nativeErrorMessage = errorCode == Interop.Errors.ERROR_BAD_EXE_FORMAT || errorCode == Interop.Errors.ERROR_EXE_MACHINE_TYPE_MISMATCH - ? SR.InvalidApplication - : GetErrorMessage(errorCode); - - throw CreateExceptionForErrorStartingProcess(nativeErrorMessage, errorCode, startInfo.FileName, workingDirectory); + finally + { + if (passwordPtr != IntPtr.Zero) + Marshal.ZeroFreeGlobalAllocUnicode(passwordPtr); + } } } - catch + else { - procSH.Dispose(); - throw; + commandLine.NullTerminate(); + fixed (char* environmentBlockPtr = environmentBlock) + fixed (char* commandLinePtr = &commandLine.GetPinnableReference()) + { + retVal = Interop.Kernel32.CreateProcess( + null, // we don't need this since all the info is in commandLine + commandLinePtr, // pointer to the command line string + ref unused_SecAttrs, // address to process security attributes, we don't need to inherit the handle + ref unused_SecAttrs, // address to thread security attributes. + true, // handle inheritance flag + creationFlags, // creation flags + (IntPtr)environmentBlockPtr, // pointer to new environment block + workingDirectory, // pointer to current directory name + ref startupInfo, // pointer to STARTUPINFO + ref processInfo // pointer to PROCESS_INFORMATION + ); + if (!retVal) + errorCode = Marshal.GetLastWin32Error(); + } } - finally + + if (processInfo.hProcess != IntPtr.Zero && processInfo.hProcess != new IntPtr(-1)) + Marshal.InitHandle(procSH, processInfo.hProcess); + if (processInfo.hThread != IntPtr.Zero && processInfo.hThread != new IntPtr(-1)) + Interop.Kernel32.CloseHandle(processInfo.hThread); + + if (!retVal) { - inheritableStdinHandle?.Dispose(); - inheritableStdoutHandle?.Dispose(); - inheritableStderrHandle?.Dispose(); + string nativeErrorMessage = errorCode == Interop.Errors.ERROR_BAD_EXE_FORMAT || errorCode == Interop.Errors.ERROR_EXE_MACHINE_TYPE_MISMATCH + ? SR.InvalidApplication + : GetErrorMessage(errorCode); + + throw CreateExceptionForErrorStartingProcess(nativeErrorMessage, errorCode, startInfo.FileName, workingDirectory); } } + catch + { + procSH.Dispose(); + throw; + } + finally + { + inheritableStdinHandle?.Dispose(); + inheritableStdoutHandle?.Dispose(); + inheritableStderrHandle?.Dispose(); + + ProcessUtils.s_processStartLock.ExitWriteLock(); - commandLine.Dispose(); + commandLine.Dispose(); + } if (procSH.IsInvalid) { @@ -615,16 +610,16 @@ ref processInfo // pointer to PROCESS_INFORMATION } /// Duplicates a handle as inheritable so the child process can use it. - private static SafeFileHandle DuplicateAsInheritable(SafeFileHandle handle) + private static SafeFileHandle DuplicateAsInheritable(SafeFileHandle sourceHandle) { IntPtr currentProcHandle = Interop.Kernel32.GetCurrentProcess(); if (!Interop.Kernel32.DuplicateHandle(currentProcHandle, - handle, - currentProcHandle, - out SafeFileHandle duplicatedHandle, - 0, - bInheritHandle: true, - Interop.Kernel32.HandleOptions.DUPLICATE_SAME_ACCESS)) + sourceHandle, + currentProcHandle, + out SafeFileHandle duplicatedHandle, + 0, + bInheritHandle: true, + Interop.Kernel32.HandleOptions.DUPLICATE_SAME_ACCESS)) { throw new Win32Exception(Marshal.GetLastWin32Error()); } diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs index 7f2ba1cbe37606..e2754e6447a380 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs @@ -1278,6 +1278,11 @@ public bool Start() } } } + bool anyRedirection = startInfo.RedirectStandardInput || startInfo.RedirectStandardOutput || startInfo.RedirectStandardError; + if (startInfo.UseShellExecute && anyRedirection) + { + throw new InvalidOperationException(SR.CantRedirectStreams); + } //Cannot start a new process and store its handle if the object has been disposed, since finalization has been suppressed. CheckDisposed(); @@ -1294,33 +1299,58 @@ public bool Start() try { + // On Windows, ShellExecute does not provide an ability to set standard handles. + // On Unix, we emulate it and require the handles. if (!startInfo.UseShellExecute || !OperatingSystem.IsWindows()) { - if (startInfo.RedirectStandardInput) - { - SafeFileHandle.CreateAnonymousPipe(out childInputPipeHandle, out parentInputPipeHandle); - } - else - { - childInputPipeHandle = Console.OpenStandardInputHandle(); - } + // Windows supports creating non-inheritable pipe in atomic way. + // When it comes to Unixes, it depends whether they support pipe2 sys-call or not. + // If they don't, the pipe is created as inheritable and made non-inheritable with another sys-call. + // Some process could be started in the meantime, so in order to prevent accidental handle inheritance, + // a WriterLock is used around the pipe creation code. - if (startInfo.RedirectStandardOutput) - { - SafeFileHandle.CreateAnonymousPipe(out parentOutputPipeHandle, out childOutputPipeHandle, asyncRead: OperatingSystem.IsWindows()); - } - else + bool requiresLock = anyRedirection && !SupportsAtomicNonInheritablePipeCreation; + + if (requiresLock) { - childOutputPipeHandle = Console.OpenStandardOutputHandle(); + ProcessUtils.s_processStartLock.EnterWriteLock(); } - if (startInfo.RedirectStandardError) + try { - SafeFileHandle.CreateAnonymousPipe(out parentErrorPipeHandle, out childErrorPipeHandle, asyncRead: OperatingSystem.IsWindows()); + if (startInfo.RedirectStandardInput) + { + SafeFileHandle.CreateAnonymousPipe(out childInputPipeHandle, out parentInputPipeHandle); + } + else + { + childInputPipeHandle = Console.OpenStandardInputHandle(); + } + + if (startInfo.RedirectStandardOutput) + { + SafeFileHandle.CreateAnonymousPipe(out parentOutputPipeHandle, out childOutputPipeHandle, asyncRead: OperatingSystem.IsWindows()); + } + else + { + childOutputPipeHandle = Console.OpenStandardOutputHandle(); + } + + if (startInfo.RedirectStandardError) + { + SafeFileHandle.CreateAnonymousPipe(out parentErrorPipeHandle, out childErrorPipeHandle, asyncRead: OperatingSystem.IsWindows()); + } + else + { + childErrorPipeHandle = Console.OpenStandardErrorHandle(); + } } - else + finally { - childErrorPipeHandle = Console.OpenStandardErrorHandle(); + if (requiresLock) + { + ProcessUtils.s_processStartLock.ExitWriteLock(); + } } } @@ -1334,10 +1364,13 @@ public bool Start() parentInputPipeHandle?.Dispose(); parentOutputPipeHandle?.Dispose(); parentErrorPipeHandle?.Dispose(); + throw; } finally { + // We MUST close the parent copies of the child handles, otherwise the parent + // process will not receive EOF when the child process closes its handles. childInputPipeHandle?.Dispose(); childOutputPipeHandle?.Dispose(); childErrorPipeHandle?.Dispose(); @@ -1347,7 +1380,9 @@ public bool Start() { _standardInput = new StreamWriter(OpenStream(parentInputPipeHandle!, FileAccess.Write), startInfo.StandardInputEncoding ?? GetStandardInputEncoding(), StreamBufferSize) - { AutoFlush = true }; + { + AutoFlush = true + }; } if (startInfo.RedirectStandardOutput) { diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.cs index c3ead1de0030bf..9753f0b7d6370e 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.cs @@ -2,11 +2,14 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.IO; +using System.Threading; namespace System.Diagnostics { internal static partial class ProcessUtils { + internal static readonly ReaderWriterLockSlim s_processStartLock = new ReaderWriterLockSlim(); + internal static string? FindProgramInPath(string program) { string? pathEnvVar = System.Environment.GetEnvironmentVariable("PATH"); diff --git a/src/native/libs/System.Native/entrypoints.c b/src/native/libs/System.Native/entrypoints.c index 458980b89bd75e..a9e608ae9f0832 100644 --- a/src/native/libs/System.Native/entrypoints.c +++ b/src/native/libs/System.Native/entrypoints.c @@ -70,6 +70,7 @@ static const Entry s_sysNative[] = DllImportEntry(SystemNative_ReadDir) DllImportEntry(SystemNative_OpenDir) DllImportEntry(SystemNative_CloseDir) + DllImportEntry(SystemNative_IsAtomicNonInheritablePipeCreationSupported) DllImportEntry(SystemNative_Pipe) DllImportEntry(SystemNative_FcntlSetFD) DllImportEntry(SystemNative_FcntlGetFD) diff --git a/src/native/libs/System.Native/pal_io.c b/src/native/libs/System.Native/pal_io.c index cd488c01c2c339..1452f24908265c 100644 --- a/src/native/libs/System.Native/pal_io.c +++ b/src/native/libs/System.Native/pal_io.c @@ -560,6 +560,15 @@ int32_t SystemNative_CloseDir(DIR* dir) return result; } +int32_t SystemNative_IsAtomicNonInheritablePipeCreationSupported() +{ +#if HAVE_PIPE2 + return 1; +#else + return 0; +#endif +} + int32_t SystemNative_Pipe(int32_t pipeFds[2], int32_t flags) { #ifdef TARGET_WASM diff --git a/src/native/libs/System.Native/pal_io.h b/src/native/libs/System.Native/pal_io.h index bc6c108828d256..31e2f81d50b0ff 100644 --- a/src/native/libs/System.Native/pal_io.h +++ b/src/native/libs/System.Native/pal_io.h @@ -447,6 +447,13 @@ PALEXPORT int32_t SystemNative_CloseDir(DIR* dir); PALEXPORT int32_t SystemNative_Pipe(int32_t pipefd[2], // [out] pipefds[0] gets read end, pipefd[1] gets write end. int32_t flags); // 0 for defaults. Use PAL_O_CLOEXEC, PAL_O_NONBLOCK_READ, and PAL_O_NONBLOCK_WRITE for additional behavior. +/** + * Determines if the current platform supports atomically creating pipes with non-inheritable file descriptors (pipe2). + * + * Returns 1 if supported, 0 if not supported. + */ +PALEXPORT int32_t SystemNative_IsAtomicNonInheritablePipeCreationSupported(void); + // NOTE: Rather than a general fcntl shim, we opt to export separate functions // for each command. This allows use to have strongly typed arguments and saves // complexity around converting command codes. From 43b6301944bfaecd3a6d063d5a95255b1f4b8326 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 19 Mar 2026 13:58:28 +0100 Subject: [PATCH 05/22] more polishing after reading the code again --- .../Interop.ForkAndExecProcess.cs | 3 -- .../src/System/Diagnostics/Process.Unix.cs | 12 ++----- .../src/System/Diagnostics/Process.Windows.cs | 8 +---- src/native/libs/System.Native/pal_process.c | 31 +++++-------------- src/native/libs/System.Native/pal_process.h | 9 ++---- .../libs/System.Native/pal_process_wasi.c | 3 -- 6 files changed, 14 insertions(+), 52 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs index 24e82a12dc2727..e03d68e0a6e90e 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs @@ -14,7 +14,6 @@ internal static partial class Sys { internal static unsafe int ForkAndExecProcess( string filename, string[] argv, string[] envp, string? cwd, - bool redirectStdin, bool redirectStdout, bool redirectStderr, bool setUser, uint userId, uint groupId, uint[]? groups, out int lpChildPid, SafeFileHandle stdinFd, SafeFileHandle stdoutFd, SafeFileHandle stderrFd, bool shouldThrow = true) { @@ -28,7 +27,6 @@ internal static unsafe int ForkAndExecProcess( { result = ForkAndExecProcess( filename, argvPtr, envpPtr, cwd, - redirectStdin ? 1 : 0, redirectStdout ? 1 : 0, redirectStderr ? 1 : 0, setUser ? 1 : 0, userId, groupId, pGroups, groups?.Length ?? 0, out lpChildPid, stdinFd, stdoutFd, stderrFd); } @@ -44,7 +42,6 @@ internal static unsafe int ForkAndExecProcess( [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_ForkAndExecProcess", StringMarshalling = StringMarshalling.Utf8, SetLastError = true)] private static unsafe partial int ForkAndExecProcess( string filename, byte** argv, byte** envp, string? cwd, - int redirectStdin, int redirectStdout, int redirectStderr, int setUser, uint userId, uint groupId, uint* groups, int groupsLength, out int lpChildPid, SafeFileHandle stdinFd, SafeFileHandle stdoutFd, SafeFileHandle stderrFd); diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index ec99a5ee96cfae..0e274824d6f528 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -447,8 +447,6 @@ private bool StartCore(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, return true; } - private static bool SupportsAtomicNonInheritablePipeCreation => Interop.Sys.IsAtomicNonInheritablePipeCreationSupported; - private bool ForkAndExecProcess( ProcessStartInfo startInfo, string? resolvedFilename, string[] argv, string[] envp, string? cwd, bool setCredentials, uint userId, @@ -481,12 +479,8 @@ private bool ForkAndExecProcess( // the calling thread will transfer, thread IDs aren't stable across the fork, etc.) int errno = Interop.Sys.ForkAndExecProcess( resolvedFilename, argv, envp, cwd, - startInfo.RedirectStandardInput, startInfo.RedirectStandardOutput, startInfo.RedirectStandardError, setCredentials, userId, groupId, groups, - out childPid, - stdinHandle, - stdoutHandle, - stderrHandle); + out childPid, stdinHandle, stdoutHandle, stderrHandle); if (errno == 0) { @@ -730,10 +724,10 @@ private static AnonymousPipeClientStream OpenStream(SafeFileHandle handle, FileA return new AnonymousPipeClientStream(direction, safePipeHandle); } - /// Gets the default encoding for standard input. + private static bool SupportsAtomicNonInheritablePipeCreation => Interop.Sys.IsAtomicNonInheritablePipeCreationSupported; + private static Encoding GetStandardInputEncoding() => Encoding.Default; - /// Gets the default encoding for standard output/error. private static Encoding GetStandardOutputEncoding() => Encoding.Default; /// Parses a command-line argument string into a list of arguments. 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 bce23b6b6f81d8..25fd3cb15d6cbe 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 @@ -765,16 +765,10 @@ private SafeProcessHandle GetProcessHandle(int access, bool throwIfExited = true } } - /// Opens a stream around the specified file handle. - private static FileStream OpenStream(SafeFileHandle handle, FileAccess access) - { - return new FileStream(handle, access, StreamBufferSize, handle.IsAsync); - } + private static FileStream OpenStream(SafeFileHandle handle, FileAccess access) => new(handle, access, StreamBufferSize, handle.IsAsync); - /// Gets the default encoding for standard input. private static ConsoleEncoding GetStandardInputEncoding() => GetEncoding((int)Interop.Kernel32.GetConsoleCP()); - /// Gets the default encoding for standard output/error. private static ConsoleEncoding GetStandardOutputEncoding() => GetEncoding((int)Interop.Kernel32.GetConsoleOutputCP()); private static string GetEnvironmentVariablesBlock(DictionaryWrapper sd) diff --git a/src/native/libs/System.Native/pal_process.c b/src/native/libs/System.Native/pal_process.c index 858827ed58cad2..2c77fd87fd65c2 100644 --- a/src/native/libs/System.Native/pal_process.c +++ b/src/native/libs/System.Native/pal_process.c @@ -70,18 +70,10 @@ static void CloseIfOpen(int fd) } } -static int Dup2WithInterruptedRetry(int oldfd, int newfd, int removeCloExec) +static int Dup2WithInterruptedRetry(int oldfd, int newfd) { int result; while (CheckInterrupted(result = dup2(oldfd, newfd))); - if (result != -1 && removeCloExec) - { - int flags = fcntl(newfd, F_GETFD); - if (flags != -1 && (flags & FD_CLOEXEC)) - { - fcntl(newfd, F_SETFD, flags & ~FD_CLOEXEC); - } - } return result; } @@ -217,9 +209,6 @@ int32_t SystemNative_ForkAndExecProcess(const char* filename, char* const argv[], char* const envp[], const char* cwd, - int32_t redirectStdin, - int32_t redirectStdout, - int32_t redirectStderr, int32_t setCredentials, uint32_t userId, uint32_t groupId, @@ -248,10 +237,6 @@ int32_t SystemNative_ForkAndExecProcess(const char* filename, assert(NULL != filename && NULL != argv && NULL != envp && NULL != childPid && (groupsLength == 0 || groups != NULL) && "null argument."); - assert((redirectStdin & ~1) == 0 && (redirectStdout & ~1) == 0 && - (redirectStderr & ~1) == 0 && (setCredentials & ~1) == 0 && - "Boolean redirect* inputs must be 0 or 1."); - if (setCredentials && groupsLength > 0) { getGroupsBuffer = (uint32_t*)(malloc(sizeof(uint32_t) * Int32ToSizeT(groupsLength))); @@ -370,11 +355,12 @@ int32_t SystemNative_ForkAndExecProcess(const char* filename, } pthread_sigmask(SIG_SETMASK, &old_signal_set, &junk_signal_set); // Not all architectures allow NULL here - // For any redirections that should happen, dup the provided file descriptors onto stdin/out/err. - // The provided file handles have CLOEXEC enabled by default, so we clear it after dup2. - if ((redirectStdin && Dup2WithInterruptedRetry(stdinFd, STDIN_FILENO, 1) == -1) || - (redirectStdout && Dup2WithInterruptedRetry(stdoutFd, STDOUT_FILENO, 1) == -1) || - (redirectStderr && Dup2WithInterruptedRetry(stderrFd, STDERR_FILENO, 1) == -1)) + // The provided file handles might have CLOEXEC enabled, + // but dup2 doesn't copy the file descriptor flags, + // so the new file descriptors won't have CLOEXEC enabled. + if (Dup2WithInterruptedRetry(stdinFd, STDIN_FILENO) == -1 || + Dup2WithInterruptedRetry(stdoutFd, STDOUT_FILENO) == -1 || + Dup2WithInterruptedRetry(stderrFd, STDERR_FILENO) == -1) { ExitChild(waitForChildToExecPipe[WRITE_END_OF_PIPE], errno); } @@ -470,9 +456,6 @@ done:; (void)argv; (void)envp; (void)cwd; - (void)redirectStdin; - (void)redirectStdout; - (void)redirectStderr; (void)setCredentials; (void)userId; (void)groupId; diff --git a/src/native/libs/System.Native/pal_process.h b/src/native/libs/System.Native/pal_process.h index 81a850c7362ca1..94c3c9c57afe70 100644 --- a/src/native/libs/System.Native/pal_process.h +++ b/src/native/libs/System.Native/pal_process.h @@ -23,18 +23,15 @@ PALEXPORT int32_t SystemNative_ForkAndExecProcess( char* const argv[], // argv argument to execve char* const envp[], // envp argument to execve const char* cwd, // path passed to chdir in child process - int32_t redirectStdin, // whether to redirect standard input from the parent - int32_t redirectStdout, // whether to redirect standard output to the parent - int32_t redirectStderr, // whether to redirect standard error to the parent int32_t setCredentials, // whether to set the userId and groupId for the child process uint32_t userId, // the user id under which the child process should run uint32_t groupId, // the group id under which the child process should run uint32_t* groups, // the groups under which the child process should run int32_t groupsLength, // the length of groups int32_t* childPid, // [out] the child process' id - int32_t stdinFd, // if redirectStdin, the fd for the child's stdin - int32_t stdoutFd, // if redirectStdout, the fd for the child's stdout - int32_t stderrFd); // if redirectStderr, the fd for the child's stderr + int32_t stdinFd, // the fd for the child's stdin + int32_t stdoutFd, // the fd for the child's stdout + int32_t stderrFd); // the fd for the child's stderr /************ * The values below in the header are fixed and correct for managed callers to use forever. diff --git a/src/native/libs/System.Native/pal_process_wasi.c b/src/native/libs/System.Native/pal_process_wasi.c index 061240920ee8c1..b83f5a2ef42478 100644 --- a/src/native/libs/System.Native/pal_process_wasi.c +++ b/src/native/libs/System.Native/pal_process_wasi.c @@ -21,9 +21,6 @@ int32_t SystemNative_ForkAndExecProcess(const char* filename, char* const argv[], char* const envp[], const char* cwd, - int32_t redirectStdin, - int32_t redirectStdout, - int32_t redirectStderr, int32_t setCredentials, uint32_t userId, uint32_t groupId, From dc85b035417df8a7dc3dafb0e896b139ac863f72 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 19 Mar 2026 14:45:47 +0100 Subject: [PATCH 06/22] fix Unix build --- src/native/libs/System.Native/pal_io.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/native/libs/System.Native/pal_io.c b/src/native/libs/System.Native/pal_io.c index 1452f24908265c..c7365d967ebec0 100644 --- a/src/native/libs/System.Native/pal_io.c +++ b/src/native/libs/System.Native/pal_io.c @@ -560,7 +560,7 @@ int32_t SystemNative_CloseDir(DIR* dir) return result; } -int32_t SystemNative_IsAtomicNonInheritablePipeCreationSupported() +int32_t SystemNative_IsAtomicNonInheritablePipeCreationSupported(void) { #if HAVE_PIPE2 return 1; From 9ee188105cc0bcf678677ae0dea16bf758e7db2e Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 19 Mar 2026 15:04:15 +0100 Subject: [PATCH 07/22] trigger the CI as it seems to got stuck --- .../src/System/Diagnostics/Process.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs index e2754e6447a380..1199f495ee37ac 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs @@ -1371,6 +1371,8 @@ public bool Start() { // We MUST close the parent copies of the child handles, otherwise the parent // process will not receive EOF when the child process closes its handles. + // It's OK to do it for handles returned by Console.OpenStandard*Handle APIs, + // because these handles are not owned and won't be closed by Dispose. childInputPipeHandle?.Dispose(); childOutputPipeHandle?.Dispose(); childErrorPipeHandle?.Dispose(); From 17bc483c72f1e5a655ab92b1321df68ec858f90b Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 19 Mar 2026 18:53:24 +0100 Subject: [PATCH 08/22] fix the tests: - STARTF_USESTDHANDLES is always provided now - we may use more flags in the future - so ProcessWindowStyle tests should check only if STARTF_USESHOWWINDOW (1) was applied --- .../src/System.Diagnostics.Process.csproj | 2 +- .../tests/ProcessStartInfoTests.Windows.cs | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 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 3499b5c439110a..6826261afefaf2 100644 --- a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -192,7 +192,7 @@ + Link="Common\Interop\Windows\Kernel32\Interop.HandleOptions.cs" /> (1, 0), - ProcessWindowStyle.Minimized => (1, 2), - ProcessWindowStyle.Maximized => (1, 3), + ProcessWindowStyle.Hidden => (true, 0), // SW_HIDE is 0 + ProcessWindowStyle.Minimized => (true, 2), // SW_SHOWMINIMIZED is 2 + ProcessWindowStyle.Maximized => (true, 3), // SW_SHOWMAXIMIZED is 3 // UseShellExecute always sets the flag but no shell does not for Normal. - _ => useShellExecute ? (1, 1) : (0, 0), + _ => useShellExecute ? (true, 1) : (false, 0), // SW_SHOWNORMAL is 1 }; using Process p = CreateProcess((string procArg) => @@ -104,13 +104,13 @@ public void TestWindowStyle(ProcessWindowStyle windowStyle, bool useShellExecute Interop.GetStartupInfoW(out Interop.STARTUPINFO si); string[] argSplit = procArg.Split(" "); - int expectedDwFlag = int.Parse(argSplit[0]); + bool expectUsesShowWindow = bool.Parse(argSplit[0]); short expectedWindowFlag = short.Parse(argSplit[1]); - Assert.Equal(expectedDwFlag, si.dwFlags); + Assert.Equal(expectUsesShowWindow, (si.dwFlags & 0x1) != 0); // STARTF_USESHOWWINDOW is 0x1 Assert.Equal(expectedWindowFlag, si.wShowWindow); return RemoteExecutor.SuccessExitCode; - }, $"{expectedDwFlag} {expectedWindowFlag}"); + }, $"{expectUsesShowWindow} {expectedWindowFlag}"); p.StartInfo.UseShellExecute = useShellExecute; p.StartInfo.WindowStyle = windowStyle; p.Start(); From 7cf7f0cbba1a9e83ce9faab31349f3e93c51839b Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 19 Mar 2026 19:41:35 +0100 Subject: [PATCH 09/22] handle INVALID_HANDLE_VALUE on Windows --- .../src/System/Diagnostics/Process.Windows.cs | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) 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 25fd3cb15d6cbe..172e85705cb2c1 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 @@ -453,15 +453,29 @@ private unsafe bool StartWithCreateProcess(ProcessStartInfo startInfo, SafeFileH { startupInfo.cb = sizeof(Interop.Kernel32.STARTUPINFO); - inheritableStdinHandle = DuplicateAsInheritable(stdinHandle); - inheritableStdoutHandle = DuplicateAsInheritable(stdoutHandle); - inheritableStderrHandle = DuplicateAsInheritable(stderrHandle); + // A process could be started with INVALID_HANDLE_VALUE, we need to take this into account. + if (!stdinHandle.IsInvalid) + { + inheritableStdinHandle = DuplicateAsInheritable(stdinHandle); + startupInfo.hStdInput = inheritableStdinHandle.DangerousGetHandle(); + } - startupInfo.hStdInput = inheritableStdinHandle.DangerousGetHandle(); - startupInfo.hStdOutput = inheritableStdoutHandle.DangerousGetHandle(); - startupInfo.hStdError = inheritableStderrHandle.DangerousGetHandle(); + if (!stdoutHandle.IsInvalid) + { + inheritableStdoutHandle = DuplicateAsInheritable(stdoutHandle); + startupInfo.hStdOutput = inheritableStdoutHandle.DangerousGetHandle(); + } - startupInfo.dwFlags = Interop.Advapi32.StartupInfoOptions.STARTF_USESTDHANDLES; + if (!stderrHandle.IsInvalid) + { + inheritableStderrHandle = DuplicateAsInheritable(stderrHandle); + startupInfo.hStdError = inheritableStderrHandle.DangerousGetHandle(); + } + + if (!stdinHandle.IsInvalid || !stdoutHandle.IsInvalid || !stderrHandle.IsInvalid) + { + startupInfo.dwFlags = Interop.Advapi32.StartupInfoOptions.STARTF_USESTDHANDLES; + } if (startInfo.WindowStyle != ProcessWindowStyle.Normal) { From 2c8c4442318e50f4b5a5c2615f904bb3378a8592 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 19 Mar 2026 22:21:10 +0100 Subject: [PATCH 10/22] address code review feedback: don't use Console.OpenStandard*Handle on Android (it's not supported) --- .../src/System/Diagnostics/Process.cs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs index 1199f495ee37ac..cef30af43bccc0 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs @@ -1322,6 +1322,10 @@ public bool Start() { SafeFileHandle.CreateAnonymousPipe(out childInputPipeHandle, out parentInputPipeHandle); } + else if (OperatingSystem.IsAndroid()) + { + childInputPipeHandle = new SafeFileHandle(0, ownsHandle: false); + } else { childInputPipeHandle = Console.OpenStandardInputHandle(); @@ -1331,6 +1335,10 @@ public bool Start() { SafeFileHandle.CreateAnonymousPipe(out parentOutputPipeHandle, out childOutputPipeHandle, asyncRead: OperatingSystem.IsWindows()); } + else if (OperatingSystem.IsAndroid()) + { + childOutputPipeHandle = new SafeFileHandle(1, ownsHandle: false); + } else { childOutputPipeHandle = Console.OpenStandardOutputHandle(); @@ -1340,6 +1348,10 @@ public bool Start() { SafeFileHandle.CreateAnonymousPipe(out parentErrorPipeHandle, out childErrorPipeHandle, asyncRead: OperatingSystem.IsWindows()); } + else if (OperatingSystem.IsAndroid()) + { + childErrorPipeHandle = new SafeFileHandle(2, ownsHandle: false); + } else { childErrorPipeHandle = Console.OpenStandardErrorHandle(); From bba0144c10d7256babcc3f1b5d7036403928a028 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 19 Mar 2026 23:36:13 +0000 Subject: [PATCH 11/22] Make Console.OpenStandard*Handle APIs work on Android by returning fd 0/1/2; remove Android checks from Process.cs Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- src/libraries/System.Console/ref/System.Console.cs | 3 --- src/libraries/System.Console/src/System/Console.cs | 3 --- .../src/System/ConsolePal.Android.cs | 6 +++--- .../System.Console/tests/ConsoleHandles.cs | 14 +++++++------- .../src/System/Diagnostics/Process.cs | 12 ------------ 5 files changed, 10 insertions(+), 28 deletions(-) diff --git a/src/libraries/System.Console/ref/System.Console.cs b/src/libraries/System.Console/ref/System.Console.cs index ed6a329434b08b..0660f552e3452f 100644 --- a/src/libraries/System.Console/ref/System.Console.cs +++ b/src/libraries/System.Console/ref/System.Console.cs @@ -100,7 +100,6 @@ public static void MoveBufferArea(int sourceLeft, int sourceTop, int sourceWidth public static void MoveBufferArea(int sourceLeft, int sourceTop, int sourceWidth, int sourceHeight, int targetLeft, int targetTop, char sourceChar, System.ConsoleColor sourceForeColor, System.ConsoleColor sourceBackColor) { } public static System.IO.Stream OpenStandardError() { throw null; } public static System.IO.Stream OpenStandardError(int bufferSize) { throw null; } - [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("android")] [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] public static Microsoft.Win32.SafeHandles.SafeFileHandle OpenStandardErrorHandle() { throw null; } @@ -112,14 +111,12 @@ public static void MoveBufferArea(int sourceLeft, int sourceTop, int sourceWidth [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("android")] [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")] public static System.IO.Stream OpenStandardInput(int bufferSize) { throw null; } - [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("android")] [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")] [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] public static Microsoft.Win32.SafeHandles.SafeFileHandle OpenStandardInputHandle() { throw null; } public static System.IO.Stream OpenStandardOutput() { throw null; } public static System.IO.Stream OpenStandardOutput(int bufferSize) { throw null; } - [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("android")] [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] public static Microsoft.Win32.SafeHandles.SafeFileHandle OpenStandardOutputHandle() { throw null; } diff --git a/src/libraries/System.Console/src/System/Console.cs b/src/libraries/System.Console/src/System/Console.cs index e72ef2f4d2ec60..100680f394d3e5 100644 --- a/src/libraries/System.Console/src/System/Console.cs +++ b/src/libraries/System.Console/src/System/Console.cs @@ -660,7 +660,6 @@ public static Stream OpenStandardError(int bufferSize) /// /// The returned handle does not own the underlying resource, so disposing it will not close the standard input handle. /// - [UnsupportedOSPlatform("android")] [UnsupportedOSPlatform("browser")] [UnsupportedOSPlatform("ios")] [UnsupportedOSPlatform("tvos")] @@ -676,7 +675,6 @@ public static SafeFileHandle OpenStandardInputHandle() /// /// The returned handle does not own the underlying resource, so disposing it will not close the standard output handle. /// - [UnsupportedOSPlatform("android")] [UnsupportedOSPlatform("ios")] [UnsupportedOSPlatform("tvos")] public static SafeFileHandle OpenStandardOutputHandle() @@ -691,7 +689,6 @@ public static SafeFileHandle OpenStandardOutputHandle() /// /// The returned handle does not own the underlying resource, so disposing it will not close the standard error handle. /// - [UnsupportedOSPlatform("android")] [UnsupportedOSPlatform("ios")] [UnsupportedOSPlatform("tvos")] public static SafeFileHandle OpenStandardErrorHandle() diff --git a/src/libraries/System.Console/src/System/ConsolePal.Android.cs b/src/libraries/System.Console/src/System/ConsolePal.Android.cs index 58e29f3ab758bb..22123a553d33ab 100644 --- a/src/libraries/System.Console/src/System/ConsolePal.Android.cs +++ b/src/libraries/System.Console/src/System/ConsolePal.Android.cs @@ -31,11 +31,11 @@ internal static void EnsureConsoleInitialized() { } public static Stream OpenStandardError() => new LogcatStream(OutputEncoding); - public static SafeFileHandle OpenStandardInputHandle() => throw new PlatformNotSupportedException(); + public static SafeFileHandle OpenStandardInputHandle() => new SafeFileHandle(0, ownsHandle: false); - public static SafeFileHandle OpenStandardOutputHandle() => throw new PlatformNotSupportedException(); + public static SafeFileHandle OpenStandardOutputHandle() => new SafeFileHandle(1, ownsHandle: false); - public static SafeFileHandle OpenStandardErrorHandle() => throw new PlatformNotSupportedException(); + public static SafeFileHandle OpenStandardErrorHandle() => new SafeFileHandle(2, ownsHandle: false); public static Encoding InputEncoding => throw new PlatformNotSupportedException(); diff --git a/src/libraries/System.Console/tests/ConsoleHandles.cs b/src/libraries/System.Console/tests/ConsoleHandles.cs index b09c94b71497c8..89e76b899179a2 100644 --- a/src/libraries/System.Console/tests/ConsoleHandles.cs +++ b/src/libraries/System.Console/tests/ConsoleHandles.cs @@ -12,7 +12,7 @@ namespace System.Tests public partial class ConsoleTests { [Fact] - [PlatformSpecific(TestPlatforms.Any & ~TestPlatforms.Browser & ~TestPlatforms.iOS & ~TestPlatforms.tvOS & ~TestPlatforms.Android)] + [PlatformSpecific(TestPlatforms.Any & ~TestPlatforms.Browser & ~TestPlatforms.iOS & ~TestPlatforms.tvOS)] public void OpenStandardInputHandle_ReturnsValidHandle() { using SafeFileHandle inputHandle = Console.OpenStandardInputHandle(); @@ -22,7 +22,7 @@ public void OpenStandardInputHandle_ReturnsValidHandle() } [Fact] - [PlatformSpecific(TestPlatforms.Any & ~TestPlatforms.iOS & ~TestPlatforms.tvOS & ~TestPlatforms.Android)] + [PlatformSpecific(TestPlatforms.Any & ~TestPlatforms.iOS & ~TestPlatforms.tvOS)] public void OpenStandardOutputHandle_ReturnsValidHandle() { using SafeFileHandle outputHandle = Console.OpenStandardOutputHandle(); @@ -32,7 +32,7 @@ public void OpenStandardOutputHandle_ReturnsValidHandle() } [Fact] - [PlatformSpecific(TestPlatforms.Any & ~TestPlatforms.iOS & ~TestPlatforms.tvOS & ~TestPlatforms.Android)] + [PlatformSpecific(TestPlatforms.Any & ~TestPlatforms.iOS & ~TestPlatforms.tvOS)] public void OpenStandardErrorHandle_ReturnsValidHandle() { using SafeFileHandle errorHandle = Console.OpenStandardErrorHandle(); @@ -42,7 +42,7 @@ public void OpenStandardErrorHandle_ReturnsValidHandle() } [Fact] - [PlatformSpecific(TestPlatforms.Any & ~TestPlatforms.Browser & ~TestPlatforms.iOS & ~TestPlatforms.tvOS & ~TestPlatforms.Android)] + [PlatformSpecific(TestPlatforms.Any & ~TestPlatforms.Browser & ~TestPlatforms.iOS & ~TestPlatforms.tvOS)] public void OpenStandardHandles_DoNotOwnHandle() { SafeFileHandle inputHandle = Console.OpenStandardInputHandle(); @@ -87,21 +87,21 @@ public void OpenStandardHandles_CanBeUsedWithStream() } [Fact] - [PlatformSpecific(TestPlatforms.Android | TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.Browser)] + [PlatformSpecific(TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.Browser)] public void OpenStandardInputHandle_ThrowsOnUnsupportedPlatforms() { Assert.Throws(() => Console.OpenStandardInputHandle()); } [Fact] - [PlatformSpecific(TestPlatforms.Android | TestPlatforms.iOS | TestPlatforms.tvOS)] + [PlatformSpecific(TestPlatforms.iOS | TestPlatforms.tvOS)] public void OpenStandardOutputHandle_ThrowsOnUnsupportedPlatforms() { Assert.Throws(() => Console.OpenStandardOutputHandle()); } [Fact] - [PlatformSpecific(TestPlatforms.Android | TestPlatforms.iOS | TestPlatforms.tvOS)] + [PlatformSpecific(TestPlatforms.iOS | TestPlatforms.tvOS)] public void OpenStandardErrorHandle_ThrowsOnUnsupportedPlatforms() { Assert.Throws(() => Console.OpenStandardErrorHandle()); diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs index cef30af43bccc0..1199f495ee37ac 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs @@ -1322,10 +1322,6 @@ public bool Start() { SafeFileHandle.CreateAnonymousPipe(out childInputPipeHandle, out parentInputPipeHandle); } - else if (OperatingSystem.IsAndroid()) - { - childInputPipeHandle = new SafeFileHandle(0, ownsHandle: false); - } else { childInputPipeHandle = Console.OpenStandardInputHandle(); @@ -1335,10 +1331,6 @@ public bool Start() { SafeFileHandle.CreateAnonymousPipe(out parentOutputPipeHandle, out childOutputPipeHandle, asyncRead: OperatingSystem.IsWindows()); } - else if (OperatingSystem.IsAndroid()) - { - childOutputPipeHandle = new SafeFileHandle(1, ownsHandle: false); - } else { childOutputPipeHandle = Console.OpenStandardOutputHandle(); @@ -1348,10 +1340,6 @@ public bool Start() { SafeFileHandle.CreateAnonymousPipe(out parentErrorPipeHandle, out childErrorPipeHandle, asyncRead: OperatingSystem.IsWindows()); } - else if (OperatingSystem.IsAndroid()) - { - childErrorPipeHandle = new SafeFileHandle(2, ownsHandle: false); - } else { childErrorPipeHandle = Console.OpenStandardErrorHandle(); From 7a345e239c7728a0ba5d2276092af604e34a5854 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 20 Mar 2026 12:00:14 +0100 Subject: [PATCH 12/22] Revert "Make Console.OpenStandard*Handle APIs work on Android by returning fd 0/1/2; remove Android checks from Process.cs" This reverts commit bba0144c10d7256babcc3f1b5d7036403928a028. --- src/libraries/System.Console/ref/System.Console.cs | 3 +++ src/libraries/System.Console/src/System/Console.cs | 3 +++ .../src/System/ConsolePal.Android.cs | 6 +++--- .../System.Console/tests/ConsoleHandles.cs | 14 +++++++------- .../src/System/Diagnostics/Process.cs | 12 ++++++++++++ 5 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.Console/ref/System.Console.cs b/src/libraries/System.Console/ref/System.Console.cs index 0660f552e3452f..ed6a329434b08b 100644 --- a/src/libraries/System.Console/ref/System.Console.cs +++ b/src/libraries/System.Console/ref/System.Console.cs @@ -100,6 +100,7 @@ public static void MoveBufferArea(int sourceLeft, int sourceTop, int sourceWidth public static void MoveBufferArea(int sourceLeft, int sourceTop, int sourceWidth, int sourceHeight, int targetLeft, int targetTop, char sourceChar, System.ConsoleColor sourceForeColor, System.ConsoleColor sourceBackColor) { } public static System.IO.Stream OpenStandardError() { throw null; } public static System.IO.Stream OpenStandardError(int bufferSize) { throw null; } + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("android")] [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] public static Microsoft.Win32.SafeHandles.SafeFileHandle OpenStandardErrorHandle() { throw null; } @@ -111,12 +112,14 @@ public static void MoveBufferArea(int sourceLeft, int sourceTop, int sourceWidth [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("android")] [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")] public static System.IO.Stream OpenStandardInput(int bufferSize) { throw null; } + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("android")] [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("browser")] [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] public static Microsoft.Win32.SafeHandles.SafeFileHandle OpenStandardInputHandle() { throw null; } public static System.IO.Stream OpenStandardOutput() { throw null; } public static System.IO.Stream OpenStandardOutput(int bufferSize) { throw null; } + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("android")] [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("ios")] [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("tvos")] public static Microsoft.Win32.SafeHandles.SafeFileHandle OpenStandardOutputHandle() { throw null; } diff --git a/src/libraries/System.Console/src/System/Console.cs b/src/libraries/System.Console/src/System/Console.cs index 100680f394d3e5..e72ef2f4d2ec60 100644 --- a/src/libraries/System.Console/src/System/Console.cs +++ b/src/libraries/System.Console/src/System/Console.cs @@ -660,6 +660,7 @@ public static Stream OpenStandardError(int bufferSize) /// /// The returned handle does not own the underlying resource, so disposing it will not close the standard input handle. /// + [UnsupportedOSPlatform("android")] [UnsupportedOSPlatform("browser")] [UnsupportedOSPlatform("ios")] [UnsupportedOSPlatform("tvos")] @@ -675,6 +676,7 @@ public static SafeFileHandle OpenStandardInputHandle() /// /// The returned handle does not own the underlying resource, so disposing it will not close the standard output handle. /// + [UnsupportedOSPlatform("android")] [UnsupportedOSPlatform("ios")] [UnsupportedOSPlatform("tvos")] public static SafeFileHandle OpenStandardOutputHandle() @@ -689,6 +691,7 @@ public static SafeFileHandle OpenStandardOutputHandle() /// /// The returned handle does not own the underlying resource, so disposing it will not close the standard error handle. /// + [UnsupportedOSPlatform("android")] [UnsupportedOSPlatform("ios")] [UnsupportedOSPlatform("tvos")] public static SafeFileHandle OpenStandardErrorHandle() diff --git a/src/libraries/System.Console/src/System/ConsolePal.Android.cs b/src/libraries/System.Console/src/System/ConsolePal.Android.cs index 22123a553d33ab..58e29f3ab758bb 100644 --- a/src/libraries/System.Console/src/System/ConsolePal.Android.cs +++ b/src/libraries/System.Console/src/System/ConsolePal.Android.cs @@ -31,11 +31,11 @@ internal static void EnsureConsoleInitialized() { } public static Stream OpenStandardError() => new LogcatStream(OutputEncoding); - public static SafeFileHandle OpenStandardInputHandle() => new SafeFileHandle(0, ownsHandle: false); + public static SafeFileHandle OpenStandardInputHandle() => throw new PlatformNotSupportedException(); - public static SafeFileHandle OpenStandardOutputHandle() => new SafeFileHandle(1, ownsHandle: false); + public static SafeFileHandle OpenStandardOutputHandle() => throw new PlatformNotSupportedException(); - public static SafeFileHandle OpenStandardErrorHandle() => new SafeFileHandle(2, ownsHandle: false); + public static SafeFileHandle OpenStandardErrorHandle() => throw new PlatformNotSupportedException(); public static Encoding InputEncoding => throw new PlatformNotSupportedException(); diff --git a/src/libraries/System.Console/tests/ConsoleHandles.cs b/src/libraries/System.Console/tests/ConsoleHandles.cs index 89e76b899179a2..b09c94b71497c8 100644 --- a/src/libraries/System.Console/tests/ConsoleHandles.cs +++ b/src/libraries/System.Console/tests/ConsoleHandles.cs @@ -12,7 +12,7 @@ namespace System.Tests public partial class ConsoleTests { [Fact] - [PlatformSpecific(TestPlatforms.Any & ~TestPlatforms.Browser & ~TestPlatforms.iOS & ~TestPlatforms.tvOS)] + [PlatformSpecific(TestPlatforms.Any & ~TestPlatforms.Browser & ~TestPlatforms.iOS & ~TestPlatforms.tvOS & ~TestPlatforms.Android)] public void OpenStandardInputHandle_ReturnsValidHandle() { using SafeFileHandle inputHandle = Console.OpenStandardInputHandle(); @@ -22,7 +22,7 @@ public void OpenStandardInputHandle_ReturnsValidHandle() } [Fact] - [PlatformSpecific(TestPlatforms.Any & ~TestPlatforms.iOS & ~TestPlatforms.tvOS)] + [PlatformSpecific(TestPlatforms.Any & ~TestPlatforms.iOS & ~TestPlatforms.tvOS & ~TestPlatforms.Android)] public void OpenStandardOutputHandle_ReturnsValidHandle() { using SafeFileHandle outputHandle = Console.OpenStandardOutputHandle(); @@ -32,7 +32,7 @@ public void OpenStandardOutputHandle_ReturnsValidHandle() } [Fact] - [PlatformSpecific(TestPlatforms.Any & ~TestPlatforms.iOS & ~TestPlatforms.tvOS)] + [PlatformSpecific(TestPlatforms.Any & ~TestPlatforms.iOS & ~TestPlatforms.tvOS & ~TestPlatforms.Android)] public void OpenStandardErrorHandle_ReturnsValidHandle() { using SafeFileHandle errorHandle = Console.OpenStandardErrorHandle(); @@ -42,7 +42,7 @@ public void OpenStandardErrorHandle_ReturnsValidHandle() } [Fact] - [PlatformSpecific(TestPlatforms.Any & ~TestPlatforms.Browser & ~TestPlatforms.iOS & ~TestPlatforms.tvOS)] + [PlatformSpecific(TestPlatforms.Any & ~TestPlatforms.Browser & ~TestPlatforms.iOS & ~TestPlatforms.tvOS & ~TestPlatforms.Android)] public void OpenStandardHandles_DoNotOwnHandle() { SafeFileHandle inputHandle = Console.OpenStandardInputHandle(); @@ -87,21 +87,21 @@ public void OpenStandardHandles_CanBeUsedWithStream() } [Fact] - [PlatformSpecific(TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.Browser)] + [PlatformSpecific(TestPlatforms.Android | TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.Browser)] public void OpenStandardInputHandle_ThrowsOnUnsupportedPlatforms() { Assert.Throws(() => Console.OpenStandardInputHandle()); } [Fact] - [PlatformSpecific(TestPlatforms.iOS | TestPlatforms.tvOS)] + [PlatformSpecific(TestPlatforms.Android | TestPlatforms.iOS | TestPlatforms.tvOS)] public void OpenStandardOutputHandle_ThrowsOnUnsupportedPlatforms() { Assert.Throws(() => Console.OpenStandardOutputHandle()); } [Fact] - [PlatformSpecific(TestPlatforms.iOS | TestPlatforms.tvOS)] + [PlatformSpecific(TestPlatforms.Android | TestPlatforms.iOS | TestPlatforms.tvOS)] public void OpenStandardErrorHandle_ThrowsOnUnsupportedPlatforms() { Assert.Throws(() => Console.OpenStandardErrorHandle()); diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs index 1199f495ee37ac..cef30af43bccc0 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs @@ -1322,6 +1322,10 @@ public bool Start() { SafeFileHandle.CreateAnonymousPipe(out childInputPipeHandle, out parentInputPipeHandle); } + else if (OperatingSystem.IsAndroid()) + { + childInputPipeHandle = new SafeFileHandle(0, ownsHandle: false); + } else { childInputPipeHandle = Console.OpenStandardInputHandle(); @@ -1331,6 +1335,10 @@ public bool Start() { SafeFileHandle.CreateAnonymousPipe(out parentOutputPipeHandle, out childOutputPipeHandle, asyncRead: OperatingSystem.IsWindows()); } + else if (OperatingSystem.IsAndroid()) + { + childOutputPipeHandle = new SafeFileHandle(1, ownsHandle: false); + } else { childOutputPipeHandle = Console.OpenStandardOutputHandle(); @@ -1340,6 +1348,10 @@ public bool Start() { SafeFileHandle.CreateAnonymousPipe(out parentErrorPipeHandle, out childErrorPipeHandle, asyncRead: OperatingSystem.IsWindows()); } + else if (OperatingSystem.IsAndroid()) + { + childErrorPipeHandle = new SafeFileHandle(2, ownsHandle: false); + } else { childErrorPipeHandle = Console.OpenStandardErrorHandle(); From b362a72ad34f6d461f675b1b84e2a3ad691e6118 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 20 Mar 2026 17:21:03 +0100 Subject: [PATCH 13/22] address code review feedback: don't dup 0/1/2 on Unix --- .../src/System/Diagnostics/Process.cs | 18 +++++++++--------- src/native/libs/System.Native/pal_process.c | 6 +++--- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs index cef30af43bccc0..d377f83e84576f 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs @@ -1322,39 +1322,39 @@ public bool Start() { SafeFileHandle.CreateAnonymousPipe(out childInputPipeHandle, out parentInputPipeHandle); } - else if (OperatingSystem.IsAndroid()) + else if (OperatingSystem.IsWindows()) { - childInputPipeHandle = new SafeFileHandle(0, ownsHandle: false); + childInputPipeHandle = Console.OpenStandardInputHandle(); } else { - childInputPipeHandle = Console.OpenStandardInputHandle(); + childInputPipeHandle = new SafeFileHandle(0, ownsHandle: false); } if (startInfo.RedirectStandardOutput) { SafeFileHandle.CreateAnonymousPipe(out parentOutputPipeHandle, out childOutputPipeHandle, asyncRead: OperatingSystem.IsWindows()); } - else if (OperatingSystem.IsAndroid()) + else if (OperatingSystem.IsWindows()) { - childOutputPipeHandle = new SafeFileHandle(1, ownsHandle: false); + childOutputPipeHandle = Console.OpenStandardOutputHandle(); } else { - childOutputPipeHandle = Console.OpenStandardOutputHandle(); + childOutputPipeHandle = new SafeFileHandle(1, ownsHandle: false); } if (startInfo.RedirectStandardError) { SafeFileHandle.CreateAnonymousPipe(out parentErrorPipeHandle, out childErrorPipeHandle, asyncRead: OperatingSystem.IsWindows()); } - else if (OperatingSystem.IsAndroid()) + else if (OperatingSystem.IsWindows()) { - childErrorPipeHandle = new SafeFileHandle(2, ownsHandle: false); + childErrorPipeHandle = Console.OpenStandardErrorHandle(); } else { - childErrorPipeHandle = Console.OpenStandardErrorHandle(); + childErrorPipeHandle = new SafeFileHandle(2, ownsHandle: false); } } finally diff --git a/src/native/libs/System.Native/pal_process.c b/src/native/libs/System.Native/pal_process.c index 2c77fd87fd65c2..71f74f0849c59a 100644 --- a/src/native/libs/System.Native/pal_process.c +++ b/src/native/libs/System.Native/pal_process.c @@ -358,9 +358,9 @@ int32_t SystemNative_ForkAndExecProcess(const char* filename, // The provided file handles might have CLOEXEC enabled, // but dup2 doesn't copy the file descriptor flags, // so the new file descriptors won't have CLOEXEC enabled. - if (Dup2WithInterruptedRetry(stdinFd, STDIN_FILENO) == -1 || - Dup2WithInterruptedRetry(stdoutFd, STDOUT_FILENO) == -1 || - Dup2WithInterruptedRetry(stderrFd, STDERR_FILENO) == -1) + if ((stdinFd != 0 && Dup2WithInterruptedRetry(stdinFd, STDIN_FILENO) == -1) || + (stdoutFd != 1 && Dup2WithInterruptedRetry(stdoutFd, STDOUT_FILENO) == -1) || + (stderrFd != 2 && Dup2WithInterruptedRetry(stderrFd, STDERR_FILENO) == -1)) { ExitChild(waitForChildToExecPipe[WRITE_END_OF_PIPE], errno); } From 419d7abccae1616d0ddea11bcc964b5a0b6a538c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Mar 2026 17:30:43 +0000 Subject: [PATCH 14/22] Add StandardInput/Output/Error SafeFileHandle properties to ProcessStartInfo Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/CompatibilitySuppressions.xml | 199 +++++++++++++ .../ref/System.Diagnostics.Process.cs | 3 + .../src/Resources/Strings.resx | 6 + .../src/System/Diagnostics/Process.cs | 55 +++- .../System/Diagnostics/ProcessStartInfo.cs | 22 ++ .../tests/ProcessHandlesTests.cs | 281 ++++++++++++++++++ .../System.Diagnostics.Process.Tests.csproj | 1 + .../Attributes/JsonNamingPolicyAttribute.cs | 72 +++++ 8 files changed, 633 insertions(+), 6 deletions(-) create mode 100644 src/libraries/Microsoft.Bcl.Cryptography/src/CompatibilitySuppressions.xml create mode 100644 src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.cs create mode 100644 src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Attributes/JsonNamingPolicyAttribute.cs diff --git a/src/libraries/Microsoft.Bcl.Cryptography/src/CompatibilitySuppressions.xml b/src/libraries/Microsoft.Bcl.Cryptography/src/CompatibilitySuppressions.xml new file mode 100644 index 00000000000000..c5a74a9d8396cf --- /dev/null +++ b/src/libraries/Microsoft.Bcl.Cryptography/src/CompatibilitySuppressions.xml @@ -0,0 +1,199 @@ + + + + + CP0014 + M:System.Security.Cryptography.X509Certificates.X509CertificateKeyAccessors.CopyWithPrivateKey(System.Security.Cryptography.X509Certificates.X509Certificate2,System.Security.Cryptography.MLKem):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net10.0/Microsoft.Bcl.Cryptography.dll + true + + + CP0014 + M:System.Security.Cryptography.X509Certificates.X509CertificateKeyAccessors.GetMLKemPrivateKey(System.Security.Cryptography.X509Certificates.X509Certificate2):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net10.0/Microsoft.Bcl.Cryptography.dll + true + + + CP0014 + M:System.Security.Cryptography.X509Certificates.X509CertificateKeyAccessors.GetMLKemPublicKey(System.Security.Cryptography.X509Certificates.X509Certificate2):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net10.0/Microsoft.Bcl.Cryptography.dll + true + + + CP0014 + M:System.Security.Cryptography.MLKem.ExportEncryptedPkcs8PrivateKey(System.ReadOnlySpan{System.Byte},System.Security.Cryptography.PbeParameters):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net11.0/Microsoft.Bcl.Cryptography.dll + + + CP0014 + M:System.Security.Cryptography.MLKem.ExportEncryptedPkcs8PrivateKey(System.ReadOnlySpan{System.Char},System.Security.Cryptography.PbeParameters):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net11.0/Microsoft.Bcl.Cryptography.dll + + + CP0014 + M:System.Security.Cryptography.MLKem.ExportEncryptedPkcs8PrivateKey(System.String,System.Security.Cryptography.PbeParameters):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net11.0/Microsoft.Bcl.Cryptography.dll + + + CP0014 + M:System.Security.Cryptography.MLKem.ExportEncryptedPkcs8PrivateKeyPem(System.ReadOnlySpan{System.Byte},System.Security.Cryptography.PbeParameters):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net11.0/Microsoft.Bcl.Cryptography.dll + + + CP0014 + M:System.Security.Cryptography.MLKem.ExportEncryptedPkcs8PrivateKeyPem(System.ReadOnlySpan{System.Char},System.Security.Cryptography.PbeParameters):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net11.0/Microsoft.Bcl.Cryptography.dll + + + CP0014 + M:System.Security.Cryptography.MLKem.ExportEncryptedPkcs8PrivateKeyPem(System.String,System.Security.Cryptography.PbeParameters):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net11.0/Microsoft.Bcl.Cryptography.dll + + + CP0014 + M:System.Security.Cryptography.MLKem.ExportPkcs8PrivateKey:[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net11.0/Microsoft.Bcl.Cryptography.dll + + + CP0014 + M:System.Security.Cryptography.MLKem.ExportPkcs8PrivateKeyPem:[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net11.0/Microsoft.Bcl.Cryptography.dll + + + CP0014 + M:System.Security.Cryptography.MLKem.ExportSubjectPublicKeyInfo:[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net11.0/Microsoft.Bcl.Cryptography.dll + + + CP0014 + M:System.Security.Cryptography.MLKem.ExportSubjectPublicKeyInfoPem:[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net11.0/Microsoft.Bcl.Cryptography.dll + + + CP0014 + M:System.Security.Cryptography.MLKem.ImportEncryptedPkcs8PrivateKey(System.ReadOnlySpan{System.Byte},System.ReadOnlySpan{System.Byte}):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net11.0/Microsoft.Bcl.Cryptography.dll + + + CP0014 + M:System.Security.Cryptography.MLKem.ImportEncryptedPkcs8PrivateKey(System.ReadOnlySpan{System.Char},System.ReadOnlySpan{System.Byte}):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net11.0/Microsoft.Bcl.Cryptography.dll + + + CP0014 + M:System.Security.Cryptography.MLKem.ImportEncryptedPkcs8PrivateKey(System.String,System.Byte[]):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net11.0/Microsoft.Bcl.Cryptography.dll + + + CP0014 + M:System.Security.Cryptography.MLKem.ImportFromEncryptedPem(System.ReadOnlySpan{System.Char},System.ReadOnlySpan{System.Byte}):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net11.0/Microsoft.Bcl.Cryptography.dll + + + CP0014 + M:System.Security.Cryptography.MLKem.ImportFromEncryptedPem(System.ReadOnlySpan{System.Char},System.ReadOnlySpan{System.Char}):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net11.0/Microsoft.Bcl.Cryptography.dll + + + CP0014 + M:System.Security.Cryptography.MLKem.ImportFromEncryptedPem(System.String,System.Byte[]):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net11.0/Microsoft.Bcl.Cryptography.dll + + + CP0014 + M:System.Security.Cryptography.MLKem.ImportFromEncryptedPem(System.String,System.String):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net11.0/Microsoft.Bcl.Cryptography.dll + + + CP0014 + M:System.Security.Cryptography.MLKem.ImportFromPem(System.ReadOnlySpan{System.Char}):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net11.0/Microsoft.Bcl.Cryptography.dll + + + CP0014 + M:System.Security.Cryptography.MLKem.ImportFromPem(System.String):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net11.0/Microsoft.Bcl.Cryptography.dll + + + CP0014 + M:System.Security.Cryptography.MLKem.ImportPkcs8PrivateKey(System.Byte[]):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net11.0/Microsoft.Bcl.Cryptography.dll + + + CP0014 + M:System.Security.Cryptography.MLKem.ImportPkcs8PrivateKey(System.ReadOnlySpan{System.Byte}):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net11.0/Microsoft.Bcl.Cryptography.dll + + + CP0014 + M:System.Security.Cryptography.MLKem.ImportSubjectPublicKeyInfo(System.Byte[]):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net11.0/Microsoft.Bcl.Cryptography.dll + + + CP0014 + M:System.Security.Cryptography.MLKem.ImportSubjectPublicKeyInfo(System.ReadOnlySpan{System.Byte}):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net11.0/Microsoft.Bcl.Cryptography.dll + + + CP0014 + M:System.Security.Cryptography.MLKem.TryExportEncryptedPkcs8PrivateKey(System.ReadOnlySpan{System.Byte},System.Security.Cryptography.PbeParameters,System.Span{System.Byte},System.Int32@):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net11.0/Microsoft.Bcl.Cryptography.dll + + + CP0014 + M:System.Security.Cryptography.MLKem.TryExportEncryptedPkcs8PrivateKey(System.ReadOnlySpan{System.Char},System.Security.Cryptography.PbeParameters,System.Span{System.Byte},System.Int32@):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net11.0/Microsoft.Bcl.Cryptography.dll + + + CP0014 + M:System.Security.Cryptography.MLKem.TryExportEncryptedPkcs8PrivateKey(System.String,System.Security.Cryptography.PbeParameters,System.Span{System.Byte},System.Int32@):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net11.0/Microsoft.Bcl.Cryptography.dll + + + CP0014 + M:System.Security.Cryptography.MLKem.TryExportPkcs8PrivateKey(System.Span{System.Byte},System.Int32@):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net11.0/Microsoft.Bcl.Cryptography.dll + + + CP0014 + M:System.Security.Cryptography.MLKem.TryExportPkcs8PrivateKeyCore(System.Span{System.Byte},System.Int32@):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net11.0/Microsoft.Bcl.Cryptography.dll + + + CP0014 + M:System.Security.Cryptography.MLKem.TryExportSubjectPublicKeyInfo(System.Span{System.Byte},System.Int32@):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] + lib/net10.0/Microsoft.Bcl.Cryptography.dll + lib/net11.0/Microsoft.Bcl.Cryptography.dll + + \ No newline at end of file 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 e05e8c2867db5f..bb4d56b8e102be 100644 --- a/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs +++ b/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs @@ -253,6 +253,9 @@ public ProcessStartInfo(string fileName, System.Collections.Generic.IEnumerable< public bool RedirectStandardInput { get { throw null; } set { } } public bool RedirectStandardOutput { get { throw null; } set { } } public System.Text.Encoding? StandardErrorEncoding { get { throw null; } set { } } + public Microsoft.Win32.SafeHandles.SafeFileHandle? StandardError { get { throw null; } set { } } + public Microsoft.Win32.SafeHandles.SafeFileHandle? StandardInput { get { throw null; } set { } } + public Microsoft.Win32.SafeHandles.SafeFileHandle? StandardOutput { get { throw null; } set { } } public System.Text.Encoding? StandardInputEncoding { get { throw null; } set { } } public System.Text.Encoding? StandardOutputEncoding { get { throw null; } set { } } [System.Diagnostics.CodeAnalysis.AllowNullAttribute] diff --git a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx index 67d05840e9796b..3677016da3fdf5 100644 --- a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx +++ b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx @@ -210,6 +210,12 @@ The Process object must have the UseShellExecute property set to false in order to redirect IO streams. + + The StandardInput, StandardOutput, and StandardError handle properties cannot be used together with RedirectStandardInput, RedirectStandardOutput, and RedirectStandardError. + + + The Process object must have the UseShellExecute property set to false in order to use StandardInput, StandardOutput, or StandardError handles. + The FileName property should not be a directory unless UseShellExecute is set. diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs index d377f83e84576f..b562690dc710d3 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs @@ -1284,6 +1284,27 @@ public bool Start() throw new InvalidOperationException(SR.CantRedirectStreams); } + bool anyHandle = startInfo.StandardInput is not null || startInfo.StandardOutput is not null || startInfo.StandardError is not null; + if (anyHandle) + { + if (startInfo.UseShellExecute) + { + throw new InvalidOperationException(SR.CantUseHandlesWithShellExecute); + } + if (startInfo.StandardInput is not null && startInfo.RedirectStandardInput) + { + throw new InvalidOperationException(SR.CantSetHandleAndRedirect); + } + if (startInfo.StandardOutput is not null && startInfo.RedirectStandardOutput) + { + throw new InvalidOperationException(SR.CantSetHandleAndRedirect); + } + if (startInfo.StandardError is not null && startInfo.RedirectStandardError) + { + throw new InvalidOperationException(SR.CantSetHandleAndRedirect); + } + } + //Cannot start a new process and store its handle if the object has been disposed, since finalization has been suppressed. CheckDisposed(); @@ -1318,7 +1339,11 @@ public bool Start() try { - if (startInfo.RedirectStandardInput) + if (startInfo.StandardInput is not null) + { + childInputPipeHandle = startInfo.StandardInput; + } + else if (startInfo.RedirectStandardInput) { SafeFileHandle.CreateAnonymousPipe(out childInputPipeHandle, out parentInputPipeHandle); } @@ -1331,7 +1356,11 @@ public bool Start() childInputPipeHandle = new SafeFileHandle(0, ownsHandle: false); } - if (startInfo.RedirectStandardOutput) + if (startInfo.StandardOutput is not null) + { + childOutputPipeHandle = startInfo.StandardOutput; + } + else if (startInfo.RedirectStandardOutput) { SafeFileHandle.CreateAnonymousPipe(out parentOutputPipeHandle, out childOutputPipeHandle, asyncRead: OperatingSystem.IsWindows()); } @@ -1344,7 +1373,11 @@ public bool Start() childOutputPipeHandle = new SafeFileHandle(1, ownsHandle: false); } - if (startInfo.RedirectStandardError) + if (startInfo.StandardError is not null) + { + childErrorPipeHandle = startInfo.StandardError; + } + else if (startInfo.RedirectStandardError) { SafeFileHandle.CreateAnonymousPipe(out parentErrorPipeHandle, out childErrorPipeHandle, asyncRead: OperatingSystem.IsWindows()); } @@ -1385,9 +1418,19 @@ public bool Start() // process will not receive EOF when the child process closes its handles. // It's OK to do it for handles returned by Console.OpenStandard*Handle APIs, // because these handles are not owned and won't be closed by Dispose. - childInputPipeHandle?.Dispose(); - childOutputPipeHandle?.Dispose(); - childErrorPipeHandle?.Dispose(); + // We must NOT close handles that were passed in by the caller via StartInfo.StandardInput/Output/Error. + if (startInfo.StandardInput is null) + { + childInputPipeHandle?.Dispose(); + } + if (startInfo.StandardOutput is null) + { + childOutputPipeHandle?.Dispose(); + } + if (startInfo.StandardError is null) + { + childErrorPipeHandle?.Dispose(); + } } if (startInfo.RedirectStandardInput) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs index 0f4fc0a0cb72c1..c1805cc49bfac1 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs @@ -8,6 +8,7 @@ using System.ComponentModel; using System.Diagnostics.CodeAnalysis; using System.Text; +using Microsoft.Win32.SafeHandles; namespace System.Diagnostics { @@ -117,6 +118,27 @@ public string Arguments public bool RedirectStandardOutput { get; set; } public bool RedirectStandardError { get; set; } + /// + /// Gets or sets a that will be used as the standard input of the child process. + /// When set, the handle is passed directly to the child process and must be . + /// + /// A to use as the standard input handle of the child process, or to use the default behavior. + public SafeFileHandle? StandardInput { get; set; } + + /// + /// Gets or sets a that will be used as the standard output of the child process. + /// When set, the handle is passed directly to the child process and must be . + /// + /// A to use as the standard output handle of the child process, or to use the default behavior. + public SafeFileHandle? StandardOutput { get; set; } + + /// + /// Gets or sets a that will be used as the standard error of the child process. + /// When set, the handle is passed directly to the child process and must be . + /// + /// A to use as the standard error handle of the child process, or to use the default behavior. + public SafeFileHandle? StandardError { get; set; } + public Encoding? StandardInputEncoding { get; set; } public Encoding? StandardErrorEncoding { get; set; } diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.cs new file mode 100644 index 00000000000000..483ad7e9a0eefc --- /dev/null +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.cs @@ -0,0 +1,281 @@ +// 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 System.Threading.Tasks; +using Microsoft.Win32.SafeHandles; +using Xunit; + +namespace System.Diagnostics.Tests +{ + public class ProcessHandlesTests : ProcessTestBase + { + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task CanRedirectOutputToPipe(bool readAsync) + { + ProcessStartInfo startInfo = OperatingSystem.IsWindows() + ? new("cmd") { ArgumentList = { "/c", "echo Test" } } + : new("sh") { ArgumentList = { "-c", "echo 'Test'" } }; + + SafeFileHandle.CreateAnonymousPipe(out SafeFileHandle readPipe, out SafeFileHandle writePipe, asyncRead: readAsync); + + startInfo.StandardOutput = writePipe; + + using (readPipe) + using (writePipe) + { + using Process process = Process.Start(startInfo)!; + + // Close the parent copy of the child handle, so the pipe will signal EOF when the child exits + writePipe.Close(); + + using FileStream fileStream = new(readPipe, FileAccess.Read, bufferSize: 1, isAsync: readAsync); + using StreamReader streamReader = new(fileStream); + + string output = readAsync + ? await streamReader.ReadToEndAsync() + : streamReader.ReadToEnd(); + + Assert.Equal(OperatingSystem.IsWindows() ? "Test\r\n" : "Test\n", output); + + process.WaitForExit(); + Assert.Equal(0, process.ExitCode); + } + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task CanRedirectOutputAndErrorToDifferentPipes(bool readAsync) + { + ProcessStartInfo startInfo = OperatingSystem.IsWindows() + ? new("cmd") { ArgumentList = { "/c", "echo Hello from stdout && echo Error from stderr 1>&2" } } + : new("sh") { ArgumentList = { "-c", "echo 'Hello from stdout' && echo 'Error from stderr' >&2" } }; + + SafeFileHandle.CreateAnonymousPipe(out SafeFileHandle outputRead, out SafeFileHandle outputWrite, asyncRead: readAsync); + SafeFileHandle.CreateAnonymousPipe(out SafeFileHandle errorRead, out SafeFileHandle errorWrite, asyncRead: readAsync); + + startInfo.StandardOutput = outputWrite; + startInfo.StandardError = errorWrite; + + using (outputRead) + using (outputWrite) + using (errorRead) + using (errorWrite) + { + using Process process = Process.Start(startInfo)!; + + // Close the parent copy of the child handles, so the pipes will signal EOF when the child exits + outputWrite.Close(); + errorWrite.Close(); + + using FileStream outputStream = new(outputRead, FileAccess.Read, bufferSize: 1, isAsync: readAsync); + using FileStream errorStream = new(errorRead, FileAccess.Read, bufferSize: 1, isAsync: readAsync); + using StreamReader outputReader = new(outputStream); + using StreamReader errorReader = new(errorStream); + + Task outputTask = outputReader.ReadToEndAsync(); + Task errorTask = errorReader.ReadToEndAsync(); + + Assert.Equal(OperatingSystem.IsWindows() ? "Hello from stdout \r\n" : "Hello from stdout\n", await outputTask); + Assert.Equal(OperatingSystem.IsWindows() ? "Error from stderr \r\n" : "Error from stderr\n", await errorTask); + + process.WaitForExit(); + Assert.Equal(0, process.ExitCode); + } + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task CanRedirectToInheritedHandles(bool useAsync) + { + ProcessStartInfo startInfo = OperatingSystem.IsWindows() + ? new("cmd") { ArgumentList = { "/c", "exit 42" } } + : new("sh") { ArgumentList = { "-c", "exit 42" } }; + + using SafeFileHandle inputHandle = Console.OpenStandardInputHandle(); + using SafeFileHandle outputHandle = Console.OpenStandardOutputHandle(); + using SafeFileHandle errorHandle = Console.OpenStandardErrorHandle(); + + startInfo.StandardInput = inputHandle; + startInfo.StandardOutput = outputHandle; + startInfo.StandardError = errorHandle; + + using Process process = Process.Start(startInfo)!; + + if (useAsync) + { + await process.WaitForExitAsync(); + } + else + { + process.WaitForExit(); + } + + Assert.Equal(42, process.ExitCode); + } + + [Fact] + public async Task CanImplementPiping() + { + SafeFileHandle.CreateAnonymousPipe(out SafeFileHandle readPipe, out SafeFileHandle writePipe); + string? tempFile = null; + + try + { + tempFile = Path.GetTempFileName(); + + ProcessStartInfo producerInfo; + ProcessStartInfo consumerInfo; + string expectedOutput; + + if (OperatingSystem.IsWindows()) + { + producerInfo = new("cmd") + { + ArgumentList = { "/c", "echo hello world & echo test line & echo another test" } + }; + consumerInfo = new("findstr") + { + ArgumentList = { "test" } + }; + // findstr adds a trailing space on Windows + expectedOutput = "test line \nanother test\n"; + } + else + { + producerInfo = new("sh") + { + ArgumentList = { "-c", "printf 'hello world\\ntest line\\nanother test\\n'" } + }; + consumerInfo = new("grep") + { + ArgumentList = { "test" } + }; + expectedOutput = "test line\nanother test\n"; + } + + producerInfo.StandardOutput = writePipe; + + using SafeFileHandle outputHandle = File.OpenHandle(tempFile, FileMode.Create, FileAccess.Write, FileShare.ReadWrite); + + consumerInfo.StandardInput = readPipe; + consumerInfo.StandardOutput = outputHandle; + + using Process producer = Process.Start(producerInfo)!; + + writePipe.Close(); // close the parent copy of child handle + + using Process consumer = Process.Start(consumerInfo)!; + + readPipe.Close(); // close the parent copy of child handle + + producer.WaitForExit(); + consumer.WaitForExit(); + + string result = File.ReadAllText(tempFile); + Assert.Equal(expectedOutput, result, ignoreLineEndingDifferences: true); + } + finally + { + readPipe.Dispose(); + writePipe.Dispose(); + + if (tempFile is not null && File.Exists(tempFile)) + { + File.Delete(tempFile); + } + } + } + + [Fact] + public void StandardInput_WithRedirectStandardInput_Throws() + { + ProcessStartInfo startInfo = new("cmd") + { + RedirectStandardInput = true, + StandardInput = Console.OpenStandardInputHandle() + }; + + using (startInfo.StandardInput) + { + Assert.Throws(() => Process.Start(startInfo)); + } + } + + [Fact] + public void StandardOutput_WithRedirectStandardOutput_Throws() + { + ProcessStartInfo startInfo = new("cmd") + { + RedirectStandardOutput = true, + StandardOutput = Console.OpenStandardOutputHandle() + }; + + using (startInfo.StandardOutput) + { + Assert.Throws(() => Process.Start(startInfo)); + } + } + + [Fact] + public void StandardError_WithRedirectStandardError_Throws() + { + ProcessStartInfo startInfo = new("cmd") + { + RedirectStandardError = true, + StandardError = Console.OpenStandardErrorHandle() + }; + + using (startInfo.StandardError) + { + Assert.Throws(() => Process.Start(startInfo)); + } + } + + [Fact] + public void StandardHandles_WithUseShellExecute_Throws() + { + ProcessStartInfo startInfo = new("cmd") + { + UseShellExecute = true, + StandardOutput = Console.OpenStandardOutputHandle() + }; + + using (startInfo.StandardOutput) + { + Assert.Throws(() => Process.Start(startInfo)); + } + } + + [Fact] + public void StandardHandles_DefaultIsNull() + { + ProcessStartInfo startInfo = new("cmd"); + Assert.Null(startInfo.StandardInput); + Assert.Null(startInfo.StandardOutput); + Assert.Null(startInfo.StandardError); + } + + [Fact] + public void StandardHandles_CanSetAndGet() + { + using SafeFileHandle handle = Console.OpenStandardOutputHandle(); + + ProcessStartInfo startInfo = new("cmd") + { + StandardInput = handle, + StandardOutput = handle, + StandardError = handle + }; + + Assert.Same(handle, startInfo.StandardInput); + Assert.Same(handle, startInfo.StandardOutput); + Assert.Same(handle, startInfo.StandardError); + } + } +} 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 8b99686373df90..ae97fec4051f24 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 @@ -26,6 +26,7 @@ + diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Attributes/JsonNamingPolicyAttribute.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Attributes/JsonNamingPolicyAttribute.cs new file mode 100644 index 00000000000000..da0b2e246876c4 --- /dev/null +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Attributes/JsonNamingPolicyAttribute.cs @@ -0,0 +1,72 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Text.Json.Serialization +{ + /// + /// When placed on a type, property, or field, indicates what + /// should be used to convert property names. + /// + /// + /// When placed on a property or field, the naming policy specified by this attribute + /// takes precedence over the type-level attribute and the . + /// When placed on a type, the naming policy specified by this attribute takes precedence + /// over the . + /// The takes precedence over this attribute. + /// + [AttributeUsage( + AttributeTargets.Class | AttributeTargets.Struct | AttributeTargets.Interface | + AttributeTargets.Property | AttributeTargets.Field, + AllowMultiple = false)] + public class JsonNamingPolicyAttribute : JsonAttribute + { + /// + /// Initializes a new instance of + /// with the specified known naming policy. + /// + /// The known naming policy to use for name conversion. + /// + /// The specified is not a valid known naming policy value. + /// + public JsonNamingPolicyAttribute(JsonKnownNamingPolicy namingPolicy) + { + NamingPolicy = ResolveNamingPolicy(namingPolicy); + } + + /// + /// Initializes a new instance of + /// with a custom naming policy. + /// + /// The naming policy to use for name conversion. + /// + /// is . + /// + protected JsonNamingPolicyAttribute(JsonNamingPolicy namingPolicy) + { + if (namingPolicy is null) + { + ThrowHelper.ThrowArgumentNullException(nameof(namingPolicy)); + } + + NamingPolicy = namingPolicy; + } + + /// + /// Gets the naming policy to use for name conversion. + /// + public JsonNamingPolicy NamingPolicy { get; } + + internal static JsonNamingPolicy ResolveNamingPolicy(JsonKnownNamingPolicy namingPolicy) + { + return namingPolicy switch + { + JsonKnownNamingPolicy.CamelCase => JsonNamingPolicy.CamelCase, + JsonKnownNamingPolicy.SnakeCaseLower => JsonNamingPolicy.SnakeCaseLower, + JsonKnownNamingPolicy.SnakeCaseUpper => JsonNamingPolicy.SnakeCaseUpper, + JsonKnownNamingPolicy.KebabCaseLower => JsonNamingPolicy.KebabCaseLower, + JsonKnownNamingPolicy.KebabCaseUpper => JsonNamingPolicy.KebabCaseUpper, + _ => throw new ArgumentOutOfRangeException(nameof(namingPolicy)), + }; + } + } +} From 18995de5c582205f31d3ad60464409abf8d7aa58 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Mar 2026 17:42:31 +0000 Subject: [PATCH 15/22] Remove unrelated files from commit Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../src/CompatibilitySuppressions.xml | 199 ------------------ .../Attributes/JsonNamingPolicyAttribute.cs | 72 ------- 2 files changed, 271 deletions(-) delete mode 100644 src/libraries/Microsoft.Bcl.Cryptography/src/CompatibilitySuppressions.xml delete mode 100644 src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Attributes/JsonNamingPolicyAttribute.cs diff --git a/src/libraries/Microsoft.Bcl.Cryptography/src/CompatibilitySuppressions.xml b/src/libraries/Microsoft.Bcl.Cryptography/src/CompatibilitySuppressions.xml deleted file mode 100644 index c5a74a9d8396cf..00000000000000 --- a/src/libraries/Microsoft.Bcl.Cryptography/src/CompatibilitySuppressions.xml +++ /dev/null @@ -1,199 +0,0 @@ - - - - - CP0014 - M:System.Security.Cryptography.X509Certificates.X509CertificateKeyAccessors.CopyWithPrivateKey(System.Security.Cryptography.X509Certificates.X509Certificate2,System.Security.Cryptography.MLKem):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net10.0/Microsoft.Bcl.Cryptography.dll - true - - - CP0014 - M:System.Security.Cryptography.X509Certificates.X509CertificateKeyAccessors.GetMLKemPrivateKey(System.Security.Cryptography.X509Certificates.X509Certificate2):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net10.0/Microsoft.Bcl.Cryptography.dll - true - - - CP0014 - M:System.Security.Cryptography.X509Certificates.X509CertificateKeyAccessors.GetMLKemPublicKey(System.Security.Cryptography.X509Certificates.X509Certificate2):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net10.0/Microsoft.Bcl.Cryptography.dll - true - - - CP0014 - M:System.Security.Cryptography.MLKem.ExportEncryptedPkcs8PrivateKey(System.ReadOnlySpan{System.Byte},System.Security.Cryptography.PbeParameters):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net11.0/Microsoft.Bcl.Cryptography.dll - - - CP0014 - M:System.Security.Cryptography.MLKem.ExportEncryptedPkcs8PrivateKey(System.ReadOnlySpan{System.Char},System.Security.Cryptography.PbeParameters):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net11.0/Microsoft.Bcl.Cryptography.dll - - - CP0014 - M:System.Security.Cryptography.MLKem.ExportEncryptedPkcs8PrivateKey(System.String,System.Security.Cryptography.PbeParameters):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net11.0/Microsoft.Bcl.Cryptography.dll - - - CP0014 - M:System.Security.Cryptography.MLKem.ExportEncryptedPkcs8PrivateKeyPem(System.ReadOnlySpan{System.Byte},System.Security.Cryptography.PbeParameters):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net11.0/Microsoft.Bcl.Cryptography.dll - - - CP0014 - M:System.Security.Cryptography.MLKem.ExportEncryptedPkcs8PrivateKeyPem(System.ReadOnlySpan{System.Char},System.Security.Cryptography.PbeParameters):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net11.0/Microsoft.Bcl.Cryptography.dll - - - CP0014 - M:System.Security.Cryptography.MLKem.ExportEncryptedPkcs8PrivateKeyPem(System.String,System.Security.Cryptography.PbeParameters):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net11.0/Microsoft.Bcl.Cryptography.dll - - - CP0014 - M:System.Security.Cryptography.MLKem.ExportPkcs8PrivateKey:[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net11.0/Microsoft.Bcl.Cryptography.dll - - - CP0014 - M:System.Security.Cryptography.MLKem.ExportPkcs8PrivateKeyPem:[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net11.0/Microsoft.Bcl.Cryptography.dll - - - CP0014 - M:System.Security.Cryptography.MLKem.ExportSubjectPublicKeyInfo:[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net11.0/Microsoft.Bcl.Cryptography.dll - - - CP0014 - M:System.Security.Cryptography.MLKem.ExportSubjectPublicKeyInfoPem:[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net11.0/Microsoft.Bcl.Cryptography.dll - - - CP0014 - M:System.Security.Cryptography.MLKem.ImportEncryptedPkcs8PrivateKey(System.ReadOnlySpan{System.Byte},System.ReadOnlySpan{System.Byte}):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net11.0/Microsoft.Bcl.Cryptography.dll - - - CP0014 - M:System.Security.Cryptography.MLKem.ImportEncryptedPkcs8PrivateKey(System.ReadOnlySpan{System.Char},System.ReadOnlySpan{System.Byte}):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net11.0/Microsoft.Bcl.Cryptography.dll - - - CP0014 - M:System.Security.Cryptography.MLKem.ImportEncryptedPkcs8PrivateKey(System.String,System.Byte[]):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net11.0/Microsoft.Bcl.Cryptography.dll - - - CP0014 - M:System.Security.Cryptography.MLKem.ImportFromEncryptedPem(System.ReadOnlySpan{System.Char},System.ReadOnlySpan{System.Byte}):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net11.0/Microsoft.Bcl.Cryptography.dll - - - CP0014 - M:System.Security.Cryptography.MLKem.ImportFromEncryptedPem(System.ReadOnlySpan{System.Char},System.ReadOnlySpan{System.Char}):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net11.0/Microsoft.Bcl.Cryptography.dll - - - CP0014 - M:System.Security.Cryptography.MLKem.ImportFromEncryptedPem(System.String,System.Byte[]):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net11.0/Microsoft.Bcl.Cryptography.dll - - - CP0014 - M:System.Security.Cryptography.MLKem.ImportFromEncryptedPem(System.String,System.String):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net11.0/Microsoft.Bcl.Cryptography.dll - - - CP0014 - M:System.Security.Cryptography.MLKem.ImportFromPem(System.ReadOnlySpan{System.Char}):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net11.0/Microsoft.Bcl.Cryptography.dll - - - CP0014 - M:System.Security.Cryptography.MLKem.ImportFromPem(System.String):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net11.0/Microsoft.Bcl.Cryptography.dll - - - CP0014 - M:System.Security.Cryptography.MLKem.ImportPkcs8PrivateKey(System.Byte[]):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net11.0/Microsoft.Bcl.Cryptography.dll - - - CP0014 - M:System.Security.Cryptography.MLKem.ImportPkcs8PrivateKey(System.ReadOnlySpan{System.Byte}):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net11.0/Microsoft.Bcl.Cryptography.dll - - - CP0014 - M:System.Security.Cryptography.MLKem.ImportSubjectPublicKeyInfo(System.Byte[]):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net11.0/Microsoft.Bcl.Cryptography.dll - - - CP0014 - M:System.Security.Cryptography.MLKem.ImportSubjectPublicKeyInfo(System.ReadOnlySpan{System.Byte}):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net11.0/Microsoft.Bcl.Cryptography.dll - - - CP0014 - M:System.Security.Cryptography.MLKem.TryExportEncryptedPkcs8PrivateKey(System.ReadOnlySpan{System.Byte},System.Security.Cryptography.PbeParameters,System.Span{System.Byte},System.Int32@):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net11.0/Microsoft.Bcl.Cryptography.dll - - - CP0014 - M:System.Security.Cryptography.MLKem.TryExportEncryptedPkcs8PrivateKey(System.ReadOnlySpan{System.Char},System.Security.Cryptography.PbeParameters,System.Span{System.Byte},System.Int32@):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net11.0/Microsoft.Bcl.Cryptography.dll - - - CP0014 - M:System.Security.Cryptography.MLKem.TryExportEncryptedPkcs8PrivateKey(System.String,System.Security.Cryptography.PbeParameters,System.Span{System.Byte},System.Int32@):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net11.0/Microsoft.Bcl.Cryptography.dll - - - CP0014 - M:System.Security.Cryptography.MLKem.TryExportPkcs8PrivateKey(System.Span{System.Byte},System.Int32@):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net11.0/Microsoft.Bcl.Cryptography.dll - - - CP0014 - M:System.Security.Cryptography.MLKem.TryExportPkcs8PrivateKeyCore(System.Span{System.Byte},System.Int32@):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net11.0/Microsoft.Bcl.Cryptography.dll - - - CP0014 - M:System.Security.Cryptography.MLKem.TryExportSubjectPublicKeyInfo(System.Span{System.Byte},System.Int32@):[T:System.Diagnostics.CodeAnalysis.ExperimentalAttribute] - lib/net10.0/Microsoft.Bcl.Cryptography.dll - lib/net11.0/Microsoft.Bcl.Cryptography.dll - - \ No newline at end of file diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Attributes/JsonNamingPolicyAttribute.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Attributes/JsonNamingPolicyAttribute.cs deleted file mode 100644 index da0b2e246876c4..00000000000000 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Attributes/JsonNamingPolicyAttribute.cs +++ /dev/null @@ -1,72 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -namespace System.Text.Json.Serialization -{ - /// - /// When placed on a type, property, or field, indicates what - /// should be used to convert property names. - /// - /// - /// When placed on a property or field, the naming policy specified by this attribute - /// takes precedence over the type-level attribute and the . - /// When placed on a type, the naming policy specified by this attribute takes precedence - /// over the . - /// The takes precedence over this attribute. - /// - [AttributeUsage( - AttributeTargets.Class | AttributeTargets.Struct | AttributeTargets.Interface | - AttributeTargets.Property | AttributeTargets.Field, - AllowMultiple = false)] - public class JsonNamingPolicyAttribute : JsonAttribute - { - /// - /// Initializes a new instance of - /// with the specified known naming policy. - /// - /// The known naming policy to use for name conversion. - /// - /// The specified is not a valid known naming policy value. - /// - public JsonNamingPolicyAttribute(JsonKnownNamingPolicy namingPolicy) - { - NamingPolicy = ResolveNamingPolicy(namingPolicy); - } - - /// - /// Initializes a new instance of - /// with a custom naming policy. - /// - /// The naming policy to use for name conversion. - /// - /// is . - /// - protected JsonNamingPolicyAttribute(JsonNamingPolicy namingPolicy) - { - if (namingPolicy is null) - { - ThrowHelper.ThrowArgumentNullException(nameof(namingPolicy)); - } - - NamingPolicy = namingPolicy; - } - - /// - /// Gets the naming policy to use for name conversion. - /// - public JsonNamingPolicy NamingPolicy { get; } - - internal static JsonNamingPolicy ResolveNamingPolicy(JsonKnownNamingPolicy namingPolicy) - { - return namingPolicy switch - { - JsonKnownNamingPolicy.CamelCase => JsonNamingPolicy.CamelCase, - JsonKnownNamingPolicy.SnakeCaseLower => JsonNamingPolicy.SnakeCaseLower, - JsonKnownNamingPolicy.SnakeCaseUpper => JsonNamingPolicy.SnakeCaseUpper, - JsonKnownNamingPolicy.KebabCaseLower => JsonNamingPolicy.KebabCaseLower, - JsonKnownNamingPolicy.KebabCaseUpper => JsonNamingPolicy.KebabCaseUpper, - _ => throw new ArgumentOutOfRangeException(nameof(namingPolicy)), - }; - } - } -} From d40cf1821ae0af4a4fd350858181f9dea68f9ee9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Mar 2026 17:43:55 +0000 Subject: [PATCH 16/22] Address code review: fix pipe handle cleanup in tests Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../tests/ProcessHandlesTests.cs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.cs index 483ad7e9a0eefc..b531e39f80bb07 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.cs @@ -56,7 +56,19 @@ public async Task CanRedirectOutputAndErrorToDifferentPipes(bool readAsync) : new("sh") { ArgumentList = { "-c", "echo 'Hello from stdout' && echo 'Error from stderr' >&2" } }; SafeFileHandle.CreateAnonymousPipe(out SafeFileHandle outputRead, out SafeFileHandle outputWrite, asyncRead: readAsync); - SafeFileHandle.CreateAnonymousPipe(out SafeFileHandle errorRead, out SafeFileHandle errorWrite, asyncRead: readAsync); + + SafeFileHandle errorRead; + SafeFileHandle errorWrite; + try + { + SafeFileHandle.CreateAnonymousPipe(out errorRead, out errorWrite, asyncRead: readAsync); + } + catch + { + outputRead.Dispose(); + outputWrite.Dispose(); + throw; + } startInfo.StandardOutput = outputWrite; startInfo.StandardError = errorWrite; @@ -122,11 +134,13 @@ public async Task CanRedirectToInheritedHandles(bool useAsync) [Fact] public async Task CanImplementPiping() { - SafeFileHandle.CreateAnonymousPipe(out SafeFileHandle readPipe, out SafeFileHandle writePipe); + SafeFileHandle readPipe = null!; + SafeFileHandle writePipe = null!; string? tempFile = null; try { + SafeFileHandle.CreateAnonymousPipe(out readPipe, out writePipe); tempFile = Path.GetTempFileName(); ProcessStartInfo producerInfo; From 60844f3a4c66e69e125f9f485ef48dee1a0ce460 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Mar 2026 22:59:25 +0000 Subject: [PATCH 17/22] Address review feedback: reuse CantRedirectStreams, add LeaveHandlesOpen property, improve XML docs Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/9f61c68e-edfd-42e9-b86b-bfb9ee7d8946 --- .../ref/System.Diagnostics.Process.cs | 1 + .../src/Resources/Strings.resx | 3 -- .../src/System/Diagnostics/Process.cs | 11 ++-- .../System/Diagnostics/ProcessStartInfo.cs | 53 +++++++++++++++++++ .../tests/ProcessHandlesTests.cs | 10 ++-- 5 files changed, 63 insertions(+), 15 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 bb4d56b8e102be..3065c70da1de0f 100644 --- a/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs +++ b/src/libraries/System.Diagnostics.Process/ref/System.Diagnostics.Process.cs @@ -256,6 +256,7 @@ public ProcessStartInfo(string fileName, System.Collections.Generic.IEnumerable< public Microsoft.Win32.SafeHandles.SafeFileHandle? StandardError { get { throw null; } set { } } public Microsoft.Win32.SafeHandles.SafeFileHandle? StandardInput { get { throw null; } set { } } public Microsoft.Win32.SafeHandles.SafeFileHandle? StandardOutput { get { throw null; } set { } } + public bool LeaveHandlesOpen { get { throw null; } set { } } public System.Text.Encoding? StandardInputEncoding { get { throw null; } set { } } public System.Text.Encoding? StandardOutputEncoding { get { throw null; } set { } } [System.Diagnostics.CodeAnalysis.AllowNullAttribute] diff --git a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx index 3677016da3fdf5..95f6bb6ebf30ed 100644 --- a/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx +++ b/src/libraries/System.Diagnostics.Process/src/Resources/Strings.resx @@ -213,9 +213,6 @@ The StandardInput, StandardOutput, and StandardError handle properties cannot be used together with RedirectStandardInput, RedirectStandardOutput, and RedirectStandardError. - - The Process object must have the UseShellExecute property set to false in order to use StandardInput, StandardOutput, or StandardError handles. - The FileName property should not be a directory unless UseShellExecute is set. diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs index b562690dc710d3..1f566f2232faa4 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs @@ -1289,7 +1289,7 @@ public bool Start() { if (startInfo.UseShellExecute) { - throw new InvalidOperationException(SR.CantUseHandlesWithShellExecute); + throw new InvalidOperationException(SR.CantRedirectStreams); } if (startInfo.StandardInput is not null && startInfo.RedirectStandardInput) { @@ -1418,16 +1418,17 @@ public bool Start() // process will not receive EOF when the child process closes its handles. // It's OK to do it for handles returned by Console.OpenStandard*Handle APIs, // because these handles are not owned and won't be closed by Dispose. - // We must NOT close handles that were passed in by the caller via StartInfo.StandardInput/Output/Error. - if (startInfo.StandardInput is null) + // When LeaveHandlesOpen is true, we must NOT close handles that were passed in + // by the caller via StartInfo.StandardInput/Output/Error. + if (startInfo.StandardInput is null || !startInfo.LeaveHandlesOpen) { childInputPipeHandle?.Dispose(); } - if (startInfo.StandardOutput is null) + if (startInfo.StandardOutput is null || !startInfo.LeaveHandlesOpen) { childOutputPipeHandle?.Dispose(); } - if (startInfo.StandardError is null) + if (startInfo.StandardError is null || !startInfo.LeaveHandlesOpen) { childErrorPipeHandle?.Dispose(); } diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs index c1805cc49bfac1..4478dcaf5f0809 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs @@ -122,6 +122,20 @@ public string Arguments /// Gets or sets a that will be used as the standard input of the child process. /// When set, the handle is passed directly to the child process and must be . /// + /// + /// + /// The handle must be inheritable. Use to create a pair of + /// connected pipe handles, or use to open a file handle. + /// + /// + /// By default, will close this handle after starting the child process. + /// Set to to keep the handle open. + /// + /// + /// This property cannot be used together with + /// and requires to be . + /// + /// /// A to use as the standard input handle of the child process, or to use the default behavior. public SafeFileHandle? StandardInput { get; set; } @@ -129,6 +143,20 @@ public string Arguments /// Gets or sets a that will be used as the standard output of the child process. /// When set, the handle is passed directly to the child process and must be . /// + /// + /// + /// The handle must be inheritable. Use to create a pair of + /// connected pipe handles, or use to open a file handle. + /// + /// + /// By default, will close this handle after starting the child process. + /// Set to to keep the handle open. + /// + /// + /// This property cannot be used together with + /// and requires to be . + /// + /// /// A to use as the standard output handle of the child process, or to use the default behavior. public SafeFileHandle? StandardOutput { get; set; } @@ -136,9 +164,34 @@ public string Arguments /// Gets or sets a that will be used as the standard error of the child process. /// When set, the handle is passed directly to the child process and must be . /// + /// + /// + /// The handle must be inheritable. Use to create a pair of + /// connected pipe handles, or use to open a file handle. + /// + /// + /// By default, will close this handle after starting the child process. + /// Set to to keep the handle open. + /// + /// + /// This property cannot be used together with + /// and requires to be . + /// + /// /// A to use as the standard error handle of the child process, or to use the default behavior. public SafeFileHandle? StandardError { get; set; } + /// + /// Gets or sets a value indicating whether the , , + /// and handles should be left open after the process is started. + /// + /// + /// When (the default), the handles are closed by + /// after starting the child process. When , the caller is responsible for closing the handles. + /// + /// to leave the handles open; to close them after the process starts. The default is . + public bool LeaveHandlesOpen { get; set; } + public Encoding? StandardInputEncoding { get; set; } public Encoding? StandardErrorEncoding { get; set; } diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.cs index b531e39f80bb07..4f6a591e2c971f 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.cs @@ -109,13 +109,9 @@ public async Task CanRedirectToInheritedHandles(bool useAsync) ? new("cmd") { ArgumentList = { "/c", "exit 42" } } : new("sh") { ArgumentList = { "-c", "exit 42" } }; - using SafeFileHandle inputHandle = Console.OpenStandardInputHandle(); - using SafeFileHandle outputHandle = Console.OpenStandardOutputHandle(); - using SafeFileHandle errorHandle = Console.OpenStandardErrorHandle(); - - startInfo.StandardInput = inputHandle; - startInfo.StandardOutput = outputHandle; - startInfo.StandardError = errorHandle; + startInfo.StandardInput = Console.OpenStandardInputHandle(); + startInfo.StandardOutput = Console.OpenStandardOutputHandle(); + startInfo.StandardError = Console.OpenStandardErrorHandle(); using Process process = Process.Start(startInfo)!; From c8b87232db039a9c1e1ac078c1ec2af830ea2bcc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Mar 2026 23:51:24 +0000 Subject: [PATCH 18/22] Address feedback: handles don't need to be inheritable, mention OpenNullHandle/OpenStandard*Handle APIs, remove redundant Close() calls Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/29b43a66-dcf9-47b0-95df-28d834129305 --- .../System/Diagnostics/ProcessStartInfo.cs | 21 +++++++++++++------ .../tests/ProcessHandlesTests.cs | 7 ------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs index 4478dcaf5f0809..15c1c1f3299198 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessStartInfo.cs @@ -124,8 +124,11 @@ public string Arguments /// /// /// - /// The handle must be inheritable. Use to create a pair of - /// connected pipe handles, or use to open a file handle. + /// The handle does not need to be inheritable; the runtime will make it inheritable as needed. + /// Use to create a pair of connected pipe handles, + /// to open a file handle, + /// to discard input, + /// or to inherit the parent's standard input. /// /// /// By default, will close this handle after starting the child process. @@ -145,8 +148,11 @@ public string Arguments /// /// /// - /// The handle must be inheritable. Use to create a pair of - /// connected pipe handles, or use to open a file handle. + /// The handle does not need to be inheritable; the runtime will make it inheritable as needed. + /// Use to create a pair of connected pipe handles, + /// to open a file handle, + /// to discard output, + /// or to inherit the parent's standard output. /// /// /// By default, will close this handle after starting the child process. @@ -166,8 +172,11 @@ public string Arguments /// /// /// - /// The handle must be inheritable. Use to create a pair of - /// connected pipe handles, or use to open a file handle. + /// The handle does not need to be inheritable; the runtime will make it inheritable as needed. + /// Use to create a pair of connected pipe handles, + /// to open a file handle, + /// to discard error output, + /// or to inherit the parent's standard error. /// /// /// By default, will close this handle after starting the child process. diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.cs index 4f6a591e2c971f..032083cfdb2232 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.cs @@ -29,9 +29,6 @@ public async Task CanRedirectOutputToPipe(bool readAsync) { using Process process = Process.Start(startInfo)!; - // Close the parent copy of the child handle, so the pipe will signal EOF when the child exits - writePipe.Close(); - using FileStream fileStream = new(readPipe, FileAccess.Read, bufferSize: 1, isAsync: readAsync); using StreamReader streamReader = new(fileStream); @@ -80,10 +77,6 @@ public async Task CanRedirectOutputAndErrorToDifferentPipes(bool readAsync) { using Process process = Process.Start(startInfo)!; - // Close the parent copy of the child handles, so the pipes will signal EOF when the child exits - outputWrite.Close(); - errorWrite.Close(); - using FileStream outputStream = new(outputRead, FileAccess.Read, bufferSize: 1, isAsync: readAsync); using FileStream errorStream = new(errorRead, FileAccess.Read, bufferSize: 1, isAsync: readAsync); using StreamReader outputReader = new(outputStream); From 671858962916672f3ab93ba842f15e6c13e170ff Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 21 Mar 2026 14:26:15 +0000 Subject: [PATCH 19/22] Only create pipe handles for redirected streams, update callers to handle nullable handles Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/264e5c0b-daf2-4f32-80ec-10fdf888c120 --- .../Interop.ForkAndExecProcess.cs | 35 +++++++++++++++++-- .../src/System/Diagnostics/Process.Unix.cs | 8 ++--- .../src/System/Diagnostics/Process.Win32.cs | 4 +-- .../src/System/Diagnostics/Process.Windows.cs | 13 +++---- .../src/System/Diagnostics/Process.cs | 34 ++---------------- src/native/libs/System.Native/pal_process.c | 7 ++-- 6 files changed, 49 insertions(+), 52 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs index e03d68e0a6e90e..c595a120a4b4d4 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs @@ -15,12 +15,34 @@ internal static partial class Sys internal static unsafe int ForkAndExecProcess( string filename, string[] argv, string[] envp, string? cwd, bool setUser, uint userId, uint groupId, uint[]? groups, - out int lpChildPid, SafeFileHandle stdinFd, SafeFileHandle stdoutFd, SafeFileHandle stderrFd, bool shouldThrow = true) + out int lpChildPid, SafeFileHandle? stdinFd, SafeFileHandle? stdoutFd, SafeFileHandle? stderrFd, bool shouldThrow = true) { byte** argvPtr = null, envpPtr = null; int result = -1; + + bool stdinRefAdded = false, stdoutRefAdded = false, stderrRefAdded = false; try { + int stdinRawFd = -1, stdoutRawFd = -1, stderrRawFd = -1; + + if (stdinFd is not null) + { + stdinFd.DangerousAddRef(ref stdinRefAdded); + stdinRawFd = stdinFd.DangerousGetHandle().ToInt32(); + } + + if (stdoutFd is not null) + { + stdoutFd.DangerousAddRef(ref stdoutRefAdded); + stdoutRawFd = stdoutFd.DangerousGetHandle().ToInt32(); + } + + if (stderrFd is not null) + { + stderrFd.DangerousAddRef(ref stderrRefAdded); + stderrRawFd = stderrFd.DangerousGetHandle().ToInt32(); + } + AllocNullTerminatedArray(argv, ref argvPtr); AllocNullTerminatedArray(envp, ref envpPtr); fixed (uint* pGroups = groups) @@ -28,7 +50,7 @@ internal static unsafe int ForkAndExecProcess( result = ForkAndExecProcess( filename, argvPtr, envpPtr, cwd, setUser ? 1 : 0, userId, groupId, pGroups, groups?.Length ?? 0, - out lpChildPid, stdinFd, stdoutFd, stderrFd); + out lpChildPid, stdinRawFd, stdoutRawFd, stderrRawFd); } return result == 0 ? 0 : Marshal.GetLastPInvokeError(); } @@ -36,6 +58,13 @@ internal static unsafe int ForkAndExecProcess( { FreeArray(envpPtr, envp.Length); FreeArray(argvPtr, argv.Length); + + if (stdinRefAdded) + stdinFd!.DangerousRelease(); + if (stdoutRefAdded) + stdoutFd!.DangerousRelease(); + if (stderrRefAdded) + stderrFd!.DangerousRelease(); } } @@ -43,7 +72,7 @@ internal static unsafe int ForkAndExecProcess( private static unsafe partial int ForkAndExecProcess( string filename, byte** argv, byte** envp, string? cwd, int setUser, uint userId, uint groupId, uint* groups, int groupsLength, - out int lpChildPid, SafeFileHandle stdinFd, SafeFileHandle stdoutFd, SafeFileHandle stderrFd); + out int lpChildPid, int stdinFd, int stdoutFd, int stderrFd); private static unsafe void AllocNullTerminatedArray(string[] arr, ref byte** arrPtr) { diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index 0e274824d6f528..fe4e45a38d991b 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -360,8 +360,6 @@ private SafeProcessHandle GetProcessHandle() private bool StartCore(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, SafeFileHandle? stdoutHandle, SafeFileHandle? stderrHandle) { - Debug.Assert(stdinHandle is not null && stdoutHandle is not null && stderrHandle is not null); - if (PlatformDoesNotSupportProcessStartAndKill) { throw new PlatformNotSupportedException(); @@ -388,7 +386,9 @@ private bool StartCore(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, // Unix applications expect the terminal to be in an echoing state by default. // To support processes that interact with the terminal (e.g. 'vi'), we need to configure the // terminal to echo. We keep this configuration as long as there are children possibly using the terminal. - bool usesTerminal = Interop.Sys.IsATty(stdinHandle) || Interop.Sys.IsATty(stdoutHandle) || Interop.Sys.IsATty(stderrHandle); + bool usesTerminal = (stdinHandle is not null && Interop.Sys.IsATty(stdinHandle)) + || (stdoutHandle is not null && Interop.Sys.IsATty(stdoutHandle)) + || (stderrHandle is not null && Interop.Sys.IsATty(stderrHandle)); if (startInfo.UseShellExecute) { @@ -451,7 +451,7 @@ private bool ForkAndExecProcess( ProcessStartInfo startInfo, string? resolvedFilename, string[] argv, string[] envp, string? cwd, bool setCredentials, uint userId, uint groupId, uint[]? groups, - SafeFileHandle stdinHandle, SafeFileHandle stdoutHandle, SafeFileHandle stderrHandle, + SafeFileHandle? stdinHandle, SafeFileHandle? stdoutHandle, SafeFileHandle? stderrHandle, bool usesTerminal, bool throwOnNoExec = true) { if (string.IsNullOrEmpty(resolvedFilename)) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Win32.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Win32.cs index 40dd848d1df136..58678c36e442e5 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Win32.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Win32.cs @@ -24,11 +24,9 @@ public partial class Process : IDisposable private bool StartCore(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, SafeFileHandle? stdoutHandle, SafeFileHandle? stderrHandle) { - Debug.Assert(startInfo.UseShellExecute == (stdinHandle is null && stdoutHandle is null && stderrHandle is null)); - return startInfo.UseShellExecute ? StartWithShellExecuteEx(startInfo) - : StartWithCreateProcess(startInfo, stdinHandle!, stdoutHandle!, stderrHandle!); + : StartWithCreateProcess(startInfo, stdinHandle, stdoutHandle, stderrHandle); } private unsafe bool StartWithShellExecuteEx(ProcessStartInfo startInfo) 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 172e85705cb2c1..081e9006d72b95 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 @@ -422,13 +422,11 @@ private void SetWorkingSetLimitsCore(IntPtr? newMin, IntPtr? newMax, out IntPtr } /// Starts the process using the supplied start info. - private unsafe bool StartWithCreateProcess(ProcessStartInfo startInfo, SafeFileHandle stdinHandle, SafeFileHandle stdoutHandle, SafeFileHandle stderrHandle) + private unsafe bool StartWithCreateProcess(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, SafeFileHandle? stdoutHandle, SafeFileHandle? stderrHandle) { // See knowledge base article Q190351 for an explanation of the following code. Noteworthy tricky points: // * The handles are duplicated as inheritable before they are passed to CreateProcess so // that the child process can use them - // * CreateProcess allows you to redirect all or none of the standard IO handles, so we use - // Console.OpenStandard*Handle for the handles that are not being redirected var commandLine = new ValueStringBuilder(stackalloc char[256]); BuildCommandLine(startInfo, ref commandLine); @@ -453,26 +451,25 @@ private unsafe bool StartWithCreateProcess(ProcessStartInfo startInfo, SafeFileH { startupInfo.cb = sizeof(Interop.Kernel32.STARTUPINFO); - // A process could be started with INVALID_HANDLE_VALUE, we need to take this into account. - if (!stdinHandle.IsInvalid) + if (stdinHandle is not null && !stdinHandle.IsInvalid) { inheritableStdinHandle = DuplicateAsInheritable(stdinHandle); startupInfo.hStdInput = inheritableStdinHandle.DangerousGetHandle(); } - if (!stdoutHandle.IsInvalid) + if (stdoutHandle is not null && !stdoutHandle.IsInvalid) { inheritableStdoutHandle = DuplicateAsInheritable(stdoutHandle); startupInfo.hStdOutput = inheritableStdoutHandle.DangerousGetHandle(); } - if (!stderrHandle.IsInvalid) + if (stderrHandle is not null && !stderrHandle.IsInvalid) { inheritableStderrHandle = DuplicateAsInheritable(stderrHandle); startupInfo.hStdError = inheritableStderrHandle.DangerousGetHandle(); } - if (!stdinHandle.IsInvalid || !stdoutHandle.IsInvalid || !stderrHandle.IsInvalid) + if (stdinHandle is not null || stdoutHandle is not null || stderrHandle is not null) { startupInfo.dwFlags = Interop.Advapi32.StartupInfoOptions.STARTF_USESTDHANDLES; } diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs index d377f83e84576f..6a068b7468608e 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs @@ -1299,9 +1299,7 @@ public bool Start() try { - // On Windows, ShellExecute does not provide an ability to set standard handles. - // On Unix, we emulate it and require the handles. - if (!startInfo.UseShellExecute || !OperatingSystem.IsWindows()) + if (anyRedirection) { // Windows supports creating non-inheritable pipe in atomic way. // When it comes to Unixes, it depends whether they support pipe2 sys-call or not. @@ -1309,7 +1307,7 @@ public bool Start() // Some process could be started in the meantime, so in order to prevent accidental handle inheritance, // a WriterLock is used around the pipe creation code. - bool requiresLock = anyRedirection && !SupportsAtomicNonInheritablePipeCreation; + bool requiresLock = !SupportsAtomicNonInheritablePipeCreation; if (requiresLock) { @@ -1322,40 +1320,16 @@ public bool Start() { SafeFileHandle.CreateAnonymousPipe(out childInputPipeHandle, out parentInputPipeHandle); } - else if (OperatingSystem.IsWindows()) - { - childInputPipeHandle = Console.OpenStandardInputHandle(); - } - else - { - childInputPipeHandle = new SafeFileHandle(0, ownsHandle: false); - } if (startInfo.RedirectStandardOutput) { SafeFileHandle.CreateAnonymousPipe(out parentOutputPipeHandle, out childOutputPipeHandle, asyncRead: OperatingSystem.IsWindows()); } - else if (OperatingSystem.IsWindows()) - { - childOutputPipeHandle = Console.OpenStandardOutputHandle(); - } - else - { - childOutputPipeHandle = new SafeFileHandle(1, ownsHandle: false); - } if (startInfo.RedirectStandardError) { SafeFileHandle.CreateAnonymousPipe(out parentErrorPipeHandle, out childErrorPipeHandle, asyncRead: OperatingSystem.IsWindows()); } - else if (OperatingSystem.IsWindows()) - { - childErrorPipeHandle = Console.OpenStandardErrorHandle(); - } - else - { - childErrorPipeHandle = new SafeFileHandle(2, ownsHandle: false); - } } finally { @@ -1381,10 +1355,8 @@ public bool Start() } finally { - // We MUST close the parent copies of the child handles, otherwise the parent + // We MUST close the child handles, otherwise the parent // process will not receive EOF when the child process closes its handles. - // It's OK to do it for handles returned by Console.OpenStandard*Handle APIs, - // because these handles are not owned and won't be closed by Dispose. childInputPipeHandle?.Dispose(); childOutputPipeHandle?.Dispose(); childErrorPipeHandle?.Dispose(); diff --git a/src/native/libs/System.Native/pal_process.c b/src/native/libs/System.Native/pal_process.c index 71f74f0849c59a..194a5ec06c44b1 100644 --- a/src/native/libs/System.Native/pal_process.c +++ b/src/native/libs/System.Native/pal_process.c @@ -358,9 +358,10 @@ int32_t SystemNative_ForkAndExecProcess(const char* filename, // The provided file handles might have CLOEXEC enabled, // but dup2 doesn't copy the file descriptor flags, // so the new file descriptors won't have CLOEXEC enabled. - if ((stdinFd != 0 && Dup2WithInterruptedRetry(stdinFd, STDIN_FILENO) == -1) || - (stdoutFd != 1 && Dup2WithInterruptedRetry(stdoutFd, STDOUT_FILENO) == -1) || - (stderrFd != 2 && Dup2WithInterruptedRetry(stderrFd, STDERR_FILENO) == -1)) + // A value of -1 means the caller did not provide a handle for that stream. + if ((stdinFd != -1 && stdinFd != STDIN_FILENO && Dup2WithInterruptedRetry(stdinFd, STDIN_FILENO) == -1) || + (stdoutFd != -1 && stdoutFd != STDOUT_FILENO && Dup2WithInterruptedRetry(stdoutFd, STDOUT_FILENO) == -1) || + (stderrFd != -1 && stderrFd != STDERR_FILENO && Dup2WithInterruptedRetry(stderrFd, STDERR_FILENO) == -1)) { ExitChild(waitForChildToExecPipe[WRITE_END_OF_PIPE], errno); } From 6ddf417c01f48875f6205eb7e5413c6307953c07 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 21 Mar 2026 14:29:55 +0000 Subject: [PATCH 20/22] Fix usesTerminal logic: null handle means child inherits parent's stream (possibly a terminal) Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/264e5c0b-daf2-4f32-80ec-10fdf888c120 --- .../src/System/Diagnostics/Process.Unix.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index fe4e45a38d991b..5c05ab4ff61f26 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -386,9 +386,10 @@ private bool StartCore(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, // Unix applications expect the terminal to be in an echoing state by default. // To support processes that interact with the terminal (e.g. 'vi'), we need to configure the // terminal to echo. We keep this configuration as long as there are children possibly using the terminal. - bool usesTerminal = (stdinHandle is not null && Interop.Sys.IsATty(stdinHandle)) - || (stdoutHandle is not null && Interop.Sys.IsATty(stdoutHandle)) - || (stderrHandle is not null && Interop.Sys.IsATty(stderrHandle)); + // When a handle is null, the child inherits the parent's standard stream which could be a terminal. + bool usesTerminal = (stdinHandle is null || Interop.Sys.IsATty(stdinHandle)) + || (stdoutHandle is null || Interop.Sys.IsATty(stdoutHandle)) + || (stderrHandle is null || Interop.Sys.IsATty(stderrHandle)); if (startInfo.UseShellExecute) { From 017f286422091eb124d73bd7fa87014dfc75d025 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 23 Mar 2026 10:01:24 +0000 Subject: [PATCH 21/22] Address review feedback: simplify test try/catch, remove manual Close() calls, add LeaveHandlesOpen test Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/6c515d2e-34b7-483e-a1e5-2b0fae3caad8 --- .../tests/ProcessHandlesTests.cs | 50 ++++++++++++------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.cs index 032083cfdb2232..996fb332465b02 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.cs @@ -53,19 +53,7 @@ public async Task CanRedirectOutputAndErrorToDifferentPipes(bool readAsync) : new("sh") { ArgumentList = { "-c", "echo 'Hello from stdout' && echo 'Error from stderr' >&2" } }; SafeFileHandle.CreateAnonymousPipe(out SafeFileHandle outputRead, out SafeFileHandle outputWrite, asyncRead: readAsync); - - SafeFileHandle errorRead; - SafeFileHandle errorWrite; - try - { - SafeFileHandle.CreateAnonymousPipe(out errorRead, out errorWrite, asyncRead: readAsync); - } - catch - { - outputRead.Dispose(); - outputWrite.Dispose(); - throw; - } + SafeFileHandle.CreateAnonymousPipe(out SafeFileHandle errorRead, out SafeFileHandle errorWrite, asyncRead: readAsync); startInfo.StandardOutput = outputWrite; startInfo.StandardError = errorWrite; @@ -170,13 +158,8 @@ public async Task CanImplementPiping() consumerInfo.StandardOutput = outputHandle; using Process producer = Process.Start(producerInfo)!; - - writePipe.Close(); // close the parent copy of child handle - using Process consumer = Process.Start(consumerInfo)!; - readPipe.Close(); // close the parent copy of child handle - producer.WaitForExit(); consumer.WaitForExit(); @@ -195,6 +178,37 @@ public async Task CanImplementPiping() } } + [Fact] + public void LeaveHandlesOpen_KeepsHandleOpen() + { + string tempFile = Path.GetTempFileName(); + + try + { + ProcessStartInfo startInfo = new("hostname"); + + SafeFileHandle outputHandle = File.OpenHandle(tempFile, FileMode.Create, FileAccess.Write, FileShare.ReadWrite); + + startInfo.StandardOutput = outputHandle; + startInfo.LeaveHandlesOpen = true; + + using Process process = Process.Start(startInfo)!; + process.WaitForExit(); + + Assert.Equal(0, process.ExitCode); + Assert.False(outputHandle.IsClosed); + + outputHandle.Dispose(); + } + finally + { + if (File.Exists(tempFile)) + { + File.Delete(tempFile); + } + } + } + [Fact] public void StandardInput_WithRedirectStandardInput_Throws() { From 95247af8e8c6e65c7dc880c0b12434fb843c8715 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 23 Mar 2026 19:06:18 +0100 Subject: [PATCH 22/22] address my own feedback: - add more tests - fix bugs - handle edge cases --- .../Interop.ForkAndExecProcess.cs | 20 +++++------ .../src/System/Diagnostics/Process.Unix.cs | 6 ++-- .../src/System/Diagnostics/Process.Windows.cs | 27 +++++++-------- .../src/System/Diagnostics/Process.cs | 27 ++++++++++----- .../tests/ProcessHandlesTests.cs | 34 ++++++++++++++++++- 5 files changed, 76 insertions(+), 38 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs index c595a120a4b4d4..85b2dd80c6433c 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs @@ -20,26 +20,26 @@ internal static unsafe int ForkAndExecProcess( byte** argvPtr = null, envpPtr = null; int result = -1; - bool stdinRefAdded = false, stdoutRefAdded = false, stderrRefAdded = false; + bool ignore = false; try { int stdinRawFd = -1, stdoutRawFd = -1, stderrRawFd = -1; if (stdinFd is not null) { - stdinFd.DangerousAddRef(ref stdinRefAdded); + stdinFd.DangerousAddRef(ref ignore); stdinRawFd = stdinFd.DangerousGetHandle().ToInt32(); } if (stdoutFd is not null) { - stdoutFd.DangerousAddRef(ref stdoutRefAdded); + stdoutFd.DangerousAddRef(ref ignore); stdoutRawFd = stdoutFd.DangerousGetHandle().ToInt32(); } if (stderrFd is not null) { - stderrFd.DangerousAddRef(ref stderrRefAdded); + stderrFd.DangerousAddRef(ref ignore); stderrRawFd = stderrFd.DangerousGetHandle().ToInt32(); } @@ -59,12 +59,12 @@ internal static unsafe int ForkAndExecProcess( FreeArray(envpPtr, envp.Length); FreeArray(argvPtr, argv.Length); - if (stdinRefAdded) - stdinFd!.DangerousRelease(); - if (stdoutRefAdded) - stdoutFd!.DangerousRelease(); - if (stderrRefAdded) - stderrFd!.DangerousRelease(); + if (stdinFd is not null) + stdinFd.DangerousRelease(); + if (stdoutFd is not null) + stdoutFd.DangerousRelease(); + if (stderrFd is not null) + stderrFd.DangerousRelease(); } } diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index 5c05ab4ff61f26..47aede70fe9930 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -387,9 +387,9 @@ private bool StartCore(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, // To support processes that interact with the terminal (e.g. 'vi'), we need to configure the // terminal to echo. We keep this configuration as long as there are children possibly using the terminal. // When a handle is null, the child inherits the parent's standard stream which could be a terminal. - bool usesTerminal = (stdinHandle is null || Interop.Sys.IsATty(stdinHandle)) - || (stdoutHandle is null || Interop.Sys.IsATty(stdoutHandle)) - || (stderrHandle is null || Interop.Sys.IsATty(stderrHandle)); + bool usesTerminal = (stdinHandle is not null && Interop.Sys.IsATty(stdinHandle)) + || (stdoutHandle is not null && Interop.Sys.IsATty(stdoutHandle)) + || (stderrHandle is not null && Interop.Sys.IsATty(stderrHandle)); if (startInfo.UseShellExecute) { 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 081e9006d72b95..077e49d62e459e 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 @@ -451,26 +451,23 @@ private unsafe bool StartWithCreateProcess(ProcessStartInfo startInfo, SafeFileH { startupInfo.cb = sizeof(Interop.Kernel32.STARTUPINFO); - if (stdinHandle is not null && !stdinHandle.IsInvalid) + if (stdinHandle is not null || stdoutHandle is not null || stderrHandle is not null) { - inheritableStdinHandle = DuplicateAsInheritable(stdinHandle); - startupInfo.hStdInput = inheritableStdinHandle.DangerousGetHandle(); - } + Debug.Assert(stdinHandle is not null && stdoutHandle is not null && stderrHandle is not null, "All or none of the standard handles must be provided."); - if (stdoutHandle is not null && !stdoutHandle.IsInvalid) - { - inheritableStdoutHandle = DuplicateAsInheritable(stdoutHandle); - startupInfo.hStdOutput = inheritableStdoutHandle.DangerousGetHandle(); - } + // The user can't specify invalid handle via ProcessStartInfo.Standar*Handle APIs. + // However, Console.OpenStandard*Handle() can return INVALID_HANDLE_VALUE for a process + // that was started with INVALID_HANDLE_VALUE as given standard handle. + // As soon as SafeFileHandle.IsInheritabe() is added, we can use it here to avoid unnecessary duplication. + inheritableStdinHandle = !stdinHandle.IsInvalid ? DuplicateAsInheritable(stdinHandle) : stdinHandle; + inheritableStdoutHandle = !stdoutHandle.IsInvalid ? DuplicateAsInheritable(stdoutHandle) : stdoutHandle; + inheritableStderrHandle = !stderrHandle.IsInvalid ? DuplicateAsInheritable(stderrHandle) : stderrHandle; - if (stderrHandle is not null && !stderrHandle.IsInvalid) - { - inheritableStderrHandle = DuplicateAsInheritable(stderrHandle); + startupInfo.hStdInput = inheritableStdinHandle.DangerousGetHandle(); + startupInfo.hStdOutput = inheritableStdoutHandle.DangerousGetHandle(); startupInfo.hStdError = inheritableStderrHandle.DangerousGetHandle(); - } - if (stdinHandle is not null || stdoutHandle is not null || stderrHandle is not null) - { + // If STARTF_USESTDHANDLES is not set, the new process will inherit the standard handles. startupInfo.dwFlags = Interop.Advapi32.StartupInfoOptions.STARTF_USESTDHANDLES; } diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs index 4d9ae60dd2f836..9af89ac1eee993 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs @@ -1278,19 +1278,16 @@ public bool Start() } } } + bool anyRedirection = startInfo.RedirectStandardInput || startInfo.RedirectStandardOutput || startInfo.RedirectStandardError; - if (startInfo.UseShellExecute && anyRedirection) + bool anyHandle = startInfo.StandardInput is not null || startInfo.StandardOutput is not null || startInfo.StandardError is not null; + if (startInfo.UseShellExecute && (anyRedirection || anyHandle)) { throw new InvalidOperationException(SR.CantRedirectStreams); } - bool anyHandle = startInfo.StandardInput is not null || startInfo.StandardOutput is not null || startInfo.StandardError is not null; if (anyHandle) { - if (startInfo.UseShellExecute) - { - throw new InvalidOperationException(SR.CantRedirectStreams); - } if (startInfo.StandardInput is not null && startInfo.RedirectStandardInput) { throw new InvalidOperationException(SR.CantSetHandleAndRedirect); @@ -1320,15 +1317,15 @@ public bool Start() try { - if (anyRedirection) + if (anyRedirection || anyHandle) { // Windows supports creating non-inheritable pipe in atomic way. // When it comes to Unixes, it depends whether they support pipe2 sys-call or not. // If they don't, the pipe is created as inheritable and made non-inheritable with another sys-call. // Some process could be started in the meantime, so in order to prevent accidental handle inheritance, - // a WriterLock is used around the pipe creation code. + // a writer lock is used around the pipe creation code. - bool requiresLock = !SupportsAtomicNonInheritablePipeCreation; + bool requiresLock = anyRedirection && !SupportsAtomicNonInheritablePipeCreation; if (requiresLock) { @@ -1345,6 +1342,10 @@ public bool Start() { SafeFileHandle.CreateAnonymousPipe(out childInputPipeHandle, out parentInputPipeHandle); } + else if (!OperatingSystem.IsAndroid()) + { + childInputPipeHandle = Console.OpenStandardInputHandle(); + } if (startInfo.StandardOutput is not null) { @@ -1354,6 +1355,10 @@ public bool Start() { SafeFileHandle.CreateAnonymousPipe(out parentOutputPipeHandle, out childOutputPipeHandle, asyncRead: OperatingSystem.IsWindows()); } + else if (!OperatingSystem.IsAndroid()) + { + childOutputPipeHandle = Console.OpenStandardOutputHandle(); + } if (startInfo.StandardError is not null) { @@ -1363,6 +1368,10 @@ public bool Start() { SafeFileHandle.CreateAnonymousPipe(out parentErrorPipeHandle, out childErrorPipeHandle, asyncRead: OperatingSystem.IsWindows()); } + else if (!OperatingSystem.IsAndroid()) + { + childErrorPipeHandle = Console.OpenStandardErrorHandle(); + } } finally { diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.cs index 996fb332465b02..3ea7920ceba15e 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessHandlesTests.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.IO; -using System.Text; using System.Threading.Tasks; using Microsoft.Win32.SafeHandles; using Xunit; @@ -81,6 +80,39 @@ public async Task CanRedirectOutputAndErrorToDifferentPipes(bool readAsync) } } + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task CanRedirectOutputAndErrorToSamePipe(bool readAsync) + { + ProcessStartInfo startInfo = OperatingSystem.IsWindows() + ? new("cmd") { ArgumentList = { "/c", "echo Hello from stdout && echo Error from stderr 1>&2" } } + : new("sh") { ArgumentList = { "-c", "echo 'Hello from stdout' && echo 'Error from stderr' >&2" } }; + + string expectedOutput = OperatingSystem.IsWindows() + ? "Hello from stdout \r\nError from stderr \r\n" + : "Hello from stdout\nError from stderr\n"; + + SafeFileHandle.CreateAnonymousPipe(out SafeFileHandle readPipe, out SafeFileHandle writePipe, asyncRead: readAsync); + + startInfo.StandardOutput = writePipe; + startInfo.StandardError = writePipe; + + using (readPipe) + using (writePipe) + { + using Process process = Process.Start(startInfo)!; + + using FileStream combinedStream = new(readPipe, FileAccess.Read, bufferSize: 1, isAsync: readAsync); + using StreamReader combinedReader = new(combinedStream); + + Assert.Equal(expectedOutput, await combinedReader.ReadToEndAsync()); + + process.WaitForExit(); + Assert.Equal(0, process.ExitCode); + } + } + [Theory] [InlineData(true)] [InlineData(false)]