Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add managed blocking info for lock and monitor waits #101192

Merged
merged 3 commits into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ private static Waiter GetWaiterForCurrentThread()
private Waiter? _waitersHead;
private Waiter? _waitersTail;

internal Lock AssociatedLock => _lock;

private unsafe void AssertIsInList(Waiter waiter)
{
Debug.Assert(_waitersHead != null && _waitersTail != null);
Expand Down Expand Up @@ -106,6 +108,8 @@ public unsafe bool Wait(int millisecondsTimeout, object? associatedObjectForMoni
if (!_lock.IsHeldByCurrentThread)
throw new SynchronizationLockException();

using ThreadBlockingInfo.Scope threadBlockingScope = new(this, millisecondsTimeout);

Waiter waiter = GetWaiterForCurrentThread();
AddWaiter(waiter);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ public sealed partial class Lock
/// </summary>
public Lock() => _spinCount = SpinCountNotInitialized;

#pragma warning disable CA1822 // can be marked as static - varies between runtimes
internal ulong OwningOSThreadId => 0;
#pragma warning restore CA1822

internal int OwningManagedThreadId => (int)_owningThreadId;

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal bool TryEnterOneShot(int currentManagedThreadId)
{
Expand Down
7 changes: 7 additions & 0 deletions src/coreclr/vm/appdomain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1158,6 +1158,13 @@ void SystemDomain::Init()

// Finish loading CoreLib now.
m_pSystemAssembly->GetDomainAssembly()->EnsureActive();

// Set AwareLock's offset of the holding OS thread ID field into ThreadBlockingInfo's static field. That can be used
// when doing managed debugging to get the OS ID of the thread holding the lock. The offset is currently not zero, and
// zero is used in managed code to determine if the static variable has been initialized.
_ASSERTE(AwareLock::GetOffsetOfHoldingOSThreadId() != 0);
CoreLibBinder::GetField(FIELD__THREAD_BLOCKING_INFO__OFFSET_OF_LOCK_OWNER_OS_THREAD_ID)
->SetStaticValue32(AwareLock::GetOffsetOfHoldingOSThreadId());
}

#ifdef _DEBUG
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/vm/corelib.h
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,10 @@ END_ILLINK_FEATURE_SWITCH()
DEFINE_CLASS(MONITOR, Threading, Monitor)
DEFINE_METHOD(MONITOR, ENTER, Enter, SM_Obj_RetVoid)

DEFINE_CLASS(THREAD_BLOCKING_INFO, Threading, ThreadBlockingInfo)
DEFINE_FIELD(THREAD_BLOCKING_INFO, OFFSET_OF_LOCK_OWNER_OS_THREAD_ID, s_monitorObjectOffsetOfLockOwnerOSThreadId)
DEFINE_FIELD(THREAD_BLOCKING_INFO, FIRST, t_first)

DEFINE_CLASS(PARAMETER, Reflection, ParameterInfo)

DEFINE_CLASS(PARAMETER_MODIFIER, Reflection, ParameterModifier)
Expand Down
6 changes: 6 additions & 0 deletions src/coreclr/vm/syncblk.h
Original file line number Diff line number Diff line change
Expand Up @@ -602,6 +602,12 @@ class AwareLock
LIMITED_METHOD_CONTRACT;
return m_HoldingThread;
}

static int GetOffsetOfHoldingOSThreadId()
{
LIMITED_METHOD_CONTRACT;
return (int)offsetof(AwareLock, m_HoldingOSThreadId);
}
};

