Skip to content
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

[7.0] Check for pending IO in the portable thread pool's worker threads #82246

Merged
merged 2 commits into from
Mar 9, 2023

Conversation

kouvel
Copy link
Member

@kouvel kouvel commented Feb 16, 2023

  • Port of Check for pending IO in the portable thread pool's worker threads #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

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 and 6.0. In 6.0 the portable thread pool is used by default for worker threads. In 7.0 the portable thread pool is used by default for IO on Windows.

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.

@kouvel kouvel added this to the 7.0.x milestone Feb 16, 2023
@kouvel kouvel requested a review from mangod9 February 16, 2023 17:56
@kouvel kouvel self-assigned this Feb 16, 2023
@ghost
Copy link

ghost commented Feb 16, 2023

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

Issue Details
  • Port of Check for pending IO in the portable thread pool's worker threads #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

Author: kouvel
Assignees: kouvel
Labels:

area-System.Threading

Milestone: 7.0.x

@kouvel kouvel added Servicing-consider Issue for next servicing release review and removed Servicing-consider Issue for next servicing release review labels Mar 1, 2023
Copy link
Member

@jeffschwMSFT jeffschwMSFT left a 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 7.0.x

@jeffschwMSFT jeffschwMSFT added the Servicing-consider Issue for next servicing release review label Mar 1, 2023
@rbhanda rbhanda modified the milestones: 7.0.x, 7.0.5 Mar 7, 2023
@rbhanda rbhanda added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Mar 7, 2023
- Port of dotnet#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 dotnet#82207
@kouvel
Copy link
Member Author

kouvel commented Mar 8, 2023

Rebased

@kouvel
Copy link
Member Author

kouvel commented Mar 9, 2023

The CI failures look unrelated to this change.

@carlossanlop carlossanlop merged commit 24fd641 into dotnet:release/7.0 Mar 9, 2023
@kouvel kouvel deleted the IoPendingFix7 branch March 9, 2023 04:16
@ghost ghost locked as resolved and limited conversation to collaborators Apr 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Threading Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants