Wait for process exit independently of waiting for stream flushing (#612)
authorJuan Hoyos <juan.hoyos@microsoft.com>
Wed, 13 Nov 2019 23:53:48 +0000 (15:53 -0800)
committerGitHub <noreply@github.com>
Wed, 13 Nov 2019 23:53:48 +0000 (15:53 -0800)
* 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.

src/Microsoft.Diagnostics.TestHelpers/ProcessRunner.cs

index 5dad0e5d0491fa705b8bd577710c637c6e9b73ca..94dd27d5f97a58e2af47a9c84ee7a670e8bb8120 100644 (file)
@@ -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)
             {