ensure MetadataReader fast path works with every FileStream on every OS (#50367)
authorAdam Sitnik <adam.sitnik@gmail.com>
Wed, 31 Mar 2021 06:53:52 +0000 (08:53 +0200)
committerGitHub <noreply@github.com>
Wed, 31 Mar 2021 06:53:52 +0000 (08:53 +0200)
src/libraries/System.Reflection.Metadata/src/System.Reflection.Metadata.csproj
src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/MemoryBlocks/StreamMemoryBlockProvider.cs
src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/FileStreamReadLightUp.netcoreapp.cs [new file with mode: 0644]
src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/FileStreamReadLightUp.netstandard1.1.cs
src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/FileStreamReadLightUp.netstandard2.0.cs [moved from src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/FileStreamReadLightUp.cs with 61% similarity]

index 2417558..e099ac3 100644 (file)
@@ -1,4 +1,4 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
     <DefaultLanguage>en-US</DefaultLanguage>
   </PropertyGroup>
   <ItemGroup>
     <Compile Include="$(CommonPath)Interop\Windows\Interop.Libraries.cs"
-             Link="Common\Interop\Windows\Interop.Libraries.cs" />
+             Link="Common\Interop\Windows\Interop.Libraries.cs"
+             Condition="'$(TargetFramework)' != '$(NetCoreAppCurrent)' "/>
     <Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.ReadFile_SafeHandle_IntPtr.cs"
-             Link="Common\Interop\Windows\kernel32\Interop.ReadFile_SafeHandle_IntPtr.cs" />
+             Link="Common\Interop\Windows\kernel32\Interop.ReadFile_SafeHandle_IntPtr.cs"
+             Condition="'$(TargetFramework)' != '$(NetCoreAppCurrent)' "/>
     <Compile Include="System\Reflection\Internal\Utilities\PinnedObject.cs" />
     <Compile Include="System\Reflection\Internal\Utilities\CriticalDisposableObject.cs" Condition="'$(TargetFramework)' != 'netstandard1.1'" />
     <Compile Include="System\Reflection\Internal\Utilities\CriticalDisposableObject.netstandard1.1.cs" Condition="'$(TargetFramework)' == 'netstandard1.1'" />
     <Compile Include="System\Reflection\Internal\Utilities\EmptyArray.cs" />
     <Compile Include="System\Reflection\Internal\Utilities\EncodingHelper.cs" Condition="$(TargetFramework.StartsWith('netstandard')) Or $(TargetFramework.StartsWith('net4'))" />
     <Compile Include="System\Reflection\Internal\Utilities\EncodingHelper.netcoreapp.cs" Condition="'$(TargetFramework)' == '$(NetCoreAppCurrent)'" />
-    <Compile Include="System\Reflection\Internal\Utilities\FileStreamReadLightUp.cs" Condition="'$(TargetFramework)' != 'netstandard1.1'" />
+    <Compile Include="System\Reflection\Internal\Utilities\FileStreamReadLightUp.netcoreapp.cs" Condition="'$(TargetFramework)' == '$(NetCoreAppCurrent)'" />
     <Compile Include="System\Reflection\Internal\Utilities\FileStreamReadLightUp.netstandard1.1.cs" Condition="'$(TargetFramework)' == 'netstandard1.1'" />
+    <Compile Include="System\Reflection\Internal\Utilities\FileStreamReadLightUp.netstandard2.0.cs" Condition="'$(TargetFramework)' == 'netstandard2.0' Or '$(TargetFramework)' == 'net461'" />
     <Compile Include="System\Reflection\Internal\Utilities\Hash.cs" />
     <Compile Include="System\Reflection\Internal\Utilities\ImmutableByteArrayInterop.cs" />
     <Compile Include="System\Reflection\Internal\Utilities\ImmutableMemoryStream.cs" />
index 7581e90..1c9f68d 100644 (file)
@@ -76,9 +76,11 @@ namespace System.Reflection.Internal
             {
                 stream.Seek(start, SeekOrigin.Begin);
 
-                if (!isFileStream || !FileStreamReadLightUp.TryReadFile(stream, block.Pointer, start, size))
+                int bytesRead = 0;
+
+                if (!isFileStream || (bytesRead = FileStreamReadLightUp.ReadFile(stream, block.Pointer, size)) != size)
                 {
-                    stream.CopyTo(block.Pointer, size);
+                    stream.CopyTo(block.Pointer + bytesRead, size - bytesRead);
                 }
 
                 fault = false;
diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/FileStreamReadLightUp.netcoreapp.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/Internal/Utilities/FileStreamReadLightUp.netcoreapp.cs
new file mode 100644 (file)
index 0000000..aabc673
--- /dev/null
@@ -0,0 +1,15 @@
+// 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;
+
+namespace System.Reflection.Internal
+{
+    internal static class FileStreamReadLightUp
+    {
+        internal static bool IsFileStream(Stream stream) => stream is FileStream;
+
+        internal static unsafe int ReadFile(Stream stream, byte* buffer, int size)
+            => stream.Read(new Span<byte>(buffer, size));
+    }
+}
index 1bcc69e..774def6 100644 (file)
@@ -86,43 +86,29 @@ namespace System.Reflection.Internal
             return handle;
         }
 
-        internal static unsafe bool TryReadFile(Stream stream, byte* buffer, long start, int size)
+        internal static unsafe int ReadFile(Stream stream, byte* buffer, int size)
         {
             if (readFileNotAvailable)
             {
-                return false;
+                return 0;
             }
 
             SafeHandle handle = GetSafeFileHandle(stream);
             if (handle == null)
             {
-                return false;
+                return 0;
             }
 
-            int result;
-            int bytesRead;
-
             try
             {
-                result = Interop.Kernel32.ReadFile(handle, buffer, size, out bytesRead, IntPtr.Zero);
+                int result = Interop.Kernel32.ReadFile(handle, buffer, size, out int bytesRead, IntPtr.Zero);
+                return result == 0 ? 0 : bytesRead;
             }
             catch
             {
                 readFileNotAvailable = true;
-                return false;
+                return 0;
             }
-
-            if (result == 0 || bytesRead != size)
-            {
-                // We used to throw here, but this is where we land if the FileStream was
-                // opened with useAsync: true, which is currently the default on .NET Core.
-                // https://github.com/dotnet/corefx/pull/987 filed to look in to how best to
-                // handle this, but in the meantime, we'll fall back to the slower code path
-                // just as in the case where the native API is unavailable in the current platform.
-                return false;
-            }
-
-            return true;
         }
     }
 }
