diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ReadFile_SafeHandle_NativeOverlapped.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ReadFile_SafeHandle_NativeOverlapped.cs index 8419932249299..c6fb2c4311cf3 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ReadFile_SafeHandle_NativeOverlapped.cs +++ b/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); } } diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.WriteFile_SafeHandle_NativeOverlapped.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.WriteFile_SafeHandle_NativeOverlapped.cs index cca93e0b3ac89..3eaff9ca07626 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.WriteFile_SafeHandle_NativeOverlapped.cs +++ b/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); } } diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/SafeFileHandle.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/SafeFileHandle.cs index 57ce00dea3d21..c726519b01375 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/SafeFileHandle.cs +++ b/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); diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileStream.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStream.cs index 439f00c0c5c8f..eb9d98750f758 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileStream.cs +++ b/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) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/AsyncWindowsFileStreamStrategy.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/AsyncWindowsFileStreamStrategy.cs index b02366ecfd13d..b7f24351a0bda 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/AsyncWindowsFileStreamStrategy.cs +++ b/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 ReadAsyncInternal(Memory 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 ReadAsyncInternal(Memory 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 ReadAsyncInternal(Memory 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 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,7 +302,7 @@ private unsafe Task WriteAsyncInternalCore(ReadOnlyMemory source, Cancella { if (!_fileHandle.IsClosed && CanSeek) // Update Position - It could be anywhere. { - SeekCore(_fileHandle, 0, SeekOrigin.Current); + _filePosition = positionBefore; } completionSource.ReleaseNativeResource(); @@ -382,24 +364,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; } } } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Unix.cs index 4d3639185feb4..9af81a5136d04 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Unix.cs +++ b/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) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Windows.cs index f565d1f1e0f3b..661e84b10f670 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Windows.cs @@ -19,7 +19,7 @@ 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) { @@ -27,8 +27,8 @@ private static FileStreamStrategy ChooseStrategyCore(SafeFileHandle handle, File } 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 bytes, NativeOverlapped* overlapped, out int errorCode) + internal static unsafe int ReadFileNative(SafeFileHandle handle, Span 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 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 } } - internal static unsafe int WriteFileNative(SafeFileHandle handle, ReadOnlySpan buffer, NativeOverlapped* overlapped, out int errorCode) + internal static unsafe int WriteFileNative(SafeFileHandle handle, ReadOnlySpan 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= 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. diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.cs index 5eb9eabaeeb97..62e344db6e4f0 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.cs +++ b/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)); diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/LegacyFileStreamStrategy.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/LegacyFileStreamStrategy.Windows.cs index 8890bf3053474..e562f0c2e2151 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/LegacyFileStreamStrategy.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/LegacyFileStreamStrategy.Windows.cs @@ -1079,14 +1079,14 @@ private unsafe int ReadFileNative(SafeFileHandle handle, Span 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 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) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/SyncWindowsFileStreamStrategy.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/SyncWindowsFileStreamStrategy.cs index bcd9c67e7f8ae..3639b4b5fb4da 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/SyncWindowsFileStreamStrategy.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/SyncWindowsFileStreamStrategy.cs @@ -11,7 +11,7 @@ namespace System.IO.Strategies { internal sealed class SyncWindowsFileStreamStrategy : WindowsFileStreamStrategy { - internal SyncWindowsFileStreamStrategy(SafeFileHandle handle, FileAccess access) : base(handle, access) + internal SyncWindowsFileStreamStrategy(SafeFileHandle handle, FileAccess access, FileShare share) : base(handle, access, share) { } @@ -104,10 +104,8 @@ private unsafe int ReadSpan(Span destination) Debug.Assert(!_fileHandle.IsClosed, "!_handle.IsClosed"); - // Make sure we are reading from the right spot - VerifyOSHandlePosition(); - - 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) { @@ -139,10 +137,8 @@ private unsafe void WriteSpan(ReadOnlySpan source) Debug.Assert(!_fileHandle.IsClosed, "!_handle.IsClosed"); - // Make sure we are writing to the position that we think we are - VerifyOSHandlePosition(); - - 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) { @@ -163,7 +159,17 @@ private unsafe void WriteSpan(ReadOnlySpan source) } Debug.Assert(r >= 0, "FileStream's WriteCore is likely broken."); _filePosition += r; - return; + UpdateLengthOnChangePosition(); + } + + 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; } } } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/WindowsFileStreamStrategy.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/WindowsFileStreamStrategy.cs index d238bc23e4c4e..c178c5e47daa6 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/WindowsFileStreamStrategy.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/WindowsFileStreamStrategy.cs @@ -21,22 +21,22 @@ internal abstract class WindowsFileStreamStrategy : FileStreamStrategy protected readonly SafeFileHandle _fileHandle; // only ever null if ctor throws protected readonly string? _path; // The path to the opened file. private readonly FileAccess _access; // What file was opened for. + private readonly FileShare _share; private readonly bool _canSeek; // Whether can seek (file) or not (pipe). private readonly bool _isPipe; // Whether to disable async buffering code. protected long _filePosition; - protected bool _exposedHandle; // Whether the file stream's handle has been exposed. private long _appendStart; // When appending, prevent overwriting file. + private long _length = -1; // When the file is locked for writes (_share <= FileShare.Read) cache file length in-memory, negative means that hasn't been fetched. - internal WindowsFileStreamStrategy(SafeFileHandle handle, FileAccess access) + internal WindowsFileStreamStrategy(SafeFileHandle handle, FileAccess access, FileShare share) { - _exposedHandle = true; - InitFromHandle(handle, access, out _canSeek, out _isPipe); // Note: Cleaner to set the following fields in ValidateAndInitFromHandle, // but we can't as they're readonly. _access = access; + _share = share; // As the handle was passed in, we must set the handle field at the very end to // avoid the finalizer closing the handle when we throw errors. @@ -49,6 +49,7 @@ internal WindowsFileStreamStrategy(string path, FileMode mode, FileAccess access _path = fullPath; _access = access; + _share = share; _fileHandle = FileStreamHelpers.OpenHandle(fullPath, mode, access, share, options); @@ -74,35 +75,51 @@ internal WindowsFileStreamStrategy(string path, FileMode mode, FileAccess access public sealed override bool CanWrite => !_fileHandle.IsClosed && (_access & FileAccess.Write) != 0; - public unsafe sealed override long Length => FileStreamHelpers.GetFileLength(_fileHandle, _path); + // When the file is locked for writes we can cache file length in memory + // and avoid subsequent native calls which are expensive. + public unsafe sealed override long Length => _share > FileShare.Read ? + FileStreamHelpers.GetFileLength(_fileHandle, _path) : + _length < 0 ? _length = FileStreamHelpers.GetFileLength(_fileHandle, _path) : _length; - /// Gets or sets the position within the current stream - public override long Position + protected void UpdateLengthOnChangePosition() { - get + // Do not update the cached length if the file is not locked + // or if the length hasn't been fetched. + if (_share > FileShare.Read || _length < 0) { - VerifyOSHandlePosition(); - - return _filePosition; + Debug.Assert(_length < 0); + return; } - set + + if (_filePosition > _length) { - Seek(value, SeekOrigin.Begin); + _length = _filePosition; } } + /// Gets or sets the position within the current stream + public override long Position + { + get => _filePosition; + set => _filePosition = value; + } + internal sealed override string Name => _path ?? SR.IO_UnknownFileName; internal sealed override bool IsClosed => _fileHandle.IsClosed; internal sealed override bool IsPipe => _isPipe; - + // Flushing is the responsibility of BufferedFileStreamStrategy internal sealed override SafeFileHandle SafeFileHandle { get { - // Flushing is the responsibility of BufferedFileStreamStrategy - _exposedHandle = true; + if (CanSeek) + { + // Update the file offset before exposing it since it's possible that + // in memory position is out-of-sync with the actual file position. + FileStreamHelpers.Seek(_fileHandle, _path, _filePosition, SeekOrigin.Begin); + } return _fileHandle; } } @@ -165,33 +182,34 @@ public sealed override long Seek(long offset, SeekOrigin origin) if (_fileHandle.IsClosed) ThrowHelper.ThrowObjectDisposedException_FileClosed(); if (!CanSeek) ThrowHelper.ThrowNotSupportedException_UnseekableStream(); - // Verify that internal position is in sync with the handle - VerifyOSHandlePosition(); - 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); @@ -209,7 +227,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 { @@ -243,9 +261,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) @@ -256,45 +280,16 @@ 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"); - VerifyOSHandlePosition(); FileStreamHelpers.SetFileLength(_fileHandle, _path, value); + _length = value; if (_filePosition > value) { - SeekCore(_fileHandle, 0, SeekOrigin.End); - } - } - - /// - /// Verify that the actual position of the OS's handle equals what we expect it to. - /// This will fail if someone else moved the UnixFileStream's handle or if - /// our position updating code is incorrect. - /// - [MethodImpl(MethodImplOptions.AggressiveInlining)] - protected void VerifyOSHandlePosition() - { - bool verifyPosition = _exposedHandle; // in release, only verify if we've given out the handle such that someone else could be manipulating it -#if DEBUG - verifyPosition = true; // in debug, always make sure our position matches what the OS says it should be -#endif - if (verifyPosition && CanSeek) - { - long oldPos = _filePosition; // SeekCore will override the current _position, so save it now - long curPos = SeekCore(_fileHandle, 0, SeekOrigin.Current); - if (oldPos != curPos) - { - // For reads, this is non-fatal but we still could have returned corrupted - // data in some cases, so discard the internal buffer. For writes, - // this is a problem; discard the buffer and error out. - - throw new IOException(SR.IO_FileStreamHandlePosition); - } + _filePosition = value; } } }