FileStreamStrategies refactor

src/libraries/System.IO.FileSystem/tests/LegacyTests/runtimeconfig.template.json

@@ -1,5 +1,5 @@
 {
     "configProperties": {
-        "System.IO.UseLegacyFileStream": true
+        "System.IO.UseNet5CompatFileStream": false
     }
 }

src/libraries/System.IO/tests/LegacyTests/runtimeconfig.template.json

@@ -1,5 +1,5 @@
 {
     "configProperties": {
-        "System.IO.UseLegacyFileStream": true
+        "System.IO.UseNet5CompatFileStream": false
     }
 }

src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems

@@ -392,8 +392,6 @@
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\BinaryReader.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\BinaryWriter.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\BufferedStream.cs" />
-    <Compile Include="$(MSBuildThisFileDirectory)System\IO\BufferedFileStreamStrategy.cs" />
-    <Compile Include="$(MSBuildThisFileDirectory)System\IO\DerivedFileStreamStrategy.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\DirectoryNotFoundException.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\EncodingCache.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\EndOfStreamException.cs" />
@@ -405,12 +403,10 @@
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\FileOptions.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\FileShare.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\FileStream.cs" />
-    <Compile Include="$(MSBuildThisFileDirectory)System\IO\FileStreamStrategy.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\HandleInheritability.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\InvalidDataException.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\IOException.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\MemoryStream.cs" />
-    <Compile Include="$(MSBuildThisFileDirectory)System\IO\LegacyFileStreamStrategy.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\Path.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\PathInternal.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\PathTooLongException.cs" />
@@ -426,6 +422,11 @@
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\UnmanagedMemoryAccessor.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\UnmanagedMemoryStream.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\UnmanagedMemoryStreamWrapper.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)System\IO\Strategies\BufferedFileStreamStrategy.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)System\IO\Strategies\DerivedFileStreamStrategy.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)System\IO\Strategies\FileStreamHelpers.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)System\IO\Strategies\FileStreamStrategy.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)System\IO\Strategies\LegacyFileStreamStrategy.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IObservable.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IObserver.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IProgress.cs" />
@@ -1635,17 +1636,17 @@
     <Compile Include="$(MSBuildThisFileDirectory)System\Globalization\GlobalizationMode.Windows.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Globalization\HijriCalendar.Win32.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Guid.Windows.cs" />
-    <Compile Include="$(MSBuildThisFileDirectory)System\IO\AsyncWindowsFileStreamStrategy.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\DisableMediaInsertionPrompt.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\DriveInfoInternal.Windows.cs" />
-    <Compile Include="$(MSBuildThisFileDirectory)System\IO\FileStreamHelpers.Windows.cs" />
-    <Compile Include="$(MSBuildThisFileDirectory)System\IO\FileStreamCompletionSource.Win32.cs" />
-    <Compile Include="$(MSBuildThisFileDirectory)System\IO\LegacyFileStreamStrategy.Windows.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\Path.Windows.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\PathHelper.Windows.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\PathInternal.Windows.cs" />
-    <Compile Include="$(MSBuildThisFileDirectory)System\IO\SyncWindowsFileStreamStrategy.cs" />
-    <Compile Include="$(MSBuildThisFileDirectory)System\IO\WindowsFileStreamStrategy.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)System\IO\Strategies\AsyncWindowsFileStreamStrategy.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)System\IO\Strategies\FileStreamHelpers.Windows.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)System\IO\Strategies\FileStreamCompletionSource.Win32.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)System\IO\Strategies\LegacyFileStreamStrategy.Windows.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)System\IO\Strategies\SyncWindowsFileStreamStrategy.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)System\IO\Strategies\WindowsFileStreamStrategy.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\PasteArguments.Windows.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Runtime\Loader\LibraryNameVariation.Windows.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Runtime\MemoryFailPoint.Windows.cs" />
@@ -1846,13 +1847,13 @@
     <Compile Include="$(MSBuildThisFileDirectory)System\Globalization\GlobalizationMode.LoadICU.iOS.cs" Condition="'$(IsiOSLike)' == 'true'" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Globalization\HijriCalendar.Unix.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Guid.Unix.cs" />
