Blame SOURCES/roslyn-57003-mono-named-mutex.patch

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