Revert "Use socketpair to implement Process.Start redirection" (#47461)…

… (#47644)

* Revert "Use socketpair to implement Process.Start redirection" (#47461)

* Revert #34861

* reinstate removed test

* Update src/libraries/System.Diagnostics.Process/tests/ProcessStreamReadTests.cs

Co-authored-by: Stephen Toub <stoub@microsoft.com>

Co-authored-by: Stephen Toub <stoub@microsoft.com>

* Add test validating against regression in #46469 (#47643)

* Add test validating against regression in #46469

* fix test bug

Co-authored-by: Stephen Toub <stoub@microsoft.com>

src/libraries/Native/Unix/System.Native/pal_process.c

@@ -20,8 +20,9 @@
 #if HAVE_CRT_EXTERNS_H
 #include <crt_externs.h>
 #endif
+#if HAVE_PIPE2
 #include <fcntl.h>
-#include <sys/socket.h>
+#endif
 #include <pthread.h>
 
 #if HAVE_SCHED_SETAFFINITY || HAVE_SCHED_GETAFFINITY
@@ -44,7 +45,7 @@ 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);
 
-#ifndef SOCK_CLOEXEC
+#if !HAVE_PIPE2
 static pthread_mutex_t ProcessCreateLock = PTHREAD_MUTEX_INITIALIZER;
 #endif
 
@@ -179,31 +180,6 @@ static int SetGroups(uint32_t* userGroups, int32_t userGroupsLength, uint32_t* p
     return rv;
 }
 
-static int32_t SocketPair(int32_t sv[2])
-{
-    int32_t result;
-
-    int type = SOCK_STREAM;
-#ifdef SOCK_CLOEXEC
-    type |= SOCK_CLOEXEC;
-#endif
-
-    while ((result = socketpair(AF_UNIX, type, 0, sv)) < 0 && errno == EINTR);
-
-#ifndef SOCK_CLOEXEC
-    if (result == 0)
-    {
-        while ((result = fcntl(sv[READ_END_OF_PIPE], F_SETFD, FD_CLOEXEC)) < 0 && errno == EINTR);
-        if (result == 0)
-        {
-            while ((result = fcntl(sv[WRITE_END_OF_PIPE], F_SETFD, FD_CLOEXEC)) < 0 && errno == EINTR);
-        }
-    }
-#endif
-
-    return result;
-}
-
 int32_t SystemNative_ForkAndExecProcess(const char* filename,
                                       char* const argv[],
                                       char* const envp[],
@@ -222,7 +198,7 @@ int32_t SystemNative_ForkAndExecProcess(const char* filename,
                                       int32_t* stderrFd)
 {
 #if HAVE_FORK
-#ifndef SOCK_CLOEXEC
+#if !HAVE_PIPE2
     bool haveProcessCreateLock = false;
 #endif
     bool success = true;
@@ -278,11 +254,11 @@ int32_t SystemNative_ForkAndExecProcess(const char* filename,
         goto done;
     }
 
-#ifndef SOCK_CLOEXEC
-    // We do not have SOCK_CLOEXEC; take the lock to emulate it race free.
-    // If another process were to be launched between the socket creation and the fcntl call to set CLOEXEC on it, that
-    // file descriptor would be inherited into the other child process, eventually causing a deadlock either in the loop
-    // below that waits for that socket to be closed or in StreamReader.ReadToEnd() in the calling code.
+#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.
@@ -294,9 +270,9 @@ int32_t SystemNative_ForkAndExecProcess(const char* filename,
 
     // Open pipes for any requests to redirect stdin/stdout/stderr and set the
     // close-on-exec flag to the pipe file descriptors.
-    if ((redirectStdin  && SocketPair(stdinFds) != 0) ||
-        (redirectStdout && SocketPair(stdoutFds) != 0) ||
-        (redirectStderr && SocketPair(stderrFds) != 0))
+    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;
@@ -447,7 +423,7 @@ int32_t SystemNative_ForkAndExecProcess(const char* filename,
     *stderrFd = stderrFds[READ_END_OF_PIPE];
 
 done:;
-#ifndef SOCK_CLOEXEC
+#if !HAVE_PIPE2
     if (haveProcessCreateLock)
     {
         pthread_mutex_unlock(&ProcessCreateLock);

src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj

@@ -337,7 +337,6 @@
     <Reference Include="System.IO.FileSystem.DriveInfo" />
     <Reference Include="System.Linq" />
     <Reference Include="System.Memory" />
-    <Reference Include="System.Net.Sockets" />
     <Reference Include="System.Runtime" />
     <Reference Include="System.Runtime.Extensions" />
     <Reference Include="System.Runtime.InteropServices" />

src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs

@@ -5,7 +5,6 @@
 using System.Collections.Generic;
 using System.ComponentModel;
 using System.IO;
-using System.Net.Sockets;
 using System.Security;
 using System.Text;
 using System.Threading;
@@ -754,31 +753,16 @@ internal static TimeSpan TicksToTimeSpan(double ticks)
             return TimeSpan.FromSeconds(ticks / (double)ticksPerSecond);
         }
 
-        /// <summary>Opens a stream around the specified socket file descriptor and with the specified access.</summary>
-        /// <param name="fd">The socket file descriptor.</param>
+        /// <summary>Opens a stream around the specified file descriptor and with the specified access.</summary>
+        /// <param name="fd">The file descriptor.</param>
         /// <param name="access">The access mode.</param>
         /// <returns>The opened stream.</returns>
-        private static Stream OpenStream(int fd, FileAccess access)
+        private static FileStream OpenStream(int fd, FileAccess access)
         {
             Debug.Assert(fd >= 0);
-            var socketHandle = new SafeSocketHandle((IntPtr)fd, ownsHandle: true);
-            var socket = new Socket(socketHandle);
-
-            if (!socket.Connected)
-            {
-                // WSL1 workaround -- due to issues with sockets syscalls
-                // socket pairs fd's are erroneously inferred as not connected.
-                // Fall back to using FileStream instead.
-
-                GC.SuppressFinalize(socket);
-                GC.SuppressFinalize(socketHandle);
-
-                return new FileStream(
-                    new SafeFileHandle((IntPtr)fd, ownsHandle: true),
-                    access, StreamBufferSize, isAsync: false);
-            }
-
-            return new NetworkStream(socket, access, ownsSocket: true);
+            return new FileStream(
+                new SafeFileHandle((IntPtr)fd, ownsHandle: true),
+                access, StreamBufferSize, isAsync: false);
         }
 
         /// <summary>Parses a command-line argument string into a list of arguments.</summary>

src/libraries/System.Diagnostics.Process/tests/ProcessStreamReadTests.cs

@@ -344,7 +344,8 @@ async private Task<bool> WaitPipeSignal(PipeStream pipe, int millisecond)
             }
         }
 
-        [PlatformSpecific(~TestPlatforms.Windows)] // currently on Windows these operations async-over-sync on Windows
+        [ActiveIssue("https://github.com/dotnet/runtime/issues/44329")]
+        [PlatformSpecific(~TestPlatforms.Windows)] // currently on Windows these operations async-over-sync on Windows	
         [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
         public async Task ReadAsync_OutputStreams_Cancel_RespondsQuickly()
         {

src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs

@@ -4,6 +4,7 @@
 using System.Collections;
 using System.Collections.Generic;
 using System.ComponentModel;
+using System.Globalization;
 using System.IO;
 using System.Linq;
 using System.Reflection;
@@ -810,6 +811,24 @@ public void TestProcessRecycledPid()
             Assert.True(foundRecycled);
         }
 
+        [PlatformSpecific(TestPlatforms.AnyUnix)]
+        [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
+        [InlineData("/dev/stdin",  O_RDONLY)]
+        [InlineData("/dev/stdout", O_WRONLY)]
+        [InlineData("/dev/stderr", O_WRONLY)]
+        public void ChildProcessRedirectedIO_FilePathOpenShouldSucceed(string filename, int flags)
+        {
+            var options = new RemoteInvokeOptions { StartInfo = new ProcessStartInfo { RedirectStandardOutput = true, RedirectStandardInput = true, RedirectStandardError = true }};
+            using (RemoteInvokeHandle handle = RemoteExecutor.Invoke(ExecuteChildProcess, filename, flags.ToString(CultureInfo.InvariantCulture), options))
+            { }
+
+            static void ExecuteChildProcess(string filename, string flags)
+            {
+                int result = open(filename, int.Parse(flags, CultureInfo.InvariantCulture));
+                Assert.True(result >= 0, $"failed to open file with {result} and errno {Marshal.GetLastWin32Error()}.");
+            }
+        }
+
         [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
         [InlineData(true)]
         [InlineData(false)]
@@ -940,6 +959,12 @@ private static unsafe HashSet<uint> GetGroups()
         [DllImport("libc", SetLastError = true)]
         private static extern int kill(int pid, int sig);
 
+        [DllImport("libc", SetLastError = true)]
+        private static extern int open(string pathname, int flags);
+
+        private const int O_RDONLY = 0;
+        private const int O_WRONLY = 1;
+
         private static readonly string[] s_allowedProgramsToRun = new string[] { "xdg-open", "gnome-open", "kfmclient" };
 
         private string WriteScriptFile(string directory, string name, int returnValue)