Wait for process kill in Geekbench (#559)

Author: yangpanMS

src/VirtualClient/VirtualClient.Actions/GeekBench/GeekbenchExecutor.cs

@@ -79,11 +79,10 @@ public string CommandLine
         protected override async Task ExecuteAsync(EventContext telemetryContext, CancellationToken cancellationToken)
         {
             this.DeleteResultsFile(telemetryContext);
-            string commandLineArguments = this.GetCommandLineArguments();
 
             using (BackgroundOperations profiling = BackgroundOperations.BeginProfiling(this, cancellationToken))
             {
-                await this.ExecuteWorkloadAsync(this.ExecutablePath, commandLineArguments, telemetryContext, cancellationToken);
+                await this.ExecuteWorkloadAsync(this.ExecutablePath, this.CommandLine, telemetryContext, cancellationToken);
             }
         }
 
@@ -190,7 +189,7 @@ protected override async Task InitializeAsync(EventContext telemetryContext, Can
                     // GeekBench runs a secondary process on both Windows and Linux systems. When we
                     // kill the parent process, it does not kill the processes the parent spun off. This
                     // ensures that all of the process are stopped/killed.
-                    this.processManager.SafeKill(this.SupportingExecutables.ToArray(), this.Logger);
+                    this.processManager.Kill(this.SupportingExecutables.ToArray(), this.Logger);
                 }
             }
         }
@@ -295,14 +294,9 @@ private Task ExecuteWorkloadAsync(string pathToExe, string commandLineArguments,
                     // GeekBench runs a secondary process on both Windows and Linux systems. When we
                     // kill the parent process, it does not kill the processes the parent spun off. This
                     // ensures that all of the process are stopped/killed.
-                    this.processManager.SafeKill(this.SupportingExecutables.ToArray(), this.Logger);
+                    this.processManager.Kill(this.SupportingExecutables.ToArray(), this.Logger);
                 }
             });
         }
-
-        private string GetCommandLineArguments()
-        {
-            return $"{this.CommandLine}";
-        }
     }
 }

src/VirtualClient/VirtualClient.Core/ProcessExtensions.cs

@@ -5,10 +5,12 @@ namespace VirtualClient
 {
     using System;
     using System.Collections.Generic;
+    using System.Diagnostics;
     using System.Diagnostics.CodeAnalysis;
     using System.IO;
     using System.Linq;
     using System.Text.RegularExpressions;
+    using System.Threading;
     using Microsoft.Extensions.Logging;
     using VirtualClient.Common;
     using VirtualClient.Common.Extensions;
@@ -86,18 +88,19 @@ public static bool IsErrored(this IProcessProxy process, IEnumerable<int> succes
         }
 
         /// <summary>
-        /// Kills the process if it is still running and handles any errors that
-        /// can occurs if the process has gone out of scope.
+        /// Kills the associated process and/or it's child/dependent processes if it is still running and 
+        /// handles any errors that can occurs if the process has gone out of scope.
         /// </summary>
         /// <param name="process">The process to kill.</param>
+        /// <param name="entireProcessTree">true to kill asociated process and it's descendents, false to only kill the process.</param>
         /// <param name="logger">The logger to use to write trace information.</param>
-        public static void SafeKill(this IProcessProxy process, ILogger logger = null)
+        public static void SafeKill(this IProcessProxy process, ILogger logger = null, bool entireProcessTree = false)
         {
             if (process != null)
             {
                 try
                 {
-                    process.Kill();
+                    process.Kill(entireProcessTree);
                 }
                 catch (Exception exc)
                 {
@@ -108,26 +111,46 @@ public static void SafeKill(this IProcessProxy process, ILogger logger = null)
         }
 
         /// <summary>
-        /// Kills the associated process and it's child/dependent processes if it is still running and 
-        /// handles any errors that can occurs if the process has gone out of scope.
+        /// Kills the associated process and/or its child/dependent processes if it is still running.
+        /// Retries up to 5 times using exponential backoff with a total wait limit of 3 minutes.
+        /// Logs error when failed to kill.
         /// </summary>
         /// <param name="process">The process to kill.</param>
-        /// <param name="entireProcessTree">true to kill asociated process and it's descendents, false to only kill the process.</param>
+        /// <param name="entireProcessTree">true to kill associated process and its descendants; false to only kill the process.</param>
+        /// <param name="timeout">Max duration to wait for exit, default to 3 minutes.</param>
         /// <param name="logger">The logger to use to write trace information.</param>
-        public static void SafeKill(this IProcessProxy process, bool entireProcessTree, ILogger logger = null)
+        public static void Kill(this IProcessProxy process, ILogger logger = null, bool entireProcessTree = false, TimeSpan timeout = default)
         {
-            if (process != null)
+            timeout = timeout == default ? TimeSpan.FromMinutes(3) : timeout;
+            const int maxRetries = 5;
+
+            Stopwatch stopWatch = Stopwatch.StartNew();
+
+            for (int attempt = 1; attempt <= maxRetries; attempt++)
             {
+                int delaySeconds = Math.Min((int)Math.Pow(2, attempt), (int)(timeout - stopWatch.Elapsed).TotalSeconds);
+                if (process.HasExited || delaySeconds <= 0)
+                {
+                    break; // Exit if timeout is reached or process exited.
+                }
+
                 try
                 {
                     process.Kill(entireProcessTree);
+                    var cts = new CancellationTokenSource(TimeSpan.FromSeconds(delaySeconds));
+                    // CancellationToken should cancel first, the timeout here is precaution.
+                    process.WaitForExitAsync(cts.Token, timeout).GetAwaiter().GetResult();
                 }
-                catch (Exception exc)
+                catch (Exception ex)
                 {
-                    // Best effort here.
-                    logger?.LogTraceMessage($"Kill Process Failure. Error = {exc.Message}");
+                    logger?.LogTraceMessage($"Attempt {attempt}: Kill failed. {ex.Message}");
                 }
             }
+
+            if (!process.HasExited)
+            {
+                logger?.LogError("Failed to kill process after retries.");
+            }
         }
 
         /// <summary>
@@ -137,7 +160,7 @@ public static void SafeKill(this IProcessProxy process, bool entireProcessTree,
         /// <param name="processManager">The process manager used to find the processes.</param>
         /// <param name="processNames">The names/paths of the processes to kill.</param>
         /// <param name="logger">The logger to use to write trace information.</param>
-        public static void SafeKill(this ProcessManager processManager, IEnumerable<string> processNames, ILogger logger = null)
+        public static void Kill(this ProcessManager processManager, IEnumerable<string> processNames, ILogger logger = null)
         {
             processManager.ThrowIfNull(nameof(processManager));
 
@@ -155,7 +178,7 @@ public static void SafeKill(this ProcessManager processManager, IEnumerable<stri
                         {
                             foreach (IProcessProxy sideProcess in processes)
                             {
-                                sideProcess.SafeKill(logger);
+                                sideProcess.Kill(logger);
                             }
                         }
                     }