-    <Compile Include="$(MSBuildThisFileDirectory)System\IO\FileStreamHelpers.Unix.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\Path.Unix.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\PathInternal.Unix.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\IO\PersistedFiles.Names.Unix.cs" />
-    <Compile Include="$(MSBuildThisFileDirectory)System\IO\LegacyFileStreamStrategy.Unix.cs" />
-    <Compile Include="$(MSBuildThisFileDirectory)System\IO\LegacyFileStreamStrategy.Lock.OSX.cs" Condition="'$(IsOSXLike)' == 'true'" />
-    <Compile Include="$(MSBuildThisFileDirectory)System\IO\LegacyFileStreamStrategy.Lock.Unix.cs" Condition="'$(IsOSXLike)' != 'true'" />
+    <Compile Include="$(MSBuildThisFileDirectory)System\IO\Strategies\FileStreamHelpers.Unix.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)System\IO\Strategies\LegacyFileStreamStrategy.Unix.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)System\IO\Strategies\LegacyFileStreamStrategy.Lock.OSX.cs" Condition="'$(IsOSXLike)' == 'true'" />
+    <Compile Include="$(MSBuildThisFileDirectory)System\IO\Strategies\LegacyFileStreamStrategy.Lock.Unix.cs" Condition="'$(IsOSXLike)' != 'true'" />
     <Compile Include="$(MSBuildThisFileDirectory)System\PasteArguments.Unix.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Runtime\Loader\LibraryNameVariation.Unix.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Runtime\MemoryFailPoint.Unix.cs" />

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

@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.IO.Strategies;
 using System.Runtime.Serialization;
 using System.Runtime.Versioning;
 using System.Threading;
