Fixed Selenium test run hang after stopping the debugger (#3995)

src/Microsoft.TestPlatform.PlatformAbstractions/common/System/ProcessHelper.cs

@@ -26,6 +26,20 @@ public partial class ProcessHelper : IProcessHelper
     private static readonly string Arm = "arm";
     private readonly Process _currentProcess = Process.GetCurrentProcess();
 
+    private IEnvironment _environment;
+
+    /// <summary>
+    /// Default constructor.
+    /// </summary>
+    public ProcessHelper() : this(new PlatformEnvironment())
+    {
+    }
+
+    internal ProcessHelper(IEnvironment environment)
+    {
+        _environment = environment;
+    }
+
     /// <inheritdoc/>
     public object LaunchProcess(string processPath, string? arguments, string? workingDirectory, IDictionary<string, string?>? envVariables, Action<object?, string?>? errorCallback, Action<object?>? exitCallBack, Action<object?, string?>? outputCallBack)
     {
@@ -86,6 +100,8 @@ void InitializeAndStart()
             {
                 process.Exited += async (sender, args) =>
                 {
+                    const int timeout = 500;
+
                     if (sender is Process p)
                     {
                         try
@@ -102,11 +118,47 @@ void InitializeAndStart()
                             //
                             // For older frameworks, the solution is more tricky but it seems we can get the expected
                             // behavior using the parameterless 'WaitForExit()' combined with an awaited Task.Run call.
-                            var cts = new CancellationTokenSource(500);
+                            var cts = new CancellationTokenSource(timeout);
 #if NET5_0_OR_GREATER
                             await p.WaitForExitAsync(cts.Token);
 #else
-                            await Task.Run(() => p.WaitForExit(), cts.Token);
+                            // NOTE: In case we run on Windows we must call 'WaitForExit(timeout)' instead of calling
+                            // the parameterless overload. The reason for this requirement stems from the behavior of
+                            // the Selenium WebDriver when debugging a test. If the debugger is detached, the default
+                            // action is to kill the testhost process that it was attached to, but for some reason we
+                            // end up with a zombie process that would make us wait indefinitely with a simple
+                            // 'WaitForExit()' call. This in turn causes the vstest.console to block waiting for the
+                            // test request to finish and this behavior will be visible to the user since TW will
+                            // show the Selenium test as still running. Only killing the Edge Driver process would help
+                            // unblock vstest.console, but this is not a reasonable ask to our users.
+                            //
+                            // TODO: This fix is not ideal, it's only a workaround to make Selenium tests usable again.
+                            // Ideally, we should spend some more time here in order to better understand what causes
+                            // the testhost to become a zombie process in the first place.
+                            if (_environment.OperatingSystem is PlatformOperatingSystem.Windows)
+                            {
+                                p.WaitForExit(timeout);
+                            }
+                            else
+                            {
+                                cts.Token.Register(() =>
+                                {
+                                    try
+                                    {
+                                        if (!p.HasExited)
+                                        {
+                                            p.Kill();
+                                        }
+                                    }
+                                    catch
+                                    {
+                                        // Ignore all exceptions thrown when trying to kill a process that may be
+                                        // left hanging. This is a best effort to kill it, but should we fail for
+                                        // any reason we'd probably block on 'WaitForExit()' anyway.
+                                    }
+                                });
+                                await Task.Run(() => p.WaitForExit(), cts.Token).ConfigureAwait(false);
+                            }
 #endif
                         }
                         catch