Fix MutexStopsServerStarting suite

This suite was flaky because it relied on timeouts and cross process
communication.  The latter in particular was problematic because it
depended on being able to control the lifetime of VBCSCompiler.exe.
This is a process frequently killed in our scripts as it can lock DLLs
and is always safe to kill.  Fixed the flakiness by refactoring the APIs
and eliminating the timeouts

src/Compilers/Server/PortableServer/CoreClrCompilerServerHost.cs

@@ -15,17 +15,15 @@ namespace Microsoft.CodeAnalysis.CompilerServer
 {
     internal sealed class CoreClrCompilerServerHost : CompilerServerHost
     {
-        private readonly Func<string, MetadataReferenceProperties, PortableExecutableReference> _assemblyReferenceProvider = (path, properties) => new CachingMetadataReference(path, properties);
-        private readonly IAnalyzerAssemblyLoader _analyzerAssemblyLoader = CoreClrAnalyzerAssemblyLoader.CreateAndSetDefault();
+        public override IAnalyzerAssemblyLoader AnalyzerAssemblyLoader { get; }
 
-        public override IAnalyzerAssemblyLoader AnalyzerAssemblyLoader => _analyzerAssemblyLoader;
-
-        public override Func<string, MetadataReferenceProperties, PortableExecutableReference> AssemblyReferenceProvider => _assemblyReferenceProvider;
+        public override Func<string, MetadataReferenceProperties, PortableExecutableReference> AssemblyReferenceProvider { get; }
 
         internal CoreClrCompilerServerHost(string clientDirectory)
             :base(clientDirectory : clientDirectory, sdkDirectory: null)
         {
-
+            AssemblyReferenceProvider = (path, properties) => new CachingMetadataReference(path, properties);
+            AnalyzerAssemblyLoader = CoreClrAnalyzerAssemblyLoader.CreateAndSetDefault();
         }
 
         public override bool CheckAnalyzers(string baseDirectory, ImmutableArray<CommandLineAnalyzerReference> analyzers)

src/Compilers/Server/VBCSCompiler/VBCSCompiler.cs

@@ -17,18 +17,14 @@
 
 namespace Microsoft.CodeAnalysis.CompilerServer
 {
-    internal static class VBCSCmopiler
+    internal static class VBCSCompiler
     {
         public static int Main(string[] args)
         {
             CompilerServerLogger.Initialize("SRV");
             CompilerServerLogger.Log("Process started");
 
-            TimeSpan? keepAliveTimeout = null;
-
-            // VBCSCompiler is installed in the same directory as csc.exe and vbc.exe which is also the 
-            // location of the response files.
-            var clientDirectory = AppDomain.CurrentDomain.BaseDirectory;
+            var keepAliveTimeout = GetKeepAliveTimeout();
 
             // Pipename should be passed as the first and only argument to the server process
             // and it must have the form "-pipename:name". Otherwise, exit with a non-zero
@@ -42,33 +38,18 @@ public static int Main(string[] args)
             }
 
             var pipeName = args[0].Substring(pipeArgPrefix.Length);
-
-            // Grab the server mutex to prevent multiple servers from starting with the same
-            // pipename and consuming excess resources. If someone else holds the mutex
-            // exit immediately with a non-zero exit code
             var serverMutexName = $"{pipeName}.server";
-            bool holdsMutex;
-            using (var serverMutex = new Mutex(initiallyOwned: true,
-                                               name: serverMutexName,
-                                               createdNew: out holdsMutex))
-            {
-                if (!holdsMutex)
-                {
-                    return CommonCompiler.Failed;
-                }
 
-                try
-                {
-                    return Run(keepAliveTimeout, clientDirectory, pipeName);
-                }
-                finally
-                {
-                    serverMutex.ReleaseMutex();
-                }
-            }
+            // VBCSCompiler is installed in the same directory as csc.exe and vbc.exe which is also the 
+            // location of the response files.
+            var clientDirectory = AppDomain.CurrentDomain.BaseDirectory;
+            var sdkDirectory = RuntimeEnvironment.GetRuntimeDirectory();
+            var compilerServerHost = new DesktopCompilerServerHost(clientDirectory, sdkDirectory);
+            var clientConnectionHost = new NamedPipeClientConnectionHost(compilerServerHost, pipeName);
+            return Run(serverMutexName, clientConnectionHost, keepAliveTimeout);
         }
 
-        private static int Run(TimeSpan? keepAliveTimeout, string clientDirectory, string pipeName)
+        private static TimeSpan? GetKeepAliveTimeout()
         {
             try
             {
@@ -80,31 +61,57 @@ private static int Run(TimeSpan? keepAliveTimeout, string clientDirectory, strin
                     if (keepAliveValue == 0)
                     {
                         // This is a one time server entry.
-                        keepAliveTimeout = null;
+                        return null;
                     }
                     else
                     {
-                        keepAliveTimeout = TimeSpan.FromSeconds(keepAliveValue);
+                        return TimeSpan.FromSeconds(keepAliveValue);
                     }
                 }
                 else
                 {
-                    keepAliveTimeout = ServerDispatcher.DefaultServerKeepAlive;
+                    return ServerDispatcher.DefaultServerKeepAlive;
                 }
             }
             catch (ConfigurationErrorsException e)
             {
-                keepAliveTimeout = ServerDispatcher.DefaultServerKeepAlive;
                 CompilerServerLogger.LogException(e, "Could not read AppSettings");
+                return ServerDispatcher.DefaultServerKeepAlive;
             }
+        }
+
+        internal static int Run(string mutexName, IClientConnectionHost connectionHost, TimeSpan? keepAlive)
+        {
+            // Grab the server mutex to prevent multiple servers from starting with the same
+            // pipename and consuming excess resources. If someone else holds the mutex
+            // exit immediately with a non-zero exit code
+            bool holdsMutex;
+            using (var serverMutex = new Mutex(initiallyOwned: true,
+                                               name: mutexName,
+                                               createdNew: out holdsMutex))
+            {
+                if (!holdsMutex)
+                {
+                    return CommonCompiler.Failed;
+                }
 
+                try
+                {
+                    return RunCore(connectionHost, keepAlive);
+                }
+                finally
+                {
+                    serverMutex.ReleaseMutex();
+                }
+            }
+        }
+
+        private static int RunCore(IClientConnectionHost connectionHost, TimeSpan? keepAliveTimeout)
+        {
             CompilerServerLogger.Log("Keep alive timeout is: {0} milliseconds.", keepAliveTimeout?.TotalMilliseconds ?? 0);
             FatalError.Handler = FailFast.OnFatalException;
 
-            var sdkDirectory = RuntimeEnvironment.GetRuntimeDirectory();
-            var compilerServerHost = new DesktopCompilerServerHost(clientDirectory, sdkDirectory);
-            var clientConnectionHost = new NamedPipeClientConnectionHost(compilerServerHost, pipeName);
-            var dispatcher = new ServerDispatcher(clientConnectionHost, new EmptyDiagnosticListener());
+            var dispatcher = new ServerDispatcher(connectionHost, new EmptyDiagnosticListener());
             dispatcher.ListenAndDispatchConnections(keepAliveTimeout);
             return CommonCompiler.Succeeded;
         }

src/Compilers/Server/VBCSCompilerTests/CompilerServerApiTest.cs

@@ -320,5 +320,54 @@ public void ClientExceptionShouldBeginShutdown()
             Assert.True(listener.HasDetectedBadConnection);
             Assert.True(listenCancellationToken.IsCancellationRequested);
         }
+
+        [Fact]
+        public void MutexStopsServerStarting()
+        {
+            var mutexName = Guid.NewGuid().ToString("N");
+
+            bool holdsMutex;
+            using (var mutex = new Mutex(initiallyOwned: true,
+                                         name: mutexName,
+                                         createdNew: out holdsMutex))
+            {
+                Assert.True(holdsMutex);
+                try
+                {
+                    var host = new Mock<IClientConnectionHost>(MockBehavior.Strict);
+                    var result = VBCSCompiler.Run(mutexName, host.Object, keepAlive: null);
+                    Assert.Equal(CommonCompiler.Failed, result);
+                }
+                finally
+                {
+                    mutex.ReleaseMutex();
+                }
+            }
+        }
+
+        [Fact]
+        public void MutexAcquiredWhenRunningServer()
+        {
+            var mutexName = Guid.NewGuid().ToString("N");
+            var host = new Mock<IClientConnectionHost>(MockBehavior.Strict);
+            host
+                .Setup(x => x.CreateListenTask(It.IsAny<CancellationToken>()))
+                .Returns(() =>
+                {
+                    var task = new Task(() =>
+                    {
+                        Mutex mutex;
+                        Assert.True(Mutex.TryOpenExisting(mutexName, out mutex));
+                        Assert.False(mutex.WaitOne(millisecondsTimeout: 0));
+                    });
+                    task.Start(TaskScheduler.Default);
+                    task.Wait();
+
+                    return new TaskCompletionSource<IClientConnection>().Task;
+                });
+
+            var result = VBCSCompiler.Run(mutexName, host.Object, keepAlive: TimeSpan.FromSeconds(1));
+            Assert.Equal(CommonCompiler.Succeeded, result);
+        }
     }
 }

