FileStream optimizations

src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ReadFile_SafeHandle_NativeOverlapped.cs

@@ -16,5 +16,13 @@ internal static extern unsafe int ReadFile(
             int numBytesToRead,
             IntPtr numBytesRead_mustBeZero,
             NativeOverlapped* overlapped);
+
+        [DllImport(Libraries.Kernel32, SetLastError = true)]
+        internal static extern unsafe int ReadFile(
+            SafeHandle handle,
+            byte* bytes,
+            int numBytesToRead,
+            out int numBytesRead,
+            NativeOverlapped* overlapped);
     }
 }

src/libraries/Common/src/Interop/Windows/Kernel32/Interop.WriteFile_SafeHandle_NativeOverlapped.cs

@@ -17,5 +17,8 @@ internal static partial class Kernel32
         // and pass in an address for the numBytesRead parameter.
         [DllImport(Libraries.Kernel32, SetLastError = true)]
         internal static extern unsafe int WriteFile(SafeHandle handle, byte* bytes, int numBytesToWrite, IntPtr numBytesWritten_mustBeZero, NativeOverlapped* lpOverlapped);
+
+        [DllImport(Libraries.Kernel32, SetLastError = true)]
+        internal static extern unsafe int WriteFile(SafeHandle handle, byte* bytes, int numBytesToWrite, out int numBytesWritten, NativeOverlapped* lpOverlapped);
     }
 }

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

@@ -66,13 +66,13 @@ public void AccessFlushesFileClosesHandle()
             }
         }
 
-        [Fact]
+        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsLegacyFileStreamEnabled))]
         public async Task ThrowWhenHandlePositionIsChanged_sync()
         {
             await ThrowWhenHandlePositionIsChanged(useAsync: false);
         }
 