@@ -1,7 +1,6 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System.Diagnostics;
 using System.IO;
 using System.Runtime.InteropServices;
 
@@ -9,12 +8,7 @@ namespace System.Reflection.Internal
 {
     internal static class FileStreamReadLightUp
     {
-        private static bool IsReadFileAvailable =>
-#if NETCOREAPP
-            OperatingSystem.IsWindows();
-#else
-            Path.DirectorySeparatorChar == '\\';
-#endif
+        private static bool IsReadFileAvailable => Path.DirectorySeparatorChar == '\\';
 
         internal static bool IsFileStream(Stream stream) => stream is FileStream;
 
@@ -42,32 +36,21 @@ namespace System.Reflection.Internal
             return handle;
         }
 
-        internal static unsafe bool TryReadFile(Stream stream, byte* buffer, long start, int size)
+        internal static unsafe int ReadFile(Stream stream, byte* buffer, int size)
         {
             if (!IsReadFileAvailable)
             {
-                return false;
+                return 0;
             }
 
             SafeHandle? handle = GetSafeFileHandle(stream);
             if (handle == null)
             {
-                return false;
+                return 0;
             }
 
             int result = Interop.Kernel32.ReadFile(handle, buffer, size, out int bytesRead, IntPtr.Zero);
-
-            if (result == 0 || bytesRead != size)
-            {
-                // We used to throw here, but this is where we land if the FileStream was
-                // opened with useAsync: true, which is currently the default on .NET Core.
-                // https://github.com/dotnet/corefx/pull/987 filed to look in to how best to
-                // handle this, but in the meantime, we'll fall back to the slower code path
-                // just as in the case where the native API is unavailable in the current platform.
-                return false;
-            }
-
-            return true;
+            return result == 0 ? 0 : bytesRead;
         }
     }
 }