[NativeAOT] Fix assert in Thread.SpinWait(0) (dotnet#73033)

* do not inline long wait
careri · Jul 29, 2022 · 5331568 · 5331568
1 parent 9b63001
commit 5331568
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 1 deletion.
diff --git a/src/coreclr/nativeaot/Runtime/MiscHelpers.cpp b/src/coreclr/nativeaot/Runtime/MiscHelpers.cpp
@@ -48,6 +48,12 @@ COOP_PINVOKE_HELPER(void, RhDebugBreak, ())
 // Busy spin for the given number of iterations.
 EXTERN_C NATIVEAOT_API void __cdecl RhSpinWait(int32_t iterations)
 {
+    ASSERT(iterations > 0);
+
+    // limit the spin count in coop mode.
+    ASSERT_MSG(iterations <= 10000 || !ThreadStore::GetCurrentThread()->IsCurrentThreadInCooperativeMode(),
+        "This is too long wait for coop mode. You must p/invoke with GC transition.");
+
     YieldProcessorNormalizationInfo normalizationInfo;
     YieldProcessorNormalizedForPreSkylakeCount(normalizationInfo, iterations);
 }

diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs
@@ -388,6 +388,11 @@ internal static unsafe void RhUnbox(object? obj, ref byte data, EETypePtr pUnbox
         [UnmanagedCallConv(CallConvs = new Type[] { typeof(CallConvCdecl) })]
         internal static partial void RhSpinWait(int iterations);
 
+        // Call RhSpinWait with a GC transition
+        [LibraryImport(RuntimeLibrary, EntryPoint = "RhSpinWait")]
+        [UnmanagedCallConv(CallConvs = new Type[] { typeof(CallConvCdecl) })]
+        internal static partial void RhLongSpinWait(int iterations);
+
         // Yield the cpu to another thread ready to process, if one is available.
         [LibraryImport(RuntimeLibrary, EntryPoint = "RhYield")]
         [UnmanagedCallConv(CallConvs = new Type[] { typeof(CallConvCdecl) })]

diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.cs
@@ -322,7 +322,30 @@ public bool Join(int millisecondsTimeout)
         /// </summary>
         internal const int OptimalMaxSpinWaitsPerSpinIteration = 64;
 
-        public static void SpinWait(int iterations) => RuntimeImports.RhSpinWait(iterations);
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        private static void LongSpinWait(int iterations)
+        {
+            RuntimeImports.RhLongSpinWait(iterations);
+        }
+
+        public static void SpinWait(int iterations)
+        {
+            if (iterations <= 0)
+                return;
+
+            // Max iterations to be done in RhSpinWait.
+            // RhSpinWait does not switch GC modes and we want to avoid native spinning in coop mode for too long.
+            const int spinWaitCoopThreshold = 10000;
+
+            if (iterations > spinWaitCoopThreshold)
+            {
+                LongSpinWait(iterations);
+            }
+            else
+            {
+                RuntimeImports.RhSpinWait(iterations);
+            }
+        }
 
         [MethodImpl(MethodImplOptions.NoInlining)] // Slow path method. Make sure that the caller frame does not pay for PInvoke overhead.
         public static bool Yield() => RuntimeImports.RhYield();