-
Notifications
You must be signed in to change notification settings - Fork 5.3k
[6.0] Check for pending IO in the portable thread pool's worker threads #82248
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
Conversation
|
Tagging subscribers to this area: @mangod9 Issue Details
Port of fix for #82207
|
|
Not sure how relevant this is for 6.0 since the change that exposed the weird perfmon behavior was in 7.0: #64834 |
The issue can occur on 6.0 too since it uses the portable thread pool by default for worker threads. A similar type of IO issued on a worker thread would also be affected in 6.0. |
jeffschwMSFT
left a comment
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.
approved. we will take for consideration in 6.0.x
|
@kouvel can you please confirm there's nothing concering in the CI failures? I am seeing a failure in |
- Port of #82245 - When Resource Monitor is attached, some async IO operations are bound to the thread that issued it even though the IO handle is bound to an IOCP. If the thread exits, the async IO operation is aborted. This can lead to hangs or unexpected exceptions. - Added a check that was missing in the portable thread pool implementation to prevent exiting a worker thread when it has pending IO Port of fix for #82207
|
Last time I checked it didn't look related to this change. I've rebased, will check again. |
|
The CI failures look unrelated to this change, and this change only affects behavior on Windows. |
I see a Unix.cs file being changed too in this PR, but I agree that the failures look unrelated. Approved by Tactics. |
Yea there is a change on Unixes too but it is not a behavioral change. |
Port of fix for #82207
Customer Impact
When Resource Monitor is attached to a .NET process, some async IO operations are aborted when thread pool worker threads exit. This leads to a hang in ASP.NET apps, where it's unable to accept new connections. The issue may also manifest as an unexpected exception. Customers have reported seeing regular instances of this issue in several apps, and the apps have to be restarted. The issue may also occur with other perf monitor tools attached. A workaround is to configure the runtime to not use the portable thread pool.
Regression?
Yes, from 5.0. In 6.0 the portable thread pool is used by default for worker threads.
Testing
Verified with the repro that the behavior is the same as before.
Risk
Low. The check for pending IO already existed in the native thread pool implementation and this change adds the check to the portable thread pool implementation.