[release/6.0] Fallback to read/write if pread/pwrite fails (#60280)

* Fallback to read/write if pread/pwrite fails

* * Cache whether a handle supports random access * Keep avoiding pread/pwrite for unseekable files * Fallback to read/write only for known error ENXIO

* Test more device paths and add tests for WriteAll*

* Fix CI: skip test when device can't be opened

* * Do not use SkipTestException * Exclude EBADF from assertion

* use NullableBool in _supportsRandomAccess

* Add tests for mkfifo and socketpair

* Rename test file

* Address feedback in src

* Address suggestions

* Address suggestions

* Update src/libraries/System.IO.FileSystem/tests/FileStream/DevicesPipesAndSockets.cs

Use Lazy for thread-safety

Co-authored-by: Adam Sitnik <adam.sitnik@gmail.com>

* Use IEnumerable<string> in Lazy and fix usages

Co-authored-by: David Cantu <jozkee@macbook.lan>
Co-authored-by: Jozkee <dacantu@microsoft.com>
Co-authored-by: Adam Sitnik <adam.sitnik@gmail.com>

Author: 4 people

src/libraries/System.IO.FileSystem/tests/FileStream/DevicesPipesAndSockets.cs

@@ -0,0 +1,219 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using Microsoft.DotNet.XUnitExtensions;
+using Microsoft.Win32.SafeHandles;
+using System.Collections.Generic;
+using System.IO.Pipes;
+using System.Runtime.InteropServices;
+using System.Text;
+using System.Threading.Tasks;
+using Xunit;
+
+namespace System.IO.Tests
+{
+    [PlatformSpecific(TestPlatforms.AnyUnix)]
+    public class DevicesPipesAndSockets : FileSystemTest
+    {
+        [Theory]
+        [MemberData(nameof(DevicePath_FileOptions_TestData))]
+        public void CharacterDevice_FileStream_Write(string devicePath, FileOptions fileOptions)
+        {
+            FileStreamOptions options = new() { Options = fileOptions, Access = FileAccess.Write, Share = FileShare.Write };
+            using FileStream fs = new(devicePath, options);
+            fs.Write(Encoding.UTF8.GetBytes("foo"));
+        }
+
+        [Theory]
+        [MemberData(nameof(DevicePath_FileOptions_TestData))]
+        public async Task CharacterDevice_FileStream_WriteAsync(string devicePath, FileOptions fileOptions)
+        {
+            FileStreamOptions options = new() { Options = fileOptions, Access = FileAccess.Write, Share = FileShare.Write };
+            using FileStream fs = new(devicePath, options);
+            await fs.WriteAsync(Encoding.UTF8.GetBytes("foo"));
+        }
+
+        [Theory]
+        [MemberData(nameof(DevicePath_TestData))]
+        public void CharacterDevice_WriteAllBytes(string devicePath)
+        {
+            File.WriteAllBytes(devicePath, Encoding.UTF8.GetBytes("foo"));
+        }
+
+        [Theory]
+        [MemberData(nameof(DevicePath_TestData))]
+        public async Task CharacterDevice_WriteAllBytesAsync(string devicePath)
+        {
+            await File.WriteAllBytesAsync(devicePath, Encoding.UTF8.GetBytes("foo"));
+        }
+
+        [Theory]
+        [MemberData(nameof(DevicePath_TestData))]
+        public void CharacterDevice_WriteAllText(string devicePath)
+        {
+            File.WriteAllText(devicePath, "foo");
+        }
+
+        [Theory]
+        [MemberData(nameof(DevicePath_TestData))]
+        public async Task CharacterDevice_WriteAllTextAsync(string devicePath)
+        {
+            await File.WriteAllTextAsync(devicePath, "foo");
+        }
+
+        [Fact]
+        [PlatformSpecific(TestPlatforms.AnyUnix & ~TestPlatforms.Browser)]
+        public async Task NamedPipe_ReadWrite()
+        {
+            string fifoPath = GetTestFilePath();
+            Assert.Equal(0, mkfifo(fifoPath, 438 /* 666 in octal */ ));
+
+            await Task.WhenAll(
+                Task.Run(() => 
+                {
+                    using var fs = new FileStream(fifoPath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite);
+                    ReadByte(fs, 42);
+                }),
+                Task.Run(() => 
+                {
+                    using var fs = new FileStream(fifoPath, FileMode.Open, FileAccess.Write, FileShare.Read);
+                    WriteByte(fs, 42);
+                }));
+        }
+
+        [Fact]
+        [PlatformSpecific(TestPlatforms.AnyUnix & ~TestPlatforms.Browser)]
+        public async Task NamedPipe_ReadWrite_Async()
+        {
+            string fifoPath = GetTestFilePath();
+            Assert.Equal(0, mkfifo(fifoPath, 438 /* 666 in octal */ ));
+
+            await Task.WhenAll(
+                Task.Run(async () => {
+                    using var fs = new FileStream(fifoPath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite);
+                    await ReadByteAsync(fs, 42);
+                }),
+                Task.Run(async () => 
+                {
+                    using var fs = new FileStream(fifoPath, FileMode.Open, FileAccess.Write, FileShare.Read);
+                    await WriteByteAsync(fs, 42);
+                }));
+        }
+
+        private const int AF_UNIX = 1;
+        private const int SOCK_STREAM = 1;
+
+        [Fact]
+        [PlatformSpecific(TestPlatforms.AnyUnix & ~TestPlatforms.Browser)]
+        public unsafe void SocketPair_ReadWrite()
+        {
+            int* ptr = stackalloc int[2];
+            Assert.Equal(0, socketpair(AF_UNIX, SOCK_STREAM, 0, ptr));
+
+            using var readFileStream = new FileStream(new SafeFileHandle((IntPtr)ptr[0], ownsHandle: true), FileAccess.Read);
+            using var writeFileStream = new FileStream(new SafeFileHandle((IntPtr)ptr[1], ownsHandle: true), FileAccess.Write);
+
+            Task.WhenAll(
+                Task.Run(() => ReadByte(readFileStream, 42)),
+                Task.Run(() => WriteByte(writeFileStream, 42))).GetAwaiter().GetResult();
+        }
+
+        [Fact]
+        [PlatformSpecific(TestPlatforms.AnyUnix & ~TestPlatforms.Browser)]
+        public void SocketPair_ReadWrite_Async()
+        {
+            unsafe
+            {
+                int* ptr = stackalloc int[2];
+                Assert.Equal(0, socketpair(AF_UNIX, SOCK_STREAM, 0, ptr));
+
+                using var readFileStream = new FileStream(new SafeFileHandle((IntPtr)ptr[0], ownsHandle: true), FileAccess.Read);
+                using var writeFileStream = new FileStream(new SafeFileHandle((IntPtr)ptr[1], ownsHandle: true), FileAccess.Write);
+
+                Task.WhenAll(
+                    ReadByteAsync(readFileStream, 42),
+                    WriteByteAsync(writeFileStream, 42)).GetAwaiter().GetResult();
+            }
+        }
+
+        private static void ReadByte(FileStream fs, byte expected)
+        {
+            var buffer = new byte[1];
+            Assert.Equal(1, fs.Read(buffer));
+            Assert.Equal(expected, buffer[0]);
+        }
+
+        private static void WriteByte(FileStream fs, byte value)
+        {
+            fs.Write(new byte[] { value });
+            fs.Flush();
+        }
+
+        private static async Task ReadByteAsync(FileStream fs, byte expected)
+        {
+            var buffer = new byte[1];
+            Assert.Equal(1, await fs.ReadAsync(buffer));
+            Assert.Equal(expected, buffer[0]);
+        }
+
+        private static async Task WriteByteAsync(FileStream fs, byte value)
+        {
+            await fs.WriteAsync(new byte[] { value });
+            await fs.FlushAsync();
+        }
+
+        private static Lazy<IEnumerable<string>> AvailableDevicePaths = new Lazy<IEnumerable<string>>(() =>
+        { 
+            List<string> paths = new();
+            FileStreamOptions options = new() { Access = FileAccess.Write, Share = FileShare.Write };
+
+            foreach (string devicePath in new[] { "/dev/tty", "/dev/console", "/dev/null", "/dev/zero" })
+            {
+                if (!File.Exists(devicePath))
+                {
+                    continue;
+                }
+
+                try
+                {
+                    File.Open(devicePath, options).Dispose();
+                }
+                catch (Exception ex)
+                {
+                    if (ex is IOException || ex is UnauthorizedAccessException)
+                    {
+                        continue;
+                    }
+
+                    throw;
+                }
+
+                paths.Add(devicePath);
+            }
+
+            return paths;
+        });
+
+        public static IEnumerable<object[]> DevicePath_FileOptions_TestData()
+        {
+            foreach (string devicePath in AvailableDevicePaths.Value)
+            {
+                foreach (FileOptions options in new[] { FileOptions.None, FileOptions.Asynchronous })
+                {
+                    yield return new object[] { devicePath, options};
+                }
+            }
+        }
+
+        public static IEnumerable<object[]> DevicePath_TestData()
+        {
+            foreach (string devicePath in AvailableDevicePaths.Value)
+            {
+                yield return new object[] { devicePath };
+            }
+        }
+
+        [DllImport("libc")]
+        private static unsafe extern int socketpair(int domain, int type, int protocol, int* ptr);
+    }
+}

src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj

@@ -118,6 +118,7 @@
     <Compile Include="FileInfo\GetSetAttributes.cs" />
     <Compile Include="FileInfo\Length.cs" />
     <Compile Include="FileInfo\Open.cs" />
+    <Compile Include="FileStream\DevicesPipesAndSockets.cs" />
     <Compile Include="FileStream\FlushAsync.cs" />
     <Compile Include="FileStream\FileStreamConformanceTests.cs" />
     <Compile Include="FileStream\SafeFileHandle.cs" />

src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs

@@ -16,6 +16,7 @@ public sealed partial class SafeFileHandle : SafeHandleZeroOrMinusOneIsInvalid
 
         // not using bool? as it's not thread safe
         private volatile NullableBool _canSeek = NullableBool.Undefined;
+        private volatile NullableBool _supportsRandomAccess = NullableBool.Undefined;
         private bool _deleteOnClose;
         private bool _isLocked;
 
@@ -33,6 +34,25 @@ private SafeFileHandle(bool ownsHandle)
 
         internal bool CanSeek => !IsClosed && GetCanSeek();
 
+        internal bool SupportsRandomAccess
+        {
+            get
+            {
+                NullableBool supportsRandomAccess = _supportsRandomAccess;
+                if (supportsRandomAccess == NullableBool.Undefined)
+                {
+                    _supportsRandomAccess = supportsRandomAccess = GetCanSeek() ? NullableBool.True : NullableBool.False;
+                }
+
+                return supportsRandomAccess == NullableBool.True;
+            }
+            set
+            {
+                Debug.Assert(value == false); // We should only use the setter to disable random access.
+                _supportsRandomAccess = value ? NullableBool.True : NullableBool.False;
+            }
+        }
+
         internal ThreadPoolBoundHandle? ThreadPoolBinding => null;
 
         internal void EnsureThreadPoolBindingInitialized() { /* nop */ }

src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Unix.cs

@@ -33,9 +33,30 @@ internal static unsafe int ReadAtOffset(SafeFileHandle handle, Span<byte> buffer
                 // The Windows implementation uses ReadFile, which ignores the offset if the handle
                 // isn't seekable.  We do the same manually with PRead vs Read, in order to enable
                 // the function to be used by FileStream for all the same situations.
-                int result = handle.CanSeek ?
-                    Interop.Sys.PRead(handle, bufPtr, buffer.Length, fileOffset) :
-                    Interop.Sys.Read(handle, bufPtr, buffer.Length);
+                int result;
+                if (handle.SupportsRandomAccess)
+                {
+                    // Try pread for seekable files.
+                    result = Interop.Sys.PRead(handle, bufPtr, buffer.Length, fileOffset);
+                    if (result == -1)
+                    {
+                        // We need to fallback to the non-offset version for certain file types
+                        // e.g: character devices (such as /dev/tty), pipes, and sockets.
+                        Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo();
+
+                        if (errorInfo.Error == Interop.Error.ENXIO ||
+                            errorInfo.Error == Interop.Error.ESPIPE)
+                        {
+                            handle.SupportsRandomAccess = false;
+                            result = Interop.Sys.Read(handle, bufPtr, buffer.Length);
+                        }
+                    }
+                }
+                else
+                {
+                    result = Interop.Sys.Read(handle, bufPtr, buffer.Length);
+                }
+
                 FileStreamHelpers.CheckFileCall(result, handle.Path);
                 return result;
             }
@@ -90,9 +111,29 @@ internal static unsafe void WriteAtOffset(SafeFileHandle handle, ReadOnlySpan<by
                     // The Windows implementation uses WriteFile, which ignores the offset if the handle
                     // isn't seekable.  We do the same manually with PWrite vs Write, in order to enable
                     // the function to be used by FileStream for all the same situations.
-                    int bytesWritten = handle.CanSeek ?
-                        Interop.Sys.PWrite(handle, bufPtr, GetNumberOfBytesToWrite(buffer.Length), fileOffset) :
-                        Interop.Sys.Write(handle, bufPtr, GetNumberOfBytesToWrite(buffer.Length));
+                    int bytesToWrite = GetNumberOfBytesToWrite(buffer.Length);
+                    int bytesWritten;
+                    if (handle.SupportsRandomAccess)
+                    {
+                        bytesWritten = Interop.Sys.PWrite(handle, bufPtr, bytesToWrite, fileOffset);
+                        if (bytesWritten == -1)
+                        {
+                            // We need to fallback to the non-offset version for certain file types
+                            // e.g: character devices (such as /dev/tty), pipes, and sockets.
+                            Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo();
+
+                            if (errorInfo.Error == Interop.Error.ENXIO ||
+                                errorInfo.Error == Interop.Error.ESPIPE)
+                            {
+                                handle.SupportsRandomAccess = false;
+                                bytesWritten = Interop.Sys.Write(handle, bufPtr, bytesToWrite);
+                            }
+                        }
+                    }
+                    else
+                    {
+                        bytesWritten = Interop.Sys.Write(handle, bufPtr, bytesToWrite);
+                    }
 
                     FileStreamHelpers.CheckFileCall(bytesWritten, handle.Path);
                     if (bytesWritten == buffer.Length)