track the file offset in memory, don't use expensive sys calls to syn…

…chronize it

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.Private.CoreLib/src/System/IO/AsyncWindowsFileStreamStrategy.cs

@@ -140,15 +140,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;
 
-                if (_filePosition + destination.Length > len)
+                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
                     {
@@ -158,23 +159,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
@@ -203,7 +198,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();
@@ -264,29 +259,30 @@ 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;
 
-                if (_filePosition + source.Length > len)
+                if (positionBefore + source.Length > len)
                 {
-                    SetLengthCore(_filePosition + source.Length);
+                    SetLengthCore(positionBefore + 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;
             }
 
             // 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
@@ -312,7 +308,7 @@ 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();
@@ -385,7 +381,7 @@ private async Task AsyncModeCopyToAsync(Stream destination, int bufferSize, Canc
                 // Make sure the stream's current position reflects where we ended up
                 if (!_fileHandle.IsClosed && CanSeek)
                 {
-                    SeekCore(_fileHandle, 0, SeekOrigin.End);
+                    _filePosition = Length;
                 }
             }
         }

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

@@ -337,7 +337,7 @@ internal static unsafe void SetLength(SafeFileHandle handle, string? path, long
         }
 
         // __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");
 
@@ -347,13 +347,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
@@ -363,7 +374,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");
 
@@ -373,13 +384,24 @@ 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)
             {
                 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 numBytesWritten;
+                }
+
                 return -1;
             }
             else
@@ -464,7 +486,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/LegacyFileStreamStrategy.Windows.cs

@@ -1079,14 +1079,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)

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

@@ -104,7 +104,8 @@ private unsafe int ReadSpan(Span<byte> destination)
 
             Debug.Assert(!_fileHandle.IsClosed, "!_handle.IsClosed");
 
-            int r = FileStreamHelpers.ReadFileNative(_fileHandle, destination, null, out int errorCode);
+            NativeOverlapped nativeOverlapped = GetNativeOverlappedForCurrentPosition();
+            int r = FileStreamHelpers.ReadFileNative(_fileHandle, destination, true, &nativeOverlapped, out int errorCode);
 
             if (r == -1)
             {
@@ -136,7 +137,8 @@ private unsafe void WriteSpan(ReadOnlySpan<byte> source)
 
             Debug.Assert(!_fileHandle.IsClosed, "!_handle.IsClosed");
 
-            int r = FileStreamHelpers.WriteFileNative(_fileHandle, source, null, out int errorCode);
+            NativeOverlapped nativeOverlapped = GetNativeOverlappedForCurrentPosition();
+            int r = FileStreamHelpers.WriteFileNative(_fileHandle, source, true, &nativeOverlapped, out int errorCode);
 
             if (r == -1)
             {
@@ -159,5 +161,15 @@ private unsafe void WriteSpan(ReadOnlySpan<byte> source)
             _filePosition += r;
             return;
         }
+
+        private NativeOverlapped GetNativeOverlappedForCurrentPosition()
+        {
+            NativeOverlapped nativeOverlapped = default;
+            // For pipes the offsets are ignored by the OS
+            nativeOverlapped.OffsetLow = unchecked((int)_filePosition);
+            nativeOverlapped.OffsetHigh = (int)(_filePosition >> 32);
+
+            return nativeOverlapped;
+        }
     }
 }

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