src/Compilers/Server/VBCSCompilerTests/CompilerServerTests.cs

@@ -2395,38 +2395,6 @@ public class Class1
         private static readonly TimeSpan s_fiveSec = TimeSpan.FromSeconds(5);
         private static readonly int s_fiveSecMillis = (int)s_fiveSec.TotalMilliseconds;
 
-        [Fact]
-        public void MutexStopsServerStarting()
-        {
-            var pipename = Guid.NewGuid().ToString("N");
-            var mutexName = $"{pipename}.server";
-
-            bool holdsMutex;
-            using (var mutex = new Mutex(initiallyOwned: true,
-                                         name: mutexName,
-                                         createdNew: out holdsMutex))
-            {
-                Assert.True(holdsMutex);
-                try
-                {
-                    var result = ProcessUtilities.StartProcess(_compilerServerExecutable,
-                                                               $"-pipename:{pipename}");
-
-                    // Wait up to 5 seconds for the process to exit
-                    var exited = result.WaitForExit(s_fiveSecMillis);
-                    if (!exited)
-                    {
-                        result.Kill();
-                    }
-                    Assert.True(exited);
-                }
-                finally
-                {
-                    mutex.ReleaseMutex();
-                }
-            }
-        }
-
         [Fact]
         public void ServerWithSamePipeNameExits()
         {