Repository navigation
Fix possible ToolTask hang - #10297
Merged
Merged
Conversation
…child process is keeping the MSBuild waiting
rainersigwald
left a comment
rainersigwald
left a comment
Member
There was a problem hiding this comment.
Remember how you mentioned this offline and said "I bet Rainer will ask for {some wise thing to ask}"?
. . . let's assume I ask for that :-P
(Was it a wait-for-flush situation? I don't know if there's a great way to do that. Maybe just putting this behind a changewave would be sufficient?)
rainersigwald
approved these changes
Jun 28, 2024
Co-authored-by: Rainer Sigwald <raines@microsoft.com>
YuliiaKovalova
approved these changes
Jul 1, 2024
|
I remember that when I analyzed the problem, the issue of WaitForExit(int) is that it doesn't wait for the pipes to empty. I'm not sure, but I think it can lead to the end of the output to be truncated. |
Member
I think I'm running into this issue after this PR: #10378 |
|
There could be a work around using some reflection hack, but without that there is no way to wait for exit cleanly on windows. Ideally the underlying bug should be fixed: dotnet/runtime#51277 |
MichalPavlik
added a commit
that referenced
this pull request
Jul 17, 2024
MichalPavlik
added a commit
that referenced
this pull request
Jul 17, 2024
This was referenced Jul 17, 2024
Merged
YuliiaKovalova
added a commit
that referenced
this pull request
Mar 23, 2026
) ## Context `ToolTask` can hang indefinitely when the tool it spawns creates grandchild processes that inherit stdout/stderr pipe handles. This is a long-standing issue reported in #2981 (opened 2018), with a previous fix attempt in #10297 that was **reverted** (#10395) because it caused output loss (#10378). ### Root Cause On .NET Framework (and .NET Core), the parameterless `Process.WaitForExit()` internally calls `AsyncStreamReader.WaitUtilEOF()`, which blocks until **all** write handles to the stdout/stderr pipes are closed. When a tool like `cl.exe` spawns a grandchild process (e.g., `mspdbsrv.exe`) that inherits the pipe handles, EOF is never reached even though the tool itself has exited - causing an infinite hang. ### Why the previous fix (#10297) failed PR #10297 changed `proc.WaitForExit()` to `proc.WaitForExit(int.MaxValue)`. The `int` overload skips `WaitUtilEOF()`, preventing the hang. **But it also skips waiting for all `DataReceived` callbacks to be delivered.** For fast tools like `command -v ls`, the `AsyncStreamReader` hadn't delivered its output before the drain calls ran, causing `ConsoleOutput` to be empty. ## Changes Made The fix (behind **ChangeWave 18.6**) uses the `Data==null` EOF sentinel that `AsyncStreamReader` sends via `DataReceived` when each pipe reaches EOF: ### Step 1: `proc.WaitForExit(int.MaxValue)` Waits for the **process handle only**, not pipe EOF. Since `_toolExited` already fired before `WaitForProcessExit` is called, the process is dead and this returns immediately. ### Step 2: `WaitHandle.WaitAll(eofEvents, 2000)` Waits for our own `_standardOutputEOF` / `_standardErrorEOF` events, which are set by `ReceiveStandardErrorOrOutputData` when `Data==null` arrives from the `AsyncStreamReader`. - **Normal case (no grandchild):** Pipe closes immediately after tool exits, EOF arrives within milliseconds, events fire, all data including final partial line is guaranteed delivered. This provides **identical guarantees** to the original `proc.WaitForExit()`. - **Grandchild case:** Grandchild holds pipe open, EOF never arrives, events time out after 2 seconds, proceed. The tool's line-by-line output was already delivered during the `HandleToolNotifications` loop. ### Why this doesn't lose data (unlike #10297) Our `_standardOutputEOF` fires **inside** `AsyncStreamReader.FlushMessageQueue()`, which is called **before** the internal `eofEvent.Set()`. By the time our event fires, the `AsyncStreamReader` has already: 1. Read all remaining bytes from the pipe buffer 2. Decoded them into characters 3. Flushed the final partial line from its `StringBuilder` 4. Delivered every line (including the final one) via `DataReceived` callbacks This is functionally equivalent to `WaitUtilEOF` - just observed from our callback instead of the internal event. ## Testing ### ToolTaskDoesNotHangWhenGrandchildInheritsPipeHandles Spawns `cmd.exe /c echo hello & start /b ping -n 120 127.0.0.1 > nul`: - `cmd.exe` writes "hello" and exits immediately - `ping` inherits pipe handles and runs for 120 seconds - **With fix:** Test completes in ~2 seconds, "hello" is captured - **Without fix (MSBUILDDISABLEFEATURESFROMVERSION=18.6):** Test hangs until 30s timeout ### ToolTaskCapturesAllOutputWithFix Spawns `cmd.exe /c echo line1 & echo line2 & echo line3` (no grandchild): - Verifies all three lines are captured - regression test for #10378 --------- Co-authored-by: Adam Sitnik <adam.sitnik@gmail.com>
dfederm
pushed a commit
to dfederm/msbuild
that referenced
this pull request
Apr 9, 2026
…net#13351) ## Context `ToolTask` can hang indefinitely when the tool it spawns creates grandchild processes that inherit stdout/stderr pipe handles. This is a long-standing issue reported in dotnet#2981 (opened 2018), with a previous fix attempt in dotnet#10297 that was **reverted** (dotnet#10395) because it caused output loss (dotnet#10378). ### Root Cause On .NET Framework (and .NET Core), the parameterless `Process.WaitForExit()` internally calls `AsyncStreamReader.WaitUtilEOF()`, which blocks until **all** write handles to the stdout/stderr pipes are closed. When a tool like `cl.exe` spawns a grandchild process (e.g., `mspdbsrv.exe`) that inherits the pipe handles, EOF is never reached even though the tool itself has exited - causing an infinite hang. ### Why the previous fix (dotnet#10297) failed PR dotnet#10297 changed `proc.WaitForExit()` to `proc.WaitForExit(int.MaxValue)`. The `int` overload skips `WaitUtilEOF()`, preventing the hang. **But it also skips waiting for all `DataReceived` callbacks to be delivered.** For fast tools like `command -v ls`, the `AsyncStreamReader` hadn't delivered its output before the drain calls ran, causing `ConsoleOutput` to be empty. ## Changes Made The fix (behind **ChangeWave 18.6**) uses the `Data==null` EOF sentinel that `AsyncStreamReader` sends via `DataReceived` when each pipe reaches EOF: ### Step 1: `proc.WaitForExit(int.MaxValue)` Waits for the **process handle only**, not pipe EOF. Since `_toolExited` already fired before `WaitForProcessExit` is called, the process is dead and this returns immediately. ### Step 2: `WaitHandle.WaitAll(eofEvents, 2000)` Waits for our own `_standardOutputEOF` / `_standardErrorEOF` events, which are set by `ReceiveStandardErrorOrOutputData` when `Data==null` arrives from the `AsyncStreamReader`. - **Normal case (no grandchild):** Pipe closes immediately after tool exits, EOF arrives within milliseconds, events fire, all data including final partial line is guaranteed delivered. This provides **identical guarantees** to the original `proc.WaitForExit()`. - **Grandchild case:** Grandchild holds pipe open, EOF never arrives, events time out after 2 seconds, proceed. The tool's line-by-line output was already delivered during the `HandleToolNotifications` loop. ### Why this doesn't lose data (unlike dotnet#10297) Our `_standardOutputEOF` fires **inside** `AsyncStreamReader.FlushMessageQueue()`, which is called **before** the internal `eofEvent.Set()`. By the time our event fires, the `AsyncStreamReader` has already: 1. Read all remaining bytes from the pipe buffer 2. Decoded them into characters 3. Flushed the final partial line from its `StringBuilder` 4. Delivered every line (including the final one) via `DataReceived` callbacks This is functionally equivalent to `WaitUtilEOF` - just observed from our callback instead of the internal event. ## Testing ### ToolTaskDoesNotHangWhenGrandchildInheritsPipeHandles Spawns `cmd.exe /c echo hello & start /b ping -n 120 127.0.0.1 > nul`: - `cmd.exe` writes "hello" and exits immediately - `ping` inherits pipe handles and runs for 120 seconds - **With fix:** Test completes in ~2 seconds, "hello" is captured - **Without fix (MSBUILDDISABLEFEATURESFROMVERSION=18.6):** Test hangs until 30s timeout ### ToolTaskCapturesAllOutputWithFix Spawns `cmd.exe /c echo line1 & echo line2 & echo line3` (no grandchild): - Verifies all three lines are captured - regression test for dotnet#10378 --------- Co-authored-by: Adam Sitnik <adam.sitnik@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2981
and probably #10286
Context
ToolTaskcan hang when child process spawns a grandchild process that doesn't exit.Changes Made
Using different overload of
WaitForExitto avoid situation when grandchild process is keeping the MSBuild waiting.See dotnet/runtime#51277 and #2981 (comment)
Testing
Manual testing with custom
ToolTaskimplementation. This tasks starts process that starts another process with longer lifetime.Notes