@@ -83,14 +83,16 @@ internal WindowsFileStreamStrategy(string path, FileMode mode, FileAccess access
         public override long Position
         {
             get => _filePosition;
-            set => Seek(value, SeekOrigin.Begin);
+            set => _filePosition = value;
         }
 
         internal sealed override string Name => _path ?? SR.IO_UnknownFileName;
 
         internal sealed override bool IsClosed => _fileHandle.IsClosed;
 
         // Flushing is the responsibility of BufferedFileStreamStrategy
+        // TODO: we might consider calling FileStreamHelpers.Seek(handle, _path, _filePosition, SeekOrigin.Begin);
+        // here to set the actual file offset
         internal sealed override SafeFileHandle SafeFileHandle => _fileHandle;
 
         // ReadByte and WriteByte methods are used only when the user has disabled buffering on purpose
@@ -152,29 +154,33 @@ public sealed override long Seek(long offset, SeekOrigin origin)
             if (!CanSeek) throw Error.GetSeekNotSupported();
 
             long oldPos = _filePosition;
-            long pos = SeekCore(_fileHandle, offset, origin);
+            long pos = origin switch
+            {
+                SeekOrigin.Begin => offset,
+                SeekOrigin.End => FileStreamHelpers.GetFileLength(_fileHandle, _path) + offset,
+                _ => _filePosition + offset // SeekOrigin.Current
+            };
+
+            if (pos >= 0)
+            {
+                _filePosition = pos;
+            }
+            else
+            {
+                // keep throwing the same exception we did when seek was causing actual offset change
+                throw Win32Marshal.GetExceptionForWin32Error(Interop.Errors.ERROR_INVALID_PARAMETER);
+            }
 
-            // Prevent users from overwriting data in a file that was opened in
-            // append mode.
+            // Prevent users from overwriting data in a file that was opened in append mode.
             if (_appendStart != -1 && pos < _appendStart)
             {
-                SeekCore(_fileHandle, oldPos, SeekOrigin.Begin);
+                _filePosition = oldPos;
                 throw new IOException(SR.IO_SeekAppendOverwrite);
             }
 
             return pos;
         }
 
-        // This doesn't do argument checking.  Necessary for SetLength, which must
-        // set the file pointer beyond the end of the file. This will update the
-        // internal position
-        protected long SeekCore(SafeFileHandle fileHandle, long offset, SeekOrigin origin, bool closeInvalidHandle = false)
-        {
-            Debug.Assert(!fileHandle.IsClosed && _canSeek, "!fileHandle.IsClosed && _canSeek");
-
-            return _filePosition = FileStreamHelpers.Seek(fileHandle, _path, offset, origin, closeInvalidHandle);
-        }
-
         internal sealed override void Lock(long position, long length) => FileStreamHelpers.Lock(_fileHandle, _path, position, length);
 
         internal sealed override void Unlock(long position, long length) => FileStreamHelpers.Unlock(_fileHandle, _path, position, length);
@@ -192,7 +198,7 @@ private void Init(FileMode mode, string originalPath)
             // For Append mode...
             if (mode == FileMode.Append)
             {
-                _appendStart = SeekCore(_fileHandle, 0, SeekOrigin.End);
+                _appendStart = _filePosition = Length;
             }
             else
             {
@@ -226,9 +232,15 @@ private void InitFromHandleImpl(SafeFileHandle handle, out bool canSeek, out boo
             OnInitFromHandle(handle);
 
             if (_canSeek)
-                SeekCore(handle, 0, SeekOrigin.Current);
+            {
+                // given strategy was created out of existing handle, so we have to perform
+                // a syscall to get the current handle offset
+                _filePosition = FileStreamHelpers.Seek(handle, _path, 0, SeekOrigin.Current);
+            }
             else
+            {
                 _filePosition = 0;
+            }
         }
 
         public sealed override void SetLength(long value)
@@ -239,8 +251,6 @@ public sealed override void SetLength(long value)
             SetLengthCore(value);
         }
 
-        // We absolutely need this method broken out so that WriteInternalCoreAsync can call
-        // a method without having to go through buffering code that might call FlushWrite.
         protected unsafe void SetLengthCore(long value)
         {
             Debug.Assert(value >= 0, "value >= 0");
@@ -249,7 +259,7 @@ protected unsafe void SetLengthCore(long value)
 
             if (_filePosition > value)
             {
-                SeekCore(_fileHandle, 0, SeekOrigin.End);
+                _filePosition = value;
             }
         }
     }