use O_TRUNC when file locking is disabled, avoid ftruncate syscall (#55456)
authorAdam Sitnik <adam.sitnik@gmail.com>
Sat, 10 Jul 2021 18:38:44 +0000 (20:38 +0200)
committerGitHub <noreply@github.com>
Sat, 10 Jul 2021 18:38:44 +0000 (20:38 +0200)
* 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 <stoub@microsoft.com>
17 files changed:
src/libraries/Common/tests/TestUtilities/System/PlatformDetection.cs
src/libraries/System.IO.FileSystem/System.IO.FileSystem.sln
src/libraries/System.IO.FileSystem/tests/DisabledFileLockingTests/DisabledFileLockingSwitchTests.cs [new file with mode: 0644]
src/libraries/System.IO.FileSystem/tests/DisabledFileLockingTests/System.IO.FileSystem.DisabledFileLocking.Tests.csproj [new file with mode: 0644]
src/libraries/System.IO.FileSystem/tests/DisabledFileLockingTests/runtimeconfig.template.json [new file with mode: 0644]
src/libraries/System.IO.FileSystem/tests/File/Copy.cs
src/libraries/System.IO.FileSystem/tests/File/Create.cs
src/libraries/System.IO.FileSystem/tests/File/ReadWriteAllBytes.cs
src/libraries/System.IO.FileSystem/tests/File/ReadWriteAllBytesAsync.cs
src/libraries/System.IO.FileSystem/tests/File/ReadWriteAllLines.cs
src/libraries/System.IO.FileSystem/tests/File/ReadWriteAllLinesAsync.cs
src/libraries/System.IO.FileSystem/tests/File/ReadWriteAllText.cs
src/libraries/System.IO.FileSystem/tests/File/ReadWriteAllTextAsync.cs
src/libraries/System.IO.FileSystem/tests/FileStream/ctor_str_fm_fa_fs.cs
src/libraries/System.IO.FileSystem/tests/FileStream/ctor_str_fm_fa_fs.write.cs
src/libraries/System.IO.FileSystem/tests/Net5CompatTests/System.IO.FileSystem.Net5Compat.Tests.csproj
src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs

index 0bd2985..e11da7e 100644 (file)
@@ -70,8 +70,8 @@ namespace System
         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 @@ namespace System
 
         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)
index 93f9297..fd94f8b 100644 (file)
@@ -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/src/libraries/System.IO.FileSystem/tests/DisabledFileLockingTests/DisabledFileLockingSwitchTests.cs b/src/libraries/System.IO.FileSystem/tests/DisabledFileLockingTests/DisabledFileLockingSwitchTests.cs
new file mode 100644 (file)
index 0000000..7369919
--- /dev/null
@@ -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/src/libraries/System.IO.FileSystem/tests/DisabledFileLockingTests/System.IO.FileSystem.DisabledFileLocking.Tests.csproj b/src/libraries/System.IO.FileSystem/tests/DisabledFileLockingTests/System.IO.FileSystem.DisabledFileLocking.Tests.csproj
new file mode 100644 (file)
index 0000000..87afaa0
--- /dev/null
@@ -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/src/libraries/System.IO.FileSystem/tests/DisabledFileLockingTests/runtimeconfig.template.json b/src/libraries/System.IO.FileSystem/tests/DisabledFileLockingTests/runtimeconfig.template.json
new file mode 100644 (file)
index 0000000..c118f2a
--- /dev/null
@@ -0,0 +1,5 @@
+{
+    "configProperties": {
+        "System.IO.DisableFileLocking": true
+    }
+}
index eb26868..643d2a9 100644 (file)
@@ -355,7 +355,7 @@ namespace System.IO.Tests
     /// <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]
index bf0417d..fb7f4be 100644 (file)
@@ -142,8 +142,7 @@ namespace System.IO.Tests
             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());
index 7f13447..54c7643 100644 (file)
@@ -80,8 +80,7 @@ namespace System.IO.Tests
             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();
index 111ecb5..748c01d 100644 (file)
@@ -94,8 +94,7 @@ namespace System.IO.Tests
             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();
index c0ba078..99c94b4 100644 (file)
@@ -77,8 +77,7 @@ namespace System.IO.Tests
             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 @@ namespace System.IO.Tests
             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();
index 76d1541..b2a1288 100644 (file)
@@ -75,8 +75,7 @@ namespace System.IO.Tests
             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();
index d24a844..1dbe303 100644 (file)
@@ -84,8 +84,7 @@ namespace System.IO.Tests
             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();
index 3481bff..3ad76c2 100644 (file)
@@ -83,8 +83,7 @@ namespace System.IO.Tests
             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();
index 386b299..205e294 100644 (file)
@@ -120,10 +120,9 @@ namespace System.IO.Tests
             }
         }
 
-        [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();
index ae93555..6ee1c6f 100644 (file)
@@ -37,8 +37,7 @@ namespace System.IO.Tests
             }
         }
 
-        [Fact]
-        [ActiveIssue("https://github.com/dotnet/runtime/issues/40065", TestPlatforms.Browser)]
+        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsFileLockingEnabled))]
         public void FileShareWithoutWriteThrows()
         {
             string fileName = GetTestFilePath();
index dd67a63..745ff60 100644 (file)
@@ -8,6 +8,7 @@
   </PropertyGroup>
   <ItemGroup>
     <Compile Include="..\**\*.cs" />
+    <Compile Remove="..\DisabledFileLockingTests\DisabledFileLockingSwitchTests.cs" />
   </ItemGroup>
   <ItemGroup Condition="'$(TargetsUnix)' == 'true'">
     <Compile Remove="..\**\*.Windows.cs" />
index 10dd1ea..ee1def8 100644 (file)
@@ -206,15 +206,29 @@ namespace Microsoft.Win32.SafeHandles
             {
                 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 @@ namespace Microsoft.Win32.SafeHandles
                     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.