Skip to content

Adjust how we access thread ID for header thin locks #124878

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.

Sign up for GitHub

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

Open
jkoritzinsky wants to merge 2 commits into
base: main
Choose a base branch
from
+15 −22
Open

Adjust how we access thread ID for header thin locks #124878

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7ef73b3
Adjust how we access thread ID for header thin locks
jkoritzinsky Feb 26, 2026
9733c23
Collapse a few more checks to align with prior implementation even be…
jkoritzinsky Feb 26, 2026
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
4 changes: 2 additions & 2 deletions src/coreclr/vm/comsynchronizable.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -855,15 +855,15 @@ FCIMPL1(ObjHeader::HeaderLockResult, ObjHeader_AcquireThinLock, Object* obj)
{
FCALL_CONTRACT;

return obj->GetHeader()->AcquireHeaderThinLock(GetThread()->GetThreadId());
return obj->GetHeader()->AcquireHeaderThinLock(GetThread());
}
FCIMPLEND

FCIMPL1(ObjHeader::HeaderLockResult, ObjHeader_ReleaseThinLock, Object* obj)
{
FCALL_CONTRACT;

return obj->GetHeader()->ReleaseHeaderThinLock(GetThread()->GetThreadId());
return obj->GetHeader()->ReleaseHeaderThinLock(GetThread());
}
FCIMPLEND

Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/vm/syncblk.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -954,9 +954,9 @@ class ObjHeader
UseSlowPath = 2
};

HeaderLockResult AcquireHeaderThinLock(DWORD tid);
HeaderLockResult AcquireHeaderThinLock(Thread* pCurThread);

HeaderLockResult ReleaseHeaderThinLock(DWORD tid);
HeaderLockResult ReleaseHeaderThinLock(Thread* pCurThread);

friend struct ::cdac_data<ObjHeader>;
};
Expand Down
29 changes: 11 additions & 18 deletions src/coreclr/vm/syncblk.inl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
#ifndef _SYNCBLK_INL_
#define _SYNCBLK_INL_

FORCEINLINE ObjHeader::HeaderLockResult ObjHeader::AcquireHeaderThinLock(DWORD tid)
FORCEINLINE ObjHeader::HeaderLockResult ObjHeader::AcquireHeaderThinLock(Thread* pCurThread)
{
CONTRACTL
{
Expand All @@ -20,6 +20,7 @@ FORCEINLINE ObjHeader::HeaderLockResult ObjHeader::AcquireHeaderThinLock(DWORD t
SBLK_MASK_LOCK_THREADID +
SBLK_MASK_LOCK_RECLEVEL)) == 0)
{
DWORD tid = pCurThread->GetThreadId();
if (tid > SBLK_MASK_LOCK_THREADID)
{
return HeaderLockResult::UseSlowPath;
Expand All @@ -38,20 +39,16 @@ FORCEINLINE ObjHeader::HeaderLockResult ObjHeader::AcquireHeaderThinLock(DWORD t
return HeaderLockResult::Failure;
}

if (oldValue & BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX)
{
return HeaderLockResult::UseSlowPath;
}

// The header is transitioning - use the slow path
if (oldValue & BIT_SBLK_SPIN_LOCK)
// The header either has a sync block, will need a sync block to represent the lock
// or is in the process of transitioning to a sync block. In any of these cases, we need to take the slow path.
if (oldValue & (BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX | BIT_SBLK_SPIN_LOCK))
{
return HeaderLockResult::UseSlowPath;
}

// Here we know we have the "thin lock" layout, but the lock is not free.
// It could still be the recursion case - compare the thread id to check
if (tid != (DWORD)(oldValue & SBLK_MASK_LOCK_THREADID))
Copy link
Member

@agocke agocke Feb 27, 2026

Choose a reason for hiding this comment

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

Could the current thread have changed here? Why do we need to refetch the id?

if (pCurThread->GetThreadId() != (DWORD)(oldValue & SBLK_MASK_LOCK_THREADID))
{
return HeaderLockResult::Failure;
}
Expand Down Expand Up @@ -80,7 +77,7 @@ FORCEINLINE ObjHeader::HeaderLockResult ObjHeader::AcquireHeaderThinLock(DWORD t

// Helper encapsulating the core logic for releasing monitor. Returns what kind of
// follow up action is necessary. This is FORCEINLINE to make it provide a very efficient implementation.
FORCEINLINE ObjHeader::HeaderLockResult ObjHeader::ReleaseHeaderThinLock(DWORD tid)
FORCEINLINE ObjHeader::HeaderLockResult ObjHeader::ReleaseHeaderThinLock(Thread* pCurThread)
{
CONTRACTL {
NOTHROW;
Expand All @@ -92,14 +89,14 @@ FORCEINLINE ObjHeader::HeaderLockResult ObjHeader::ReleaseHeaderThinLock(DWORD t

if ((syncBlockValue & (BIT_SBLK_SPIN_LOCK + BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX)) == 0)
{
if (tid > SBLK_MASK_LOCK_THREADID)
Copy link
Member

@agocke agocke Feb 27, 2026

Choose a reason for hiding this comment

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

Why isn't this early-out important?

{
return HeaderLockResult::UseSlowPath;
}
DWORD tid = pCurThread->GetThreadId();

if ((syncBlockValue & SBLK_MASK_LOCK_THREADID) != tid)
{
// This thread does not own the lock.
// We don't need to check if the thread id could be stored in the lock
// as we know we are in the thin lock case.
// The lock is definitely not being held by this thread.
return HeaderLockResult::Failure;
}

Expand All @@ -116,8 +113,6 @@ FORCEINLINE ObjHeader::HeaderLockResult ObjHeader::ReleaseHeaderThinLock(DWORD t
{
return HeaderLockResult::Success;
}

return HeaderLockResult::UseSlowPath;
}
else
{
Expand All @@ -131,8 +126,6 @@ FORCEINLINE ObjHeader::HeaderLockResult ObjHeader::ReleaseHeaderThinLock(DWORD t
{
return HeaderLockResult::Success;
}

return HeaderLockResult::UseSlowPath;
}
}

Expand Down
Loading