Support unseekable filestream when ReadAllBytes[Async] (dotnet#58434) (…

…dotnet#61410) Co-authored-by: Adam Sitnik <[email protected]> Co-authored-by: LateApexEarlySpeed <[email protected]>
timandy · Nov 12, 2021 · 278057b · 278057b
1 parent 8881fcf
commit 278057b
Show file tree

Hide file tree

Showing 5 changed files with 106 additions and 15 deletions.
diff --git a/src/libraries/System.IO.FileSystem/tests/File/ReadWriteAllBytes.cs b/src/libraries/System.IO.FileSystem/tests/File/ReadWriteAllBytes.cs
@@ -2,8 +2,11 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Text;
+using System.Threading;
 using System.Threading.Tasks;
 using Xunit;
+using System.IO.Pipes;
+using Microsoft.DotNet.XUnitExtensions;
 
 namespace System.IO.Tests
 {
@@ -172,5 +175,30 @@ public void ProcFs_NotEmpty(string path)
         {
             Assert.InRange(File.ReadAllBytes(path).Length, 1, int.MaxValue);
         }
+
+        [Fact]
+        [PlatformSpecific(TestPlatforms.AnyUnix & ~TestPlatforms.Browser)]
+        public async Task ReadAllBytes_NonSeekableFileStream_InUnix()
+        {
+            string fifoPath = GetTestFilePath();
+            Assert.Equal(0, mkfifo(fifoPath, 438 /* 666 in octal */ ));
+
+            var contentBytes = new byte[] { 1, 2, 3 };
+
+            await Task.WhenAll(
+                Task.Run(() =>
+                {
+                    byte[] readBytes = File.ReadAllBytes(fifoPath);
+                    Assert.Equal<byte>(contentBytes, readBytes);
+                }),
+                Task.Run(() =>
+                {
+                    using var fs = new FileStream(fifoPath, FileMode.Open, FileAccess.Write, FileShare.Read);
+                    foreach (byte content in contentBytes)
+                    {
+                        fs.WriteByte(content);
+                    }
+                }));
+        }
     }
 }
diff --git a/src/libraries/System.IO.FileSystem/tests/File/ReadWriteAllBytesAsync.cs b/src/libraries/System.IO.FileSystem/tests/File/ReadWriteAllBytesAsync.cs
@@ -5,6 +5,8 @@
 using System.Threading;
 using System.Threading.Tasks;
 using Xunit;
+using System.IO.Pipes;
+using Microsoft.DotNet.XUnitExtensions;
 
 namespace System.IO.Tests
 {
@@ -186,5 +188,61 @@ public async Task ProcFs_NotEmpty(string path)
         {
             Assert.InRange((await File.ReadAllBytesAsync(path)).Length, 1, int.MaxValue);
         }
+
+        [Fact]
+        [PlatformSpecific(TestPlatforms.Windows)] // DOS device paths (\\.\ and \\?\) are a Windows concept
+        public async Task ReadAllBytesAsync_NonSeekableFileStream_InWindows()
+        {
+            string pipeName = FileSystemTest.GetNamedPipeServerStreamName();
+            string pipePath = Path.GetFullPath($@"\\.\pipe\{pipeName}");
+
+            var namedPipeWriterStream = new NamedPipeServerStream(pipeName, PipeDirection.Out);
+            var contentBytes = new byte[] { 1, 2, 3 };
+
+            using (var cts = new CancellationTokenSource())
+            {
+                Task writingServerTask = WaitConnectionAndWritePipeStreamAsync(namedPipeWriterStream, contentBytes, cts.Token);
+                Task<byte[]> readTask = File.ReadAllBytesAsync(pipePath, cts.Token);
+                cts.CancelAfter(TimeSpan.FromSeconds(50));
+
+                await writingServerTask;
+                byte[] readBytes = await readTask;
+                Assert.Equal<byte>(contentBytes, readBytes);
+            }
+
+            static async Task WaitConnectionAndWritePipeStreamAsync(NamedPipeServerStream namedPipeWriterStream, byte[] contentBytes, CancellationToken cancellationToken)
+            {
+                await using (namedPipeWriterStream)
+                {
+                    await namedPipeWriterStream.WaitForConnectionAsync(cancellationToken);
+                    await namedPipeWriterStream.WriteAsync(contentBytes, cancellationToken);
+                }
+            }
+        }
+
+        [Fact]
+        [PlatformSpecific(TestPlatforms.AnyUnix & ~TestPlatforms.Browser)]
+        public async Task ReadAllBytesAsync_NonSeekableFileStream_InUnix()
+        {
+            string fifoPath = GetTestFilePath();
+            Assert.Equal(0, mkfifo(fifoPath, 438 /* 666 in octal */ ));
+
+            var contentBytes = new byte[] { 1, 2, 3 };
+
+            await Task.WhenAll(
+                Task.Run(async () =>
+                {
+                    byte[] readBytes = await File.ReadAllBytesAsync(fifoPath);
+                    Assert.Equal<byte>(contentBytes, readBytes);
+                }),
+                Task.Run(() =>
+                {
+                    using var fs = new FileStream(fifoPath, FileMode.Open, FileAccess.Write, FileShare.Read);
+                    foreach (byte content in contentBytes)
+                    {
+                        fs.WriteByte(content);
+                    }
+                }));
+        }
     }
 }
