forge/ayaports
1
0
Fork 0
Code Issues 43 Pull requests 11 Projects Releases Packages Wiki Activity Actions
ayaports/community/dotnet6-build/roslyn_57003-mono-namedmutex.patch

470 lines
20 KiB
Diff
Raw Blame History

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
Reference in a new issue View git blame Copy permalink