Merge pull request #1132 from pratiksanglikar/dev/ps/fixTyeWaitForProcess

Add timeout for `Process.WaitForExit()`

src/Microsoft.Tye.Core/ProcessUtil.cs

@@ -23,6 +23,8 @@ namespace Microsoft.Tye
 
         private static readonly bool IsWindows = RuntimeInformation.IsOSPlatform(OSPlatform.Windows);
 
+        private const int ProcessExitTimeoutMs = 30 * 1000; // 30 seconds timeout for the process to exit.
+
         public static Task<int> ExecuteAsync(
             string command,
             string args,
@@ -118,7 +120,13 @@ namespace Microsoft.Tye
                 {
                     // Even though the Exited event has been raised, WaitForExit() must still be called to ensure the output buffers
                     // have been flushed before the process is considered completely done.
-                    process.WaitForExit();
+                    // Because of the bug in the dotnet runtime https://github.com/dotnet/runtime/issues/29232, Process.WaitForExit()
+                    // hangs for processes that spawn another long-running processes.
+                    // Since these are expected to be long running processes and we're typically not concerned with capturing all of its output
+                    // i.e. it's probably ok for some output to be lost on shutdown, since Tye is shutting down anyway,
+                    // we call Process.WaitForProcessExit(ProcessExitTimeoutMs).
+                    // Also, since this is a process.Exited event, process.ExitCode is valid even if WaitForExit() times out.
+                    process.WaitForExit(ProcessExitTimeoutMs);
                 }
 
                 if (throwOnError && process.ExitCode != 0)
@@ -127,6 +135,7 @@ namespace Microsoft.Tye
                 }
                 else
                 {
+                    // Since the process has exited, no additional data will be written to either output buffer or error buffer, it's thread-safe to call ToString() on both outputBuilder and errorBuilder.
                     processLifetimeTask.TrySetResult(new ProcessResult(outputBuilder.ToString(), errorBuilder.ToString(), process.ExitCode));
                 }
             };

test/E2ETest/ReplicaStoppingTests.cs

@@ -61,7 +61,7 @@ namespace E2ETest
                 var replicasToRestart = new[] { replicaToStop.Key };
                 var restOfReplicas = host.Application.Services.SelectMany(s => s.Value.Replicas).Select(r => r.Value.Name).Where(r => r != replicaToStop.Key).ToArray();
 
-                Assert.True(await DoOperationAndWaitForReplicasToRestart(host, replicasToRestart.ToHashSet(), restOfReplicas.ToHashSet(), TimeSpan.FromSeconds(1), _ =>
+                Assert.True(await DoOperationAndWaitForReplicasToRestart(host, replicasToRestart.ToHashSet(), restOfReplicas.ToHashSet(), TimeSpan.FromSeconds(30), _ =>
                 {
                     replicaToStop.Value.StoppingTokenSource!.Cancel();
                     return Task.CompletedTask;