Skip to content

Commit c45e40e

Browse files
committed
Fix named mutex workaround for .NET Mono runtime
* Enable named mutex workaround when running on .NET using Mono runtime * Re-implement file locking logic to ensure intra- and inter-process mutual exclusion, and no longer leak lock files * Use mutex abstraction throughout test suite * Fix async race in ServerFailsWithLongTempPathUnix test case * Fixes #57002
1 parent d1a39e2 commit c45e40e

File tree

6 files changed

+54
-54
lines changed

6 files changed

+54
-54
lines changed

src/Compilers/Core/Portable/InternalUtilities/PlatformInformation.cs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,5 +31,22 @@ public static bool IsRunningOnMono
3131
}
3232
}
3333
}
34+
// Are we running on .NET 5 or later using the Mono runtime?
35+
// Will also return true when running on Mono itself; if necessary
36+
// we can use the above IsRunningOnMono to distinguish.
37+
public static bool IsUsingMonoRuntime
38+
{
39+
get
40+
{
41+
try
42+
{
43+
return !(Type.GetType("Mono.RuntimeStructs") is null);
44+
}
45+
catch
46+
{
47+
return false;
48+
}
49+
}
50+
}
3451
}
3552
}

src/Compilers/Server/VBCSCompilerTests/BuildClientTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ public void ConnectToServerFails()
7979
// to connect. When it fails it should fall back to in-proc
8080
// compilation.
8181
bool holdsMutex;
82-
using (var serverMutex = new Mutex(initiallyOwned: true,
82+
using (var serverMutex = BuildServerConnection.OpenOrCreateMutex(
8383
name: BuildServerConnection.GetServerMutexName(_pipeName),
8484
createdNew: out holdsMutex))
8585
{

src/Compilers/Server/VBCSCompilerTests/CompilerServerApiTest.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ public void MutexStopsServerStarting()
103103
var mutexName = BuildServerConnection.GetServerMutexName(pipeName);
104104

105105
bool holdsMutex;
106-
using (var mutex = new Mutex(initiallyOwned: true,
106+
using (var mutex = BuildServerConnection.OpenOrCreateMutex(
107107
name: mutexName,
108108
createdNew: out holdsMutex))
109109
{
@@ -119,7 +119,7 @@ public void MutexStopsServerStarting()
119119
}
120120
finally
121121
{
122-
mutex.ReleaseMutex();
122+
mutex.Dispose();
123123
}
124124
}
125125
}

src/Compilers/Server/VBCSCompilerTests/CompilerServerTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,7 @@ public async Task ServerFailsWithLongTempPathUnix()
304304
var newTempDir = _tempDirectory.CreateDirectory(new string('a', 100 - _tempDirectory.Path.Length));
305305
await ApplyEnvironmentVariables(
306306
new[] { new KeyValuePair<string, string>("TMPDIR", newTempDir.Path) },
307-
async () =>
307+
async () => await Task.Run(async () =>
308308
{
309309
using var serverData = await ServerUtil.CreateServer(_logger);
310310
var result = RunCommandLineCompiler(
@@ -317,7 +317,7 @@ await ApplyEnvironmentVariables(
317317

318318
var listener = await serverData.Complete();
319319
Assert.Equal(CompletionData.RequestCompleted, listener.CompletionDataList.Single());
320-
});
320+
}));
321321
}
322322

323323
[Fact]

src/Compilers/Server/VBCSCompilerTests/VBCSCompilerServerTests.cs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ public async Task NoServerConnection()
101101

102102
var thread = new Thread(() =>
103103
{
104-
using (var mutex = new Mutex(initiallyOwned: true, name: mutexName, createdNew: out created))
104+
using (var mutex = BuildServerConnection.OpenOrCreateMutex(name: mutexName, createdNew: out created))
105105
using (var stream = NamedPipeUtil.CreateServer(pipeName))
106106
{
107107
readyMre.Set();
@@ -112,7 +112,7 @@ public async Task NoServerConnection()
112112
stream.Close();
113113

114114
doneMre.WaitOne();
115-
mutex.ReleaseMutex();
115+
mutex.Dispose();
116116
}
117117
});
118118

@@ -153,15 +153,14 @@ public async Task ServerShutdownsDuringProcessing()
153153
{
154154
using (var stream = NamedPipeUtil.CreateServer(pipeName))
155155
{
156-
var mutex = new Mutex(initiallyOwned: true, name: mutexName, createdNew: out created);
156+
var mutex = BuildServerConnection.OpenOrCreateMutex(name: mutexName, createdNew: out created);
157157
readyMre.Set();
158158

159159
stream.WaitForConnection();
160160
connected = true;
161161

162162
// Client is waiting for a response. Close the mutex now. Then close the connection
163163
// so the client gets an error.
164-
mutex.ReleaseMutex();
165164
mutex.Dispose();
166165
stream.Close();
167166

src/Compilers/Shared/BuildServerConnection.cs

Lines changed: 29 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -543,19 +543,9 @@ internal static bool WasServerMutexOpen(string mutexName)
543543
{
544544
try
545545
{
546-
if (PlatformInformation.IsRunningOnMono)
546+
if (PlatformInformation.IsUsingMonoRuntime)
547547
{
548-
IServerMutex? mutex = null;
549-
bool createdNew = false;
550-
try
551-
{
552-
mutex = new ServerFileMutexPair(mutexName, false, out createdNew);
553-
return !createdNew;
554-
}
555-
finally
556-
{
557-
mutex?.Dispose();
558-
}
548+
return ServerFileMutexPair.WasOpen(mutexName);
559549
}
560550
else
561551
{
@@ -572,9 +562,9 @@ internal static bool WasServerMutexOpen(string mutexName)
572562

573563
internal static IServerMutex OpenOrCreateMutex(string name, out bool createdNew)
574564
{
575-
if (PlatformInformation.IsRunningOnMono)
565+
if (PlatformInformation.IsUsingMonoRuntime)
576566
{
577-
return new ServerFileMutexPair(name, initiallyOwned: true, out createdNew);
567+
return new ServerFileMutexPair(name, out createdNew);
578568
}
579569
else
580570
{
@@ -648,16 +638,16 @@ internal interface IServerMutex : IDisposable
648638
}
649639

650640
/// <summary>
651-
/// An interprocess mutex abstraction based on OS advisory locking (FileStream.Lock/Unlock).
641+
/// An interprocess mutex abstraction based on file sharing permission (FileShare.None).
652642
/// If multiple processes running as the same user create FileMutex instances with the same name,
653643
/// those instances will all point to the same file somewhere in a selected temporary directory.
654-
/// The TryLock method can be used to attempt to acquire the mutex, with Unlock or Dispose used to release.
644+
/// The TryLock method can be used to attempt to acquire the mutex, with Dispose used to release.
655645
/// Unlike Win32 named mutexes, there is no mechanism for detecting an abandoned mutex. The file
656646
/// will simply revert to being unlocked but remain where it is.
657647
/// </summary>
658648
internal sealed class FileMutex : IDisposable
659649
{
660-
public readonly FileStream Stream;
650+
public FileStream? Stream;
661651
public readonly string FilePath;
662652

663653
public bool IsLocked { get; private set; }
@@ -673,7 +663,11 @@ internal static string GetMutexDirectory()
673663
public FileMutex(string name)
674664
{
675665
FilePath = Path.Combine(GetMutexDirectory(), name);
676-
Stream = new FileStream(FilePath, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None);
666+
}
667+
668+
public bool WasOpen()
669+
{
670+
return File.Exists(FilePath);
677671
}
678672

679673
public bool TryLock(int timeoutMs)
@@ -686,7 +680,7 @@ public bool TryLock(int timeoutMs)
686680
{
687681
try
688682
{
689-
Stream.Lock(0, 0);
683+
Stream = new FileStream(FilePath, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None);
690684
IsLocked = true;
691685
return true;
692686
}
@@ -709,22 +703,14 @@ public bool TryLock(int timeoutMs)
709703
return false;
710704
}
711705

712-
public void Unlock()
713-
{
714-
if (!IsLocked)
715-
return;
716-
Stream.Unlock(0, 0);
717-
IsLocked = false;
718-
}
719-
720706
public void Dispose()
721707
{
722-
var wasLocked = IsLocked;
723-
if (wasLocked)
724-
Unlock();
725-
Stream.Dispose();
726-
// We do not delete the lock file here because there is no reliable way to perform a
727-
// 'delete if no one has the file open' operation atomically on *nix. This is a leak.
708+
if (IsLocked)
709+
{
710+
File.Delete(FilePath);
711+
Stream!.Dispose();
712+
}
713+
IsLocked = false;
728714
}
729715
}
730716

@@ -806,18 +792,24 @@ internal sealed class ServerFileMutexPair : IServerMutex
806792

807793
public bool IsDisposed { get; private set; }
808794

809-
public ServerFileMutexPair(string mutexName, bool initiallyOwned, out bool createdNew)
795+
public ServerFileMutexPair(string mutexName, out bool createdNew)
810796
{
811797
AliveMutex = new FileMutex(mutexName + "-alive");
812798
HeldMutex = new FileMutex(mutexName + "-held");
813799
createdNew = AliveMutex.TryLock(0);
814-
if (initiallyOwned && createdNew)
800+
if (createdNew)
815801
{
816802
if (!TryLock(0))
817803
throw new Exception("Failed to lock mutex after creating it");
818804
}
819805
}
820806

807+
public static bool WasOpen(string mutexName)
808+
{
809+
var TestAliveMutex = new FileMutex(mutexName + "-alive");
810+
return TestAliveMutex.WasOpen();
811+
}
812+
821813
public bool TryLock(int timeoutMs)
822814
{
823815
if (IsDisposed)
@@ -831,16 +823,8 @@ public void Dispose()
831823
return;
832824
IsDisposed = true;
833825

834-
try
835-
{
836-
HeldMutex.Unlock();
837-
AliveMutex.Unlock();
838-
}
839-
finally
840-
{
841-
AliveMutex.Dispose();
842-
HeldMutex.Dispose();
843-
}
826+
HeldMutex.Dispose();
827+
AliveMutex.Dispose();
844828
}
845829
}
846830

0 commit comments

Comments
 (0)