path: root/roslyn-57003-mono-named-mutex.patch

Index: tarball.6.0.1-rc2-6.0.100-rc2/src/roslyn.8e1779e16298415843e85029d8b52a1ae9bb4c30/src/Compilers/Core/Portable/InternalUtilities/PlatformInformation.cs
===================================================================
--- tarball.6.0.1-rc2-6.0.100-rc2.orig/src/roslyn.8e1779e16298415843e85029d8b52a1ae9bb4c30/src/Compilers/Core/Portable/InternalUtilities/PlatformInformation.cs
+++ tarball.6.0.1-rc2-6.0.100-rc2/src/roslyn.8e1779e16298415843e85029d8b52a1ae9bb4c30/src/Compilers/Core/Portable/InternalUtilities/PlatformInformation.cs
@@ -31,5 +31,24 @@ namespace Roslyn.Utilities
                 }
             }
         }
+        /// <summary>
+        /// Are we running on .NET 5 or later using the Mono runtime?
+        /// Will also return true when running on Mono itself; if necessary
+        /// we can use IsRunningOnMono to distinguish.
+        /// </summary>
+        public static bool IsUsingMonoRuntime
+        {
+            get
+            {
+                try
+                {
+                    return !(Type.GetType("Mono.RuntimeStructs", throwOnError: false) is null);
+                }
+                catch
+                {
+                    return false;
+                }
+            }
+        }
     }
 }
Index: tarball.6.0.1-rc2-6.0.100-rc2/src/roslyn.8e1779e16298415843e85029d8b52a1ae9bb4c30/src/Compilers/Server/VBCSCompilerTests/BuildClientTests.cs
===================================================================
--- tarball.6.0.1-rc2-6.0.100-rc2.orig/src/roslyn.8e1779e16298415843e85029d8b52a1ae9bb4c30/src/Compilers/Server/VBCSCompilerTests/BuildClientTests.cs
+++ tarball.6.0.1-rc2-6.0.100-rc2/src/roslyn.8e1779e16298415843e85029d8b52a1ae9bb4c30/src/Compilers/Server/VBCSCompilerTests/BuildClientTests.cs
@@ -79,7 +79,7 @@ namespace Microsoft.CodeAnalysis.Compile
                 // to connect. When it fails it should fall back to in-proc
                 // compilation.
                 bool holdsMutex;