@@ -46,7 +47,7 @@ public FileStream(IntPtr handle, FileAccess access, bool ownsHandle, int bufferS
             {
                 ValidateHandle(safeHandle, access, bufferSize, isAsync);
 
-                _strategy = WrapIfDerivedType(FileStreamHelpers.ChooseStrategy(safeHandle, access, bufferSize, isAsync));
+                _strategy = FileStreamHelpers.ChooseStrategy(this, safeHandle, access, bufferSize, isAsync);
             }
             catch
             {
@@ -78,9 +79,6 @@ private static void ValidateHandle(SafeFileHandle handle, FileAccess access, int
                 throw new ArgumentException(SR.Arg_HandleNotAsync, nameof(handle));
         }
 
-        private FileStreamStrategy WrapIfDerivedType(FileStreamStrategy impl)
-            => GetType() == typeof(FileStream) ? impl : new DerivedFileStreamStrategy(this, impl);
-
         public FileStream(SafeFileHandle handle, FileAccess access)
             : this(handle, access, DefaultBufferSize)
         {
@@ -95,7 +93,7 @@ public FileStream(SafeFileHandle handle, FileAccess access, int bufferSize, bool
         {
             ValidateHandle(handle, access, bufferSize, isAsync);
 
-            _strategy = WrapIfDerivedType(FileStreamHelpers.ChooseStrategy(handle, access, bufferSize, isAsync));
+            _strategy = FileStreamHelpers.ChooseStrategy(this, handle, access, bufferSize, isAsync);
         }
 
         public FileStream(string path, FileMode mode) :
@@ -164,7 +162,7 @@ public FileStream(string path, FileMode mode, FileAccess access, FileShare share
                 SerializationInfo.ThrowIfDeserializationInProgress("AllowFileWrites", ref s_cachedSerializationSwitch);
             }
 
-            _strategy = WrapIfDerivedType(FileStreamHelpers.ChooseStrategy(path, mode, access, share, bufferSize, options));
+            _strategy = FileStreamHelpers.ChooseStrategy(this, path, mode, access, share, bufferSize, options);
         }
 
         [Obsolete("This property has been deprecated.  Please use FileStream's SafeFileHandle property instead.  https://go.microsoft.com/fwlink/?linkid=14202")]
@@ -479,21 +477,5 @@ internal IAsyncResult BaseBeginWrite(byte[] buffer, int offset, int count, Async
             => base.BeginWrite(buffer, offset, count, callback, state);
 
         internal void BaseEndWrite(IAsyncResult asyncResult) => base.EndWrite(asyncResult);
-
-        internal static bool IsIoRelatedException(Exception e) =>
-            // These all derive from IOException
-            //     DirectoryNotFoundException
-            //     DriveNotFoundException
-            //     EndOfStreamException
-            //     FileLoadException
-            //     FileNotFoundException
-            //     PathTooLongException
-            //     PipeException
-            e is IOException ||
-            // Note that SecurityException is only thrown on runtimes that support CAS
-            // e is SecurityException ||
-            e is UnauthorizedAccessException ||
-            e is NotSupportedException ||
-            (e is ArgumentException && !(e is ArgumentNullException));
     }
 }

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

@@ -6,7 +6,7 @@
 using System.Threading.Tasks;
 using Microsoft.Win32.SafeHandles;
 
-namespace System.IO
+namespace System.IO.Strategies
 {
     internal sealed partial class AsyncWindowsFileStreamStrategy : WindowsFileStreamStrategy, IFileStreamCompletionSourceStrategy
     {
@@ -397,7 +397,7 @@ await FileStreamHelpers
             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);
                 }

src/libraries/System.Private.CoreLib/src/System/IO/BufferedFileStreamStrategy.cs → src/libraries/System.Private.CoreLib/src/System/IO/Strategies/BufferedFileStreamStrategy.cs

@@ -6,7 +6,7 @@
 using System.Threading.Tasks;
 using Microsoft.Win32.SafeHandles;
 
-namespace System.IO
+namespace System.IO.Strategies
 {
     // this type exists so we can avoid duplicating the buffering logic in every FileStreamStrategy implementation
     internal sealed class BufferedFileStreamStrategy : FileStreamStrategy
@@ -37,7 +37,7 @@ internal BufferedFileStreamStrategy(FileStreamStrategy strategy, int bufferSize)
                 // so we enforce it by passing always true
                 Dispose(true);
             }
-            catch (Exception e) when (FileStream.IsIoRelatedException(e))
+            catch (Exception e) when (FileStreamHelpers.IsIoRelatedException(e))
             {
                 // On finalization, ignore failures from trying to flush the write buffer,
                 // e.g. if this stream is wrapping a pipe and the pipe is now broken.
@@ -74,7 +74,7 @@ public override long Position
             {
                 Debug.Assert(!(_writePos > 0 && _readPos != _readLen), "Read and Write buffers cannot both have data in them at the same time.");
 
-                return _strategy.Position + (_readPos - _readLen + _writePos);
+                return _strategy.Position + _readPos - _readLen + _writePos;
             }
             set
             {
@@ -598,7 +598,7 @@ private void WriteByteSlow(byte value)
                 ClearReadBufferBeforeWrite();
                 EnsureBufferAllocated();
             }
-            else if (_writePos >= _bufferSize - 1)
+            else if (_writePos == _bufferSize - 1)
             {
                 FlushWrite();
             }
@@ -1055,7 +1055,7 @@ private void EnsureCanWrite()
 
         private void EnsureBufferAllocated()
         {
-            Debug.Assert(_bufferSize > 0);
+            Debug.Assert(_bufferSize > 1);
 
             // BufferedFileStreamStrategy is not intended for multi-threaded use, so no worries about the get/set race on _buffer.
             if (_buffer == null)

src/libraries/System.Private.CoreLib/src/System/IO/DerivedFileStreamStrategy.cs → src/libraries/System.Private.CoreLib/src/System/IO/Strategies/DerivedFileStreamStrategy.cs

@@ -5,7 +5,7 @@
 using System.Threading.Tasks;
 using Microsoft.Win32.SafeHandles;
 
-namespace System.IO
+namespace System.IO.Strategies
 {
     // this type exists so we can avoid GetType() != typeof(FileStream) checks in FileStream
     // when FileStream was supposed to call base.Method() for such cases, we just call _fileStream.BaseMethod()

src/libraries/System.Private.CoreLib/src/System/IO/FileStreamCompletionSource.Win32.cs → src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamCompletionSource.Win32.cs

@@ -8,7 +8,7 @@
 using System.Threading.Tasks;
 using Microsoft.Win32.SafeHandles;
 
-namespace System.IO
+namespace System.IO.Strategies
 {
     // to avoid code duplicaiton of FileStreamCompletionSource for LegacyFileStreamStrategy and AsyncWindowsFileStreamStrategy
     // we have created the following interface that is a common contract for both of them

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

@@ -8,16 +8,16 @@
 using System.Threading;
 using System.Threading.Tasks;
 
-namespace System.IO
+namespace System.IO.Strategies
 {
     // this type defines a set of stateless FileStream/FileStreamStrategy helper methods
-    internal static class FileStreamHelpers
+    internal static partial class FileStreamHelpers
     {
         // in the future we are most probably going to introduce more strategies (io_uring etc)
-        internal static FileStreamStrategy ChooseStrategy(SafeFileHandle handle, FileAccess access, int bufferSize, bool isAsync)
+        private static FileStreamStrategy ChooseStrategy(SafeFileHandle handle, FileAccess access, int bufferSize, bool isAsync)
             => new LegacyFileStreamStrategy(handle, access, bufferSize, isAsync);
 
-        internal static FileStreamStrategy ChooseStrategy(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, FileOptions options)
+        private static FileStreamStrategy ChooseStrategy(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, FileOptions options)
             => new LegacyFileStreamStrategy(path, mode, access, share, bufferSize, options);
 
         internal static SafeFileHandle OpenHandle(string path, FileMode mode, FileAccess access, FileShare share, FileOptions options)

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

@@ -9,21 +9,17 @@
 using System.Threading.Tasks;
 using Microsoft.Win32.SafeHandles;
 
-namespace System.IO
+namespace System.IO.Strategies
 {
     // this type defines a set of stateless FileStream/FileStreamStrategy helper methods
-    internal static class FileStreamHelpers
+    internal static partial class FileStreamHelpers
     {
         internal const int ERROR_BROKEN_PIPE = 109;
         internal const int ERROR_NO_DATA = 232;
         private const int ERROR_HANDLE_EOF = 38;
         private const int ERROR_IO_PENDING = 997;
 
-        // It's enabled by default. We are going to change that (by removing !) once we fix #16354, #25905 and #24847.
-        internal static bool UseLegacyStrategy { get; }
-            = !AppContextConfigHelper.GetBooleanConfig("System.IO.UseLegacyFileStream", "DOTNET_SYSTEM_IO_USELEGACYFILESTREAM");
-
-        internal static FileStreamStrategy ChooseStrategy(SafeFileHandle handle, FileAccess access, int bufferSize, bool isAsync)
+        private static FileStreamStrategy ChooseStrategy(SafeFileHandle handle, FileAccess access, int bufferSize, bool isAsync)
         {
             if (UseLegacyStrategy)
             {
@@ -37,7 +33,7 @@ internal static FileStreamStrategy ChooseStrategy(SafeFileHandle handle, FileAcc
             return EnableBufferingIfNeeded(strategy, bufferSize);
         }
 
-        internal static FileStreamStrategy ChooseStrategy(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, FileOptions options)
+        private static FileStreamStrategy ChooseStrategy(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, FileOptions options)
         {
             if (UseLegacyStrategy)
             {

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

@@ -0,0 +1,53 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using Microsoft.Win32.SafeHandles;
+
+namespace System.IO.Strategies
+{
+    internal static partial class FileStreamHelpers
+    {
+        // It's enabled by default. We are going to change that once we fix #16354, #25905 and #24847.
+        internal static bool UseLegacyStrategy { get; } = GetLegacyFileStreamSetting();
+
+        private static bool GetLegacyFileStreamSetting()
+        {
+            if (AppContext.TryGetSwitch("System.IO.UseNet5CompatFileStream", out bool fileConfig))
+            {
+                return fileConfig;
+            }
+
+            string? envVar = Environment.GetEnvironmentVariable("DOTNET_SYSTEM_IO_USENET5COMPATFILESTREAM");
+            return envVar is null
+                ? true // legacy is currently enabled by default;
+                : bool.IsTrueStringIgnoreCase(envVar) || envVar.Equals("1");
+        }
+
+        internal static FileStreamStrategy ChooseStrategy(FileStream fileStream, SafeFileHandle handle, FileAccess access, int bufferSize, bool isAsync)
+            => WrapIfDerivedType(fileStream, ChooseStrategy(handle, access, bufferSize, isAsync));
+
+        internal static FileStreamStrategy ChooseStrategy(FileStream fileStream, string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, FileOptions options)
+            => WrapIfDerivedType(fileStream, ChooseStrategy(path, mode, access, share, bufferSize, options));
+
+        private static FileStreamStrategy WrapIfDerivedType(FileStream fileStream, FileStreamStrategy strategy)
+            => fileStream.GetType() == typeof(FileStream)
+                ? strategy
+                : new DerivedFileStreamStrategy(fileStream, strategy);
+
+        internal static bool IsIoRelatedException(Exception e) =>
+            // These all derive from IOException
+            //     DirectoryNotFoundException
+            //     DriveNotFoundException
+            //     EndOfStreamException
+            //     FileLoadException
+            //     FileNotFoundException
+            //     PathTooLongException
+            //     PipeException
+            e is IOException ||
+            // Note that SecurityException is only thrown on runtimes that support CAS
+            // e is SecurityException ||
+            e is UnauthorizedAccessException ||
+            e is NotSupportedException ||
+            (e is ArgumentException && !(e is ArgumentNullException));
+    }
+}

src/libraries/System.Private.CoreLib/src/System/IO/FileStreamStrategy.cs → src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamStrategy.cs

@@ -3,7 +3,7 @@
 
 using Microsoft.Win32.SafeHandles;
 
-namespace System.IO
+namespace System.IO.Strategies
 {
     internal abstract class FileStreamStrategy : Stream
     {

src/libraries/System.Private.CoreLib/src/System/IO/LegacyFileStreamStrategy.Lock.OSX.cs → src/libraries/System.Private.CoreLib/src/System/IO/Strategies/LegacyFileStreamStrategy.Lock.OSX.cs

@@ -1,7 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-namespace System.IO
+namespace System.IO.Strategies
 {
     internal sealed partial class LegacyFileStreamStrategy : FileStreamStrategy
     {

src/libraries/System.Private.CoreLib/src/System/IO/LegacyFileStreamStrategy.Lock.Unix.cs → src/libraries/System.Private.CoreLib/src/System/IO/Strategies/LegacyFileStreamStrategy.Lock.Unix.cs

@@ -1,7 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-namespace System.IO
+namespace System.IO.Strategies
 {
     internal sealed partial class LegacyFileStreamStrategy : FileStreamStrategy
     {