[NativeAOT] Use QueueUserAPC2 in GC suspension, if available. (#80087)

* QueueUserAPC2 * fixes, do not stack APCs * adjust for newer SDK * force a failure - to see if lab machines have support for this * cleanup some includes * FEATURE_SUSPEND_APC2 * remove intentional failure * one more case of FEATURE_SUSPEND_APC2 * comment * fail if QUEUE_USER_APC_CALLBACK_DATA_CONTEXT supported * Apply suggestions from code review Co-authored-by: Jan Kotas <jkotas@microsoft.com> * FEATURE_SUSPEND_APC2 -> FEATURE_SPECIAL_USER_MODE_APC * throw - to make sure this fails on Release builds * un-break the feature * fix for a stress issue. * check for a failure to queue an APC. * use the same activation optimization on Unix * PR feedback * unmark ActivationPending if APC failed to queue. * a bit more robust against sharing the signal with something else. * initialize QueueUserAPC2 on demand * Check for STATUS_INVALID_PARAMETER * print the error * STATUS_INVALID_PARAMETER -> ERROR_INVALID_PARAMETER Co-authored-by: Jan Kotas <jkotas@microsoft.com>
dotnet · Jan 6, 2023 · 7a6f33b · 7a6f33b
1 parent afc963e
commit 7a6f33b
Show file tree

Hide file tree

Showing 7 changed files with 173 additions and 23 deletions.
diff --git a/src/coreclr/nativeaot/Runtime/CMakeLists.txt b/src/coreclr/nativeaot/Runtime/CMakeLists.txt
@@ -229,6 +229,7 @@ if(WIN32)
   add_definitions(-DFEATURE_ETW)
   add_definitions(-DFEATURE_EVENT_TRACE)
   add_definitions(-DFEATURE_SUSPEND_REDIRECTION)
+  add_definitions(-DFEATURE_SPECIAL_USER_MODE_APC)
 else()
   add_definitions(-DFEATURE_READONLY_GS_COOKIE)
   add_definitions(-DNO_UI_ASSERT)

diff --git a/src/coreclr/nativeaot/Runtime/StackFrameIterator.h b/src/coreclr/nativeaot/Runtime/StackFrameIterator.h
@@ -1,5 +1,9 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
+
+#ifndef __StackFrameIterator_h__
+#define __StackFrameIterator_h__
+
 #include "ICodeManager.h"
 
 struct ExInfo;
@@ -215,3 +219,4 @@ class StackFrameIterator
     PTR_VOID            m_OriginalControlPC;
 };
 
+#endif // __StackFrameIterator_h__
diff --git a/src/coreclr/nativeaot/Runtime/regdisplay.h b/src/coreclr/nativeaot/Runtime/regdisplay.h
@@ -1,6 +1,9 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+#ifndef __regdisplay_h__
+#define __regdisplay_h__
+
 #if defined(TARGET_X86) || defined(TARGET_AMD64)
 
 struct REGDISPLAY
@@ -160,3 +163,5 @@ struct REGDISPLAY
 #endif // HOST_X86 || HOST_AMD64 || HOST_ARM || HOST_ARM64 || HOST_WASM
 
 typedef REGDISPLAY * PREGDISPLAY;
+
+#endif //__regdisplay_h__
diff --git a/src/coreclr/nativeaot/Runtime/thread.cpp b/src/coreclr/nativeaot/Runtime/thread.cpp
@@ -628,10 +628,20 @@ void Thread::HijackCallback(NATIVE_CONTEXT* pThreadContext, void* pThreadToHijac
     Thread* pThread = (Thread*) pThreadToHijack;
     if (pThread == NULL)
     {
-        pThread = ThreadStore::GetCurrentThread();
+        pThread = ThreadStore::GetCurrentThreadIfAvailable();
+        if (pThread == NULL)
+        {
+            ASSERT(!"a not attached thread got signaled");
+            // perhaps we share the signal with something else?
+            return;
+        }
 
-        ASSERT(pThread != NULL);
-        ASSERT(pThread != ThreadStore::GetSuspendingThread());
+        if (pThread == ThreadStore::GetSuspendingThread())
+        {
+            ASSERT(!"trying to suspend suspending thread");
+            // perhaps we share the signal with something else?
+            return;
+        }
     }
 
     // we have a thread stopped, and we do not know where exactly.
@@ -1074,6 +1084,23 @@ void Thread::SetDetached()
     SetState(TSF_Detached);
 }
 
