diff --git a/src/Microsoft.TestPlatform.PlatformAbstractions/common/System/ProcessHelper.cs b/src/Microsoft.TestPlatform.PlatformAbstractions/common/System/ProcessHelper.cs index ef7e1a97db..f1aa22de84 100644 --- a/src/Microsoft.TestPlatform.PlatformAbstractions/common/System/ProcessHelper.cs +++ b/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; + + /// + /// Default constructor. + /// + public ProcessHelper() : this(new PlatformEnvironment()) + { + } + + internal ProcessHelper(IEnvironment environment) + { + _environment = environment; + } + /// public object LaunchProcess(string processPath, string? arguments, string? workingDirectory, IDictionary? envVariables, Action? errorCallback, Action? exitCallBack, Action? 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