-                using (var serverMutex = new Mutex(initiallyOwned: true,
+                using (var serverMutex = BuildServerConnection.OpenOrCreateMutex(
                                                    name: BuildServerConnection.GetServerMutexName(_pipeName),
                                                    createdNew: out holdsMutex))
                 {
Index: tarball.6.0.1-rc2-6.0.100-rc2/src/roslyn.8e1779e16298415843e85029d8b52a1ae9bb4c30/src/Compilers/Server/VBCSCompilerTests/CompilerServerApiTest.cs
===================================================================
--- tarball.6.0.1-rc2-6.0.100-rc2.orig/src/roslyn.8e1779e16298415843e85029d8b52a1ae9bb4c30/src/Compilers/Server/VBCSCompilerTests/CompilerServerApiTest.cs
+++ tarball.6.0.1-rc2-6.0.100-rc2/src/roslyn.8e1779e16298415843e85029d8b52a1ae9bb4c30/src/Compilers/Server/VBCSCompilerTests/CompilerServerApiTest.cs
@@ -103,7 +103,7 @@ class Hello
             var mutexName = BuildServerConnection.GetServerMutexName(pipeName);
 
             bool holdsMutex;
-            using (var mutex = new Mutex(initiallyOwned: true,
+            using (var mutex = BuildServerConnection.OpenOrCreateMutex(
                                          name: mutexName,
                                          createdNew: out holdsMutex))
             {
@@ -119,7 +119,7 @@ class Hello
                 }
                 finally
                 {
-                    mutex.ReleaseMutex();
+                    mutex.Dispose();
                 }
             }
         }
Index: tarball.6.0.1-rc2-6.0.100-rc2/src/roslyn.8e1779e16298415843e85029d8b52a1ae9bb4c30/src/Compilers/Server/VBCSCompilerTests/CompilerServerTests.cs
===================================================================
--- tarball.6.0.1-rc2-6.0.100-rc2.orig/src/roslyn.8e1779e16298415843e85029d8b52a1ae9bb4c30/src/Compilers/Server/VBCSCompilerTests/CompilerServerTests.cs
+++ tarball.6.0.1-rc2-6.0.100-rc2/src/roslyn.8e1779e16298415843e85029d8b52a1ae9bb4c30/src/Compilers/Server/VBCSCompilerTests/CompilerServerTests.cs
@@ -304,7 +304,7 @@ End Module")
             var newTempDir = _tempDirectory.CreateDirectory(new string('a', 100 - _tempDirectory.Path.Length));
             await ApplyEnvironmentVariables(
                 new[] { new KeyValuePair<string, string>("TMPDIR", newTempDir.Path) },
-                async () =>
+                async () => await Task.Run(async () =>
             {
                 using var serverData = await ServerUtil.CreateServer(_logger);
                 var result = RunCommandLineCompiler(
@@ -317,7 +317,7 @@ End Module")
 
                 var listener = await serverData.Complete();
                 Assert.Equal(CompletionData.RequestCompleted, listener.CompletionDataList.Single());
-            });
+            }));
         }
 
         [Fact]
Index: tarball.6.0.1-rc2-6.0.100-rc2/src/roslyn.8e1779e16298415843e85029d8b52a1ae9bb4c30/src/Compilers/Server/VBCSCompilerTests/VBCSCompilerServerTests.cs
===================================================================
--- tarball.6.0.1-rc2-6.0.100-rc2.orig/src/roslyn.8e1779e16298415843e85029d8b52a1ae9bb4c30/src/Compilers/Server/VBCSCompilerTests/VBCSCompilerServerTests.cs
+++ tarball.6.0.1-rc2-6.0.100-rc2/src/roslyn.8e1779e16298415843e85029d8b52a1ae9bb4c30/src/Compilers/Server/VBCSCompilerTests/VBCSCompilerServerTests.cs
@@ -101,7 +101,7 @@ namespace Microsoft.CodeAnalysis.Compile
 
                     var thread = new Thread(() =>
                     {
-                        using (var mutex = new Mutex(initiallyOwned: true, name: mutexName, createdNew: out created))
+                        using (var mutex = BuildServerConnection.OpenOrCreateMutex(name: mutexName, createdNew: out created))
                         using (var stream = NamedPipeUtil.CreateServer(pipeName))
                         {
                             readyMre.Set();
@@ -112,7 +112,7 @@ namespace Microsoft.CodeAnalysis.Compile
                             stream.Close();
 
                             doneMre.WaitOne();
-                            mutex.ReleaseMutex();
+                            mutex.Dispose();
                         }
                     });
 
@@ -153,7 +153,7 @@ namespace Microsoft.CodeAnalysis.Compile
                     {
                         using (var stream = NamedPipeUtil.CreateServer(pipeName))
                         {
-                            var mutex = new Mutex(initiallyOwned: true, name: mutexName, createdNew: out created);
+                            var mutex = BuildServerConnection.OpenOrCreateMutex(name: mutexName, createdNew: out created);
                             readyMre.Set();
 
                             stream.WaitForConnection();
@@ -161,7 +161,6 @@ namespace Microsoft.CodeAnalysis.Compile
 
                             // Client is waiting for a response.  Close the mutex now.  Then close the connection 
                             // so the client gets an error.
-                            mutex.ReleaseMutex();
                             mutex.Dispose();
                             stream.Close();
 
Index: tarball.6.0.1-rc2-6.0.100-rc2/src/roslyn.8e1779e16298415843e85029d8b52a1ae9bb4c30/src/Compilers/Shared/BuildServerConnection.cs
===================================================================
--- tarball.6.0.1-rc2-6.0.100-rc2.orig/src/roslyn.8e1779e16298415843e85029d8b52a1ae9bb4c30/src/Compilers/Shared/BuildServerConnection.cs
+++ tarball.6.0.1-rc2-6.0.100-rc2/src/roslyn.8e1779e16298415843e85029d8b52a1ae9bb4c30/src/Compilers/Shared/BuildServerConnection.cs
@@ -543,19 +543,10 @@ namespace Microsoft.CodeAnalysis.Command
         {
             try
             {
-                if (PlatformInformation.IsRunningOnMono)
+                if (PlatformInformation.IsUsingMonoRuntime)
                 {
-                    IServerMutex? mutex = null;
-                    bool createdNew = false;
-                    try
-                    {
-                        mutex = new ServerFileMutexPair(mutexName, false, out createdNew);
-                        return !createdNew;
-                    }
-                    finally
-                    {
-                        mutex?.Dispose();
-                    }
+                    using var mutex = new ServerFileMutex(mutexName);
+                    return !mutex.CouldLock();
                 }
                 else
                 {
@@ -572,9 +563,11 @@ namespace Microsoft.CodeAnalysis.Command
 
         internal static IServerMutex OpenOrCreateMutex(string name, out bool createdNew)
         {
-            if (PlatformInformation.IsRunningOnMono)
+            if (PlatformInformation.IsUsingMonoRuntime)
             {
-                return new ServerFileMutexPair(name, initiallyOwned: true, out createdNew);
+                var mutex = new ServerFileMutex(name);
+                createdNew = mutex.TryLock(0);
+                return mutex;
             }
             else
             {
@@ -648,19 +641,22 @@ namespace Microsoft.CodeAnalysis.Command
     }
 
     /// <summary>
-    /// An interprocess mutex abstraction based on OS advisory locking (FileStream.Lock/Unlock).
+    /// An interprocess mutex abstraction based on file sharing permission (FileShare.None).
     /// If multiple processes running as the same user create FileMutex instances with the same name,
     ///  those instances will all point to the same file somewhere in a selected temporary directory.
-    /// The TryLock method can be used to attempt to acquire the mutex, with Unlock or Dispose used to release.
+    /// The TryLock method can be used to attempt to acquire the mutex, with Dispose used to release.
+    /// The CouldLock method can be used to check whether an attempt to acquire the mutex would have
+    ///  succeeded at the current time, without actually acquiring it.
     /// Unlike Win32 named mutexes, there is no mechanism for detecting an abandoned mutex. The file
     ///  will simply revert to being unlocked but remain where it is.
     /// </summary>
-    internal sealed class FileMutex : IDisposable
+    internal sealed class ServerFileMutex : IServerMutex
     {
-        public readonly FileStream Stream;
+        public FileStream? Stream;
         public readonly string FilePath;
+        public readonly string GuardPath;
 
-        public bool IsLocked { get; private set; }
+        public bool IsDisposed { get; private set; }
 
         internal static string GetMutexDirectory()
         {
@@ -670,61 +666,176 @@ namespace Microsoft.CodeAnalysis.Command
             return result;
         }
 
-        public FileMutex(string name)
+        public ServerFileMutex(string name)
         {
-            FilePath = Path.Combine(GetMutexDirectory(), name);
-            Stream = new FileStream(FilePath, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None);
+            var mutexDirectory = GetMutexDirectory();
+            FilePath = Path.Combine(mutexDirectory, name);
+            GuardPath = Path.Combine(mutexDirectory, ".guard");
         }
 
-        public bool TryLock(int timeoutMs)
+        /// <summary>
+        /// Acquire the guard by opening the guard file with FileShare.None.  The guard must only ever
+        /// be held for very brief amounts of time, so we can simply spin until it is acquired.  The
+        /// guard must be released by disposing the FileStream returned from this routine.  Note the
+        /// guard file is never deleted; this is a leak, but only of a single file.
+        /// </summary>
+        internal FileStream LockGuard()
         {
-            if (IsLocked)
-                throw new InvalidOperationException("Lock already held");
-
-            var sw = Stopwatch.StartNew();
-            do
+            // We should be able to acquire the guard quickly.  Limit the number of retries anyway
+            // by some arbitrary bound to avoid getting hung up in a possibly infinite loop.
+            for (var i = 0; i < 100; i++)
             {
                 try
                 {
-                    Stream.Lock(0, 0);
-                    IsLocked = true;
-                    return true;
+                    return new FileStream(GuardPath, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None);
                 }
                 catch (IOException)
                 {
-                    // Lock currently held by someone else.
+                    // Guard currently held by someone else.
                     // We want to sleep for a short period of time to ensure that other processes
                     //  have an opportunity to finish their work and relinquish the lock.
                     // Spinning here (via Yield) would work but risks creating a priority
                     //  inversion if the lock is held by a lower-priority process.
                     Thread.Sleep(1);
                 }
+            }
+            // Handle unexpected failure to acquire guard as error.
+            throw new InvalidOperationException("Unable to acquire guard");
+        }
+
+        /// <summary>
+        /// Attempt to acquire the lock by opening the lock file with FileShare.None.  Sets "Stream"
+        /// and returns true if successful, returns false if the lock is already held by another
+        /// thread or process.  Guard must be held when calling this routine.
+        /// </summary>
+        internal bool TryLockFile()
+        {
+            Debug.Assert(Stream is null);
+            FileStream? stream = null;
+            try
+            {
+                stream = new FileStream(FilePath, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None);
+                // On some targets, the file locking used to implement FileShare.None may not be
+                // atomic with opening/creating the file.   This creates a race window when another
+                // thread holds the lock and is just about to unlock: we may be able to open the
+                // file here, then the other thread unlocks and deletes the file, and then we
+                // acquire the lock on our file handle - but the actual file is already deleted.
+                // To close this race, we verify that the file does in fact still exist now that
+                // we have successfull acquired the locked FileStream.   (Note that this check is
+                // safe because we cannot race with an other attempt to create the file since we
+                // hold the guard, and after the FileStream constructor returned we can no race
+                // with file deletion because we hold the lock.)
+                if (!File.Exists(FilePath))
+                {
+                    // To simplify the logic, we treat this case as "unable to acquire the lock"
+                    // because it we caught another process while it owned the lock and was just
+                    // giving it up.  If the caller retries, we'll likely acquire the lock then.
+                    stream.Dispose();
+                    return false;
+                }
+            }
+            catch (Exception)
+            {
+                stream?.Dispose();
+                return false;
+            }
+            Stream = stream;
+            return true;
+        }
+
+        /// <summary>
+        /// Release the lock by deleting the lock file and disposing "Stream".
+        /// </summary>
+        internal void UnlockFile()
+        {
+            Debug.Assert(Stream is not null);
+            try
+            {
+                // Delete the lock file while the stream is not yet disposed
+                // and we therefore still hold the FileShare.None exclusion.
+                // There may still be a race with another thread attempting a
+                // TryLockFile in parallel, but that is safely handled there.
+                File.Delete(FilePath);
+            }
+            finally
+            {
+                Stream.Dispose();
+                Stream = null;
+            }
+        }
+
+        public bool TryLock(int timeoutMs)
+        {
+            if (IsDisposed)
+                throw new ObjectDisposedException("Mutex");
+            if (Stream is not null)
+                throw new InvalidOperationException("Lock already held");
+
+            var sw = Stopwatch.StartNew();
+            do
+            {
+                try
+                {
+                    // Attempt to acquire lock while holding guard.
+                    using var guard = LockGuard();
+                    if (TryLockFile())
+                        return true;
+                }
                 catch (Exception)
                 {
-                    // Something else went wrong.
                     return false;
                 }
+
+                // See comment in LockGuard.
+                Thread.Sleep(1);
             } while (sw.ElapsedMilliseconds < timeoutMs);
 
             return false;
         }
 
-        public void Unlock()
+        public bool CouldLock()
         {
-            if (!IsLocked)
-                return;
-            Stream.Unlock(0, 0);
-            IsLocked = false;
+            if (IsDisposed)
+                return false;
+            if (Stream is not null)
+                return false;
+
+            try
+            {
+                // Attempt to acquire lock while holding guard, and if successful
+                // immediately unlock again while still holding guard.  This ensures
+                // no other thread will spuriously observe the lock as held due to
+                // the lock attempt here.
+                using var guard = LockGuard();
+                if (TryLockFile())
+                {
+                    UnlockFile();
+                    return true;
+                }
+            }
+            catch (Exception)
+            {
+                return false;
+            }
+
+            return false;
         }
 
         public void Dispose()
         {
-            var wasLocked = IsLocked;
-            if (wasLocked)
-                Unlock();
-            Stream.Dispose();
-            // We do not delete the lock file here because there is no reliable way to perform a
-            //  'delete if no one has the file open' operation atomically on *nix. This is a leak.
+            if (IsDisposed)
+                return;
+            IsDisposed = true;
+            if (Stream is not null)
+            {
+                try
+                {
+                    UnlockFile();
+                }
+                catch (Exception)
+                {
+                }
+            }
         }
     }
 
@@ -792,56 +903,4 @@ namespace Microsoft.CodeAnalysis.Command
             }
         }
     }
-
-    /// <summary>
-    /// Approximates a named mutex with 'locked', 'unlocked' and 'abandoned' states.
-    /// There is no reliable way to detect whether a mutex has been abandoned on some target platforms,
-    ///  so we use the AliveMutex to manually track whether the creator of a mutex is still running,
-    ///  while the HeldMutex represents the actual lock state of the mutex.
-    /// </summary>
-    internal sealed class ServerFileMutexPair : IServerMutex
-    {
-        public readonly FileMutex AliveMutex;
-        public readonly FileMutex HeldMutex;
-
-        public bool IsDisposed { get; private set; }
-
-        public ServerFileMutexPair(string mutexName, bool initiallyOwned, out bool createdNew)
-        {
-            AliveMutex = new FileMutex(mutexName + "-alive");
-            HeldMutex = new FileMutex(mutexName + "-held");
-            createdNew = AliveMutex.TryLock(0);
-            if (initiallyOwned && createdNew)
-            {
-                if (!TryLock(0))
-                    throw new Exception("Failed to lock mutex after creating it");
-            }
-        }
-
-        public bool TryLock(int timeoutMs)
-        {
-            if (IsDisposed)
-                throw new ObjectDisposedException("Mutex");
-            return HeldMutex.TryLock(timeoutMs);
-        }
-
-        public void Dispose()
-        {
-            if (IsDisposed)
-                return;
-            IsDisposed = true;
-
-            try
-            {
-                HeldMutex.Unlock();
-                AliveMutex.Unlock();
-            }
-            finally
-            {
-                AliveMutex.Dispose();
-                HeldMutex.Dispose();
-            }
-        }
-    }
-
 }