Fix correct usage of the /keepalive option in the managed client

src/Compilers/Core/Desktop/CommandLine/CommonCommandLineParser.cs

@@ -317,28 +317,51 @@ internal static string RemoveTrailingSpacesAndDots(string path)
         }
 
         /// <summary>
-        /// Returns an error message if any of the client arguments are invalid
-        /// and null otherwise.
+        /// Returns false if any of the client arguments are invalid and true otherwise.
         /// </summary>
-        internal static string CheckArgsForClientErrors(
+        /// <param name="args">
+        /// The original args to the client.
+        /// </param>
+        /// <param name="parsedArgs">
+        /// The original args minus the client args, if no errors were encountered.
+        /// </param>
+        /// <param name="containsShared">
+        /// Only defined if no errors were encountered.
+        /// True if '/shared' was an argument, false otherwise.
+        /// </param>
+        /// <param name="keepAliveValue">
+        /// Only defined if no errors were encountered.
+        /// The value to the '/keepalive' argument if one was specified, null otherwise.
+        /// </param>
+        /// <param name="errorMessage">
+        /// Only defined if errors were encountered.
+        /// The error message for the encountered error.
+        /// </param>
+        internal static bool TryParseClientArgs(
             IEnumerable<string> args,
-            out bool containsShared)
+            out List<string> parsedArgs,
+            out bool containsShared,
+            out string keepAliveValue,
+            out string errorMessage)
         {
             const string keepAlive = "/keepalive";
             const string shared = "/shared";
-            bool hasKeepAlive = false;
             containsShared = false;
+            keepAliveValue = null;
+            errorMessage = null;
+            parsedArgs = null;
+            var newArgs = new List<string>();
             foreach (var arg in args)
             {
                 var prefixLength = keepAlive.Length;
                 if (arg.StartsWith(keepAlive, StringComparison.OrdinalIgnoreCase))
                 {
-                    hasKeepAlive = true;
                     if (arg.Length < prefixLength + 2 ||
                         arg[prefixLength] != ':' &&
                         arg[prefixLength] != '=')
                     {
-                        return CodeAnalysisDesktopResources.MissingKeepAlive;
+                        errorMessage = CodeAnalysisDesktopResources.MissingKeepAlive;
+                        return false;
                     }
 
                     var value = arg.Substring(prefixLength + 1).Trim('"');
@@ -347,23 +370,37 @@ internal static string RemoveTrailingSpacesAndDots(string path)
                     {
                         if (intValue < -1)
                         {
-                            return CodeAnalysisDesktopResources.KeepAliveIsTooSmall;
+                            errorMessage = CodeAnalysisDesktopResources.KeepAliveIsTooSmall;
+                            return false;
                         }
+                        keepAliveValue = value;
                     }
                     else
                     {
-                        return CodeAnalysisDesktopResources.KeepAliveIsNotAnInteger;
+                        errorMessage = CodeAnalysisDesktopResources.KeepAliveIsNotAnInteger;
+                        return false;
                     }
+                    continue;
                 }
 
                 if (string.Equals(arg, shared, StringComparison.OrdinalIgnoreCase))
                 {
                     containsShared = true;
+                    continue;
                 }
+                newArgs.Add(arg);
+            }
+
+            if (keepAliveValue != null && !containsShared)
+            {
+                errorMessage = CodeAnalysisDesktopResources.KeepAliveWithoutShared;
+                return false;
+            }
+            else
+            {
+                parsedArgs = newArgs;
+                return true;
             }
-            return hasKeepAlive && !containsShared
-                ? CodeAnalysisDesktopResources.KeepAliveWithoutShared
-                : null;
         }
 
         internal static string MismatchedVersionErrorText => CodeAnalysisDesktopResources.MismatchedVersion;

src/Compilers/Core/MSBuildTask/BuildClient.cs

@@ -55,25 +55,28 @@ private static string GetExpectedServerExeDir()
             args = args.Select(arg => arg.Trim()).ToArray();
 
             bool hasShared;
