Skip to content

Bound post-kill WaitForExit to prevent scenario-test hangs - #354

Merged
mthalman merged 1 commit into
dotnet:mainfrom
nagilson:nagilson/bound-process-kill-waitforexit
Jul 31, 2026
Merged

Bound post-kill WaitForExit to prevent scenario-test hangs#354
mthalman merged 1 commit into
dotnet:mainfrom
nagilson:nagilson/bound-process-kill-waitforexit

Conversation

@nagilson

Copy link
Copy Markdown
Member

Problem

The scenario-test harness intermittently hangs a test leg until the pipeline task timeout (observed as a 4h hang producing no .trx). It shows up most on the newest distro image in the source-build offline validation matrix — SB_Fedora43_Offline_PreviousSourceBuiltSdk_Validation_x64 — and a retry sometimes passes, so it's a race, not a deterministic failure. Tracked in dotnet/source-build#5624.

Root cause

Every process-teardown path kills a long-running process (e.g. dotnet run on a web template) with Kill(true) and then calls the parameterless WaitForExit():

process.Kill(true);
process.WaitForExit();   // blocks until stdout/stderr pipes hit EOF

Because stdout/stderr are redirected and read asynchronously (BeginOutputReadLine), WaitForExit() waits for the async readers to reach EOF on the pipes, not merely for the process to exit. dotnet run launches the app as a child/grandchild that inherits those pipe write handles. Kill(true) signals the process tree known at that instant, but if a descendant survives the kill for a moment (tree-walk race / reparent to pid 1), it keeps the write end open, EOF never arrives, and WaitForExit() blocks forever. Newer Fedora 43 (kernel + systemd + cgroup v2 reaping differences) makes the lingering-child race most likely, and the offline PreviousSourceBuiltSdk legs hit the timeout→kill path most often.

Fix

Bound every post-kill wait with a grace timeout (ExecuteHelper.KillGraceMilliseconds = 30_000) so the run fails fast instead of hanging. On the already-failing/timeout path, accepting possibly-truncated output is fine. Three sites:

  • Microsoft.DotNet.ScenarioTests.Common/ExecuteHelper.csExecuteProcess
  • Microsoft.DotNet.ScenarioTests.SdkTemplateTests/DotNetSdkHelper.csExecuteRunWeb and ExecuteRunUIApp

Minimal, behavior-preserving for the healthy path; only the pathological "won't die / pipe held open" case changes (from infinite hang to fast failure). Built locally with build.cmd (0 warnings, 0 errors).

Note: the VMR carries a sibling copy at test/TestUtilities/ExecuteHelper.cs (dotnet/dotnet) with the identical pattern; that one should get the same bound separately.

Investigation

Full analysis and signals: dotnet/source-build#5624

cc @akoeplinger @ViktorHofer — you're the most active maintainers here; would appreciate a review.

The scenario-test harness kills long-running processes (e.g. `dotnet run` on
web templates) via Kill(true) followed by a parameterless WaitForExit(). With
stdout/stderr redirected and read asynchronously, WaitForExit() blocks until the
pipes reach EOF, not merely until the process exits. A grandchild that inherited
the pipe handles and survives the tree-kill race keeps the write end open, so
WaitForExit() never returns and the leg runs until the pipeline task timeout
(observed as a 4h hang with no .trx). This is most frequent on the newest distro
image (Fedora 43) in the source-build offline validation matrix.

Bound every post-kill wait with a grace timeout so the run fails fast instead of
hanging. Sites: ExecuteHelper.ExecuteProcess and DotNetSdkHelper
ExecuteRunWeb/ExecuteRunUIApp.

Investigation: dotnet/source-build#5624

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 367a8152-b3c0-447a-99f1-ea5d9a9d1d79
Comment thread src/Microsoft.DotNet.ScenarioTests.Common/ExecuteHelper.cs
@nagilson

Copy link
Copy Markdown
Member Author

@ViktorHofer @mthalman Thanks for your review, I don't have permission to merge this so would appreciate a merge now.

@mthalman
mthalman merged commit 2ae7f98 into dotnet:main Jul 31, 2026
8 checks passed
@dotnet-milestone-bot dotnet-milestone-bot Bot added this to the 11.0-rc1 milestone Aug 3, 2026
@mthalman

mthalman commented Aug 3, 2026

Copy link
Copy Markdown
Member

/backport to release/10.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants