diff --git a/build/dependencies.props b/build/dependencies.props index b3d0015da9..846ef0a842 100644 --- a/build/dependencies.props +++ b/build/dependencies.props @@ -42,8 +42,8 @@ 2.0.1 11.0.1 1.1.92 - 4.5.0-preview2-26313-01 - 4.5.0-preview2-26313-01 + 4.5.0-preview2-26326-04 + 4.5.0-preview2-26326-04 9.0.1 2.7.0-beta3-62512-06 2.7.0-beta3-62512-06 diff --git a/src/Microsoft.AspNetCore.Razor.Tools/ServerCommand.cs b/src/Microsoft.AspNetCore.Razor.Tools/ServerCommand.cs index 65a70b51ed..5a153eee80 100644 --- a/src/Microsoft.AspNetCore.Razor.Tools/ServerCommand.cs +++ b/src/Microsoft.AspNetCore.Razor.Tools/ServerCommand.cs @@ -34,37 +34,51 @@ namespace Microsoft.AspNetCore.Razor.Tools protected override Task ExecuteCoreAsync() { // Make sure there's only one server with the same identity at a time. - using (var mutex = new Mutex(initiallyOwned: true, name: MutexName.GetServerMutexName(Pipe.Value()), createdNew: out var holdsMutex)) + var serverMutexName = MutexName.GetServerMutexName(Pipe.Value()); + Mutex serverMutex = null; + var holdsMutex = false; + + try { - if (!holdsMutex) + serverMutex = new Mutex(initiallyOwned: true, name: serverMutexName, createdNew: out holdsMutex); + } + catch (Exception ex) + { + // The Mutex constructor can throw in certain cases. One specific example is docker containers + // where the /tmp directory is restricted. In those cases there is no reliable way to execute + // the server and we need to fall back to the command line. + // Example: https://github.com/dotnet/roslyn/issues/24124 + + Error.Write($"Server mutex creation failed. {ex.Message}"); + + return Task.FromResult(-1); + } + + if (!holdsMutex) + { + // Another server is running, just exit. + Error.Write("Another server already running..."); + return Task.FromResult(1); + } + + try + { + TimeSpan? keepAlive = null; + if (KeepAlive.HasValue() && int.TryParse(KeepAlive.Value(), out var result)) { - // Another server is running, just exit. - Error.Write("Another server already running..."); - return Task.FromResult(1); + // Keep alive times are specified in seconds + keepAlive = TimeSpan.FromSeconds(result); } - try - { - TimeSpan? keepAlive = null; - if (KeepAlive.HasValue()) - { - var value = KeepAlive.Value(); - if (int.TryParse(value, out var result)) - { - // Keep alive times are specified in seconds - keepAlive = TimeSpan.FromSeconds(result); - } - } + var host = ConnectionHost.Create(Pipe.Value()); - var host = ConnectionHost.Create(Pipe.Value()); - - var compilerHost = CompilerHost.Create(); - ExecuteServerCore(host, compilerHost, Cancelled, eventBus: null, keepAlive: keepAlive); - } - finally - { - mutex.ReleaseMutex(); - } + var compilerHost = CompilerHost.Create(); + ExecuteServerCore(host, compilerHost, Cancelled, eventBus: null, keepAlive: keepAlive); + } + finally + { + serverMutex.ReleaseMutex(); + serverMutex.Dispose(); } return Task.FromResult(0); diff --git a/src/Microsoft.AspNetCore.Razor.Tools/ServerProtocol/ServerConnection.cs b/src/Microsoft.AspNetCore.Razor.Tools/ServerProtocol/ServerConnection.cs index 1c9f122ace..1bd9a1365b 100644 --- a/src/Microsoft.AspNetCore.Razor.Tools/ServerProtocol/ServerConnection.cs +++ b/src/Microsoft.AspNetCore.Razor.Tools/ServerProtocol/ServerConnection.cs @@ -23,13 +23,21 @@ namespace Microsoft.AspNetCore.Razor.Tools public static bool WasServerMutexOpen(string mutexName) { - var open = Mutex.TryOpenExisting(mutexName, out var mutex); - if (open) + Mutex mutex = null; + var open = false; + try { - mutex.Dispose(); - return true; + open = Mutex.TryOpenExisting(mutexName, out mutex); } - return false; + catch + { + // In the case an exception occured trying to open the Mutex then + // the assumption is that it's not open. + } + + mutex?.Dispose(); + + return open; } /// @@ -129,44 +137,63 @@ namespace Microsoft.AspNetCore.Razor.Tools var timeoutExistingProcess = timeoutOverride ?? TimeOutMsExistingProcess; var clientMutexName = MutexName.GetClientMutexName(pipeName); Task pipeTask = null; - using (var clientMutex = new Mutex(initiallyOwned: true, name: clientMutexName, createdNew: out var holdsMutex)) + + Mutex clientMutex = null; + var holdsMutex = false; + + try { try { - if (!holdsMutex) - { - try - { - holdsMutex = clientMutex.WaitOne(timeoutNewProcess); - - if (!holdsMutex) - { - return new RejectedServerResponse(); - } - } - catch (AbandonedMutexException) - { - holdsMutex = true; - } - } - - // Check for an already running server - var serverMutexName = MutexName.GetServerMutexName(pipeName); - var wasServerRunning = WasServerMutexOpen(serverMutexName); - var timeout = wasServerRunning ? timeoutExistingProcess : timeoutNewProcess; - - if (wasServerRunning || tryCreateServerFunc(clientDir, pipeName, debug)) - { - pipeTask = Client.ConnectAsync(pipeName, TimeSpan.FromMilliseconds(timeout), cancellationToken); - } + clientMutex = new Mutex(initiallyOwned: true, name: clientMutexName, createdNew: out holdsMutex); } - finally + catch (Exception ex) { - if (holdsMutex) + // The Mutex constructor can throw in certain cases. One specific example is docker containers + // where the /tmp directory is restricted. In those cases there is no reliable way to execute + // the server and we need to fall back to the command line. + // Example: https://github.com/dotnet/roslyn/issues/24124 + + ServerLogger.LogException(ex, "Client mutex creation failed."); + + return new RejectedServerResponse(); + } + + if (!holdsMutex) + { + try { - clientMutex.ReleaseMutex(); + holdsMutex = clientMutex.WaitOne(timeoutNewProcess); + + if (!holdsMutex) + { + return new RejectedServerResponse(); + } + } + catch (AbandonedMutexException) + { + holdsMutex = true; } } + + // Check for an already running server + var serverMutexName = MutexName.GetServerMutexName(pipeName); + var wasServerRunning = WasServerMutexOpen(serverMutexName); + var timeout = wasServerRunning ? timeoutExistingProcess : timeoutNewProcess; + + if (wasServerRunning || tryCreateServerFunc(clientDir, pipeName, debug)) + { + pipeTask = Client.ConnectAsync(pipeName, TimeSpan.FromMilliseconds(timeout), cancellationToken); + } + } + finally + { + if (holdsMutex) + { + clientMutex?.ReleaseMutex(); + } + + clientMutex?.Dispose(); } if (pipeTask != null) diff --git a/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/BuildServerIntegrationTest.cs b/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/BuildServerIntegrationTest.cs index 52f02f4aea..d033449e9c 100644 --- a/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/BuildServerIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/BuildServerIntegrationTest.cs @@ -103,5 +103,35 @@ namespace Microsoft.AspNetCore.Razor.Design.IntegrationTests Assert.FileExists(result, IntermediateOutputPath, "SimpleMvc.dll"); Assert.FileDoesNotExist(result, IntermediateOutputPath, "SimpleMvc.Views.dll"); } + + [Fact] + [InitializeTestProject("SimpleMvc")] + public async Task Build_ServerConnectionMutexCreationFails_FallsBackToInProcessRzc() + { + // Use a pipe name longer that 260 characters to make the Mutex constructor throw. + var pipeName = new string('P', 261); + var result = await DotnetMSBuild( + "Build", + "/p:_RazorForceBuildServer=true", + buildServerPipeName: pipeName); + + // We expect this to fail because we don't allow it to fallback to in process execution. + Assert.BuildFailed(result); + Assert.FileDoesNotExist(result, IntermediateOutputPath, "SimpleMvc.Views.dll"); + + // Try to build again without forcing it to run on build server. + result = await DotnetMSBuild( + "Build", + "/p:_RazorForceBuildServer=false", + buildServerPipeName: pipeName); + + Assert.BuildPassed(result); + + Assert.FileExists(result, IntermediateOutputPath, "SimpleMvc.dll"); + Assert.FileExists(result, IntermediateOutputPath, "SimpleMvc.Views.dll"); + + // Note: We don't need to handle server clean up here because it will fail before + // it reaches server creation part. + } } } diff --git a/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/MSBuildIntegrationTestBase.cs b/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/MSBuildIntegrationTestBase.cs index d039aa6345..a0abcb93df 100644 --- a/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/MSBuildIntegrationTestBase.cs +++ b/test/Microsoft.AspNetCore.Razor.Design.Test/IntegrationTests/MSBuildIntegrationTestBase.cs @@ -58,6 +58,7 @@ namespace Microsoft.AspNetCore.Razor.Design.IntegrationTests bool suppressRestore = false, bool suppressTimeout = false, bool suppressBuildServer = false, + string buildServerPipeName = null, MSBuildProcessKind msBuildProcessKind = MSBuildProcessKind.Dotnet) { var timeout = suppressTimeout ? (TimeSpan?)Timeout.InfiniteTimeSpan : null; @@ -70,7 +71,7 @@ namespace Microsoft.AspNetCore.Razor.Design.IntegrationTests if (!suppressBuildServer) { - buildArgumentList.Add($"/p:_RazorBuildServerPipeName={BuildServer.PipeName}"); + buildArgumentList.Add($"/p:_RazorBuildServerPipeName={buildServerPipeName ?? BuildServer.PipeName}"); } buildArgumentList.Add($"/t:{target} /p:Configuration={Configuration} {args}");