Add an internal mode to Lock to have it use non-alertable waits (do…

…tnet#97227)

* Add an internal mode to `Lock` to have it use trivial (non-alertable) waits

- Added an internal constructor that enables the lock to use trivial waits
- Trivial waits are non-alertable waits that are not forwarded to `SynchronizationContext` wait overrides, are non-message-pumping waits, and are not interruptible
- Updated most of the uses of `Lock` in NativeAOT to use trivial waits
- Also simplified the fix in dotnet#94873 to avoid having to do the relevant null checks in various places along the wait path, by limiting the scope of the null checks to the initialization phase

Fixes dotnet#97105

src/coreclr/System.Private.CoreLib/src/System/Threading/WaitHandle.CoreCLR.cs

@@ -9,7 +9,7 @@ namespace System.Threading
     public abstract partial class WaitHandle
     {
         [MethodImpl(MethodImplOptions.InternalCall)]
-        private static extern int WaitOneCore(IntPtr waitHandle, int millisecondsTimeout);
+        private static extern int WaitOneCore(IntPtr waitHandle, int millisecondsTimeout, bool useTrivialWaits);
 
         private static unsafe int WaitMultipleIgnoringSyncContextCore(Span<IntPtr> waitHandles, bool waitAll, int millisecondsTimeout)
         {

src/coreclr/nativeaot/Common/src/System/Collections/Concurrent/ConcurrentUnifier.cs

@@ -65,7 +65,7 @@ internal abstract class ConcurrentUnifier<K, V>
     {
         protected ConcurrentUnifier()
         {
-            _lock = new Lock();
+            _lock = new Lock(useTrivialWaits: true);
             _container = new Container(this);
         }
 

src/coreclr/nativeaot/Common/src/System/Collections/Concurrent/ConcurrentUnifierW.cs

@@ -75,7 +75,7 @@ internal abstract class ConcurrentUnifierW<K, V>
     {
         protected ConcurrentUnifierW()
         {
-            _lock = new Lock();
+            _lock = new Lock(useTrivialWaits: true);
             _container = new Container(this);
         }
 

src/coreclr/nativeaot/Common/src/System/Collections/Concurrent/ConcurrentUnifierWKeyed.cs

@@ -84,7 +84,7 @@ internal abstract class ConcurrentUnifierWKeyed<K, V>
     {
         protected ConcurrentUnifierWKeyed()
         {
-            _lock = new Lock();
+            _lock = new Lock(useTrivialWaits: true);
             _container = new Container(this);
         }
 

src/coreclr/nativeaot/System.Private.CoreLib/src/CompatibilitySuppressions.xml

@@ -913,6 +913,10 @@
     <DiagnosticId>CP0002</DiagnosticId>
     <Target>M:System.Reflection.MethodBase.GetParametersAsSpan</Target>
   </Suppression>
+  <Suppression>
+    <DiagnosticId>CP0002</DiagnosticId>
+    <Target>M:System.Threading.Lock.#ctor(System.Boolean)</Target>
+  </Suppression>
   <Suppression>
     <DiagnosticId>CP0015</DiagnosticId>
     <Target>M:System.Diagnostics.Tracing.EventSource.Write``1(System.String,``0):[T:System.Diagnostics.CodeAnalysis.RequiresUnreferencedCodeAttribute]</Target>

src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/FrozenObjectHeapManager.cs

@@ -16,7 +16,7 @@ internal unsafe partial class FrozenObjectHeapManager
     {
         public static readonly FrozenObjectHeapManager Instance = new FrozenObjectHeapManager();
 
-        private readonly LowLevelLock m_Crst = new LowLevelLock();
+        private readonly Lock m_Crst = new Lock(useTrivialWaits: true);
         private FrozenObjectSegment m_CurrentSegment;
 
         // Default size to reserve for a frozen segment
@@ -34,9 +34,7 @@ internal unsafe partial class FrozenObjectHeapManager
         {
             HalfBakedObject* obj = null;
 
-            m_Crst.Acquire();
-
-            try
+            using (m_Crst.EnterScope())
             {
                 Debug.Assert(type != null);
                 // _ASSERT(FOH_COMMIT_SIZE >= MIN_OBJECT_SIZE);
@@ -84,10 +82,6 @@ internal unsafe partial class FrozenObjectHeapManager
                     Debug.Assert(obj != null);
                 }
             } // end of m_Crst lock
-            finally
-            {
-                m_Crst.Release();
-            }
 
             IntPtr result = (IntPtr)obj;
 

src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/CompilerServices/ClassConstructorRunner.cs

@@ -275,7 +275,7 @@ public static CctorHandle GetCctor(StaticClassConstructionContext* pContext)
 #if TARGET_WASM
                 if (s_cctorGlobalLock == null)
                 {
-                    Interlocked.CompareExchange(ref s_cctorGlobalLock, new Lock(), null);
+                    Interlocked.CompareExchange(ref s_cctorGlobalLock, new Lock(useTrivialWaits: true), null);
                 }
                 if (s_cctorArrays == null)
                 {
@@ -342,7 +342,7 @@ public static CctorHandle GetCctor(StaticClassConstructionContext* pContext)
 
                         Debug.Assert(resultArray[resultIndex]._pContext == default(StaticClassConstructionContext*));
                         resultArray[resultIndex]._pContext = pContext;
-                        resultArray[resultIndex].Lock = new Lock();
+                        resultArray[resultIndex].Lock = new Lock(useTrivialWaits: true);
                         s_count++;
                     }
 
@@ -489,7 +489,7 @@ public static CctorHandle GetCctorThatThreadIsBlockedOn(int managedThreadId)
         internal static void Initialize()
         {
             s_cctorArrays = new Cctor[10][];
-            s_cctorGlobalLock = new Lock();
+            s_cctorGlobalLock = new Lock(useTrivialWaits: true);
         }
 
         [Conditional("ENABLE_NOISY_CCTOR_LOG")]

src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/ComWrappers.NativeAot.cs

@@ -44,7 +44,7 @@ public abstract partial class ComWrappers
         private static readonly List<GCHandle> s_referenceTrackerNativeObjectWrapperCache = new List<GCHandle>();
 
         private readonly ConditionalWeakTable<object, ManagedObjectWrapperHolder> _ccwTable = new ConditionalWeakTable<object, ManagedObjectWrapperHolder>();
-        private readonly Lock _lock = new Lock();
+        private readonly Lock _lock = new Lock(useTrivialWaits: true);
         private readonly Dictionary<IntPtr, GCHandle> _rcwCache = new Dictionary<IntPtr, GCHandle>();
 
         internal static bool TryGetComInstanceForIID(object obj, Guid iid, out IntPtr unknown, out long wrapperId)

src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Condition.cs

@@ -114,6 +114,7 @@ public unsafe bool Wait(int millisecondsTimeout, object? associatedObjectForMoni
                 success =
                     waiter.ev.WaitOneNoCheck(
                         millisecondsTimeout,
+                        false, // useTrivialWaits
                         associatedObjectForMonitorWait,
                         associatedObjectForMonitorWait != null
                             ? NativeRuntimeEventSource.WaitHandleWaitSourceMap.MonitorWait

src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Lock.NativeAot.cs

@@ -92,6 +92,18 @@ internal void Reenter(uint previousRecursionCount)
             _recursionCount = previousRecursionCount;
         }
 
+        private static bool IsFullyInitialized
+        {
+            get
+            {
+                // If NativeRuntimeEventSource is already being class-constructed by this thread earlier in the stack, Log can
+                // be null. This property is used to avoid going down the wait path in that case to avoid null checks in several
+                // other places.
+                Debug.Assert((StaticsInitializationStage)s_staticsInitializationStage == StaticsInitializationStage.Complete);
+                return NativeRuntimeEventSource.Log != null;
+            }
+        }
+
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private TryLockResult LazyInitializeOrEnter()
         {
@@ -101,6 +113,10 @@ private TryLockResult LazyInitializeOrEnter()
                 case StaticsInitializationStage.Complete:
                     if (_spinCount == SpinCountNotInitialized)
                     {
+                        if (!IsFullyInitialized)
+                        {
+                            goto case StaticsInitializationStage.Started;
+                        }
                         _spinCount = s_maxSpinCount;
                     }
                     return TryLockResult.Spin;
@@ -121,7 +137,7 @@ private TryLockResult LazyInitializeOrEnter()
                         }
 
                         stage = (StaticsInitializationStage)Volatile.Read(ref s_staticsInitializationStage);
-                        if (stage == StaticsInitializationStage.Complete)
+                        if (stage == StaticsInitializationStage.Complete && IsFullyInitialized)
                         {
                             goto case StaticsInitializationStage.Complete;
                         }
@@ -166,14 +182,17 @@ private static bool TryInitializeStatics()
                     return true;
             }
 
+            bool isFullyInitialized;
             try
             {
                 s_isSingleProcessor = Environment.IsSingleProcessor;
                 s_maxSpinCount = DetermineMaxSpinCount();
                 s_minSpinCount = DetermineMinSpinCount();
 
-                // Also initialize some types that are used later to prevent potential class construction cycles
-                _ = NativeRuntimeEventSource.Log;
+                // Also initialize some types that are used later to prevent potential class construction cycles. If
+                // NativeRuntimeEventSource is already being class-constructed by this thread earlier in the stack, Log can be
+                // null. Avoid going down the wait path in that case to avoid null checks in several other places.
+                isFullyInitialized = NativeRuntimeEventSource.Log != null;
             }
             catch
             {
@@ -182,7 +201,7 @@ private static bool TryInitializeStatics()
             }
 
             Volatile.Write(ref s_staticsInitializationStage, (int)StaticsInitializationStage.Complete);
-            return true;
+            return isFullyInitialized;
         }
 
         // Returns false until the static variable is lazy-initialized

src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/SyncTable.cs

@@ -65,7 +65,7 @@ internal static class SyncTable
         /// <summary>
         /// Protects all mutable operations on s_entries, s_freeEntryList, s_unusedEntryIndex. Also protects growing the table.
         /// </summary>
-        internal static readonly Lock s_lock = new Lock();
+        internal static readonly Lock s_lock = new Lock(useTrivialWaits: true);
 
         /// <summary>
         /// The dynamically growing array of sync entries.

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

@@ -167,7 +167,7 @@ private bool JoinInternal(int millisecondsTimeout)
                 }
                 else
                 {
-                    result = WaitHandle.WaitOneCore(waitHandle.DangerousGetHandle(), millisecondsTimeout);
+                    result = WaitHandle.WaitOneCore(waitHandle.DangerousGetHandle(), millisecondsTimeout, useTrivialWaits: false);
                 }
 
                 return result == (int)Interop.Kernel32.WAIT_OBJECT_0;

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

@@ -31,7 +31,7 @@ public sealed partial class Thread
         private Exception? _startException;
 
         // Protects starting the thread and setting its priority
-        private Lock _lock = new Lock();
+        private Lock _lock = new Lock(useTrivialWaits: true);
 
         // This is used for a quick check on thread pool threads after running a work item to determine if the name, background
         // state, or priority were changed by the work item, and if so to reset it. Other threads may also change some of those,

src/coreclr/nativeaot/System.Private.CoreLib/src/System/Type.NativeAot.cs

@@ -31,15 +31,14 @@ internal static unsafe RuntimeType GetTypeFromMethodTable(MethodTable* pMT)
 
         private static class AllocationLockHolder
         {
-            public static LowLevelLock AllocationLock = new LowLevelLock();
+            public static Lock AllocationLock = new Lock(useTrivialWaits: true);
         }
 
         [MethodImpl(MethodImplOptions.NoInlining)]
         private static unsafe RuntimeType GetTypeFromMethodTableSlow(MethodTable* pMT)
         {
             // Allocate and set the RuntimeType under a lock - there's no way to free it if there is a race.
-            AllocationLockHolder.AllocationLock.Acquire();
-            try
+            using (AllocationLockHolder.AllocationLock.EnterScope())
             {
                 ref RuntimeType? runtimeTypeCache = ref Unsafe.AsRef<RuntimeType?>(pMT->WritableData);
                 if (runtimeTypeCache != null)
@@ -55,10 +54,6 @@ private static unsafe RuntimeType GetTypeFromMethodTableSlow(MethodTable* pMT)
 
                 return type;
             }
-            finally
-            {
-                AllocationLockHolder.AllocationLock.Release();
-            }
         }
 
         //

src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeLoaderEnvironment.ConstructedGenericsRegistration.cs

@@ -24,7 +24,7 @@ internal struct DynamicGenericsRegistrationData
         }
 
         // To keep the synchronization simple, we execute all dynamic generic type registration/lookups under a global lock
-        private Lock _dynamicGenericsLock = new Lock();
+        private Lock _dynamicGenericsLock = new Lock(useTrivialWaits: true);
 
         internal void RegisterDynamicGenericTypesAndMethods(DynamicGenericsRegistrationData registrationData)
         {

src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeLoaderEnvironment.StaticsLookup.cs

@@ -15,7 +15,7 @@ namespace Internal.Runtime.TypeLoader
     public sealed partial class TypeLoaderEnvironment
     {
         // To keep the synchronization simple, we execute all TLS registration/lookups under a global lock
-        private Lock _threadStaticsLock = new Lock();
+        private Lock _threadStaticsLock = new Lock(useTrivialWaits: true);
 
         // Counter to keep track of generated offsets for TLS cells of dynamic types;
         private LowLevelDictionary<IntPtr, uint> _maxThreadLocalIndex = new LowLevelDictionary<IntPtr, uint>();

src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeLoaderEnvironment.cs

@@ -145,7 +145,7 @@ internal static void Initialize()
         }
 
         // To keep the synchronization simple, we execute all type loading under a global lock
-        private Lock _typeLoaderLock = new Lock();
+        private Lock _typeLoaderLock = new Lock(useTrivialWaits: true);
 
         public void VerifyTypeLoaderLockHeld()
         {

src/coreclr/nativeaot/System.Private.TypeLoader/src/Internal/Runtime/TypeLoader/TypeSystemContextFactory.cs

@@ -18,7 +18,7 @@ public static class TypeSystemContextFactory
         // This allows us to avoid recreating the type resolution context again and again, but still allows it to go away once the types are no longer being built
         private static GCHandle s_cachedContext = GCHandle.Alloc(null, GCHandleType.Weak);
 
-        private static Lock s_lock = new Lock();
+        private static Lock s_lock = new Lock(useTrivialWaits: true);
 
         public static TypeSystemContext Create()
         {

src/coreclr/vm/comwaithandle.cpp

@@ -16,7 +16,7 @@
 #include "excep.h"
 #include "comwaithandle.h"
 
-FCIMPL2(INT32, WaitHandleNative::CorWaitOneNative, HANDLE handle, INT32 timeout)
+FCIMPL3(INT32, WaitHandleNative::CorWaitOneNative, HANDLE handle, INT32 timeout, CLR_BOOL useTrivialWaits)
 {
     FCALL_CONTRACT;
 
@@ -28,7 +28,8 @@ FCIMPL2(INT32, WaitHandleNative::CorWaitOneNative, HANDLE handle, INT32 timeout)
 
     Thread* pThread = GET_THREAD();
 
-    retVal = pThread->DoAppropriateWait(1, &handle, TRUE, timeout, (WaitMode)(WaitMode_Alertable | WaitMode_IgnoreSyncCtx));
+    WaitMode waitMode = (WaitMode)((!useTrivialWaits ? WaitMode_Alertable : WaitMode_None) | WaitMode_IgnoreSyncCtx);
+    retVal = pThread->DoAppropriateWait(1, &handle, TRUE, timeout, waitMode);
 
     HELPER_METHOD_FRAME_END();
     return retVal;

src/coreclr/vm/comwaithandle.h

@@ -18,7 +18,7 @@
 class WaitHandleNative
 {
 public:
-    static FCDECL2(INT32, CorWaitOneNative, HANDLE handle, INT32 timeout);
+    static FCDECL3(INT32, CorWaitOneNative, HANDLE handle, INT32 timeout, CLR_BOOL useTrivialWaits);
     static FCDECL4(INT32, CorWaitMultipleNative, HANDLE *handleArray, INT32 numHandles, CLR_BOOL waitForAll, INT32 timeout);
     static FCDECL3(INT32, CorSignalAndWaitOneNative, HANDLE waitHandleSignalUNSAFE, HANDLE waitHandleWaitUNSAFE, INT32 timeout);
 };