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

[release/8.0-staging] Handle NativeOverlapped* coming from both the Windows or Portable thread pool in NativeRuntimeEventSource #99656

Merged

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Mar 13, 2024

Backport of #97365 to release/8.0-staging

/cc @eduardo-vp

Customer Impact

  • Customer reported
  • Found internally

When a customer's app uses the Windows thread pool and tries to fire ThreadPoolIOEnqueue, ThreadPoolIODequeue or ThreadPoolIOPack, it's likely to crash as these events currently expect a NativeOverlapped* from the Portable thread pool. The events are expected to handle either implementation of the thread pool. This change affects apps using the Windows thread pool (it's the default for Native AOT on Windows apps but other apps may enable its usage too).

Regression

  • Yes
  • No

Testing

The fix was verified with the following test #98432. It enables the Windows thread pool, provides the conditions to fire the events and makes some assertions to verify everything worked as expected. There were no tests using async read/write which could have showed evidence of the issue.

Risk

Low, the fix just affects apps which fire thread pool events using the Windows thread pool. The fix was locally tested several times and also a test was added to verify it works as expected.

@eduardo-vp eduardo-vp added the Servicing-consider Issue for next servicing release review label Mar 13, 2024
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 8.0.x

@jeffschwMSFT jeffschwMSFT added this to the 8.0.x milestone Mar 14, 2024
@jeffschwMSFT jeffschwMSFT added Servicing-consider Issue for next servicing release review and removed Servicing-consider Issue for next servicing release review labels Mar 14, 2024
@rbhanda rbhanda added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Mar 14, 2024
@rbhanda rbhanda modified the milestones: 8.0.x, 8.0.5 Mar 14, 2024
@eduardo-vp eduardo-vp merged commit ff80834 into release/8.0-staging Mar 16, 2024
174 of 186 checks passed
@jkotas jkotas deleted the backport/pr-97365-to-release/8.0-staging branch March 31, 2024 15:29
@github-actions github-actions bot locked and limited conversation to collaborators May 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants