use O_TRUNC when file locking is disabled, avoid ftruncate syscall (d…

…otnet#55456) * add test project with file locking disabled via config file * use O_TRUNC when file locking is disabled, avoid ftruncate syscall * Apply suggestions from code review Co-authored-by: Stephen Toub <[email protected]>
timandy · Jul 10, 2021 · 2eb0071 · 2eb0071
1 parent 004feb8
commit 2eb0071
Show file tree

Hide file tree

Showing 17 changed files with 95 additions and 26 deletions.
diff --git a/src/libraries/Common/tests/TestUtilities/System/PlatformDetection.cs b/src/libraries/Common/tests/TestUtilities/System/PlatformDetection.cs
@@ -70,8 +70,8 @@ public static partial class PlatformDetection
         public static bool IsLinqExpressionsBuiltWithIsInterpretingOnly => s_LinqExpressionsBuiltWithIsInterpretingOnly.Value;
         public static bool IsNotLinqExpressionsBuiltWithIsInterpretingOnly => !IsLinqExpressionsBuiltWithIsInterpretingOnly;
         private static readonly Lazy<bool> s_LinqExpressionsBuiltWithIsInterpretingOnly = new Lazy<bool>(GetLinqExpressionsBuiltWithIsInterpretingOnly);
-        private static bool GetLinqExpressionsBuiltWithIsInterpretingOnly() 
-        {            
+        private static bool GetLinqExpressionsBuiltWithIsInterpretingOnly()
+        {
             Type type = typeof(LambdaExpression);
             if (type != null)
             {
@@ -285,6 +285,10 @@ private static Version GetICUVersion()
 
         public static bool IsNet5CompatFileStreamEnabled => _net5CompatFileStream.Value;
 
+        private static readonly Lazy<bool> s_fileLockingDisabled = new Lazy<bool>(() => GetStaticNonPublicBooleanPropertyValue("Microsoft.Win32.SafeHandles.SafeFileHandle", "DisableFileLocking"));
+
+        public static bool IsFileLockingEnabled => IsWindows || !s_fileLockingDisabled.Value;
+
         private static bool GetIsInContainer()
         {
             if (IsWindows)

diff --git a/src/libraries/System.IO.FileSystem/System.IO.FileSystem.sln b/src/libraries/System.IO.FileSystem/System.IO.FileSystem.sln
@@ -29,6 +29,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "System.IO.FileSystem.Net5Co
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "StreamConformanceTests", "..\Common\tests\StreamConformanceTests\StreamConformanceTests.csproj", "{FEF03BCC-509F-4646-9132-9DE27FA3DA6F}"
 EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "System.IO.FileSystem.DisabledFileLocking.Tests", "tests\DisabledFileLockingTests\System.IO.FileSystem.DisabledFileLocking.Tests.csproj", "{D20CD3B7-A332-4D47-851A-FD8C80AE10B9}"
+EndProject
 Global
 	GlobalSection(NestedProjects) = preSolution
 		{D350D6E7-52F1-40A4-B646-C178F6BBB689} = {1A727AF9-4F39-4109-BB8F-813286031DC9}
@@ -43,6 +45,7 @@ Global
 		{D7DF8034-3AE5-4DEF-BCC4-6353239391BF} = {D9FB1730-B750-4C0D-8D24-8C992DEB6034}
 		{48E07F12-8597-40DE-8A37-CCBEB9D54012} = {1A727AF9-4F39-4109-BB8F-813286031DC9}
 		{FEF03BCC-509F-4646-9132-9DE27FA3DA6F} = {1A727AF9-4F39-4109-BB8F-813286031DC9}
+		{D20CD3B7-A332-4D47-851A-FD8C80AE10B9} = {1A727AF9-4F39-4109-BB8F-813286031DC9}
 	EndGlobalSection
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -97,6 +100,10 @@ Global
 		{FEF03BCC-509F-4646-9132-9DE27FA3DA6F}.Debug|Any CPU.Build.0 = Debug|Any CPU
 		{FEF03BCC-509F-4646-9132-9DE27FA3DA6F}.Release|Any CPU.ActiveCfg = Release|Any CPU
 		{FEF03BCC-509F-4646-9132-9DE27FA3DA6F}.Release|Any CPU.Build.0 = Release|Any CPU
+		{D20CD3B7-A332-4D47-851A-FD8C80AE10B9}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{D20CD3B7-A332-4D47-851A-FD8C80AE10B9}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{D20CD3B7-A332-4D47-851A-FD8C80AE10B9}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{D20CD3B7-A332-4D47-851A-FD8C80AE10B9}.Release|Any CPU.Build.0 = Release|Any CPU
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE

diff --git a/...ies/System.IO.FileSystem/tests/DisabledFileLockingTests/DisabledFileLockingSwitchTests.cs b/...ies/System.IO.FileSystem/tests/DisabledFileLockingTests/DisabledFileLockingSwitchTests.cs
@@ -0,0 +1,16 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using Xunit;
+
+namespace System.IO.Tests
+{
+    public class DisabledFileLockingSwitchTests
+    {
+        [Fact]
+        public static void ConfigSwitchIsHonored()
+        {
+            Assert.Equal(OperatingSystem.IsWindows(), PlatformDetection.IsFileLockingEnabled);
+        }
+    }
+}
diff --git a/...stem/tests/DisabledFileLockingTests/System.IO.FileSystem.DisabledFileLocking.Tests.csproj b/...stem/tests/DisabledFileLockingTests/System.IO.FileSystem.DisabledFileLocking.Tests.csproj
@@ -0,0 +1,32 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <IncludeRemoteExecutor>true</IncludeRemoteExecutor>
+    <!-- file locking can't be disabled on Windows -->
+    <TargetFrameworks>$(NetCoreAppCurrent)-Unix</TargetFrameworks>
+
+    <WasmXHarnessMonoArgs>--working-dir=/test-dir</WasmXHarnessMonoArgs>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="DisabledFileLockingSwitchTests.cs" />
+    <Compile Include="..\FSAssert.cs" />
+    <Compile Include="..\TestData.cs" />
+    <Compile Include="..\FileSystemTest.cs" />
+    <Compile Include="..\FileSystemTest.Unix.cs" />
+    <Compile Include="..\UnseekableFileStream.cs" />
+    <Compile Include="..\FileStream\**\*.cs" />
+    <Compile Include="..\PortedCommon\DllImports.cs" />
+    <Compile Include="..\PortedCommon\IOInputs.cs" />
+    <Compile Include="..\PortedCommon\IOServices.cs" />
+    <Compile Include="$(CommonTestPath)System\IO\PathFeatures.cs" Link="Common\System\IO\PathFeatures.cs" />
+  </ItemGroup>
+  <ItemGroup Condition="'$(TargetsUnix)' == 'true'">
+    <Compile Remove="..\**\*.Windows.cs" />
+    <Compile Remove="..\**\*.Browser.cs" />
+    <Compile Include="$(CommonPath)Interop\Unix\Interop.Libraries.cs" Link="Interop\Unix\Interop.Libraries.cs" />
+    <Compile Include="$(CommonPath)Interop\Unix\System.Native\Interop.Stat.cs" Link="Interop\Unix\System.Native\Interop.Stat.cs" />
+  </ItemGroup>
+  <ItemGroup>
+    <ProjectReference Include="$(CommonTestPath)StreamConformanceTests\StreamConformanceTests.csproj" />
+  </ItemGroup>
+</Project>
diff --git a/...libraries/System.IO.FileSystem/tests/DisabledFileLockingTests/runtimeconfig.template.json b/...libraries/System.IO.FileSystem/tests/DisabledFileLockingTests/runtimeconfig.template.json
@@ -0,0 +1,5 @@
+{
+    "configProperties": {
+        "System.IO.DisableFileLocking": true
+    }
+}
diff --git a/src/libraries/System.IO.FileSystem/tests/File/Copy.cs b/src/libraries/System.IO.FileSystem/tests/File/Copy.cs
@@ -355,7 +355,7 @@ public void WindowsAlternateDataStreamOverwrite(string defaultStream, string alt
     /// <summary>
     /// Single tests that shouldn't be duplicated by inheritance.
     /// </summary>
-    [SkipOnPlatform(TestPlatforms.Browser, "https://github.com/dotnet/runtime/issues/40867 - flock not supported")]
+    [ConditionalClass(typeof(PlatformDetection), nameof(PlatformDetection.IsFileLockingEnabled))]
     public sealed class File_Copy_Single : FileSystemTest
     {
         [Fact]

diff --git a/src/libraries/System.IO.FileSystem/tests/File/Create.cs b/src/libraries/System.IO.FileSystem/tests/File/Create.cs
@@ -142,8 +142,7 @@ public void InvalidDirectory()
             Assert.Throws<DirectoryNotFoundException>(() => Create(testFile));
         }
 
-        [Fact]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/40065", TestPlatforms.Browser)]
+        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsFileLockingEnabled))]
         public void FileInUse()
         {
             DirectoryInfo testDir = Directory.CreateDirectory(GetTestFilePath());

diff --git a/src/libraries/System.IO.FileSystem/tests/File/ReadWriteAllBytes.cs b/src/libraries/System.IO.FileSystem/tests/File/ReadWriteAllBytes.cs
@@ -80,8 +80,7 @@ public void Overwrite()
             Assert.Equal(overwriteBytes, File.ReadAllBytes(path));
         }
 
-        [Fact]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/40065", TestPlatforms.Browser)]
+        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsFileLockingEnabled))]
         public void OpenFile_ThrowsIOException()
         {
             string path = GetTestFilePath();

diff --git a/src/libraries/System.IO.FileSystem/tests/File/ReadWriteAllBytesAsync.cs b/src/libraries/System.IO.FileSystem/tests/File/ReadWriteAllBytesAsync.cs
@@ -94,8 +94,7 @@ public async Task OverwriteAsync()
             Assert.Equal(overwriteBytes, await File.ReadAllBytesAsync(path));
         }
 
-        [Fact]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/40065", TestPlatforms.Browser)]
+        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsFileLockingEnabled))]
         public async Task OpenFile_ThrowsIOExceptionAsync()
         {
             string path = GetTestFilePath();

diff --git a/src/libraries/System.IO.FileSystem/tests/File/ReadWriteAllLines.cs b/src/libraries/System.IO.FileSystem/tests/File/ReadWriteAllLines.cs
@@ -77,8 +77,7 @@ public virtual void Overwrite()
             Assert.Equal(overwriteLines, Read(path));
         }
 
-        [Fact]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/40065", TestPlatforms.Browser)]
+        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsFileLockingEnabled))]
         public void OpenFile_ThrowsIOException()
         {
             string path = GetTestFilePath();
@@ -267,8 +266,7 @@ public virtual void Overwrite()
             Assert.Equal(overwriteLines, Read(path));
         }
 
-        [Fact]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/40065", TestPlatforms.Browser)]
+        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsFileLockingEnabled))]
         public void OpenFile_ThrowsIOException()
         {
             string path = GetTestFilePath();

diff --git a/src/libraries/System.IO.FileSystem/tests/File/ReadWriteAllLinesAsync.cs b/src/libraries/System.IO.FileSystem/tests/File/ReadWriteAllLinesAsync.cs
@@ -75,8 +75,7 @@ public virtual async Task OverwriteAsync()
             Assert.Equal(overwriteLines, await ReadAsync(path));
         }
 
