Fix ToolTask output loss: increase EOF pipe timeout from 2s to 30s#13767
Fix ToolTask output loss: increase EOF pipe timeout from 2s to 30s#13767huulinhnguyen-dev wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses intermittent loss of final stdout/stderr lines captured by ToolTask under Wave18_6, by giving the async pipe readers more time to reach EOF after the tool process has exited and emitting a low-importance diagnostic when EOF still doesn’t arrive.
Changes:
- Increased the post-exit EOF wait from 2s to 30s in
ToolTask.WaitForProcessExitand acted on theWaitAlltimeout result. - On EOF-wait timeout, drains any already-delivered stdout/stderr and logs a
MessageImportance.Lowdiagnostic (ToolTask.PipeEOFTimeout). - Added the new localized resource string across
Strings.resxand the localized.xlffiles.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Utilities/ToolTask.cs | Extends EOF wait and adds a low-importance diagnostic + drain behavior on timeout (Wave18_6 path). |
| src/Utilities/Resources/Strings.resx | Adds ToolTask.PipeEOFTimeout resource string used by the new diagnostic message. |
| src/Utilities/Resources/xlf/Strings.cs.xlf | Adds localized entry stub for ToolTask.PipeEOFTimeout. |
| src/Utilities/Resources/xlf/Strings.de.xlf | Adds localized entry stub for ToolTask.PipeEOFTimeout. |
| src/Utilities/Resources/xlf/Strings.es.xlf | Adds localized entry stub for ToolTask.PipeEOFTimeout. |
| src/Utilities/Resources/xlf/Strings.fr.xlf | Adds localized entry stub for ToolTask.PipeEOFTimeout. |
| src/Utilities/Resources/xlf/Strings.it.xlf | Adds localized entry stub for ToolTask.PipeEOFTimeout. |
| src/Utilities/Resources/xlf/Strings.ja.xlf | Adds localized entry stub for ToolTask.PipeEOFTimeout. |
| src/Utilities/Resources/xlf/Strings.ko.xlf | Adds localized entry stub for ToolTask.PipeEOFTimeout. |
| src/Utilities/Resources/xlf/Strings.pl.xlf | Adds localized entry stub for ToolTask.PipeEOFTimeout. |
| src/Utilities/Resources/xlf/Strings.pt-BR.xlf | Adds localized entry stub for ToolTask.PipeEOFTimeout. |
| src/Utilities/Resources/xlf/Strings.ru.xlf | Adds localized entry stub for ToolTask.PipeEOFTimeout. |
| src/Utilities/Resources/xlf/Strings.tr.xlf | Adds localized entry stub for ToolTask.PipeEOFTimeout. |
| src/Utilities/Resources/xlf/Strings.zh-Hans.xlf | Adds localized entry stub for ToolTask.PipeEOFTimeout. |
| src/Utilities/Resources/xlf/Strings.zh-Hant.xlf | Adds localized entry stub for ToolTask.PipeEOFTimeout. |
|
this pr improved the logging #13760 can you verify the flaky test output matches expectations with this better diagnosability? we'd like to have confirmation before commiting this candidate fix |
|
What is the behavior of a task that does launch a long-lived child that inherits the stdout handle? Does it now hang for 30s? |
|
Updated the grandchild test to use a longer-lived process (ping -n 40, ~39s) with a Stopwatch assertion proving the task returns within 35s — confirming the 30s EOF timeout is a hard bound. The diagnostic message ("Pipe EOF not received within 30s") also appears in the log, addressing the diagnosability ask from #13760. |
…github.com/huulinhnguyen-dev/msbuild into dev/huulinhnguyen/fix-tooltask-eof-timeout
Fixes #13734
Context
ToolTask.WaitForProcessExithad a 2-second timeout forAsyncStreamReaderto flush stdout/stderr after process exit. On loaded CI machines the timeout expired before the reader finished, and the result was silently discarded — causing final output lines to be lost.Changes Made
WaitForProcessExitWaitHandle.WaitAllreturn value and drain queues on timeoutMessageImportance.Lowdiagnostic when timeout firesTesting
Microsoft.Build.Utilities.UnitTeststests passToolTaskDoesNotHangWhenGrandchildInheritsPipeHandlesverifies no regression on grandchild pipe scenarioToolTaskCapturesAllOutputWithFixverifies end-to-end output capture still worksNotes
Fix stays within the existing
Wave18_6gate. No new infrastructure added.