diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/AsyncStreamReader.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/AsyncStreamReader.cs index fbfd3a6baaae8a..a4b1640a2f743b 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/AsyncStreamReader.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/AsyncStreamReader.cs @@ -24,6 +24,11 @@ internal sealed class AsyncStreamReader : IDisposable { private const int DefaultBufferSize = 1024; // Byte buffer size + /// + /// Default timeout in milliseconds to wait for redirected streams to complete after the process exits. + /// + private const int StreamDrainDefaultTimeoutMs = 300; + private readonly Stream _stream; private readonly Decoder _decoder; private readonly byte[] _byteBuffer; @@ -253,7 +258,29 @@ private bool FlushMessageQueue(bool rethrowInNewThread) } } - internal Task EOF => _readToBufferTask ?? Task.CompletedTask; + internal void CancelDueToProcessExit() + { + Task? task = _readToBufferTask; + if (task is not null && !task.Wait(StreamDrainDefaultTimeoutMs)) + { + _cts.Cancel(); + task.GetAwaiter().GetResult(); + } + } + + internal async Task CancelDueToProcessExitAsync(CancellationToken cancellationToken) + { + Task? task = _readToBufferTask; + if (task is not null) + { + Task completed = await Task.WhenAny(task, Task.Delay(StreamDrainDefaultTimeoutMs, cancellationToken)).ConfigureAwait(false); + if (completed != task) + { + _cts.Cancel(); + await task.ConfigureAwait(false); + } + } + } public void Dispose() { diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index f26a8f70ffba74..61d89baf60c8b5 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -205,10 +205,10 @@ private bool WaitForExitCore(int milliseconds) bool exited = GetWaitState().WaitForExit(milliseconds); Debug.Assert(exited || milliseconds != Timeout.Infinite); - if (exited && milliseconds == Timeout.Infinite) // if we have a hard timeout, we cannot wait for the streams + if (exited) { - _output?.EOF.GetAwaiter().GetResult(); - _error?.EOF.GetAwaiter().GetResult(); + _output?.CancelDueToProcessExit(); + _error?.CancelDueToProcessExit(); } return exited; diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs index f073bd1e8d54f5..98f028802aef5c 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs @@ -176,11 +176,10 @@ private bool WaitForExitCore(int milliseconds) } finally { - // If we have a hard timeout, we cannot wait for the streams - if (milliseconds == Timeout.Infinite) + if (_signaled) { - _output?.EOF.GetAwaiter().GetResult(); - _error?.EOF.GetAwaiter().GetResult(); + _output?.CancelDueToProcessExit(); + _error?.CancelDueToProcessExit(); } handle?.Dispose(); diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs index 4218596c129644..4f3d3abccf01f6 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.cs @@ -1527,7 +1527,7 @@ public async Task WaitForExitAsync(CancellationToken cancellationToken = default // exception up to the user if (HasExited) { - await WaitUntilOutputEOF(cancellationToken).ConfigureAwait(false); + await CancelStreamReadersDueToProcessExitAsync(cancellationToken).ConfigureAwait(false); return; } @@ -1554,24 +1554,24 @@ public async Task WaitForExitAsync(CancellationToken cancellationToken = default } } - // Wait until output streams have been drained - await WaitUntilOutputEOF(cancellationToken).ConfigureAwait(false); + // Cancel stream readers after process exit with a default timeout + await CancelStreamReadersDueToProcessExitAsync(cancellationToken).ConfigureAwait(false); } finally { Exited -= handler; } - async Task WaitUntilOutputEOF(CancellationToken cancellationToken) + async Task CancelStreamReadersDueToProcessExitAsync(CancellationToken cancellationToken) { if (_output is not null) { - await _output.EOF.WaitAsync(cancellationToken).ConfigureAwait(false); + await _output.CancelDueToProcessExitAsync(cancellationToken).ConfigureAwait(false); } if (_error is not null) { - await _error.EOF.WaitAsync(cancellationToken).ConfigureAwait(false); + await _error.CancelDueToProcessExitAsync(cancellationToken).ConfigureAwait(false); } } } diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessWaitingTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessWaitingTests.cs index 8ef602113be272..e1d48347386651 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessWaitingTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessWaitingTests.cs @@ -671,5 +671,61 @@ public async Task WaitForExitAsync_NotDirected_ThrowsInvalidOperationException() var process = new Process(); await Assert.ThrowsAsync(() => process.WaitForExitAsync()); } + + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData("WaitForExit")] + [InlineData("WaitForExitInt")] + [InlineData("WaitForExitTimeSpan")] + [InlineData("WaitForExitAsync")] + public async Task WaitForExit_WithGrandChildProcess_DoesNotHang(string waitMethod) + { + Process child = CreateProcess(() => + { + using Process grandChild = CreateProcess(SleepForEightHours); + grandChild.Start(); + Console.WriteLine(grandChild.Id); + + return RemoteExecutor.SuccessExitCode; + }); + child.StartInfo.RedirectStandardOutput = true; + child.Start(); + + string grandChildPidStr = await child.StandardOutput.ReadLineAsync(); + int grandChildPid = int.Parse(grandChildPidStr); + + var stopwatch = Stopwatch.StartNew(); + + switch (waitMethod) + { + case "WaitForExit": + child.WaitForExit(); + break; + case "WaitForExitInt": + Assert.True(child.WaitForExit(WaitInMS)); + break; + case "WaitForExitTimeSpan": + Assert.True(child.WaitForExit(TimeSpan.FromMilliseconds(WaitInMS))); + break; + case "WaitForExitAsync": + await child.WaitForExitAsync(); + break; + } + + stopwatch.Stop(); + Assert.True(stopwatch.Elapsed.TotalSeconds < 2, $"WaitForExit took {stopwatch.Elapsed.TotalSeconds:F1}s, expected < 2s"); + + try + { + Process.GetProcessById(grandChildPid).Kill(); + } + catch (Exception) { } + } + + private static int SleepForEightHours() + { + Thread.Sleep(TimeSpan.FromHours(8)); + + return RemoteExecutor.SuccessExitCode; + } } }