#ifdef FEATURE_COMINTEROP
Expand Down
43 changes: 41 additions & 2 deletions src/coreclr/vm/threaddebugblockinginfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,37 @@ VOID ThreadDebugBlockingInfo::VisitBlockingItems(DebugBlockingItemVisitor visito
// Holder constructor pushes a blocking item on the blocking info stack
#ifndef DACCESS_COMPILE
DebugBlockingItemHolder::DebugBlockingItemHolder(Thread *pThread, DebugBlockingItem *pItem) :
m_pThread(pThread)
m_pThread(pThread), m_ppFirstBlockingInfo(nullptr)
{
LIMITED_METHOD_CONTRACT;
CONTRACTL
{
THROWS;
GC_TRIGGERS;
MODE_COOPERATIVE;
}
CONTRACTL_END;

// Try to get the address of the thread-local slot for the managed ThreadBlockingInfo.t_first
EX_TRY
{
FieldDesc *pFD = CoreLibBinder::GetField(FIELD__THREAD_BLOCKING_INFO__FIRST);
m_ppFirstBlockingInfo = (ThreadBlockingInfo **)Thread::GetStaticFieldAddress(pFD);
}
EX_CATCH
{
}
EX_END_CATCH(RethrowTerminalExceptions);

if (m_ppFirstBlockingInfo != nullptr)
{
// Push info for the managed ThreadBlockingInfo
m_blockingInfo.objectPtr = pItem->pMonitor;
m_blockingInfo.objectKind = (ThreadBlockingInfo::ObjectKind)pItem->type;
m_blockingInfo.timeoutMs = (INT32)pItem->dwTimeout;
m_blockingInfo.next = *m_ppFirstBlockingInfo;
*m_ppFirstBlockingInfo = &m_blockingInfo;
}

pThread->DebugBlockingInfo.PushBlockingItem(pItem);
}
#endif //DACCESS_COMPILE
Expand All @@ -84,6 +112,17 @@ m_pThread(pThread)
DebugBlockingItemHolder::~DebugBlockingItemHolder()
{
LIMITED_METHOD_CONTRACT;

m_pThread->DebugBlockingInfo.PopBlockingItem();

if (m_ppFirstBlockingInfo != nullptr)
{
// Pop info for the managed ThreadBlockingInfo
_ASSERTE(
m_ppFirstBlockingInfo ==
(void *)m_pThread->GetStaticFieldAddrNoCreate(CoreLibBinder::GetField(FIELD__THREAD_BLOCKING_INFO__FIRST)));
_ASSERTE(*m_ppFirstBlockingInfo == &m_blockingInfo);
*m_ppFirstBlockingInfo = m_blockingInfo.next;
}
}
#endif //DACCESS_COMPILE
24 changes: 22 additions & 2 deletions src/coreclr/vm/threaddebugblockinginfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
// Different ways thread can block that the debugger will expose
enum DebugBlockingItemType
{
DebugBlock_MonitorCriticalSection,
DebugBlock_MonitorEvent,
DebugBlock_MonitorCriticalSection, // maps to ThreadBlockingInfo.ObjectKind.MonitorLock below and in managed code
DebugBlock_MonitorEvent, // maps to ThreadBlockingInfo.ObjectKind.MonitorWait below and in managed code
};

typedef DPTR(struct DebugBlockingItem) PTR_DebugBlockingItem;
Expand Down Expand Up @@ -65,15 +65,35 @@ class ThreadDebugBlockingInfo
};

#ifndef DACCESS_COMPILE

// This is the equivalent of the managed ThreadBlockingInfo (see ThreadBlockingInfo.cs), which is used for tracking blocking
// info from the managed side, similarly to DebugBlockingItem
struct ThreadBlockingInfo
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a 2nd definition of these types? Could we update DebugBlockingItem and DebugBlockingItemType to match instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current definition appears to be used by debugger APIs to enumerate the blocking items. It seems there would need to be a second object on the stack for maintaining a separate linked list that contains info only about Monitor blocking, as the current APIs to get info from a blocking object would not work with Lock. I was thinking it might be easier to remove the current definitions in the future after debuggers switch to using the managed surface, and maybe deprecate some of the relevant debugger APIs. Thoughts?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The types that get exposed publicly should be CorDebugBlockingObject and CorDebugBlockingReason in the ICorDebug interface. The types exposed in the DAC IDacDbiInterface are not public and can be changed as needed. The code to convert from the internal to public types occurs here:

SIZE_T dacObjIndex = dacBlockingObjects.Size()-i-1;
.

However if we removed the domain field off the internal type we'd have to repopulate it from somewhere else and it looks like we've never done the work to convert ICorDebug to understand that CoreCLR just has a single AppDomain. Its probably not that hard, but I wouldn't feel right asking you to do that refactoring when the ground work isn't there yet. So I'd say lets not worry about this, it was just a potential for some minor deduplication.

In terms of deprecation we've rarely ever done it in the past but it is possible if supporting the old APIs is ongoing hassle and we've allowed some time to pass so that the new API is easily available. I'd think next release at the earliest unless there is some pressing need. We'd need to remove the usage from our tests, remove code from the runtime, and put out notifications of the breaking change. Thanks Kount!