+bool Thread::IsActivationPending()
+{
+    return IsStateSet(TSF_ActivationPending);
+}
+
+void Thread::SetActivationPending(bool isPending)
+{
+    if (isPending)
+    {
+        SetState(TSF_ActivationPending);
+    }
+    else
+    {
+        ClearState(TSF_ActivationPending);
+    }
+}
+
 #endif // !DACCESS_COMPILE
 
 void Thread::ValidateExInfoStack()

diff --git a/src/coreclr/nativeaot/Runtime/thread.h b/src/coreclr/nativeaot/Runtime/thread.h
@@ -1,5 +1,12 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
+
+#ifndef __thread_h__
+#define __thread_h__
+
+#include "regdisplay.h"
+#include "StackFrameIterator.h"
+
 #include "forward_declarations.h"
 
 struct gc_alloc_context;
@@ -131,6 +138,14 @@ class Thread : private ThreadBuffer
                                                     // suspend once resumed.
                                                     // If we see this flag, we skip hijacking as an optimization.
 #endif //FEATURE_SUSPEND_REDIRECTION
+
+        TSF_ActivationPending   = 0x00000100,       // An APC with QUEUE_USER_APC_FLAGS_SPECIAL_USER_APC can interrupt another APC.
+                                                    // For suspension APCs it is mostly harmless, but wasteful and in extreme
+                                                    // cases may force the target thread into stack oveflow.
+                                                    // We use this flag to avoid sending another APC when one is still going through.
+                                                    // 
+                                                    // On Unix this is an optimization to not queue up more signals when one is
+                                                    // still being processed.
     };
 private:
 
@@ -282,6 +297,9 @@ class Thread : private ThreadBuffer
 #ifdef FEATURE_SUSPEND_REDIRECTION
     NATIVE_CONTEXT* EnsureRedirectionContext();
 #endif //FEATURE_SUSPEND_REDIRECTION
+
+    bool                IsActivationPending();
+    void                SetActivationPending(bool isPending);
 };
 
 #ifndef __GCENV_BASE_INCLUDED__
@@ -321,3 +339,5 @@ typedef promote_func EnumGcRefCallbackFunc;
 typedef ScanContext  EnumGcRefScanContext;
 
 #endif // DACCESS_COMPILE
+
+#endif // __thread_h__
diff --git a/src/coreclr/nativeaot/Runtime/unix/PalRedhawkUnix.cpp b/src/coreclr/nativeaot/Runtime/unix/PalRedhawkUnix.cpp
@@ -21,6 +21,8 @@
 #include "HardwareExceptions.h"
 #include "cgroupcpu.h"
 #include "threadstore.h"
+#include "thread.h"
+#include "threadstore.inl"
 
 #define _T(s) s
 #include "RhConfig.h"
@@ -966,22 +968,26 @@ static void ActivationHandler(int code, siginfo_t* siginfo, void* context)
         g_pHijackCallback((NATIVE_CONTEXT*)context, NULL);
         errno = savedErrNo;
     }
+
+    Thread* pThread = ThreadStore::GetCurrentThreadIfAvailable();
+    if (pThread)
+    {
+        pThread->SetActivationPending(false);
+    }
+
+    // Call the original handler when it is not ignored or default (terminate).
+    if (g_previousActivationHandler.sa_flags & SA_SIGINFO)
+    {
+        _ASSERTE(g_previousActivationHandler.sa_sigaction != NULL);
+        g_previousActivationHandler.sa_sigaction(code, siginfo, context);
+    }
     else
     {
-        // Call the original handler when it is not ignored or default (terminate).
-        if (g_previousActivationHandler.sa_flags & SA_SIGINFO)
-        {
-            _ASSERTE(g_previousActivationHandler.sa_sigaction != NULL);
-            g_previousActivationHandler.sa_sigaction(code, siginfo, context);
-        }
-        else
+        if (g_previousActivationHandler.sa_handler != SIG_IGN &&
+            g_previousActivationHandler.sa_handler != SIG_DFL)
         {
-            if (g_previousActivationHandler.sa_handler != SIG_IGN &&
-                g_previousActivationHandler.sa_handler != SIG_DFL)
-            {
-                _ASSERTE(g_previousActivationHandler.sa_handler != NULL);
-                g_previousActivationHandler.sa_handler(code);
-            }
+            _ASSERTE(g_previousActivationHandler.sa_handler != NULL);
+            g_previousActivationHandler.sa_handler(code);
         }
     }
 }