-        [Fact]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/40065", TestPlatforms.Browser)]
+        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsFileLockingEnabled))]
         public async Task OpenFile_ThrowsIOExceptionAsync()
         {
             string path = GetTestFilePath();

diff --git a/src/libraries/System.IO.FileSystem/tests/File/ReadWriteAllText.cs b/src/libraries/System.IO.FileSystem/tests/File/ReadWriteAllText.cs
@@ -84,8 +84,7 @@ public virtual void Overwrite()
             Assert.Equal(overwriteLines, Read(path));
         }
 
-        [Fact]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/40065", TestPlatforms.Browser)]
+        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsFileLockingEnabled))]
         public void OpenFile_ThrowsIOException()
         {
             string path = GetTestFilePath();

diff --git a/src/libraries/System.IO.FileSystem/tests/File/ReadWriteAllTextAsync.cs b/src/libraries/System.IO.FileSystem/tests/File/ReadWriteAllTextAsync.cs
@@ -83,8 +83,7 @@ public virtual async Task OverwriteAsync()
             Assert.Equal(overwriteLines, await ReadAsync(path));
         }
 
-        [Fact]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/40065", TestPlatforms.Browser)]
+        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsFileLockingEnabled))]
         public async Task OpenFile_ThrowsIOExceptionAsync()
         {
             string path = GetTestFilePath();

diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_str_fm_fa_fs.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_str_fm_fa_fs.cs
@@ -120,10 +120,9 @@ public void FileShareOpenOrCreate()
             }
         }
 
-        [Theory]
         [InlineData(FileMode.Create)]
         [InlineData(FileMode.Truncate)]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/40065", TestPlatforms.Browser)]
+        [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsFileLockingEnabled))]
         public void NoTruncateOnFileShareViolation(FileMode fileMode)
         {
             string fileName = GetTestFilePath();

diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_str_fm_fa_fs.write.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_str_fm_fa_fs.write.cs
@@ -37,8 +37,7 @@ public void FileShareWriteExisting()
             }
         }
 
-        [Fact]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/40065", TestPlatforms.Browser)]
+        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsFileLockingEnabled))]
         public void FileShareWithoutWriteThrows()
         {
             string fileName = GetTestFilePath();

diff --git a/...s/System.IO.FileSystem/tests/Net5CompatTests/System.IO.FileSystem.Net5Compat.Tests.csproj b/...s/System.IO.FileSystem/tests/Net5CompatTests/System.IO.FileSystem.Net5Compat.Tests.csproj
@@ -8,6 +8,7 @@
   </PropertyGroup>
   <ItemGroup>
     <Compile Include="..\**\*.cs" />
+    <Compile Remove="..\DisabledFileLockingTests\DisabledFileLockingSwitchTests.cs" />
   </ItemGroup>
   <ItemGroup Condition="'$(TargetsUnix)' == 'true'">
     <Compile Remove="..\**\*.Windows.cs" />

diff --git a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs
@@ -206,15 +206,29 @@ private static Interop.Sys.OpenFlags PreOpenConfigurationFromOptions(FileMode mo
             {
                 default:
                 case FileMode.Open: // Open maps to the default behavior for open(...).  No flags needed.
-                case FileMode.Truncate: // We truncate the file after getting the lock
+                    break;
+                case FileMode.Truncate:
+                    if (DisableFileLocking)
+                    {
+                        // if we don't lock the file, we can truncate it when opening
+                        // otherwise we truncate the file after getting the lock
+                        flags |= Interop.Sys.OpenFlags.O_TRUNC;
+                    }
                     break;
 
                 case FileMode.Append: // Append is the same as OpenOrCreate, except that we'll also separately jump to the end later
                 case FileMode.OpenOrCreate:
-                case FileMode.Create: // We truncate the file after getting the lock
                     flags |= Interop.Sys.OpenFlags.O_CREAT;
                     break;
 
+                case FileMode.Create:
+                    flags |= Interop.Sys.OpenFlags.O_CREAT;
+                    if (DisableFileLocking)
+                    {
+                        flags |= Interop.Sys.OpenFlags.O_TRUNC;
+                    }
+                    break;
+
                 case FileMode.CreateNew:
                     flags |= (Interop.Sys.OpenFlags.O_CREAT | Interop.Sys.OpenFlags.O_EXCL);
                     break;
@@ -292,7 +306,7 @@ private void Init(string path, FileMode mode, FileAccess access, FileShare share
                     ignoreNotSupported: true); // just a hint.
             }
 
-            if (mode == FileMode.Create || mode == FileMode.Truncate)
+            if ((mode == FileMode.Create || mode == FileMode.Truncate) && !DisableFileLocking)
             {
                 // Truncate the file now if the file mode requires it. This ensures that the file only will be truncated
                 // if opened successfully.