From dd9246a46a5e8781257b7f62091e40b6dcb704df Mon Sep 17 00:00:00 2001 From: Koundinya Veluri Date: Tue, 31 Oct 2023 12:35:45 -0700 Subject: [PATCH 1/2] Fix class construction recursion issue in `Lock` on NativeAOT - The `Monitor` type was being constructed due to the use of `Monitor.DebugBlockingScope`, added that to the initialization phase - If necessary, an alternative may be to move `DebugBlockingScope` to be under `Lock`. Based on the comments the thread-static field is apparently bound-to in debugging scenarios. - Fixes https://github.com/dotnet/runtime/issues/94227 --- .../src/System/Threading/Lock.NativeAot.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Lock.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Lock.NativeAot.cs index 690014f91691f..dc41c1d51fb52 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Lock.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Lock.NativeAot.cs @@ -178,6 +178,7 @@ private static bool TryInitializeStatics() // Also initialize some types that are used later to prevent potential class construction cycles NativeRuntimeEventSource.Log.IsEnabled(); + using (new Monitor.DebugBlockingScope(null, Monitor.DebugBlockingItemType.MonitorCriticalSection, 0, out _)) { } } catch { From 5a499084ede56d67764212bd85ea3d883660b4a3 Mon Sep 17 00:00:00 2001 From: Koundinya Veluri Date: Tue, 31 Oct 2023 18:24:02 -0700 Subject: [PATCH 2/2] Remove `DebugBlockingScope` instead --- .../SynchronizedMethodHelpers.cs | 4 +- .../src/System/Threading/Lock.NativeAot.cs | 9 +-- .../src/System/Threading/Monitor.NativeAot.cs | 69 ++----------------- .../src/System/Threading/Lock.cs | 13 ---- 4 files changed, 8 insertions(+), 87 deletions(-) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/CompilerHelpers/SynchronizedMethodHelpers.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/CompilerHelpers/SynchronizedMethodHelpers.cs index 56701f033500b..d522c4231be22 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/CompilerHelpers/SynchronizedMethodHelpers.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/CompilerHelpers/SynchronizedMethodHelpers.cs @@ -26,7 +26,7 @@ private static void MonitorEnter(object obj, ref bool lockTaken) ObjectHeader.GetLockObject(obj) : SyncTable.GetLockObject(resultOrIndex); - lck.TryEnterSlow(Timeout.Infinite, currentThreadID, obj); + lck.TryEnterSlow(Timeout.Infinite, currentThreadID); lockTaken = true; } private static void MonitorExit(object obj, ref bool lockTaken) @@ -55,7 +55,7 @@ private static unsafe void MonitorEnterStatic(MethodTable* pMT, ref bool lockTak ObjectHeader.GetLockObject(obj) : SyncTable.GetLockObject(resultOrIndex); - lck.TryEnterSlow(Timeout.Infinite, currentThreadID, obj); + lck.TryEnterSlow(Timeout.Infinite, currentThreadID); lockTaken = true; } private static unsafe void MonitorExitStatic(MethodTable* pMT, ref bool lockTaken) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Lock.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Lock.NativeAot.cs index dc41c1d51fb52..492e186b95bfc 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Lock.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Lock.NativeAot.cs @@ -54,12 +54,8 @@ internal void Exit(int currentManagedThreadId) } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private ThreadId TryEnterSlow(int timeoutMs, ThreadId currentThreadId) => - TryEnterSlow(timeoutMs, currentThreadId, this); - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal bool TryEnterSlow(int timeoutMs, int currentManagedThreadId, object associatedObject) => - TryEnterSlow(timeoutMs, new ThreadId((uint)currentManagedThreadId), associatedObject).IsInitialized; + internal bool TryEnterSlow(int timeoutMs, int currentManagedThreadId) => + TryEnterSlow(timeoutMs, new ThreadId((uint)currentManagedThreadId)).IsInitialized; [MethodImpl(MethodImplOptions.AggressiveInlining)] internal bool GetIsHeldByCurrentThread(int currentManagedThreadId) @@ -178,7 +174,6 @@ private static bool TryInitializeStatics() // Also initialize some types that are used later to prevent potential class construction cycles NativeRuntimeEventSource.Log.IsEnabled(); - using (new Monitor.DebugBlockingScope(null, Monitor.DebugBlockingItemType.MonitorCriticalSection, 0, out _)) { } } catch { diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Monitor.NativeAot.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Monitor.NativeAot.cs index 89a8505663d30..66016246231fe 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Monitor.NativeAot.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Monitor.NativeAot.cs @@ -50,7 +50,7 @@ public static void Enter(object obj) ObjectHeader.GetLockObject(obj) : SyncTable.GetLockObject(resultOrIndex); - lck.TryEnterSlow(Timeout.Infinite, currentThreadID, obj); + lck.TryEnterSlow(Timeout.Infinite, currentThreadID); } [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -82,7 +82,7 @@ public static bool TryEnter(object obj) if (currentThreadID != 0 && lck.TryEnterOneShot(currentThreadID)) return true; - return lck.TryEnterSlow(0, currentThreadID, obj); + return lck.TryEnterSlow(0, currentThreadID); } [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -114,7 +114,7 @@ public static bool TryEnter(object obj, int millisecondsTimeout) if (millisecondsTimeout == 0 && currentThreadID != 0 && lck.TryEnterOneShot(currentThreadID)) return true; - return lck.TryEnterSlow(millisecondsTimeout, currentThreadID, obj); + return lck.TryEnterSlow(millisecondsTimeout, currentThreadID); } [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -146,12 +146,7 @@ public static bool IsEntered(object obj) [UnsupportedOSPlatform("browser")] public static bool Wait(object obj, int millisecondsTimeout) { - Condition condition = GetCondition(obj); - - using (new DebugBlockingScope(obj, DebugBlockingItemType.MonitorEvent, millisecondsTimeout, out _)) - { - return condition.Wait(millisecondsTimeout); - } + return GetCondition(obj).Wait(millisecondsTimeout); } public static void Pulse(object obj) @@ -170,62 +165,6 @@ public static void PulseAll(object obj) #endregion - #region Debugger support - - // The debugger binds to the fields below by name. Do not change any names or types without - // updating the debugger! - - // The head of the list of DebugBlockingItem stack objects used by the debugger to implement - // ICorDebugThread4::GetBlockingObjects. Usually the list either is empty or contains a single - // item. However, a wait on an STA thread may reenter via the message pump and cause the thread - // to be blocked on a second object. - [ThreadStatic] - private static IntPtr t_firstBlockingItem; - - // Different ways a thread can be blocked that the debugger will expose. - // Do not change or add members without updating the debugger code. - internal enum DebugBlockingItemType - { - MonitorCriticalSection = 0, - MonitorEvent = 1 - } - - // Represents an item a thread is blocked on. This structure is allocated on the stack and accessed by the debugger. - internal struct DebugBlockingItem - { - // The object the thread is waiting on - public object _object; - - // Indicates how the thread is blocked on the item - public DebugBlockingItemType _blockingType; - - // Blocking timeout in milliseconds or Timeout.Infinite for no timeout - public int _timeout; - - // Next pointer in the linked list of DebugBlockingItem records - public IntPtr _next; - } - - internal unsafe struct DebugBlockingScope : IDisposable - { - public DebugBlockingScope(object obj, DebugBlockingItemType blockingType, int timeout, out DebugBlockingItem blockingItem) - { - blockingItem._object = obj; - blockingItem._blockingType = blockingType; - blockingItem._timeout = timeout; - blockingItem._next = t_firstBlockingItem; - - t_firstBlockingItem = (IntPtr)Unsafe.AsPointer(ref blockingItem); - } - - public void Dispose() - { - t_firstBlockingItem = Unsafe.Read((void*)t_firstBlockingItem)._next; - } - } - - #endregion - #region Metrics /// diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs index 0d7380d48b5a1..fe794fdf9426e 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs @@ -290,11 +290,7 @@ private void ExitImpl() private static bool IsAdaptiveSpinEnabled(short minSpinCount) => minSpinCount <= 0; [MethodImpl(MethodImplOptions.NoInlining)] -#if !NATIVEAOT private ThreadId TryEnterSlow(int timeoutMs, ThreadId currentThreadId) -#else - private ThreadId TryEnterSlow(int timeoutMs, ThreadId currentThreadId, object associatedObject) -#endif { Debug.Assert(timeoutMs >= -1); @@ -462,15 +458,6 @@ private ThreadId TryEnterSlow(int timeoutMs, ThreadId currentThreadId, object as // exceptional paths. try { -#if NATIVEAOT - using var debugBlockingScope = - new Monitor.DebugBlockingScope( - associatedObject, - Monitor.DebugBlockingItemType.MonitorCriticalSection, - timeoutMs, - out _); -#endif - Interlocked.Increment(ref s_contentionCount); long waitStartTimeTicks = 0;