Removed the remaining TODOS

src/Compilers/Server/PortableServer/CoreClrCompilerServerHost.cs

@@ -33,10 +33,5 @@ public override bool CheckAnalyzers(string baseDirectory, ImmutableArray<Command
             // Analyzers not supported in the portable server yet.
             return analyzers.Length == 0;
         }
-
-        public override void Log(string message)
-        {
-            // BTODO: Do we need this anymore? 
-        }
     }
 }

src/Compilers/Server/ServerShared/CompilerRequestHandler.cs

@@ -53,8 +53,6 @@ protected CompilerServerHost(string clientDirectory, string sdkDirectory)
 
         public abstract bool CheckAnalyzers(string baseDirectory, ImmutableArray<CommandLineAnalyzerReference> analyzers);
 
-        public abstract void Log(string message);
-
         public bool TryCreateCompiler(RunRequest request, out CommonCompiler compiler)
         {
             switch (request.Language)

src/Compilers/Server/ServerShared/Connection.cs

@@ -43,7 +43,6 @@ internal enum CompletionReason
 
         /// <summary>
         /// There was an unhandled exception processing the result.
-        /// BTODO: need to handle this is the server.
         /// </summary>
         ClientException,
     }
@@ -190,15 +189,15 @@ private BuildResponse ServeBuildRequestCore(BuildRequest buildRequest, Cancellat
             if (!_compilerServerHost.TryCreateCompiler(request, out compiler))
             {
                 // We can't do anything with a request we don't know about. 
-                _compilerServerHost.Log($"Got request with id '{request.Language}'");
+                Log($"Got request with id '{request.Language}'");
                 return new CompletedBuildResponse(-1, false, "", "");
             }
 
-            _compilerServerHost.Log($"CurrentDirectory = '{request.CurrentDirectory}'");
-            _compilerServerHost.Log($"LIB = '{request.LibDirectory}'");
+            Log($"CurrentDirectory = '{request.CurrentDirectory}'");
+            Log($"LIB = '{request.LibDirectory}'");
             for (int i = 0; i < request.Arguments.Length; ++i)
             {
-                _compilerServerHost.Log($"Argument[{i}] = '{request.Arguments[i]}'");
+                Log($"Argument[{i}] = '{request.Arguments[i]}'");
             }
 
             bool utf8output = compiler.Arguments.Utf8Output;
@@ -207,10 +206,10 @@ private BuildResponse ServeBuildRequestCore(BuildRequest buildRequest, Cancellat
                 return new AnalyzerInconsistencyBuildResponse();
             }
 
-            _compilerServerHost.Log($"****Running {request.Language} compiler...");
+            Log($"****Running {request.Language} compiler...");
             TextWriter output = new StringWriter(CultureInfo.InvariantCulture);
             int returnCode = compiler.Run(output, cancellationToken);
-            _compilerServerHost.Log($"****{request.Language} Compilation complete.\r\n****Return code: {returnCode}\r\n****Output:\r\n{output.ToString()}\r\n");
+            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/DiagnosticListener.cs

@@ -20,6 +20,12 @@ internal interface IDiagnosticListener
         /// processed is provided in <paramref name="count"/>.
         /// </summary>
         void ConnectionProcessed(int count);
+
+        /// <summary>
+        /// Called when a bad client connection was detected and the server will be shutting down as a 
+        /// result.
+        /// </summary>
+        void DetectedBadConnection();
     }
 
     internal sealed class EmptyDiagnosticListener : IDiagnosticListener
@@ -31,5 +37,9 @@ public void UpdateKeepAlive(TimeSpan timeSpan)
         public void ConnectionProcessed(int count)
         {
         }
+
+        public void DetectedBadConnection()
+        {
+        }
     }
 }

src/Compilers/Server/ServerShared/ICompilerServerHost.cs

@@ -12,11 +12,9 @@ namespace Microsoft.CodeAnalysis.CompilerServer
 {
     internal interface ICompilerServerHost
     {
-        // BTODO: how many of these are needed anymore? 
         IAnalyzerAssemblyLoader AnalyzerAssemblyLoader { get; }
         Func<string, MetadataReferenceProperties, PortableExecutableReference> AssemblyReferenceProvider { get; }
         bool TryCreateCompiler(RunRequest request, out CommonCompiler compiler);
         bool CheckAnalyzers(string baseDirectory, ImmutableArray<CommandLineAnalyzerReference> analyzers);
-        void Log(string message);
     }
 }