@@ -997,20 +1003,29 @@ REDHAWK_PALEXPORT UInt32_BOOL REDHAWK_PALAPI PalRegisterHijackCallback(_In_ PalH
 REDHAWK_PALEXPORT void REDHAWK_PALAPI PalHijack(HANDLE hThread, _In_opt_ void* pThreadToHijack)
 {
     ThreadUnixHandle* threadHandle = (ThreadUnixHandle*)hThread;
+    Thread* pThread = (Thread*)pThreadToHijack;
+    pThread->SetActivationPending(true);
+
     int status = pthread_kill(*threadHandle->GetObject(), INJECT_ACTIVATION_SIGNAL);
+
     // We can get EAGAIN when printing stack overflow stack trace and when other threads hit
     // stack overflow too. Those are held in the sigsegv_handler with blocked signals until
     // the process exits.
-
+    // ESRCH may happen on some OSes when the thread is exiting.
+    // The thread should leave cooperative mode, but we could have seen it in its earlier state.
+    if ((status == EAGAIN)
+     || (status == ESRCH)
 #ifdef __APPLE__
-    // On Apple, pthread_kill is not allowed to be sent to dispatch queue threads
-    if (status == ENOTSUP)
+        // On Apple, pthread_kill is not allowed to be sent to dispatch queue threads
+     || (status == ENOTSUP)
+#endif
+       )
     {
+        pThread->SetActivationPending(false);
         return;
     }
-#endif
 
-    if ((status != 0) && (status != EAGAIN) && (status != ESRCH))
+    if (status != 0)
     {
         // Failure to send the signal is fatal. There are only two cases when sending
         // the signal can fail. First, if the signal ID is invalid and second,

diff --git a/src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp b/src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp
@@ -33,6 +33,7 @@ uint32_t PalEventWrite(REGHANDLE arg1, const EVENT_DESCRIPTOR * arg2, uint32_t a
 #include "gcenv.ee.h"
 #include "gcconfig.h"
 
+#include "thread.h"
 
 #define REDHAWK_PALEXPORT extern "C"
 #define REDHAWK_PALAPI __stdcall
@@ -56,6 +57,11 @@ void __stdcall FiberDetachCallback(void* lpFlsData)
     }
 }
 
+static HMODULE LoadKernel32dll()
+{
+    return LoadLibraryExW(L"kernel32", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
+}
+
 void InitializeCurrentProcessCpuCount()
 {
     DWORD count;
@@ -442,6 +448,26 @@ REDHAWK_PALEXPORT void REDHAWK_PALAPI PalRestoreContext(CONTEXT * pCtx)
 
 static PalHijackCallback g_pHijackCallback;
 
+#ifdef FEATURE_SPECIAL_USER_MODE_APC
+typedef BOOL (WINAPI* QueueUserAPC2Proc)(PAPCFUNC ApcRoutine, HANDLE Thread, ULONG_PTR Data, QUEUE_USER_APC_FLAGS Flags);
+
+#define QUEUE_USER_APC2_UNINITIALIZED (QueueUserAPC2Proc)-1
+static QueueUserAPC2Proc g_pfnQueueUserAPC2Proc = QUEUE_USER_APC2_UNINITIALIZED;
+
+static const QUEUE_USER_APC_FLAGS SpecialUserModeApcWithContextFlags = (QUEUE_USER_APC_FLAGS)
+                                    (QUEUE_USER_APC_FLAGS_SPECIAL_USER_APC |
+                                    QUEUE_USER_APC_CALLBACK_DATA_CONTEXT);
+
+static void NTAPI ActivationHandler(ULONG_PTR parameter)
+{
+    APC_CALLBACK_DATA* data = (APC_CALLBACK_DATA*)parameter;
+    g_pHijackCallback(data->ContextRecord, NULL);
+
+    Thread* pThread = (Thread*)data->Parameter;
+    pThread->SetActivationPending(false);
+}
+#endif
+
 REDHAWK_PALEXPORT UInt32_BOOL REDHAWK_PALAPI PalRegisterHijackCallback(_In_ PalHijackCallback callback)
 {
     ASSERT(g_pHijackCallback == NULL);
@@ -453,6 +479,57 @@ REDHAWK_PALEXPORT UInt32_BOOL REDHAWK_PALAPI PalRegisterHijackCallback(_In_ PalH
 REDHAWK_PALEXPORT void REDHAWK_PALAPI PalHijack(HANDLE hThread, _In_opt_ void* pThreadToHijack)
 {
     _ASSERTE(hThread != INVALID_HANDLE_VALUE);
+
+#ifdef FEATURE_SPECIAL_USER_MODE_APC
+    // initialize g_pfnQueueUserAPC2Proc on demand.
+    // Note that only one thread at a time may perform suspension (guaranteed by the thread store lock)
+    // so simple conditional assignment is ok.
+    if (g_pfnQueueUserAPC2Proc == QUEUE_USER_APC2_UNINITIALIZED)
+    {
+        g_pfnQueueUserAPC2Proc = (QueueUserAPC2Proc)GetProcAddress(LoadKernel32dll(), "QueueUserAPC2");
+    }
+
+    if (g_pfnQueueUserAPC2Proc)
+    {
+        Thread* pThread = (Thread*)pThreadToHijack;
+
+        // An APC can be interrupted by another one, do not queue more if one is pending.
+        if (pThread->IsActivationPending())
+        {
+            return;
+        }
+
+        pThread->SetActivationPending(true);
+        BOOL success = g_pfnQueueUserAPC2Proc(
+            &ActivationHandler,
+            hThread,
+            (ULONG_PTR)pThreadToHijack,
+            SpecialUserModeApcWithContextFlags);
+
+        if (success)
+        {
+            return;
+        }
+
+        // queuing an APC failed
+        pThread->SetActivationPending(false);
+
+        DWORD lastError = GetLastError();
+        if (lastError != ERROR_INVALID_PARAMETER)
+        {
+            // An unexpected failure has happened. It is a concern.
+            ASSERT_UNCONDITIONALLY("Failed to queue an APC for unusual reason.");
+
+            // maybe it will work next time.
+            return;
+        }
+
+        // the flags that we passed are not supported.
+        // we will not try again
+        g_pfnQueueUserAPC2Proc = NULL;
+    }
+#endif
+
     if (SuspendThread(hThread) == (DWORD)-1)
     {
         return;
@@ -546,7 +623,7 @@ REDHAWK_PALEXPORT bool REDHAWK_PALAPI PalIsAvxEnabled()
     typedef DWORD64(WINAPI* PGETENABLEDXSTATEFEATURES)();
     PGETENABLEDXSTATEFEATURES pfnGetEnabledXStateFeatures = NULL;
 
-    HMODULE hMod = LoadLibraryExW(L"kernel32", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
+    HMODULE hMod = LoadKernel32dll();
     if (hMod == NULL)
         return FALSE;
 
@@ -571,7 +648,7 @@ REDHAWK_PALEXPORT bool REDHAWK_PALAPI PalIsAvx512Enabled()
     typedef DWORD64(WINAPI* PGETENABLEDXSTATEFEATURES)();
     PGETENABLEDXSTATEFEATURES pfnGetEnabledXStateFeatures = NULL;
 
-    HMODULE hMod = LoadLibraryExW(L"kernel32", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);
+    HMODULE hMod = LoadKernel32dll();
     if (hMod == NULL)
         return FALSE;