-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Implement Thread.Interrupt on NativeAOT on Windows #118968
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
Implement Thread.Interrupt on NativeAOT on Windows #118968
Conversation
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.
Pull Request Overview
This PR implements Thread.Interrupt functionality for NativeAOT on Windows, addressing issue #118293. The implementation enables thread interruption through Windows APC (Asynchronous Procedure Call) mechanism.
Key changes:
- Adds native interrupt APC callback and thread state management in the NativeAOT runtime
- Implements Thread.Interrupt() and CheckForPendingInterrupt() methods for Windows
- Integrates interrupt checks into sleep and wait operations with alertable waits
- Removes conditional test exclusions for NativeAOT now that Thread.Interrupt is supported
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
src/tests/baseservices/threading/regressions/115178/115178.cs | Removes NativeAOT conditional exclusion for interrupt test |
src/libraries/System.Threading/tests/MonitorTests.cs | Removes ActiveIssue attribute excluding NativeAOT from interrupt wait test |
src/libraries/System.Threading.Thread/tests/ThreadTests.cs | Removes ActiveIssue attributes excluding NativeAOT from interrupt tests |
src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.Windows.cs | Adds interrupt checks during APC completion in wait operations |
src/libraries/System.Private.CoreLib/src/System/Threading/Thread.Windows.cs | Changes conditional compilation to exclude Mono instead of including CoreCLR |
src/libraries/Common/src/Interop/Windows/Kernel32/Interop.Threading.cs | Adds P/Invoke declarations for QueueUserAPC and SleepEx |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.cs | Adds Interrupted thread state constant |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Windows.cs | Implements Thread.Interrupt, SleepInternal, and interrupt-aware join operations |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs | Adds runtime imports for interrupt APC callback and interrupt checking |
src/coreclr/nativeaot/Runtime/thread.h | Adds TSF_Interrupted thread state flag and interrupt management methods |
src/coreclr/nativeaot/Runtime/thread.cpp | Implements interrupt state management and native APC callback functions |
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Windows.cs
Outdated
Show resolved
Hide resolved
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Windows.cs
Outdated
Show resolved
Hide resolved
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Windows.cs
Outdated
Show resolved
Hide resolved
Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas |
Co-authored-by: Copilot <[email protected]>
…inally block around the re-entrant wait loop
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Windows.cs
Outdated
Show resolved
Hide resolved
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Windows.cs
Outdated
Show resolved
Hide resolved
Change check to assert.
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Windows.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Jan Kotas <[email protected]>
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.
LGTM otherwise. Thank you!
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Windows.cs
Outdated
Show resolved
Hide resolved
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Windows.cs
Outdated
Show resolved
Hide resolved
src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Windows.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.Windows.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.Windows.cs
Outdated
Show resolved
Hide resolved
Please don't forget to run outerloop when this is ready to merge, I don't think we run any of these newly enabled tests by default. |
…ore throwing the exception instead of in a finally block
/azp run runtime-nativeaot-outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-nativeaot-outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-nativeaot-outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
/ba-g nativeaot linux failures and windows timeout seem unrelated. Tests passed on the windows leg (manually inspected the test results on the job that completed right before the timeout) |
Fixes #118293
Primarily done via Copilot based on the issue (just done locally on a Windows machine to avoid the problems with #118302)