470 lines
20 KiB
Diff
470 lines
20 KiB
Diff
From 210c17ea60f525837a7525df73e7332598ad4089 Mon Sep 17 00:00:00 2001
|
|
Patch-Source: https://github.com/dotnet/roslyn/pull/57003
|
|
From: Antoine Martin <dev@ayakael.net>
|
|
Date: Sat, 27 Aug 2022 21:26:01 -0800
|
|
Subject: [PATCH 1/1] mono-named-mutex
|
|
|
|
---
|
|
.../InternalUtilities/PlatformInformation.cs | 19 ++
|
|
.../VBCSCompilerTests/BuildClientTests.cs | 2 +-
|
|
.../CompilerServerApiTest.cs | 4 +-
|
|
.../VBCSCompilerTests/CompilerServerTests.cs | 4 +-
|
|
.../VBCSCompilerServerTests.cs | 7 +-
|
|
src/Compilers/Shared/BuildServerConnection.cs | 251 +++++++++++-------
|
|
6 files changed, 182 insertions(+), 105 deletions(-)
|
|
|
|
diff --git a/src/roslyn/src/Compilers/Core/Portable/InternalUtilities/PlatformInformation.cs b/src/roslyn/src/Compilers/Core/Portable/InternalUtilities/PlatformInformation.cs
|
|
index 033e66cd2f2..d4fa56413fb 100644
|
|
--- a/src/roslyn/src/Compilers/Core/Portable/InternalUtilities/PlatformInformation.cs
|
|
+++ b/src/roslyn/src/Compilers/Core/Portable/InternalUtilities/PlatformInformation.cs
|
|
@@ -31,5 +31,24 @@ public static bool IsRunningOnMono
|
|
}
|
|
}
|
|
}
|
|
+ /// <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;
|
|
+ }
|
|
+ }
|
|
+ }
|
|
}
|
|
}
|
|
diff --git a/src/roslyn/src/Compilers/Server/VBCSCompilerTests/BuildClientTests.cs b/src/roslyn/src/Compilers/Server/VBCSCompilerTests/BuildClientTests.cs
|
|
index 669d1bfb676..7f1d0468823 100644
|
|
--- a/src/roslyn/src/Compilers/Server/VBCSCompilerTests/BuildClientTests.cs
|
|
+++ b/src/roslyn/src/Compilers/Server/VBCSCompilerTests/BuildClientTests.cs
|
|
@@ -79,7 +79,7 @@ public void ConnectToServerFails()
|
|
// 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))
|
|
{
|
|
diff --git a/src/roslyn/src/Compilers/Server/VBCSCompilerTests/CompilerServerApiTest.cs b/src/roslyn/src/Compilers/Server/VBCSCompilerTests/CompilerServerApiTest.cs
|
|
index 0dbd1b2e143..c00b72e8434 100644
|
|
--- a/src/roslyn/src/Compilers/Server/VBCSCompilerTests/CompilerServerApiTest.cs
|
|
+++ b/src/roslyn/src/Compilers/Server/VBCSCompilerTests/CompilerServerApiTest.cs
|
|
@@ -103,7 +103,7 @@ public void MutexStopsServerStarting()
|
|
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 @@ public void MutexStopsServerStarting()
|
|
}
|
|
finally
|
|
{
|
|
- mutex.ReleaseMutex();
|
|
+ mutex.Dispose();
|
|
}
|
|
}
|
|
}
|
|
diff --git a/src/roslyn/src/Compilers/Server/VBCSCompilerTests/CompilerServerTests.cs b/src/roslyn/src/Compilers/Server/VBCSCompilerTests/CompilerServerTests.cs
|
|
index 3b6f4f291ff..e97a6bf923f 100644
|
|
--- a/src/roslyn/src/Compilers/Server/VBCSCompilerTests/CompilerServerTests.cs
|
|
+++ b/src/roslyn/src/Compilers/Server/VBCSCompilerTests/CompilerServerTests.cs
|
|
@@ -304,7 +304,7 @@ public async Task ServerFailsWithLongTempPathUnix()
|
|
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 @@ public async Task ServerFailsWithLongTempPathUnix()
|
|
|
|
var listener = await serverData.Complete();
|
|
Assert.Equal(CompletionData.RequestCompleted, listener.CompletionDataList.Single());
|
|
- });
|
|
+ }));
|
|
}
|
|
|
|
[Fact]
|
|
diff --git a/src/roslyn/src/Compilers/Server/VBCSCompilerTests/VBCSCompilerServerTests.cs b/src/roslyn/src/Compilers/Server/VBCSCompilerTests/VBCSCompilerServerTests.cs
|
|
index d5f493fed8a..73941972e48 100644
|
|
--- a/src/roslyn/src/Compilers/Server/VBCSCompilerTests/VBCSCompilerServerTests.cs
|
|
+++ b/src/roslyn/src/Compilers/Server/VBCSCompilerTests/VBCSCompilerServerTests.cs
|
|
@@ -101,7 +101,7 @@ public async Task NoServerConnection()
|
|
|
|
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 @@ public async Task NoServerConnection()
|
|
stream.Close();
|
|
|
|
doneMre.WaitOne();
|
|
- mutex.ReleaseMutex();
|
|
+ mutex.Dispose();
|
|
}
|
|
});
|
|
|
|
@@ -153,7 +153,7 @@ public async Task ServerShutdownsDuringProcessing()
|
|
{
|
|
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 @@ public async Task ServerShutdownsDuringProcessing()
|
|
|
|
// 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();
|
|
|
|
diff --git a/src/roslyn/src/Compilers/Shared/BuildServerConnection.cs b/src/roslyn/src/Compilers/Shared/BuildServerConnection.cs
|
|
index f67c2d83957..1fe609061ee 100644
|
|
--- a/src/roslyn/src/Compilers/Shared/BuildServerConnection.cs
|
|
+++ b/src/roslyn/src/Compilers/Shared/BuildServerConnection.cs
|
|
@@ -543,19 +543,10 @@ internal static bool WasServerMutexOpen(string mutexName)
|
|
{
|
|
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 @@ internal static bool WasServerMutexOpen(string mutexName)
|
|
|
|
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 @@ internal interface IServerMutex : IDisposable
|
|
}
|
|
|
|
/// <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 @@ internal static string GetMutexDirectory()
|
|
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 @@ public void Dispose()
|
|
}
|
|
}
|
|
}
|
|
-
|
|
- /// <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();
|
|
- }
|
|
- }
|
|
- }
|
|
-
|
|
}
|
|
--
|
|
2.36.2
|