diff --git a/...reLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.OverlappedValueTaskSource.Windows.cs b/...reLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.OverlappedValueTaskSource.Windows.cs
@@ -86,8 +86,11 @@ internal static Exception GetIOError(int errorCode, string? path)
                 _bufferSize = memory.Length;
                 _memoryHandle = memory.Pin();
                 _overlapped = _fileHandle.ThreadPoolBinding!.AllocateNativeOverlapped(_preallocatedOverlapped);
-                _overlapped->OffsetLow = (int)fileOffset;
-                _overlapped->OffsetHigh = (int)(fileOffset >> 32);
+                if (_fileHandle.CanSeek)
+                {
+                    _overlapped->OffsetLow = (int)fileOffset;
+                    _overlapped->OffsetHigh = (int)(fileOffset >> 32);
+                }
                 return _overlapped;
             }
 

diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/File.cs b/src/libraries/System.Private.CoreLib/src/System/IO/File.cs
@@ -333,15 +333,15 @@ public static byte[] ReadAllBytes(string path)
             // bufferSize == 1 used to avoid unnecessary buffer in FileStream
             using (FileStream fs = new FileStream(path, FileMode.Open, FileAccess.Read, FileShare.Read, bufferSize: 1, FileOptions.SequentialScan))
             {
-                long fileLength = fs.Length;
-                if (fileLength > int.MaxValue)
+                long fileLength = 0;
+                if (fs.CanSeek && (fileLength = fs.Length) > int.MaxValue)
                 {
                     throw new IOException(SR.IO_FileTooLong2GB);
                 }
-                else if (fileLength == 0)
+                if (fileLength == 0)
                 {
 #if !MS_IO_REDIST
-                    // Some file systems (e.g. procfs on Linux) return 0 for length even when there's content.
+                    // Some file systems (e.g. procfs on Linux) return 0 for length even when there's content; also there is non-seekable file stream.
                     // Thus we need to assume 0 doesn't mean empty.
                     return ReadAllBytesUnknownLength(fs);
 #endif
@@ -729,8 +729,8 @@ private static async Task<string> InternalReadAllTextAsync(string path, Encoding
             bool returningInternalTask = false;
             try
             {
-                long fileLength = fs.Length;
-                if (fileLength > int.MaxValue)
+                long fileLength = 0L;
+                if (fs.CanSeek && (fileLength = fs.Length) > int.MaxValue)
                 {
                     var e = new IOException(SR.IO_FileTooLong2GB);
 #if !MS_IO_REDIST

diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs
@@ -74,7 +74,7 @@ private static unsafe int ReadSyncUsingAsyncHandle(SafeFileHandle handle, Span<b
 
             try
             {
-                overlapped = GetNativeOverlappedForAsyncHandle(handle.ThreadPoolBinding!, fileOffset, resetEvent);
+                overlapped = GetNativeOverlappedForAsyncHandle(handle, fileOffset, resetEvent);
 
                 fixed (byte* pinned = &MemoryMarshal.GetReference(buffer))
                 {
@@ -172,7 +172,7 @@ private static unsafe void WriteSyncUsingAsyncHandle(SafeFileHandle handle, Read
 
             try
             {
-                overlapped = GetNativeOverlappedForAsyncHandle(handle.ThreadPoolBinding!, fileOffset, resetEvent);
+                overlapped = GetNativeOverlappedForAsyncHandle(handle, fileOffset, resetEvent);
 
                 fixed (byte* pinned = &MemoryMarshal.GetReference(buffer))
                 {
@@ -698,15 +698,17 @@ private static async ValueTask WriteGatherAtOffsetMultipleSyscallsAsync(SafeFile
             }
         }
 
-        private static unsafe NativeOverlapped* GetNativeOverlappedForAsyncHandle(ThreadPoolBoundHandle threadPoolBinding, long fileOffset, CallbackResetEvent resetEvent)
+        private static unsafe NativeOverlapped* GetNativeOverlappedForAsyncHandle(SafeFileHandle handle, long fileOffset, CallbackResetEvent resetEvent)
         {
             // After SafeFileHandle is bound to ThreadPool, we need to use ThreadPoolBinding
             // to allocate a native overlapped and provide a valid callback.
-            NativeOverlapped* result = threadPoolBinding.AllocateNativeOverlapped(s_callback, resetEvent, null);
+            NativeOverlapped* result = handle.ThreadPoolBinding!.AllocateNativeOverlapped(s_callback, resetEvent, null);
 
-            // For pipes the offsets are ignored by the OS
-            result->OffsetLow = unchecked((int)fileOffset);
-            result->OffsetHigh = (int)(fileOffset >> 32);
+            if (handle.CanSeek)
+            {
+                result->OffsetLow = unchecked((int)fileOffset);
+                result->OffsetHigh = (int)(fileOffset >> 32);
+            }
 
             // From https://docs.microsoft.com/en-us/windows/win32/api/ioapiset/nf-ioapiset-getoverlappedresult:
             // "If the hEvent member of the OVERLAPPED structure is NULL, the system uses the state of the hFile handle to signal when the operation has been completed.