Test shutdown in middle of compilation

Added a test to verify the shutdown operation is passive.  If any
existing compilations are in progress they will be allowed to complete
after the shutdown is received.

This required a bit of code refactoring.  It needed to be easier to
intercept compilation requests and block on them until a given set of
events had completed.

src/Compilers/CSharp/Test/CommandLine/TouchedFileLoggingTests.cs

@@ -215,7 +215,7 @@ public void TrivialMetadataCaching()
                 filelist.Add(source1);
                 var outWriter = new StringWriter();
                 var cmd = new CSharpCompilerServer(
-                    new DesktopCompilerServerHost(),
+                    DesktopCompilerServerHost.SharedAssemblyReferenceProvider,
                     new[] { "/nologo", "/touchedfiles:" + touchedBase, source1 },
                     null,
                     _baseDirectory,

src/Compilers/Server/ServerShared/CSharpCompilerServer.cs

@@ -13,17 +13,17 @@ namespace Microsoft.CodeAnalysis.CompilerServer
 {
     internal sealed class CSharpCompilerServer : CSharpCompiler
     {
-        private readonly ICompilerServerHost _compilerServerHost;
+        private readonly Func<string, MetadataReferenceProperties, PortableExecutableReference> _metadataProvider;
 
-        internal CSharpCompilerServer(ICompilerServerHost compilerServerHost, string[] args, string clientDirectory, string baseDirectory, string sdkDirectory, string libDirectory, IAnalyzerAssemblyLoader analyzerLoader)
+        internal CSharpCompilerServer(Func<string, MetadataReferenceProperties, PortableExecutableReference> metadataProvider, string[] args, string clientDirectory, string baseDirectory, string sdkDirectory, string libDirectory, IAnalyzerAssemblyLoader analyzerLoader)
             : base(CSharpCommandLineParser.Default, clientDirectory != null ? Path.Combine(clientDirectory, ResponseFileName) : null, args, clientDirectory, baseDirectory, sdkDirectory, libDirectory, analyzerLoader)
         {
-            _compilerServerHost = compilerServerHost;
+            _metadataProvider = metadataProvider;
         }
 
         internal override Func<string, MetadataReferenceProperties, PortableExecutableReference> GetMetadataProvider()
         {
-            return _compilerServerHost.AssemblyReferenceProvider;
+            return _metadataProvider;
         }
 
         protected override uint GetSqmAppID()

src/Compilers/Server/ServerShared/CompilerRequestHandler.cs

@@ -11,6 +11,8 @@
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.CommandLine;
 
+using static Microsoft.CodeAnalysis.CommandLine.CompilerServerLogger;
+
 namespace Microsoft.CodeAnalysis.CompilerServer
 {
     internal struct RunRequest
@@ -59,7 +61,7 @@ public bool TryCreateCompiler(RunRequest request, out CommonCompiler compiler)
             {
                 case LanguageNames.CSharp:
                     compiler = new CSharpCompilerServer(
-                        this,
+                        AssemblyReferenceProvider,
                         args: request.Arguments,
                         clientDirectory: ClientDirectory,
                         baseDirectory: request.CurrentDirectory,
@@ -69,7 +71,7 @@ public bool TryCreateCompiler(RunRequest request, out CommonCompiler compiler)
                     return true;
                 case LanguageNames.VisualBasic:
                     compiler = new VisualBasicCompilerServer(
-                        this,
+                        AssemblyReferenceProvider,
                         args: request.Arguments,
                         clientDirectory: ClientDirectory,
                         baseDirectory: request.CurrentDirectory,
@@ -82,5 +84,39 @@ public bool TryCreateCompiler(RunRequest request, out CommonCompiler compiler)
                     return false;
             }
         }
+
+        public BuildResponse RunCompilation(RunRequest request, CancellationToken cancellationToken)
+        {
+            Log($"CurrentDirectory = '{request.CurrentDirectory}'");
+            Log($"LIB = '{request.LibDirectory}'");
+            for (int i = 0; i < request.Arguments.Length; ++i)
+            {
+                Log($"Argument[{i}] = '{request.Arguments[i]}'");
+            }
+
+            CommonCompiler compiler;
+            if (!TryCreateCompiler(request, out compiler))
+            {
+                // TODO: is this the right option?  Right now it will cause the fall back in the command line 
+                // to fail because it's a valid response.  Probably should add a "server failed" response
+                // for command line to deal with.
+
+                // We can't do anything with a request we don't know about. 
+                Log($"Got request with id '{request.Language}'");
+                return new CompletedBuildResponse(returnCode:  -1, utf8output: false, output:  "", errorOutput: "");
+            }
+
+            bool utf8output = compiler.Arguments.Utf8Output;
+            if (!CheckAnalyzers(request.CurrentDirectory, compiler.Arguments.AnalyzerReferences))
+            {
+                return new AnalyzerInconsistencyBuildResponse();
+            }
+
+            Log($"****Running {request.Language} compiler...");
+            TextWriter output = new StringWriter(CultureInfo.InvariantCulture);
+            int returnCode = compiler.Run(output, cancellationToken);
+            Log($"****{request.Language} Compilation complete.\r\n****Return code: {returnCode}\r\n****Output:\r\n{output.ToString()}\r\n");
+            return new CompletedBuildResponse(returnCode, utf8output, output.ToString(), "");
+        }
     }
 }

src/Compilers/Server/ServerShared/Connection.cs

@@ -200,15 +200,18 @@ private bool IsShutdownRequest(BuildRequest request)
             return request.Arguments.Length == 1 && request.Arguments[0].ArgumentId == BuildProtocolConstants.ArgumentId.Shutdown;
         }
 
-        protected virtual Task<BuildResponse> ServeBuildRequest(BuildRequest request, CancellationToken cancellationToken)
+        protected virtual Task<BuildResponse> ServeBuildRequest(BuildRequest buildRequest, CancellationToken cancellationToken)
         {
-            return Task.Run(() =>
+            Func<BuildResponse> func = () =>
             {
                 try
                 {
                     // Do the compilation
                     Log("Begin compilation");
-                    BuildResponse response = ServeBuildRequestCore(request, cancellationToken);
+
+                    var request = BuildProtocolUtil.GetRunRequest(buildRequest);
+                    var response = _compilerServerHost.RunCompilation(request, cancellationToken);
+
                     Log("End compilation");
                     return response;
                 }
@@ -216,42 +219,11 @@ protected virtual Task<BuildResponse> ServeBuildRequest(BuildRequest request, Ca
                 {
                     throw ExceptionUtilities.Unreachable;
                 }
-            });
-        }
-
-        private BuildResponse ServeBuildRequestCore(BuildRequest buildRequest, CancellationToken cancellationToken)
-        {
-            var request = BuildProtocolUtil.GetRunRequest(buildRequest);
-            CommonCompiler compiler;
-            if (!_compilerServerHost.TryCreateCompiler(request, out compiler))
-            {
-                // TODO: is this the right option?  Right now it will cause the fall back in the command line 
-                // to fail because it's a valid response.  Probably should add a "server failed" response
-                // for command line to deal with.
-
-                // We can't do anything with a request we don't know about. 
-                Log($"Got request with id '{request.Language}'");
-                return new CompletedBuildResponse(-1, false, "", "");
-            }
-
-            Log($"CurrentDirectory = '{request.CurrentDirectory}'");
-            Log($"LIB = '{request.LibDirectory}'");
-            for (int i = 0; i < request.Arguments.Length; ++i)
-            {
-                Log($"Argument[{i}] = '{request.Arguments[i]}'");
-            }
-
-            bool utf8output = compiler.Arguments.Utf8Output;
-            if (!_compilerServerHost.CheckAnalyzers(request.CurrentDirectory, compiler.Arguments.AnalyzerReferences))
-            {
-                return new AnalyzerInconsistencyBuildResponse();
-            }
+            };
 
-            Log($"****Running {request.Language} compiler...");
-            TextWriter output = new StringWriter(CultureInfo.InvariantCulture);
-            int returnCode = compiler.Run(output, cancellationToken);
-            Log($"****{request.Language} Compilation complete.\r\n****Return code: {returnCode}\r\n****Output:\r\n{output.ToString()}\r\n");
-            return new CompletedBuildResponse(returnCode, utf8output, output.ToString(), "");
+            var task = new Task<BuildResponse>(func, cancellationToken, TaskCreationOptions.LongRunning);
+            task.Start();
+            return task;
         }
 
         private void Log(string message)

src/Compilers/Server/ServerShared/ICompilerServerHost.cs

@@ -7,13 +7,12 @@
 using System.Text;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.CodeAnalysis.CommandLine;
 
 namespace Microsoft.CodeAnalysis.CompilerServer
 {
     internal interface ICompilerServerHost
     {
-        Func<string, MetadataReferenceProperties, PortableExecutableReference> AssemblyReferenceProvider { get; }
-        bool TryCreateCompiler(RunRequest request, out CommonCompiler compiler);
-        bool CheckAnalyzers(string baseDirectory, ImmutableArray<CommandLineAnalyzerReference> analyzers);
+        BuildResponse RunCompilation(RunRequest request, CancellationToken cancellationToken);
     }
 }

src/Compilers/Server/ServerShared/VisualBasicCompilerServer.cs

@@ -15,17 +15,17 @@ namespace Microsoft.CodeAnalysis.CompilerServer
 {
     internal sealed class VisualBasicCompilerServer : VisualBasicCompiler
     {
-        private readonly ICompilerServerHost _compilerServerHost;
+        private readonly Func<string, MetadataReferenceProperties, PortableExecutableReference> _metadataProvider;
 
-        internal VisualBasicCompilerServer(ICompilerServerHost compilerServerHost, string[] args, string clientDirectory, string baseDirectory, string sdkDirectory, string libDirectory, IAnalyzerAssemblyLoader analyzerLoader)
+        internal VisualBasicCompilerServer(Func<string, MetadataReferenceProperties, PortableExecutableReference> metadataProvider, string[] args, string clientDirectory, string baseDirectory, string sdkDirectory, string libDirectory, IAnalyzerAssemblyLoader analyzerLoader)
             : base(VisualBasicCommandLineParser.Default, clientDirectory != null ? Path.Combine(clientDirectory, ResponseFileName) : null, args, clientDirectory, baseDirectory, sdkDirectory, libDirectory, analyzerLoader)
         {
-            _compilerServerHost = compilerServerHost;
+            _metadataProvider = metadataProvider;
         }
 
         internal override Func<string, MetadataReferenceProperties, PortableExecutableReference> GetMetadataProvider()
         {
-            return _compilerServerHost.AssemblyReferenceProvider;
+            return _metadataProvider;
         }
 
         protected override uint GetSqmAppID()

src/Compilers/Server/VBCSCompiler/DesktopCompilerServerHost.cs

@@ -21,11 +21,11 @@ internal sealed class DesktopCompilerServerHost : CompilerServerHost
         private static readonly IAnalyzerAssemblyLoader s_analyzerLoader = new ShadowCopyAnalyzerAssemblyLoader(Path.Combine(Path.GetTempPath(), "VBCSCompiler", "AnalyzerAssemblyLoader"));
 
         // Caches are used by C# and VB compilers, and shared here.
-        private static readonly Func<string, MetadataReferenceProperties, PortableExecutableReference> s_assemblyReferenceProvider = (path, properties) => new CachingMetadataReference(path, properties);
+        public static readonly Func<string, MetadataReferenceProperties, PortableExecutableReference> SharedAssemblyReferenceProvider = (path, properties) => new CachingMetadataReference(path, properties);
 
         public override IAnalyzerAssemblyLoader AnalyzerAssemblyLoader => s_analyzerLoader;
 
-        public override Func<string, MetadataReferenceProperties, PortableExecutableReference> AssemblyReferenceProvider => s_assemblyReferenceProvider;
+        public override Func<string, MetadataReferenceProperties, PortableExecutableReference> AssemblyReferenceProvider => SharedAssemblyReferenceProvider;
 
         internal DesktopCompilerServerHost()
             : this(AppDomain.CurrentDomain.BaseDirectory, RuntimeEnvironment.GetRuntimeDirectory())

src/Compilers/Server/VBCSCompilerTests/CompilerServerApiTest.cs

@@ -11,6 +11,7 @@
 using Roslyn.Test.Utilities;
 using Xunit;
 using System.Runtime.InteropServices;
+using System.Diagnostics;
 
 namespace Microsoft.CodeAnalysis.CompilerServer.UnitTests
 {
@@ -96,6 +97,13 @@ private async Task<BuildRequest> CreateBuildRequest(string sourceText, TimeSpan?
                 builder.ToImmutable());
         }
 
+        private static async Task Verify(ServerData serverData, int connections, int completed)
+        {
+            var serverStats = await serverData.Complete().ConfigureAwait(true);
+            Assert.Equal(connections, serverStats.Connections);
+            Assert.Equal(completed, serverStats.CompletedConnections);
+        }
+
         /// <summary>
         /// Run a C# compilation against the given source text using the provided named pipe name.
         /// </summary>
@@ -353,5 +361,54 @@ public void MutexAcquiredWhenRunningServer()
             var result = VBCSCompiler.Run(mutexName, host.Object, keepAlive: TimeSpan.FromSeconds(1));
             Assert.Equal(CommonCompiler.Succeeded, result);
         }
+
+        [Fact]
+        public async Task ShutdownRequestDirect()
+        {
+            using (var serverData = ServerUtil.CreateServer())
+            {
+                var serverProcessId = await ServerUtil.SendShutdown(serverData.PipeName);
+                Assert.Equal(Process.GetCurrentProcess().Id, serverProcessId);
+                await Verify(serverData, connections: 1, completed: 1);
+            }
+        }
+
+        /// <summary>
+        /// A shutdown request should not abort an existing compilation.  It should be allowed to run to 
+        /// completion.
+        /// </summary>
+        [Fact]
+        public async Task ShutdownDoesNotAbortCompilation()
+        {
+            var host = new TestableCompilerServerHost();
+
+            using (var startedMre = new ManualResetEvent(initialState: false))
+            using (var finishedMre = new ManualResetEvent(initialState: false))
+            using (var serverData = ServerUtil.CreateServer(compilerServerHost: host))
+            {
+                // Create a compilation that is guaranteed to complete after the shutdown is seen. 
+                host.RunCompilation = (request, cancellationToken) =>
+                {
+                    startedMre.Set();
+                    finishedMre.WaitOne();
+                    return s_emptyBuildResponse;
+                };
+
+                var compileTask = ServerUtil.Send(serverData.PipeName, s_emptyCSharpBuildRequest);
+                startedMre.WaitOne();
+
+                // The compilation is now in progress, send the shutdown.
+                await ServerUtil.SendShutdown(serverData.PipeName);
+                Assert.False(compileTask.IsCompleted);
+                finishedMre.Set();
+
+                var response = await compileTask;
+                Assert.Equal(BuildResponse.ResponseType.Completed, response.Type);
+                Assert.Equal(0, ((CompletedBuildResponse)response).ReturnCode);
+
+                await Verify(serverData, connections: 2, completed: 2);
+            }
+        }
+
     }
 }

src/Compilers/Server/VBCSCompilerTests/CompilerServerTests.cs

@@ -1409,26 +1409,6 @@ public void BadKeepAlive4()
             Assert.Equal("", result.Errors);
         }
 
-        [Fact]
-        public async Task ShutdownRequestDirect()
-        {
-            using (var serverData = ServerUtil.CreateServer())
-            using (var client = new NamedPipeClientStream(serverData.PipeName))
-            {
-                await client.ConnectAsync();
-
-                var memoryStream = new MemoryStream();
-                await BuildRequest.CreateShutdown().WriteAsync(memoryStream);
-                memoryStream.Position = 0;
-                await memoryStream.CopyToAsync(client);
-
-                var response = await BuildResponse.ReadAsync(client);
-                Assert.Equal(BuildResponse.ResponseType.Shutdown, response.Type);
-                Assert.Equal(Process.GetCurrentProcess().Id, ((ShutdownBuildResponse)response).ServerProcessId);
-                await Verify(serverData, connections: 1, completed: 1);
-            }
-        }
-
         [Fact] 
         [WorkItem(1024619, "DevDiv")]
         public async Task Bug1024619_01()

src/Compilers/Server/VBCSCompilerTests/ServerUtil.cs

@@ -144,6 +144,27 @@ internal static ServerData CreateServerFailsConnection(string pipeName = null)
             return new ServerData(cts, taskSource.Task, pipeName);
         }
 
+        internal static async Task<BuildResponse> Send(string pipeName, BuildRequest request)
+        {
+            using (var client = new NamedPipeClientStream(pipeName))
+            {
+                await client.ConnectAsync();
+
+                var memoryStream = new MemoryStream();
+                await request.WriteAsync(memoryStream);
+                memoryStream.Position = 0;
+                await memoryStream.CopyToAsync(client);
+
+                return await BuildResponse.ReadAsync(client);
+            }
+        }
+
+        internal static async Task<int> SendShutdown(string pipeName)
+        {
+            var response = await Send(pipeName, BuildRequest.CreateShutdown());
+            return ((ShutdownBuildResponse)response).ServerProcessId;
+        }
+
         private static async Task<int> CreateServerFailsConnectionCore(string pipeName, CancellationToken cancellationToken)
         {
             var connections = 0;

src/Compilers/Server/VBCSCompilerTests/TestableCompilerServerHost.cs

+// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
+
+using Microsoft.CodeAnalysis.CommandLine;
+using System;
+using System.Threading;
+
+namespace Microsoft.CodeAnalysis.CompilerServer.UnitTests
+{
+    internal sealed class TestableCompilerServerHost : ICompilerServerHost
+    {
+        internal Func<RunRequest, CancellationToken, BuildResponse> RunCompilation;
+
+        internal TestableCompilerServerHost(Func<RunRequest, CancellationToken, BuildResponse> runCompilation = null)
+        {
+            RunCompilation = runCompilation;
+        }
+
+        BuildResponse ICompilerServerHost.RunCompilation(RunRequest request, CancellationToken cancellationToken)
+        {
+            return RunCompilation(request, cancellationToken);
+        }
+    }
+}

src/Compilers/Server/VBCSCompilerTests/VBCSCompilerTests.csproj

@@ -117,6 +117,7 @@
     <Compile Include="MockEngine.cs" />
     <Compile Include="RunKeepAliveTests.cs" />
     <Compile Include="ServerUtil.cs" />
+    <Compile Include="TestableCompilerServerHost.cs" />
     <Compile Include="TestableDiagnosticListener.cs" />
   </ItemGroup>
   <ItemGroup>

src/Compilers/VisualBasic/Test/CommandLine/TouchedFileLoggingTests.vb

@@ -169,7 +169,7 @@ End Class
 
                 Dim outWriter = New StringWriter()
                 Dim cmd = New VisualBasicCompilerServer(
-                    New DesktopCompilerServerHost(),
+                    DesktopCompilerServerHost.SharedAssemblyReferenceProvider,
                     {"/nologo",
                      "/touchedfiles:" + touchedBase,
                      source1},