Skip to content

Commit

Permalink
Fix class construction recursion issue in Lock on NativeAOT (#94241)
Browse files Browse the repository at this point in the history
* 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 #94227

* Remove `DebugBlockingScope` instead
  • Loading branch information
kouvel authored Nov 1, 2023
1 parent c88b783 commit 872b1fd
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 86 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,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)
Expand Down Expand Up @@ -59,7 +59,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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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)
Expand All @@ -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<DebugBlockingItem>((void*)t_firstBlockingItem)._next;
}
}

#endregion

#region Metrics

/// <summary>
Expand Down
13 changes: 0 additions & 13 deletions src/libraries/System.Private.CoreLib/src/System/Threading/Lock.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 872b1fd

Please sign in to comment.