-        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))]
+        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported), nameof(PlatformDetection.IsLegacyFileStreamEnabled))]
         public async Task ThrowWhenHandlePositionIsChanged_async()
         {
             await ThrowWhenHandlePositionIsChanged(useAsync: true);

src/libraries/System.Private.CoreLib/src/System/IO/FileStream.cs

@@ -47,7 +47,7 @@ public FileStream(IntPtr handle, FileAccess access, bool ownsHandle, int bufferS
             {
                 ValidateHandle(safeHandle, access, bufferSize, isAsync);
 
-                _strategy = FileStreamHelpers.ChooseStrategy(this, safeHandle, access, bufferSize, isAsync);
+                _strategy = FileStreamHelpers.ChooseStrategy(this, safeHandle, access, DefaultShare, bufferSize, isAsync);
             }
             catch
             {
@@ -101,7 +101,7 @@ public FileStream(SafeFileHandle handle, FileAccess access, int bufferSize, bool
         {
             ValidateHandle(handle, access, bufferSize, isAsync);
 
-            _strategy = FileStreamHelpers.ChooseStrategy(this, handle, access, bufferSize, isAsync);
+            _strategy = FileStreamHelpers.ChooseStrategy(this, handle, access, DefaultShare, bufferSize, isAsync);
         }
 
         public FileStream(string path, FileMode mode)

src/libraries/System.Private.CoreLib/src/System/IO/Strategies/AsyncWindowsFileStreamStrategy.cs

@@ -13,8 +13,8 @@ internal sealed partial class AsyncWindowsFileStreamStrategy : WindowsFileStream
         private PreAllocatedOverlapped? _preallocatedOverlapped;     // optimization for async ops to avoid per-op allocations
         private FileStreamCompletionSource? _currentOverlappedOwner; // async op currently using the preallocated overlapped
 
-        internal AsyncWindowsFileStreamStrategy(SafeFileHandle handle, FileAccess access)
-            : base(handle, access)
+        internal AsyncWindowsFileStreamStrategy(SafeFileHandle handle, FileAccess access, FileShare share)
+            : base(handle, access, share)
         {
         }
 
@@ -98,7 +98,6 @@ protected override void OnInit()
                 if (_fileHandle.ThreadPoolBinding == null)
                 {
                     // We should close the handle so that the handle is not open until SafeFileHandle GC
-                    Debug.Assert(!_exposedHandle, "Are we closing handle that we exposed/not own, how?");
                     _fileHandle.Dispose();
                 }
             }
@@ -142,18 +141,16 @@ private unsafe Task<int> ReadAsyncInternal(Memory<byte> destination, Cancellatio
             NativeOverlapped* intOverlapped = completionSource.Overlapped;
 
             // Calculate position in the file we should be at after the read is done
+            long positionBefore = _filePosition;
             if (CanSeek)
             {
                 long len = Length;
 
-                // Make sure we are reading from the position that we think we are
-                VerifyOSHandlePosition();
-
-                if (destination.Length > len - _filePosition)
+                if (positionBefore + destination.Length > len)
                 {
-                    if (_filePosition <= len)
+                    if (positionBefore <= len)
                     {
-                        destination = destination.Slice(0, (int)(len - _filePosition));
+                        destination = destination.Slice(0, (int)(len - positionBefore));
                     }
                     else
                     {
@@ -163,23 +160,17 @@ private unsafe Task<int> ReadAsyncInternal(Memory<byte> destination, Cancellatio
 
                 // Now set the position to read from in the NativeOverlapped struct
                 // For pipes, we should leave the offset fields set to 0.
-                intOverlapped->OffsetLow = unchecked((int)_filePosition);
-                intOverlapped->OffsetHigh = (int)(_filePosition >> 32);
+                intOverlapped->OffsetLow = unchecked((int)positionBefore);
+                intOverlapped->OffsetHigh = (int)(positionBefore >> 32);
 
                 // When using overlapped IO, the OS is not supposed to
                 // touch the file pointer location at all.  We will adjust it
-                // ourselves. This isn't threadsafe.
-
-                // WriteFile should not update the file pointer when writing
-                // in overlapped mode, according to MSDN.  But it does update
-                // the file pointer when writing to a UNC path!
-                // So changed the code below to seek to an absolute
-                // location, not a relative one.  ReadFile seems consistent though.
-                SeekCore(_fileHandle, destination.Length, SeekOrigin.Current);
+                // ourselves, but only in memory. This isn't threadsafe.
+                _filePosition += destination.Length;
             }
 
             // queue an async ReadFile operation and pass in a packed overlapped
-            int r = FileStreamHelpers.ReadFileNative(_fileHandle, destination.Span, intOverlapped, out int errorCode);
+            int r = FileStreamHelpers.ReadFileNative(_fileHandle, destination.Span, false, intOverlapped, out int errorCode);
 
             // ReadFile, the OS version, will return 0 on failure.  But
             // my ReadFileNative wrapper returns -1.  My wrapper will return
@@ -208,7 +199,7 @@ private unsafe Task<int> ReadAsyncInternal(Memory<byte> destination, Cancellatio
                 {
                     if (!_fileHandle.IsClosed && CanSeek)  // Update Position - It could be anywhere.
                     {
-                        SeekCore(_fileHandle, 0, SeekOrigin.Current);
+                        _filePosition = positionBefore;
                     }
 
                     completionSource.ReleaseNativeResource();
@@ -269,32 +260,23 @@ private unsafe Task WriteAsyncInternalCore(ReadOnlyMemory<byte> source, Cancella
             FileStreamCompletionSource completionSource = FileStreamCompletionSource.Create(this, _preallocatedOverlapped, 0, source);
             NativeOverlapped* intOverlapped = completionSource.Overlapped;
 
+            long positionBefore = _filePosition;
             if (CanSeek)
             {
-                // Make sure we set the length of the file appropriately.
-                long len = Length;
-
-                // Make sure we are writing to the position that we think we are
-                VerifyOSHandlePosition();
-
-                if (_filePosition + source.Length > len)
-                {
-                    SetLengthCore(_filePosition + source.Length);
-                }
-
                 // Now set the position to read from in the NativeOverlapped struct
                 // For pipes, we should leave the offset fields set to 0.
-                intOverlapped->OffsetLow = (int)_filePosition;
-                intOverlapped->OffsetHigh = (int)(_filePosition >> 32);
+                intOverlapped->OffsetLow = (int)positionBefore;
+                intOverlapped->OffsetHigh = (int)(positionBefore >> 32);
 
                 // When using overlapped IO, the OS is not supposed to
                 // touch the file pointer location at all.  We will adjust it
-                // ourselves.  This isn't threadsafe.
-                SeekCore(_fileHandle, source.Length, SeekOrigin.Current);
+                // ourselves, but only in memory.  This isn't threadsafe.
+                _filePosition += source.Length;
+                UpdateLengthOnChangePosition();
             }
 
             // queue an async WriteFile operation and pass in a packed overlapped
-            int r = FileStreamHelpers.WriteFileNative(_fileHandle, source.Span, intOverlapped, out int errorCode);
+            int r = FileStreamHelpers.WriteFileNative(_fileHandle, source.Span, false, intOverlapped, out int errorCode);
 
             // WriteFile, the OS version, will return 0 on failure.  But
             // my WriteFileNative wrapper returns -1.  My wrapper will return
@@ -320,19 +302,11 @@ private unsafe Task WriteAsyncInternalCore(ReadOnlyMemory<byte> source, Cancella
                 {
                     if (!_fileHandle.IsClosed && CanSeek)  // Update Position - It could be anywhere.
                     {
-                        SeekCore(_fileHandle, 0, SeekOrigin.Current);
+                        _filePosition = positionBefore;
                     }
 
                     completionSource.ReleaseNativeResource();
-
-                    if (errorCode == ERROR_HANDLE_EOF)
-                    {
-                        ThrowHelper.ThrowEndOfFileException();
-                    }
-                    else
-                    {
-                        throw Win32Marshal.GetExceptionForWin32Error(errorCode, _path);
-                    }
+                    throw Win32Marshal.GetExceptionForWin32Error(errorCode, _path);
                 }
                 else if (cancellationToken.CanBeCanceled) // ERROR_IO_PENDING
                 {
@@ -382,24 +356,18 @@ private async Task AsyncModeCopyToAsync(Stream destination, int bufferSize, Canc
             Debug.Assert(!_fileHandle.IsClosed, "!_handle.IsClosed");
             Debug.Assert(CanRead, "_parent.CanRead");
 
-            bool canSeek = CanSeek;
-            if (canSeek)
-            {
-                VerifyOSHandlePosition();
-            }
-
             try
             {
                 await FileStreamHelpers
-                    .AsyncModeCopyToAsync(_fileHandle, _path, canSeek, _filePosition, destination, bufferSize, cancellationToken)
+                    .AsyncModeCopyToAsync(_fileHandle, _path, CanSeek, _filePosition, destination, bufferSize, cancellationToken)
                     .ConfigureAwait(false);
             }
             finally
             {
                 // Make sure the stream's current position reflects where we ended up
-                if (!_fileHandle.IsClosed && canSeek)
+                if (!_fileHandle.IsClosed && CanSeek)
                 {
-                    SeekCore(_fileHandle, 0, SeekOrigin.End);
+                    _filePosition = Length;
                 }
             }
         }

src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Unix.cs

@@ -14,7 +14,7 @@ namespace System.IO.Strategies
     internal static partial class FileStreamHelpers
     {
         // in the future we are most probably going to introduce more strategies (io_uring etc)
-        private static FileStreamStrategy ChooseStrategyCore(SafeFileHandle handle, FileAccess access, int bufferSize, bool isAsync)
+        private static FileStreamStrategy ChooseStrategyCore(SafeFileHandle handle, FileAccess access, FileShare share, int bufferSize, bool isAsync)
             => new LegacyFileStreamStrategy(handle, access, bufferSize, isAsync);
 
         private static FileStreamStrategy ChooseStrategyCore(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, FileOptions options)

src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Windows.cs

@@ -19,16 +19,16 @@ internal static partial class FileStreamHelpers
         private const int ERROR_HANDLE_EOF = 38;
         private const int ERROR_IO_PENDING = 997;
 
-        private static FileStreamStrategy ChooseStrategyCore(SafeFileHandle handle, FileAccess access, int bufferSize, bool isAsync)
+        private static FileStreamStrategy ChooseStrategyCore(SafeFileHandle handle, FileAccess access, FileShare share, int bufferSize, bool isAsync)
         {
             if (UseLegacyStrategy)
             {
                 return new LegacyFileStreamStrategy(handle, access, bufferSize, isAsync);
             }
 
             WindowsFileStreamStrategy strategy = isAsync
-                ? new AsyncWindowsFileStreamStrategy(handle, access)
-                : new SyncWindowsFileStreamStrategy(handle, access);
+                ? new AsyncWindowsFileStreamStrategy(handle, access, share)
+                : new SyncWindowsFileStreamStrategy(handle, access, share);
 
             return EnableBufferingIfNeeded(strategy, bufferSize);
         }
@@ -333,7 +333,7 @@ internal static unsafe void SetFileLength(SafeFileHandle handle, string? path, l
         }
 
         // __ConsoleStream also uses this code.
-        internal static unsafe int ReadFileNative(SafeFileHandle handle, Span<byte> bytes, NativeOverlapped* overlapped, out int errorCode)
+        internal static unsafe int ReadFileNative(SafeFileHandle handle, Span<byte> bytes, bool syncUsingOverlapped, NativeOverlapped* overlapped, out int errorCode)
         {
             Debug.Assert(handle != null, "handle != null");
 
@@ -343,13 +343,24 @@ internal static unsafe int ReadFileNative(SafeFileHandle handle, Span<byte> byte
             fixed (byte* p = &MemoryMarshal.GetReference(bytes))
             {
                 r = overlapped != null ?
-                    Interop.Kernel32.ReadFile(handle, p, bytes.Length, IntPtr.Zero, overlapped) :
-                    Interop.Kernel32.ReadFile(handle, p, bytes.Length, out numBytesRead, IntPtr.Zero);
+                    (syncUsingOverlapped
+                        ? Interop.Kernel32.ReadFile(handle, p, bytes.Length, out numBytesRead, overlapped)
+                        : Interop.Kernel32.ReadFile(handle, p, bytes.Length, IntPtr.Zero, overlapped))
+                    : Interop.Kernel32.ReadFile(handle, p, bytes.Length, out numBytesRead, IntPtr.Zero);
             }
 
             if (r == 0)
             {
                 errorCode = GetLastWin32ErrorAndDisposeHandleIfInvalid(handle);
+
+                if (syncUsingOverlapped && errorCode == Interop.Errors.ERROR_HANDLE_EOF)
+                {
+                    // https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-readfile#synchronization-and-file-position :
+                    // "If lpOverlapped is not NULL, then when a synchronous read operation reaches the end of a file,
+                    // ReadFile returns FALSE and GetLastError returns ERROR_HANDLE_EOF"
+                    return numBytesRead;
+                }
+
                 return -1;
             }
             else
@@ -359,7 +370,7 @@ internal static unsafe int ReadFileNative(SafeFileHandle handle, Span<byte> byte
             }
         }
 
-        internal static unsafe int WriteFileNative(SafeFileHandle handle, ReadOnlySpan<byte> buffer, NativeOverlapped* overlapped, out int errorCode)
+        internal static unsafe int WriteFileNative(SafeFileHandle handle, ReadOnlySpan<byte> buffer, bool syncUsingOverlapped, NativeOverlapped* overlapped, out int errorCode)
         {
             Debug.Assert(handle != null, "handle != null");
 
@@ -369,8 +380,10 @@ internal static unsafe int WriteFileNative(SafeFileHandle handle, ReadOnlySpan<b
             fixed (byte* p = &MemoryMarshal.GetReference(buffer))
             {
                 r = overlapped != null ?
-                    Interop.Kernel32.WriteFile(handle, p, buffer.Length, IntPtr.Zero, overlapped) :
-                    Interop.Kernel32.WriteFile(handle, p, buffer.Length, out numBytesWritten, IntPtr.Zero);
+                    (syncUsingOverlapped
+                        ? Interop.Kernel32.WriteFile(handle, p, buffer.Length, out numBytesWritten, overlapped)
+                        : Interop.Kernel32.WriteFile(handle, p, buffer.Length, IntPtr.Zero, overlapped))
+                    : Interop.Kernel32.WriteFile(handle, p, buffer.Length, out numBytesWritten, IntPtr.Zero);
             }
 
             if (r == 0)
@@ -460,7 +473,7 @@ internal static async Task AsyncModeCopyToAsync(SafeFileHandle handle, string? p
                             }
 
                             // Kick off the read.
-                            synchronousSuccess = ReadFileNative(handle, copyBuffer, readAwaitable._nativeOverlapped, out errorCode) >= 0;
+                            synchronousSuccess = ReadFileNative(handle, copyBuffer, false, readAwaitable._nativeOverlapped, out errorCode) >= 0;
                         }
 
                         // If the operation did not synchronously succeed, it either failed or initiated the asynchronous operation.

src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.cs

@@ -23,8 +23,8 @@ private static bool GetLegacyFileStreamSetting()
                 : bool.IsTrueStringIgnoreCase(envVar) || envVar.Equals("1");
         }
 
-        internal static FileStreamStrategy ChooseStrategy(FileStream fileStream, SafeFileHandle handle, FileAccess access, int bufferSize, bool isAsync)
-            => WrapIfDerivedType(fileStream, ChooseStrategyCore(handle, access, bufferSize, isAsync));
+        internal static FileStreamStrategy ChooseStrategy(FileStream fileStream, SafeFileHandle handle, FileAccess access, FileShare share, int bufferSize, bool isAsync)
+            => WrapIfDerivedType(fileStream, ChooseStrategyCore(handle, access, share, bufferSize, isAsync));
 
         internal static FileStreamStrategy ChooseStrategy(FileStream fileStream, string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, FileOptions options)
             => WrapIfDerivedType(fileStream, ChooseStrategyCore(path, mode, access, share, bufferSize, options));

src/libraries/System.Private.CoreLib/src/System/IO/Strategies/LegacyFileStreamStrategy.Windows.cs

@@ -1037,15 +1037,7 @@ private unsafe Task WriteAsyncInternalCore(ReadOnlyMemory<byte> source, Cancella
                     }
 
                     completionSource.ReleaseNativeResource();
-
-                    if (errorCode == ERROR_HANDLE_EOF)
-                    {
-                        ThrowHelper.ThrowEndOfFileException();
-                    }
-                    else
-                    {
-                        throw Win32Marshal.GetExceptionForWin32Error(errorCode, _path);
-                    }
+                    throw Win32Marshal.GetExceptionForWin32Error(errorCode, _path);
                 }
                 else if (cancellationToken.CanBeCanceled) // ERROR_IO_PENDING
                 {
@@ -1079,14 +1071,14 @@ private unsafe int ReadFileNative(SafeFileHandle handle, Span<byte> bytes, Nativ
         {
             Debug.Assert((_useAsyncIO && overlapped != null) || (!_useAsyncIO && overlapped == null), "Async IO and overlapped parameters inconsistent in call to ReadFileNative.");
 
-            return FileStreamHelpers.ReadFileNative(handle, bytes, overlapped, out errorCode);
+            return FileStreamHelpers.ReadFileNative(handle, bytes, false, overlapped, out errorCode);
         }
 
         private unsafe int WriteFileNative(SafeFileHandle handle, ReadOnlySpan<byte> buffer, NativeOverlapped* overlapped, out int errorCode)
         {
             Debug.Assert((_useAsyncIO && overlapped != null) || (!_useAsyncIO && overlapped == null), "Async IO and overlapped parameters inconsistent in call to WriteFileNative.");
 
-            return FileStreamHelpers.WriteFileNative(handle, buffer, overlapped, out errorCode);
+            return FileStreamHelpers.WriteFileNative(handle, buffer, false, overlapped, out errorCode);
         }
 
         public override Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken)