Skip to content

Commit c09afe1

Browse files
jkoritzinskyCopilotjkotas
authored
Implement Thread.Interrupt on NativeAOT on Windows (#118968)
* Implement Thread.Interrupt on NativeAOT on Windows * Apply suggestions from code review Co-authored-by: Copilot <[email protected]> * Cleanup * Structure the loops like the re-entrant wait handling, insert a try-finally block around the re-entrant wait loop * Add ifdef around flag only used on Windows * Check interrupt before we wait (in case the APC ran at thread startup). Change check to assert. * Update Thread.NativeAot.Windows.cs Co-authored-by: Jan Kotas <[email protected]> * Use Timeout constants and move state clearing to be manually done before throwing the exception instead of in a finally block * Fix the "unstarted" interrupt case * Remove null checks and only attach uninitialized threads --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Jan Kotas <[email protected]>
1 parent 5621da6 commit c09afe1

File tree

12 files changed

+235
-20
lines changed

12 files changed

+235
-20
lines changed

‎src/coreclr/nativeaot/Runtime/thread.cpp‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1161,6 +1161,23 @@ bool Thread::CheckPendingRedirect(PCODE eip)
11611161

11621162
#endif // TARGET_X86
11631163

1164+
void Thread::SetInterrupted(bool isInterrupted)
1165+
{
1166+
if (isInterrupted)
1167+
{
1168+
SetState(TSF_Interrupted);
1169+
}
1170+
else
1171+
{
1172+
ClearState(TSF_Interrupted);
1173+
}
1174+
}
1175+
1176+
bool Thread::CheckInterrupted()
1177+
{
1178+
return IsStateSet(TSF_Interrupted);
1179+
}
1180+
11641181
#endif // !DACCESS_COMPILE
11651182

11661183
void Thread::ValidateExInfoStack()
@@ -1367,6 +1384,43 @@ FCIMPL0(size_t, RhGetDefaultStackSize)
13671384
}
13681385
FCIMPLEND
13691386

1387+
#ifdef TARGET_WINDOWS
1388+
// Native APC callback for Thread.Interrupt
1389+
// This callback sets the interrupt flag on the current thread
1390+
static VOID CALLBACK InterruptApcCallback(ULONG_PTR /* parameter */)
1391+
{
1392+
Thread* pCurrentThread = ThreadStore::RawGetCurrentThread();
1393+
if (!pCurrentThread->IsInitialized())
1394+
{
1395+
// If the thread was interrupted before it was started
1396+
// the thread won't have been initialized.
1397+
// Attach the thread here if it's the first time we're seeing it.
1398+
ThreadStore::AttachCurrentThread();
1399+
}
1400+
1401+
pCurrentThread->SetInterrupted(true);
1402+
}
1403+
1404+
// Function to get the address of the interrupt APC callback
1405+
FCIMPL0(void*, RhGetInterruptApcCallback)
1406+
{
1407+
return (void*)InterruptApcCallback;
1408+
}
1409+
FCIMPLEND
1410+
1411+
FCIMPL0(FC_BOOL_RET, RhCheckAndClearPendingInterrupt)
1412+
{
1413+
Thread* pCurrentThread = ThreadStore::RawGetCurrentThread();
1414+
if (pCurrentThread->CheckInterrupted())
1415+
{
1416+
pCurrentThread->SetInterrupted(false);
1417+
FC_RETURN_BOOL(true);
1418+
}
1419+
FC_RETURN_BOOL(false);
1420+
}
1421+
FCIMPLEND
1422+
#endif // TARGET_WINDOWS
1423+
13701424
// Standard calling convention variant and actual implementation for RhpReversePInvokeAttachOrTrapThread
13711425
EXTERN_C NOINLINE void FASTCALL RhpReversePInvokeAttachOrTrapThread2(ReversePInvokeFrame* pFrame)
13721426
{

‎src/coreclr/nativeaot/Runtime/thread.h‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ struct ee_alloc_context
132132

133133
struct RuntimeThreadLocals
134134
{
135-
ee_alloc_context m_eeAllocContext;
135+
ee_alloc_context m_eeAllocContext;
136136
uint32_t volatile m_ThreadStateFlags; // see Thread::ThreadStateFlags enum
137137
PInvokeTransitionFrame* m_pTransitionFrame;
138138
PInvokeTransitionFrame* m_pDeferredTransitionFrame; // see Thread::EnablePreemptiveMode
@@ -214,6 +214,7 @@ class Thread : private RuntimeThreadLocals
214214
//
215215
// On Unix this is an optimization to not queue up more signals when one is
216216
// still being processed.
217+
TSF_Interrupted = 0x00000200, // Set to indicate Thread.Interrupt() has been called on this thread
217218
};
218219
private:
219220

@@ -390,6 +391,9 @@ class Thread : private RuntimeThreadLocals
390391
void SetPendingRedirect(PCODE eip);
391392
bool CheckPendingRedirect(PCODE eip);
392393
#endif
394+
395+
void SetInterrupted(bool isInterrupted);
396+
bool CheckInterrupted();
393397
};
394398

395399
#ifndef __GCENV_BASE_INCLUDED__

‎src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -614,6 +614,14 @@ internal static IntPtr RhGetModuleSection(TypeManagerHandle module, ReadyToRunSe
614614
[RuntimeImport(RuntimeLibrary, "RhGetDefaultStackSize")]
615615
internal static extern unsafe IntPtr RhGetDefaultStackSize();
616616

617+
[MethodImplAttribute(MethodImplOptions.InternalCall)]
618+
[RuntimeImport(RuntimeLibrary, "RhGetInterruptApcCallback")]
619+
internal static extern unsafe delegate* unmanaged<nuint, void> RhGetInterruptApcCallback();
620+
621+
[MethodImplAttribute(MethodImplOptions.InternalCall)]
622+
[RuntimeImport(RuntimeLibrary, "RhCheckAndClearPendingInterrupt")]
623+
internal static extern bool RhCheckAndClearPendingInterrupt();
624+
617625
[MethodImplAttribute(MethodImplOptions.InternalCall)]
618626
[RuntimeImport("*", "RhGetCurrentThunkContext")]
619627
internal static extern IntPtr GetCurrentInteropThunkContext();

‎src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Windows.cs‎

Lines changed: 138 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,59 @@ public sealed partial class Thread
2626

2727
partial void PlatformSpecificInitialize();
2828

29+
internal static void SleepInternal(int millisecondsTimeout)
30+
{
31+
Debug.Assert(millisecondsTimeout >= Timeout.Infinite);
32+
33+
CheckForPendingInterrupt();
34+
35+
Thread currentThread = CurrentThread;
36+
if (millisecondsTimeout == Timeout.Infinite)
37+
{
38+
// Infinite wait - use alertable wait
39+
currentThread.SetWaitSleepJoinState();
40+
uint result;
41+
while (true)
42+
{
43+
result = Interop.Kernel32.SleepEx(Timeout.UnsignedInfinite, true);
44+
if (result != Interop.Kernel32.WAIT_IO_COMPLETION)
45+
{
46+
break;
47+
}
48+
CheckForPendingInterrupt();
49+
}
50+
51+
currentThread.ClearWaitSleepJoinState();
52+
}
53+
else
54+
{
55+
// Timed wait - use alertable wait
56+
currentThread.SetWaitSleepJoinState();
57+
long startTime = Environment.TickCount64;
58+
while (true)
59+
{
60+
uint result = Interop.Kernel32.SleepEx((uint)millisecondsTimeout, true);
61+
if (result != Interop.Kernel32.WAIT_IO_COMPLETION)
62+
{
63+
break;
64+
}
65+
// Check if this was our interrupt APC
66+
CheckForPendingInterrupt();
67+
// Handle APC completion by adjusting timeout and retrying
68+
long currentTime = Environment.TickCount64;
69+
long elapsed = currentTime - startTime;
70+
if (elapsed >= millisecondsTimeout)
71+
{
72+
break;
73+
}
74+
millisecondsTimeout -= (int)elapsed;
75+
startTime = currentTime;
76+
}
77+
78+
currentThread.ClearWaitSleepJoinState();
79+
}
80+
}
81+
2982
// Platform-specific initialization of foreign threads, i.e. threads not created by Thread.Start
3083
private void PlatformSpecificInitializeExistingThread()
3184
{
@@ -154,18 +207,57 @@ private bool JoinInternal(int millisecondsTimeout)
154207

155208
try
156209
{
157-
int result;
158-
159210
if (millisecondsTimeout == 0)
160211
{
161-
result = (int)Interop.Kernel32.WaitForSingleObject(waitHandle.DangerousGetHandle(), 0);
212+
int result = (int)Interop.Kernel32.WaitForSingleObject(waitHandle.DangerousGetHandle(), 0);
213+
return result == (int)Interop.Kernel32.WAIT_OBJECT_0;
162214
}
163215
else
164216
{
165-
result = WaitHandle.WaitOneCore(waitHandle.DangerousGetHandle(), millisecondsTimeout, useTrivialWaits: false);
217+
Thread currentThread = CurrentThread;
218+
currentThread.SetWaitSleepJoinState();
219+
uint result;
220+
if (millisecondsTimeout == Timeout.Infinite)
221+
{
222+
// Infinite wait
223+
while (true)
224+
{
225+
result = Interop.Kernel32.WaitForSingleObjectEx(waitHandle.DangerousGetHandle(), Timeout.UnsignedInfinite, Interop.BOOL.TRUE);
226+
if (result != Interop.Kernel32.WAIT_IO_COMPLETION)
227+
{
228+
break;
229+
}
230+
// Check if this was our interrupt APC
231+
CheckForPendingInterrupt();
232+
}
233+
}
234+
else
235+
{
236+
long startTime = Environment.TickCount64;
237+
while (true)
238+
{
239+
result = Interop.Kernel32.WaitForSingleObjectEx(waitHandle.DangerousGetHandle(), (uint)millisecondsTimeout, Interop.BOOL.TRUE);
240+
if (result != Interop.Kernel32.WAIT_IO_COMPLETION)
241+
{
242+
break;
243+
}
244+
// Check if this was our interrupt APC
245+
CheckForPendingInterrupt();
246+
// Handle APC completion by adjusting timeout and retrying
247+
long currentTime = Environment.TickCount64;
248+
long elapsed = currentTime - startTime;
249+
if (elapsed >= millisecondsTimeout)
250+
{
251+
result = Interop.Kernel32.WAIT_TIMEOUT;
252+
break;
253+
}
254+
millisecondsTimeout -= (int)elapsed;
255+
startTime = currentTime;
256+
}
257+
}
258+
currentThread.ClearWaitSleepJoinState();
259+
return result == (int)Interop.Kernel32.WAIT_OBJECT_0;
166260
}
167-
168-
return result == (int)Interop.Kernel32.WAIT_OBJECT_0;
169261
}
170262
finally
171263
{
@@ -212,6 +304,13 @@ private unsafe bool CreateThread(GCHandle<Thread> thisThreadHandle)
212304
// CoreCLR ignores OS errors while setting the priority, so do we
213305
SetPriorityLive(_priority);
214306

307+
// If the thread was interrupted before it was started, queue the interruption now
308+
if (GetThreadStateBit(Interrupted))
309+
{
310+
ClearThreadStateBit(Interrupted);
311+
Interrupt();
312+
}
313+
215314
Interop.Kernel32.ResumeThread(_osHandle);
216315
return true;
217316
}
@@ -393,7 +492,39 @@ internal static Thread EnsureThreadPoolThreadInitialized()
393492
return InitializeExistingThreadPoolThread();
394493
}
395494

396-
public void Interrupt() { throw new PlatformNotSupportedException(); }
495+
public void Interrupt()
496+
{
497+
using (_lock.EnterScope())
498+
{
499+
// Thread.Interrupt for dead thread should do nothing
500+
if (IsDead())
501+
{
502+
return;
503+
}
504+
505+
// Thread.Interrupt for thread that has not been started yet should queue a pending interrupt
506+
// for when we actually create the thread.
507+
if (_osHandle?.IsInvalid ?? true)
508+
{
509+
SetThreadStateBit(Interrupted);
510+
return;
511+
}
512+
513+
unsafe
514+
{
515+
Interop.Kernel32.QueueUserAPC(RuntimeImports.RhGetInterruptApcCallback(), _osHandle, 0);
516+
}
517+
}
518+
}
519+
520+
internal static void CheckForPendingInterrupt()
521+
{
522+
if (RuntimeImports.RhCheckAndClearPendingInterrupt())
523+
{
524+
CurrentThread.ClearWaitSleepJoinState();
525+
throw new ThreadInterruptedException();
526+
}
527+
}
397528

398529
internal static bool ReentrantWaitsEnabled =>
399530
GetCurrentApartmentType() == ApartmentType.STA;

‎src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ public sealed partial class Thread
1717
{
1818
// Extra bits used in _threadState
1919
private const ThreadState ThreadPoolThread = (ThreadState)0x1000;
20+
#if TARGET_WINDOWS
21+
private const ThreadState Interrupted = (ThreadState)0x2000;
22+
#endif
2023

2124
// Bits of _threadState that are returned by the ThreadState property
2225
private const ThreadState PublicThreadStateMask = (ThreadState)0x1FF;

‎src/libraries/Common/src/Interop/Windows/Kernel32/Interop.Threading.cs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,5 +78,12 @@ internal enum ThreadPriority : int
7878
[LibraryImport(Libraries.Kernel32, SetLastError = true)]
7979
[return: MarshalAs(UnmanagedType.Bool)]
8080
internal static partial bool GetThreadIOPendingFlag(nint hThread, out BOOL lpIOIsPending);
81+
82+
[LibraryImport(Libraries.Kernel32, SetLastError = true)]
83+
[return: MarshalAs(UnmanagedType.Bool)]
84+
internal static unsafe partial bool QueueUserAPC(delegate* unmanaged<nuint, void> pfnAPC, SafeHandle hThread, nuint dwData);
85+
86+
[LibraryImport(Libraries.Kernel32)]
87+
internal static partial uint SleepEx(uint dwMilliseconds, [MarshalAs(UnmanagedType.Bool)] bool bAlertable);
8188
}
8289
}

