Use shared lock for files opened without FileAccess.Write on Linux (#44185)

* Use shared lock for readonly files on Linux

* Add suggested tests

* Exclude test from OSX

* Add RemoteExecutor.IsSupported condition to tests

diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.LockFileRegion.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.LockFileRegion.cs
index 42da964..3276275 100644 (file)
--- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.LockFileRegion.cs
+++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.LockFileRegion.cs
@@ -9,6 +9,7 @@ internal static partial class Interop
     {
         internal enum LockType : short
         {
+            F_RDLCK = 0,    // shared or read lock
             F_WRLCK = 1,    // exclusive or write lock
             F_UNLCK = 2     // unlock
         }

diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/LockUnlock.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/LockUnlock.cs
index cec61a4..1da23bf 100644 (file)
--- a/src/libraries/System.IO.FileSystem/tests/FileStream/LockUnlock.cs
+++ b/src/libraries/System.IO.FileSystem/tests/FileStream/LockUnlock.cs
@@ -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 Microsoft.DotNet.RemoteExecutor;
 using Xunit;
 
@@ -73,6 +72,22 @@ namespace System.IO.Tests
         }
 
         [Theory]
+        [InlineData(FileAccess.Read)]
+        [InlineData(FileAccess.Write)]
+        [InlineData(FileAccess.ReadWrite)]
+        [PlatformSpecific(~TestPlatforms.OSX)]
+        public void Lock_Unlock_Successful_AlternateFileAccess(FileAccess fileAccess)
+        {
+            string path = GetTestFilePath();
+            File.WriteAllBytes(path, new byte[100]);
+
+            using FileStream fs = File.Open(path, FileMode.Open, fileAccess);
+
+            fs.Lock(0, 100);
+            fs.Unlock(0, 100);
+        }
+
+        [Theory]
         [InlineData(10, 0, 2, 3, 5)]
         [PlatformSpecific(~TestPlatforms.OSX)]
         public void NonOverlappingRegions_Success(long fileLength, long firstPosition, long firstLength, long secondPosition, long secondLength)
@@ -153,9 +168,7 @@ namespace System.IO.Tests
             }
         }
 
-        private static bool IsNotWindowsSubsystemForLinuxAndRemoteExecutorSupported => PlatformDetection.IsNotWindowsSubsystemForLinux && RemoteExecutor.IsSupported;
-
-        [ConditionalTheory(nameof(IsNotWindowsSubsystemForLinuxAndRemoteExecutorSupported))] // https://github.com/dotnet/runtime/issues/28330
+        [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
         [InlineData(10, 0, 10, 1, 2)]
         [InlineData(10, 3, 5, 3, 5)]
         [InlineData(10, 3, 5, 3, 4)]
@@ -192,5 +205,57 @@ namespace System.IO.Tests
                 }, path, secondPosition.ToString(), secondLength.ToString()).Dispose();
             }
         }
+
+        [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
+        [PlatformSpecific(TestPlatforms.Linux)]
+        public void OverlappingRegionsFromOtherProcess_With_ReadLock_AllowedOnLinux()
+        {
+            string path = GetTestFilePath();
+            File.WriteAllBytes(path, new byte[100]);
+
+            using FileStream fs1 = File.Open(path, FileMode.Open, FileAccess.Read, FileShare.ReadWrite);
+            fs1.Lock(0, 100);
+
+            RemoteExecutor.Invoke((path) =>
+            {
+                using FileStream fs2 = File.Open(path, FileMode.Open, FileAccess.Read, FileShare.ReadWrite);
+                fs2.Lock(0, 100);
+                fs2.Unlock(0, 100);
+
+            }, path).Dispose();
+
+            fs1.Unlock(0, 100);            
+        }
+
+        [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
+        [InlineData(FileAccess.Read)]
+        [InlineData(FileAccess.Write)]
+        [InlineData(FileAccess.ReadWrite)]
+        [PlatformSpecific(~TestPlatforms.OSX)]
+        public void OverlappingRegionsFromOtherProcess_With_WriteLock_ThrowsException(FileAccess fileAccess)
+        {
+            string path = GetTestFilePath();
+            File.WriteAllBytes(path, new byte[100]);
+
+            using FileStream fs1 = File.Open(path, FileMode.Open, fileAccess, FileShare.ReadWrite);
+            fs1.Lock(0, 100);
+
+            RemoteExecutor.Invoke((path) =>
+            {
+                using FileStream fs2 = File.Open(path, FileMode.Open, FileAccess.Write, FileShare.ReadWrite);
+                Assert.Throws<IOException>(() => fs2.Lock(0, 100));
+
+            }, path).Dispose();
+
+            fs1.Unlock(0, 100);
+
+            RemoteExecutor.Invoke((path) =>
+            {
+                using FileStream fs2 = File.Open(path, FileMode.Open, FileAccess.Write, FileShare.ReadWrite);
+                fs2.Lock(0, 100);
+                fs2.Unlock(0, 100);
+
+            }, path).Dispose();
+        }
     }
 }

diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileStream.Lock.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStream.Lock.Unix.cs
index 34026c1..1869552 100644 (file)
--- a/src/libraries/System.Private.CoreLib/src/System/IO/FileStream.Lock.Unix.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileStream.Lock.Unix.cs
@@ -10,7 +10,7 @@ namespace System.IO
         /// <param name="length">The range to be locked.</param>
         private void LockInternal(long position, long length)
         {
-            CheckFileCall(Interop.Sys.LockFileRegion(_fileHandle, position, length, Interop.Sys.LockType.F_WRLCK));
+            CheckFileCall(Interop.Sys.LockFileRegion(_fileHandle, position, length, CanWrite ? Interop.Sys.LockType.F_WRLCK : Interop.Sys.LockType.F_RDLCK));
         }
 
         /// <summary>Allows access by other processes to all or part of a file that was previously locked.</summary>