{
enum class ObjectKind : INT32
{
MonitorLock, // maps to DebugBlockingItemType::DebugBlock_MonitorCriticalSection
MonitorWait // maps to DebugBlockingItemType::DebugBlock_MonitorEvent
};

void *objectPtr;
ObjectKind objectKind;
INT32 timeoutMs;
ThreadBlockingInfo *next;
};

class DebugBlockingItemHolder
{
private:
Thread *m_pThread;
ThreadBlockingInfo **m_ppFirstBlockingInfo;
ThreadBlockingInfo m_blockingInfo;

public:
DebugBlockingItemHolder(Thread *pThread, DebugBlockingItem *pItem);
~DebugBlockingItemHolder();
};

#endif //!DACCESS_COMPILE

#endif // __ThreadBlockingInfo__
Original file line number Diff line number Diff line change
Expand Up @@ -1266,6 +1266,7 @@
<Compile Include="$(MSBuildThisFileDirectory)System\Threading\Thread.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Threading\ProcessorIdCache.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Threading\ThreadAbortException.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Threading\ThreadBlockingInfo.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Threading\ThreadExceptionEventArgs.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Threading\ThreadInt64PersistentCounter.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Threading\ThreadInterruptedException.cs" />
Expand Down Expand Up @@ -2766,4 +2767,4 @@
<Compile Include="$(MSBuildThisFileDirectory)System\Numerics\IUnaryPlusOperators.cs" />
<Compile Include="$(MSBuildThisFileDirectory)System\Numerics\IUnsignedNumber.cs" />
</ItemGroup>
</Project>
</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@ public sealed partial class Lock
/// </summary>
public Lock() => _spinCount = s_maxSpinCount;

internal ulong OwningOSThreadId => _owningThreadId;

#pragma warning disable CA1822 // can be marked as static - varies between runtimes
internal int OwningManagedThreadId => 0;
#pragma warning restore CA1822

private static TryLockResult LazyInitializeOrEnter() => TryLockResult.Spin;
private static bool IsSingleProcessor => Environment.IsSingleProcessor;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,8 @@ private ThreadId TryEnterSlow(int timeoutMs, ThreadId currentThreadId)
waitStartTimeTicks = Stopwatch.GetTimestamp();
}

using ThreadBlockingInfo.Scope threadBlockingScope = new(this, timeoutMs);
noahfalk marked this conversation as resolved.
Show resolved Hide resolved

bool acquiredLock = false;
int waitStartTimeMs = timeoutMs < 0 ? 0 : Environment.TickCount;
int remainingTimeoutMs = timeoutMs;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;

namespace System.Threading
{
// Tracks some kinds of blocking on the thread, like waiting on locks where it may be useful to know when debugging which
// thread owns the lock.
//
// Notes:
// - The type, some fields, and some other members may be used by debuggers (noted specifically below), so take care when
// renaming them
// - There is a native version of this struct in CoreCLR, used by Monitor to fold in its blocking info here. The struct is
// blittable with sequential layout to support that.
[StructLayout(LayoutKind.Sequential)]
internal unsafe struct ThreadBlockingInfo
{
#if CORECLR
// In CoreCLR, for the Monitor object kinds, the object ptr will be a pointer to a native AwareLock object. This
// relative offset indicates the location of the field holding the lock owner OS thread ID (the field is of type
// size_t), and is used to get that info by the LockOwnerOSThreadId property. The offset is not zero currently, so zero
// is used to determine if the static field has been initialized.
//
// This mechanism is used instead of using an FCall in the property getter such that the property can be more easily
// evaluated by a debugger.
private static int s_monitorObjectOffsetOfLockOwnerOSThreadId;
#endif

// Points to the first (most recent) blocking info for the thread. The _next field points to the next-most-recent
// blocking info for the thread, or null if there are no more. Blocking can be reentrant in some cases, such as on UI
// threads where reentrant waits are used, or if a SynchronizationContext wait override is set.
[ThreadStatic]
private static ThreadBlockingInfo* t_first; // may be used by debuggers

// This pointer can be used to obtain the object relevant to the blocking. For native object kinds, it points to the
// native object (for Monitor object kinds in CoreCLR, it points to a native AwareLock object). For managed object
// kinds, it points to a stack location containing the managed object reference.
private void* _objectPtr; // may be used by debuggers

// Indicates the type of object relevant to the blocking
private ObjectKind _objectKind; // may be used by debuggers

// The timeout in milliseconds for the wait, -1 for infinite timeout
private int _timeoutMs; // may be used by debuggers

// Points to the next-most-recent blocking info for the thread
private ThreadBlockingInfo* _next; // may be used by debuggers

private void Push(void* objectPtr, ObjectKind objectKind, int timeoutMs)
{
Debug.Assert(objectPtr != null);

_objectPtr = objectPtr;
_objectKind = objectKind;
_timeoutMs = timeoutMs;
_next = t_first;
t_first = (ThreadBlockingInfo*)Unsafe.AsPointer(ref this);
}

private void Pop()
{
Debug.Assert(_objectPtr != null);
Debug.Assert(t_first != null);
Debug.Assert(t_first->_next == _next);

t_first = _next;
_objectPtr = null;
}

// If the blocking is associated with a lock of some kind that has thread affinity and tracks the owner's OS thread ID,
// returns the OS thread ID of the thread that currently owns the lock. Otherwise, returns 0. A return value of 0 may
// indicate that the associated lock is currently not owned by a thread, or that the information could not be
// determined.
//
// Calls to native helpers are avoided in the property getter such that it can be more easily evaluated by a debugger.
public ulong LockOwnerOSThreadId // the getter may be used by debuggers
{
get
{
Debug.Assert(_objectPtr != null);

switch (_objectKind)
{
case ObjectKind.MonitorLock:
case ObjectKind.MonitorWait:
// The Monitor object kinds are only used by CoreCLR, and only the OS thread ID is reported
#if CORECLR
if (s_monitorObjectOffsetOfLockOwnerOSThreadId != 0)
{
return *(nuint*)((nint)_objectPtr + s_monitorObjectOffsetOfLockOwnerOSThreadId);
}
#endif
return 0;

case ObjectKind.Lock:
return ((Lock)Unsafe.AsRef<object>(_objectPtr)).OwningOSThreadId;

default:
Debug.Assert(_objectKind == ObjectKind.Condition);
#if NATIVEAOT
return ((Condition)Unsafe.AsRef<object>(_objectPtr)).AssociatedLock.OwningOSThreadId;
#else
return 0;
#endif
}
}
}

// If the blocking is associated with a lock of some kind that has thread affinity and tracks the owner's managed thread
// ID, returns the managed thread ID of the thread that currently owns the lock. Otherwise, returns 0. A return value of
// 0 may indicate that the associated lock is currently not owned by a thread, or that the information could not be
// determined.
//
// Calls to native helpers are avoided in the property getter such that it can be more easily evaluated by a debugger.
public int LockOwnerManagedThreadId // the getter may be used by debuggers
{
get
{
Debug.Assert(_objectPtr != null);

switch (_objectKind)
{
case ObjectKind.MonitorLock:
case ObjectKind.MonitorWait:
// The Monitor object kinds are only used by CoreCLR, and only the OS thread ID is reported
return 0;

case ObjectKind.Lock:
return ((Lock)Unsafe.AsRef<object>(_objectPtr)).OwningManagedThreadId;

default:
Debug.Assert(_objectKind == ObjectKind.Condition);
#if NATIVEAOT
return ((Condition)Unsafe.AsRef<object>(_objectPtr)).AssociatedLock.OwningManagedThreadId;
#else
return 0;
#endif
}
}
}

public unsafe ref struct Scope
{
private object? _object;
private ThreadBlockingInfo _blockingInfo;

#pragma warning disable CS9216 // casting Lock to object
public Scope(Lock lockObj, int timeoutMs) : this(lockObj, ObjectKind.Lock, timeoutMs) { }
#pragma warning restore CS9216

#if NATIVEAOT
public Scope(Condition condition, int timeoutMs) : this(condition, ObjectKind.Condition, timeoutMs) { }
#endif

private Scope(object obj, ObjectKind objectKind, int timeoutMs)
{
_object = obj;
_blockingInfo.Push(Unsafe.AsPointer(ref _object), objectKind, timeoutMs);
}

public void Dispose()
{
if (_object is not null)
{
_blockingInfo.Pop();
_object = null;
}
}
}

public enum ObjectKind // may be used by debuggers
{
MonitorLock, // maps to DebugBlockingItemType::DebugBlock_MonitorCriticalSection in coreclr
MonitorWait, // maps to DebugBlockingItemType::DebugBlock_MonitorEvent in coreclr
Lock,
Condition
}
}
}
Loading