‎src/libraries/System.Private.CoreLib/src/System/Threading/Thread.Windows.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ public sealed partial class Thread
1818
{
1919
internal static void UninterruptibleSleep0() => Interop.Kernel32.Sleep(0);
2020

21-
#if !CORECLR
21+
#if MONO
2222
private static void SleepInternal(int millisecondsTimeout)
2323
{
2424
Debug.Assert(millisecondsTimeout >= -1);

‎src/libraries/System.Private.CoreLib/src/System/Threading/WaitHandle.Windows.cs‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,9 @@ private static unsafe int WaitForMultipleObjectsIgnoringSyncContext(IntPtr* pHan
5656
}
5757

5858
int result;
59+
60+
Thread.CheckForPendingInterrupt();
61+
5962
while (true)
6063
{
6164
#if NATIVEAOT
@@ -75,8 +78,10 @@ private static unsafe int WaitForMultipleObjectsIgnoringSyncContext(IntPtr* pHan
7578
if (result != Interop.Kernel32.WAIT_IO_COMPLETION)
7679
break;
7780

81+
Thread.CheckForPendingInterrupt();
82+
7883
// Handle APC completion by adjusting timeout and retrying
79-
if (millisecondsTimeout != -1)
84+
if (millisecondsTimeout != Timeout.Infinite)
8085
{
8186
long currentTime = Environment.TickCount64;
8287
long elapsed = currentTime - startTime;
@@ -89,6 +94,7 @@ private static unsafe int WaitForMultipleObjectsIgnoringSyncContext(IntPtr* pHan
8994
startTime = currentTime;
9095
}
9196
}
97+
9298
currentThread.ClearWaitSleepJoinState();
9399

94100
if (result == Interop.Kernel32.WAIT_FAILED)
@@ -134,12 +140,16 @@ private static int SignalAndWaitCore(IntPtr handleToSignal, IntPtr handleToWaitO
134140
startTime = Environment.TickCount64;
135141
}
136142

143+
Thread.CheckForPendingInterrupt();
144+
137145
// Signal the object and wait for the first time
138146
int ret = (int)Interop.Kernel32.SignalObjectAndWait(handleToSignal, handleToWaitOn, (uint)millisecondsTimeout, Interop.BOOL.TRUE);
139147

140148
// Handle APC completion by retrying with WaitForSingleObjectEx (without signaling again)
141149
while (ret == Interop.Kernel32.WAIT_IO_COMPLETION)
142150
{
151+
Thread.CheckForPendingInterrupt();
152+
143153
if (millisecondsTimeout != -1)
144154
{
145155
long currentTime = Environment.TickCount64;

‎src/libraries/System.Threading.Thread/tests/ThreadTests.cs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -749,7 +749,7 @@ public static void AbortSuspendTest()
749749
verify();
750750

751751
e.Set();
752-
waitForThread();
752+
waitForThread();
753753
}
754754

755755
private static void VerifyLocalDataSlot(LocalDataStoreSlot slot)
@@ -916,7 +916,6 @@ public static void LocalDataSlotTest()
916916

917917
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))]
918918
[ActiveIssue("https://github.com/dotnet/runtime/issues/49521", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)]
919-
[ActiveIssue("https://github.com/dotnet/runtime/issues/69919", typeof(PlatformDetection), nameof(PlatformDetection.IsNativeAot))]
920919
public static void InterruptTest()
921920
{
922921
// Interrupting a thread that is not blocked does not do anything, but once the thread starts blocking, it gets
@@ -966,7 +965,6 @@ public static void InterruptTest()
966965
}
967966

968967
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))]
969-
[ActiveIssue("https://github.com/dotnet/runtime/issues/69919", typeof(PlatformDetection), nameof(PlatformDetection.IsNativeAot))]
970968
[ActiveIssue("https://github.com/dotnet/runtime/issues/49521", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)]
971969
public static void InterruptInFinallyBlockTest_SkipOnDesktopFramework()
972970
{

0 commit comments

Comments
 (0)