From f43ad930d26dbd280b929a67fb4ae9d595622a34 Mon Sep 17 00:00:00 2001 From: Juan Hoyos Date: Wed, 13 Nov 2019 15:53:48 -0800 Subject: [PATCH] Wait for process exit independently of waiting for stream flushing (#612) * Wait for process exit independently of waiting for stream flushing Before, the ProcessRunner waited for the process completion as well as stream buffer flushing for stdout and stderr. This is problematic if the process it ran dies, but its spawned child processes keep the defunct's standard streams alive by redirecting to them. We have had no way of harvesting the children, so this process hung the runner. After this change, we wait for the spawned process to die. As soon as that happens, we give the pipes a grace period of 15 seconds to flush, after which we assume the process is dead. This has the shortcoming of two potentially problematic situations: - We've potentially left an unobserved aggregate exception unobserved that will throw on the finalizer thread once the tasks get collected in exceptional cases. - This might leak threadpool threads. --- .../ProcessRunner.cs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.Diagnostics.TestHelpers/ProcessRunner.cs b/src/Microsoft.Diagnostics.TestHelpers/ProcessRunner.cs index 5dad0e5d0..94dd27d5f 100644 --- a/src/Microsoft.Diagnostics.TestHelpers/ProcessRunner.cs +++ b/src/Microsoft.Diagnostics.TestHelpers/ProcessRunner.cs @@ -403,9 +403,22 @@ namespace Microsoft.Diagnostics.TestHelpers }, TaskCreationOptions.LongRunning); - DebugTrace("awaiting process {0} exit, stdOut, and stdErr", p.Id); - await Task.WhenAll(processExit, stdOutTask, stdErrTask); - DebugTrace("await process {0} exit, stdOut, and stdErr complete", p.Id); + DebugTrace("awaiting process {0} exit", p.Id); + await processExit; + DebugTrace("process {0} completed with exit code {1}", p.Id, p.ExitCode); + + DebugTrace("awaiting to flush stdOut and stdErr for process {0} for up to 15 seconds", p.Id); + var streamsTask = Task.WhenAll(stdOutTask, stdErrTask); + var completedTask = await Task.WhenAny(streamsTask, Task.Delay(TimeSpan.FromSeconds(15))); + + if (completedTask != streamsTask) + { + DebugTrace("WARNING: Flushing stdOut and stdErr for process {0} timed out, threads used for the tasks might be leaked", p.Id); + } + else + { + DebugTrace("Flushed stdOut and stdErr for process {0}", p.Id); + } foreach (IProcessLogger logger in Loggers) {