-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix thread pool hang #56346
Fix thread pool hang #56346
Conversation
- In dotnet#53471 the thread count goal was moved out of `ThreadCounts`, it turns out that are a few subtle races that it was avoiding. There are other ways to fix it, but I've added the goal back into `ThreadCounts` for now. - Reverted PR dotnet#55985, which worked around the issue in the CI Fixes dotnet#55642
This reverts commit 0a5e93b.
Tagging subscribers to this area: @mangod9 Issue Details
Fixes #55642
|
Thanks for tracking this down! There was some suspicion this may also have been contributing to some of the CI hangs we've seen recently. It'll be interesting to see if their rate decreases after this goes in. |
Is there anything that could be done to create a test that would catch this kind of failure? Or do we already have that in the Quic tests 😄 |
Seems like tricky business @danmoseley. It was found by standard conformance Stream tests (with synchronous IO). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The QUIC part looks good to me. I'll give it try tomorrow.
True, but I have seen @kouvel devise unit tests that consistently repro the most sporadic issues, so I thought I'd ask 😄 |
I wonder if coyote could come in handy here https://microsoft.github.io/coyote/ |
I had some difficulty in reproing it initially when I tried but upon trying a few more things I got a fairly frequent repro now on my Windows machine, added a test. |
- Depends on dotnet/runtime#56346 - Reverted commit 3d57bee from PR dotnet#2324 since the relevant change to `ThreadCounts` was reverted in dotnet/runtime#56346
- Depends on dotnet/runtime#56346 - Reverted commit 3d57bee from PR #2324 since the relevant change to `ThreadCounts` was reverted in dotnet/runtime#56346
ThreadCounts
, it turns out that there are a few subtle races that it was avoiding. There are other ways to fix it, but I've added the goal back intoThreadCounts
for now.Fixes #55642