src/Compilers/Server/ServerShared/ServerDispatcher.cs

@@ -113,6 +113,7 @@ public void ListenAndDispatchConnections(TimeSpan? keepAlive, CancellationToken
                     // the shutdown process.  We have to assume that the client disconnected via
                     // Ctrl+C and wants the server process to terminate.  It's possible a compilation
                     // is running out of control and the client wants their machine back.  
+                    _diagnosticListener.DetectedBadConnection();
                     listenCancellationTokenSource.Cancel();
                     break;
                 }

src/Compilers/Server/VBCSCompiler/DesktopCompilerServerHost.cs

@@ -43,11 +43,5 @@ public override bool CheckAnalyzers(string baseDirectory, ImmutableArray<Command
         {
             return AnalyzerConsistencyChecker.Check(baseDirectory, analyzers, s_analyzerLoader);
         }
-
-        public override void Log(string message)
-        {
-            CompilerServerLogger.Log(message);
-        }
-
     }
 }

src/Compilers/Server/VBCSCompilerTests/CompilerServerApiTest.cs

@@ -21,6 +21,7 @@ private sealed class TestableDiagnosticListener : IDiagnosticListener
             public int ProcessedCount;
             public DateTime? LastProcessedTime;
             public TimeSpan? KeepAlive;
+            public bool HasDetectedBadConnection;
 
             public void ConnectionProcessed(int count)
             {
@@ -32,6 +33,11 @@ public void UpdateKeepAlive(TimeSpan timeSpan)
             {
                 KeepAlive = timeSpan;
             }
+
+            public void DetectedBadConnection()
+            {
+                HasDetectedBadConnection = true;
+            }
         }
 
         private static readonly BuildRequest s_emptyCSharpBuildRequest = new BuildRequest(
@@ -278,5 +284,41 @@ public async Task KeepAliveAfterSimultaneousConnection()
             Assert.True(listener.LastProcessedTime.HasValue);
             Assert.True((DateTime.Now - listener.LastProcessedTime.Value) > keepAlive);
         }
+
+        [Fact]
+        public void ClientExceptionShouldBeginShutdown()
+        {
+            var client = new Mock<IClientConnection>();
+            client
+                .Setup(x => x.HandleConnection(It.IsAny<CancellationToken>()))
+                .Throws(new Exception());
+
+            var listenCancellationToken = default(CancellationToken);
+            var first = true;
+
+            var host = new Mock<IClientConnectionHost>();
+            host
+                .Setup(x => x.CreateListenTask(It.IsAny<CancellationToken>()))
+                .Returns((CancellationToken cancellationToken) =>
+                {
+                    if (first)
+                    {
+                        first = false;
+                        return Task.FromResult(client.Object);
+                    }
+                    else
+                    {
+                        listenCancellationToken = cancellationToken;
+                        return Task.Delay(-1, cancellationToken).ContinueWith<IClientConnection>(_ => null, TaskScheduler.Default);
+                    }
+                });
+
+            var listener = new TestableDiagnosticListener();
+            var dispatcher = new ServerDispatcher(host.Object, listener);
+            dispatcher.ListenAndDispatchConnections(TimeSpan.FromSeconds(10));
+
+            Assert.True(listener.HasDetectedBadConnection);
+            Assert.True(listenCancellationToken.IsCancellationRequested);
+        }
     }
 }

src/Compilers/Shared/CoreClrBuildClient.cs

@@ -25,9 +25,10 @@ private CoreClrBuildClient(RequestLanguage language, CompileFunc compileFunc)
 
         internal static int Run(IEnumerable<string> arguments, RequestLanguage language, CompileFunc compileFunc)
         {
-            // BTODO: Should be using BuildClient.GetCommandLineArgs(arguments) here.  But the native invoke 
+            // Should be using BuildClient.GetCommandLineArgs(arguments) here.  But the native invoke 
             // ends up giving us both CoreRun and the exe file.  Need to find a good way to remove the host 
             // as well as the EXE argument. 
+            // https://github.com/dotnet/roslyn/issues/6677
             var client = new CoreClrBuildClient(language, compileFunc);
             var clientDir = AppContext.BaseDirectory;
             var workingDir = Directory.GetCurrentDirectory();