-            var errorMessage = CommandLineParser.CheckArgsForClientErrors(args, out hasShared);
-            if (errorMessage != null)
+            string keepAlive;
+            string errorMessage;
+            List<string> parsedArgs;
+            if (!CommandLineParser.TryParseClientArgs(
+                    args,
+                    out parsedArgs,
+                    out hasShared,
+                    out keepAlive,
+                    out errorMessage))
             {
                 Console.Out.WriteLine(errorMessage);
                 return CommonCompiler.Failed;
             }
 
-            IEnumerable<string> validatedArgs = args;
             if (hasShared)
             {
-                validatedArgs = validatedArgs
-                    .Where(arg => !string.Equals(arg,
-                                                 "/shared",
-                                                 StringComparison.OrdinalIgnoreCase));
                 var responseTask = TryRunServerCompilation(
                     language,
                     Environment.CurrentDirectory,
-                    validatedArgs.ToArray(),
+                    parsedArgs,
                     default(CancellationToken),
+                    keepAlive: keepAlive,
                     libEnvVariable: Environment.GetEnvironmentVariable("LIB"));
 
                 var response = responseTask.Result;
@@ -83,10 +86,7 @@ private static string GetExpectedServerExeDir()
                 }
             }
 
-            validatedArgs = validatedArgs
-                .Where(arg => !arg.StartsWith("/keepalive", StringComparison.OrdinalIgnoreCase));
-
-            return fallbackCompiler(validatedArgs.ToArray());
+            return fallbackCompiler(parsedArgs.ToArray());
         }
 
         private static int HandleResponse(BuildResponse response)
@@ -132,6 +132,7 @@ private static int HandleResponse(BuildResponse response)
             string workingDir,
             IList<string> arguments,
             CancellationToken cancellationToken,
+            string keepAlive = null,
             string libEnvVariable = null)
         {
             try
@@ -163,7 +164,7 @@ private static int HandleResponse(BuildResponse response)
 
                         if (holdsMutex)
                         {
-                            var request = BuildRequest.Create(language, workingDir, arguments, libEnvVariable);
+                            var request = BuildRequest.Create(language, workingDir, arguments, keepAlive, libEnvVariable);
                             // Check for already running processes in case someone came in before us
                             pipe = TryExistingProcesses(expectedServerExePath, cancellationToken);
                             if (pipe != null)

src/Compilers/Core/VBCSCompiler/BuildProtocol.cs

@@ -73,6 +73,7 @@ internal class BuildRequest
         public static BuildRequest Create(RequestLanguage language,
                                           string workingDirectory,
                                           IList<string> args,
+                                          string keepAlive = null,
                                           string libDirectory = null)
         {
             Log("Creating BuildRequest");
@@ -84,6 +85,9 @@ internal class BuildRequest
 
             requestArgs.Add(new Argument(ArgumentId.CurrentDirectory, 0, workingDirectory));
 
+            if (keepAlive != null)
+                requestArgs.Add(new Argument(ArgumentId.KeepAlive, 0, keepAlive));
+
             if (libDirectory != null)
                 requestArgs.Add(new Argument(ArgumentId.LibEnvVariable, 0, libDirectory));
 

src/Compilers/Core/VBCSCompilerTests/CompilerServerTests.cs

@@ -2096,6 +2096,16 @@ public void BadKeepAlive4()
             Assert.Equal("", result.Errors);
         }
 
+        [Fact]
+        public void SimpleKeepAlive()
+        {
+            var result = RunCommandLineCompiler(_csharpCompilerClientExecutable,
+                                                $"/nologo /keepalive:1 hello.cs",
+                                                _tempDirectory,
+                                                s_helloWorldSrcCs);
+            VerifyResultAndOutput(result, _tempDirectory, "Hello, world.\r\n");
+        }
+
         [Fact, WorkItem(1024619, "DevDiv")]
         public void Bug1024619_01()
         {