Skip to content

Fix possible ToolTask hang#10297

Merged
MichalPavlik merged 3 commits intomainfrom
dev/mipavlik/fix-tooltask-hang
Jul 11, 2024
Merged

Fix possible ToolTask hang#10297
MichalPavlik merged 3 commits intomainfrom
dev/mipavlik/fix-tooltask-hang

Conversation

@MichalPavlik
Copy link
Member

Fixes #2981
and probably #10286

Context

ToolTask can hang when child process spawns a grandchild process that doesn't exit.

Changes Made

Using different overload of WaitForExit to avoid situation when grandchild process is keeping the MSBuild waiting.
See dotnet/runtime#51277 and #2981 (comment)

Testing

Manual testing with custom ToolTask implementation. This tasks starts process that starts another process with longer lifetime.

Notes

…child process is keeping the MSBuild waiting
Copy link
Member

@rainersigwald rainersigwald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?)

Co-authored-by: Rainer Sigwald <raines@microsoft.com>
@MichalPavlik MichalPavlik merged commit 35bc386 into main Jul 11, 2024
@MichalPavlik MichalPavlik deleted the dev/mipavlik/fix-tooltask-hang branch July 11, 2024 11:27
@Kuinox
Copy link

Kuinox commented Jul 11, 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.

@akoeplinger
Copy link
Member

I'm not sure, but I think it can lead to the end of the output to be truncated.

I think I'm running into this issue after this PR: #10378

@Kuinox
Copy link

Kuinox commented Jul 16, 2024

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

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.

Process.WaitForExit() deadlocks when waiting on "dotnet.exe build /m /nr:true"

5 participants