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.

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

Expose a Lock type in preview mode #87672

Merged
merged 17 commits into from
Oct 30, 2023
Merged

Expose a Lock type in preview mode #87672

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
b78e117
Expose a `Lock` type in preview mode
kouvel Mar 30, 2023
2c9b16b
Add xml comments for constructor
kouvel Jun 16, 2023
79cba4d
Fix an issue
kouvel Jun 16, 2023
dab479e
Fix an issue
kouvel Jun 16, 2023
f3074be
Address feedback and fix an issue
kouvel Jun 16, 2023
5ae22a1
Fix an issue
kouvel Jun 16, 2023
32b3423
Add comments and fix test
kouvel Jun 16, 2023
35dbebc
Fix tests
kouvel Jun 16, 2023
9b22948
Address feedback, fix some issues, update test
kouvel Jun 17, 2023
9f58679
Remove assertion in Dispose
kouvel Jun 20, 2023
d20839b
Address feedback
kouvel Jun 21, 2023
779b9eb
Fix an issue
kouvel Jun 22, 2023
16384aa
Fixes from merge
kouvel Aug 17, 2023
ad2739d
Fix some things, address feedback, reduce thread-static lookups in so…
kouvel Aug 18, 2023
c3e6fc9
Reduce size of Lock, change recursion count back to `uint`
kouvel Oct 9, 2023
4910b45
Fix tests
kouvel Oct 20, 2023
c0fb16d
Fix wasm build
kouvel Oct 30, 2023
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: 3 additions & 1 deletion src/coreclr/System.Private.CoreLib/src/System/Threading/Monitor.CoreCLR.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,12 @@ public static void PulseAll(object obj)
ObjPulseAll(obj);
}

#pragma warning disable CA2252 // Opt in to preview features before using them (Lock)
/// <summary>
/// Gets the number of times there was contention upon trying to take a <see cref="Monitor"/>'s lock so far.
/// </summary>
public static long LockContentionCount => GetLockContentionCount();
public static long LockContentionCount => GetLockContentionCount() + Lock.ContentionCount;
#pragma warning restore CA2252

[LibraryImport(RuntimeHelpers.QCall, EntryPoint = "ObjectNative_GetMonitorLockContentionCount")]
private static partial long GetLockContentionCount();
Expand Down
14 changes: 7 additions & 7 deletions src/coreclr/nativeaot/Common/src/System/Collections/Concurrent/ConcurrentUnifier.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ protected ConcurrentUnifier()
public V GetOrAdd(K key)
{
Debug.Assert(key != null);
Debug.Assert(!_lock.IsAcquired, "GetOrAdd called while lock already acquired. A possible cause of this is an Equals or GetHashCode method that causes reentrancy in the table.");
Debug.Assert(!_lock.IsHeldByCurrentThread, "GetOrAdd called while lock already acquired. A possible cause of this is an Equals or GetHashCode method that causes reentrancy in the table.");

int hashCode = key.GetHashCode();
V value;
Expand All @@ -89,7 +89,7 @@ public V GetOrAdd(K key)
V checkedValue;
bool checkedFound;
// In debug builds, always exercise a locked TryGet (this is a good way to detect deadlock/reentrancy through Equals/GetHashCode()).
using (LockHolder.Hold(_lock))
using (_lock.EnterScope())
{
_container.VerifyUnifierConsistency();
int h = key.GetHashCode();
Expand All @@ -110,7 +110,7 @@ public V GetOrAdd(K key)

value = this.Factory(key);

using (LockHolder.Hold(_lock))
using (_lock.EnterScope())
{
V heyIWasHereFirst;
if (_container.TryGetValue(key, hashCode, out heyIWasHereFirst))
Expand Down Expand Up @@ -171,7 +171,7 @@ public bool TryGetValue(K key, int hashCode, out V value)

public void Add(K key, int hashCode, V value)
{
Debug.Assert(_owner._lock.IsAcquired);
Debug.Assert(_owner._lock.IsHeldByCurrentThread);

int bucket = ComputeBucket(hashCode, _buckets.Length);

Expand All @@ -194,14 +194,14 @@ public bool HasCapacity
{
get
{
Debug.Assert(_owner._lock.IsAcquired);
Debug.Assert(_owner._lock.IsHeldByCurrentThread);
return _nextFreeEntry != _entries.Length;
}
}

public void Resize()
{
Debug.Assert(_owner._lock.IsAcquired);
Debug.Assert(_owner._lock.IsHeldByCurrentThread);

int newSize = HashHelpers.GetPrime(_buckets.Length * 2);
#if DEBUG
Expand Down Expand Up @@ -257,7 +257,7 @@ public void VerifyUnifierConsistency()
if (_nextFreeEntry >= 5000 && (0 != (_nextFreeEntry % 100)))
return;

Debug.Assert(_owner._lock.IsAcquired);
Debug.Assert(_owner._lock.IsHeldByCurrentThread);
Debug.Assert(_nextFreeEntry >= 0 && _nextFreeEntry <= _entries.Length);
int numEntriesEncountered = 0;
for (int bucket = 0; bucket < _buckets.Length; bucket++)
Expand Down
14 changes: 7 additions & 7 deletions src/coreclr/nativeaot/Common/src/System/Collections/Concurrent/ConcurrentUnifierW.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ protected ConcurrentUnifierW()
public V GetOrAdd(K key)
{
Debug.Assert(key != null);
Debug.Assert(!_lock.IsAcquired, "GetOrAdd called while lock already acquired. A possible cause of this is an Equals or GetHashCode method that causes reentrancy in the table.");
Debug.Assert(!_lock.IsHeldByCurrentThread, "GetOrAdd called while lock already acquired. A possible cause of this is an Equals or GetHashCode method that causes reentrancy in the table.");

int hashCode = key.GetHashCode();
V? value;
Expand All @@ -99,7 +99,7 @@ public V GetOrAdd(K key)
V? checkedValue;
bool checkedFound;
// In debug builds, always exercise a locked TryGet (this is a good way to detect deadlock/reentrancy through Equals/GetHashCode()).
using (LockHolder.Hold(_lock))
using (_lock.EnterScope())
{
_container.VerifyUnifierConsistency();
int h = key.GetHashCode();
Expand Down Expand Up @@ -137,7 +137,7 @@ public V GetOrAdd(K key)
return null;
}

using (LockHolder.Hold(_lock))
using (_lock.EnterScope())
{
V? heyIWasHereFirst;
if (_container.TryGetValue(key, hashCode, out heyIWasHereFirst))
Expand Down Expand Up @@ -201,7 +201,7 @@ public bool TryGetValue(K key, int hashCode, out V? value)

public void Add(K key, int hashCode, V value)
{
Debug.Assert(_owner._lock.IsAcquired);
Debug.Assert(_owner._lock.IsHeldByCurrentThread);

int bucket = ComputeBucket(hashCode, _buckets.Length);

Expand Down Expand Up @@ -251,14 +251,14 @@ public bool HasCapacity
{
get
{
Debug.Assert(_owner._lock.IsAcquired);
Debug.Assert(_owner._lock.IsHeldByCurrentThread);
return _nextFreeEntry != _entries.Length;
}
}

public void Resize()
{
Debug.Assert(_owner._lock.IsAcquired);
Debug.Assert(_owner._lock.IsHeldByCurrentThread);

// Before we actually grow the size of the table, figure out how much we can recover just by dropping entries with
// expired weak references.
Expand Down Expand Up @@ -341,7 +341,7 @@ public void VerifyUnifierConsistency()
if (_nextFreeEntry >= 5000 || (0 != (_nextFreeEntry % 100)))
return;

Debug.Assert(_owner._lock.IsAcquired);
Debug.Assert(_owner._lock.IsHeldByCurrentThread);
Debug.Assert(_nextFreeEntry >= 0 && _nextFreeEntry <= _entries.Length);
int numEntriesEncountered = 0;
for (int bucket = 0; bucket < _buckets.Length; bucket++)
Expand Down
14 changes: 7 additions & 7 deletions src/coreclr/nativeaot/Common/src/System/Collections/Concurrent/ConcurrentUnifierWKeyed.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ protected ConcurrentUnifierWKeyed()
public V GetOrAdd(K key)
{
Debug.Assert(key != null);
Debug.Assert(!_lock.IsAcquired, "GetOrAdd called while lock already acquired. A possible cause of this is an Equals or GetHashCode method that causes reentrancy in the table.");
Debug.Assert(!_lock.IsHeldByCurrentThread, "GetOrAdd called while lock already acquired. A possible cause of this is an Equals or GetHashCode method that causes reentrancy in the table.");

int hashCode = key.GetHashCode();
V value;
Expand All @@ -112,7 +112,7 @@ public V GetOrAdd(K key)
V checkedValue;
bool checkedFound;
// In debug builds, always exercise a locked TryGet (this is a good way to detect deadlock/reentrancy through Equals/GetHashCode()).
using (LockHolder.Hold(_lock))
using (_lock.EnterScope())
{
_container.VerifyUnifierConsistency();
int h = key.GetHashCode();
Expand Down Expand Up @@ -154,7 +154,7 @@ public V GetOrAdd(K key)
// it needs to produce the key quickly and in a deadlock-free manner once we're inside the lock.
value.PrepareKey();

using (LockHolder.Hold(_lock))
using (_lock.EnterScope())
{
V heyIWasHereFirst;
if (_container.TryGetValue(key, hashCode, out heyIWasHereFirst))
Expand Down Expand Up @@ -220,7 +220,7 @@ public bool TryGetValue(K key, int hashCode, out V value)

public void Add(int hashCode, V value)
{
Debug.Assert(_owner._lock.IsAcquired);
Debug.Assert(_owner._lock.IsHeldByCurrentThread);

int bucket = ComputeBucket(hashCode, _buckets.Length);
int newEntryIdx = _nextFreeEntry;
Expand All @@ -241,14 +241,14 @@ public bool HasCapacity
{
get
{
Debug.Assert(_owner._lock.IsAcquired);
Debug.Assert(_owner._lock.IsHeldByCurrentThread);
return _nextFreeEntry != _entries.Length;
}
}

public void Resize()
{
Debug.Assert(_owner._lock.IsAcquired);
Debug.Assert(_owner._lock.IsHeldByCurrentThread);

// Before we actually grow the size of the table, figure out how much we can recover just by dropping entries with
// expired weak references.
Expand Down Expand Up @@ -330,7 +330,7 @@ public void VerifyUnifierConsistency()
if (_nextFreeEntry >= 5000 && (0 != (_nextFreeEntry % 100)))
return;

Debug.Assert(_owner._lock.IsAcquired);
Debug.Assert(_owner._lock.IsHeldByCurrentThread);
Debug.Assert(_nextFreeEntry >= 0 && _nextFreeEntry <= _entries.Length);
int numEntriesEncountered = 0;
for (int bucket = 0; bucket < _buckets.Length; bucket++)
Expand Down
8 changes: 0 additions & 8 deletions src/coreclr/nativeaot/System.Private.CoreLib/src/CompatibilitySuppressions.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -929,14 +929,6 @@
<DiagnosticId>CP0001</DiagnosticId>
<Target>T:System.Threading.Condition</Target>
</Suppression>
<Suppression>
<DiagnosticId>CP0001</DiagnosticId>
<Target>T:System.Threading.Lock</Target>
</Suppression>
<Suppression>
<DiagnosticId>CP0001</DiagnosticId>
<Target>T:System.Threading.LockHolder</Target>
</Suppression>
<Suppression>
<DiagnosticId>CP0002</DiagnosticId>
<Target>M:System.ModuleHandle.#ctor(System.Reflection.Module)</Target>
Expand Down
10 changes: 6 additions & 4 deletions .../System.Private.CoreLib/src/Internal/Runtime/CompilerHelpers/SynchronizedMethodHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ internal static class SynchronizedMethodHelpers
private static void MonitorEnter(object obj, ref bool lockTaken)
{
// Inlined Monitor.Enter with a few tweaks
int resultOrIndex = ObjectHeader.Acquire(obj);
int currentThreadID = ManagedThreadId.CurrentManagedThreadIdUnchecked;
int resultOrIndex = ObjectHeader.Acquire(obj, currentThreadID);
if (resultOrIndex < 0)
{
lockTaken = true;
Expand All @@ -25,7 +26,7 @@ private static void MonitorEnter(object obj, ref bool lockTaken)
ObjectHeader.GetLockObject(obj) :
SyncTable.GetLockObject(resultOrIndex);

Monitor.TryAcquireSlow(lck, obj, Timeout.Infinite);
lck.TryEnterSlow(Timeout.Infinite, currentThreadID, obj);
lockTaken = true;
}
private static void MonitorExit(object obj, ref bool lockTaken)
Expand All @@ -42,7 +43,8 @@ private static unsafe void MonitorEnterStatic(MethodTable* pMT, ref bool lockTak
{
// Inlined Monitor.Enter with a few tweaks
object obj = GetStaticLockObject(pMT);
int resultOrIndex = ObjectHeader.Acquire(obj);
int currentThreadID = ManagedThreadId.CurrentManagedThreadIdUnchecked;
int resultOrIndex = ObjectHeader.Acquire(obj, currentThreadID);
if (resultOrIndex < 0)
{
lockTaken = true;
Expand All @@ -53,7 +55,7 @@ private static unsafe void MonitorEnterStatic(MethodTable* pMT, ref bool lockTak
ObjectHeader.GetLockObject(obj) :
SyncTable.GetLockObject(resultOrIndex);

Monitor.TryAcquireSlow(lck, obj, Timeout.Infinite);
lck.TryEnterSlow(Timeout.Infinite, currentThreadID, obj);
lockTaken = true;
}
private static unsafe void MonitorExitStatic(MethodTable* pMT, ref bool lockTaken)
Expand Down
8 changes: 3 additions & 5 deletions src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
<EmitCompilerGeneratedFiles>true</EmitCompilerGeneratedFiles>
<!-- AD0001 : https://github.com/dotnet/runtime/issues/90356 -->
<NoWarn>$(NoWarn);AD0001</NoWarn>
<!-- CA2252: Opt in to preview features before using them (Lock) -->
<EnablePreviewFeatures>true</EnablePreviewFeatures>
</PropertyGroup>

<ItemGroup>
Expand Down Expand Up @@ -231,10 +233,9 @@
<Compile Include="System\RuntimeFieldHandle.cs" />
<Compile Include="System\Text\StringBuilder.NativeAot.cs" />
<Compile Include="System\Threading\ManagedThreadId.cs" />
<Compile Include="System\Threading\Lock.cs" />
<Compile Include="System\Threading\Lock.NativeAot.cs" />
<Compile Include="System\Threading\Condition.cs" />
<Compile Include="System\Threading\Interlocked.cs" />
<Compile Include="System\Threading\LockHolder.cs" />
<Compile Include="System\Threading\Monitor.NativeAot.cs" />
<Compile Include="System\Threading\ObjectHeader.cs" />
<Compile Include="System\Threading\SyncTable.cs" />
Expand Down Expand Up @@ -305,9 +306,6 @@
<Compile Include="$(CommonPath)\Interop\Unix\System.Native\Interop.Exit.cs">
<Link>Interop\Unix\System.Native\Interop.Exit.cs</Link>
</Compile>
<Compile Include="$(CommonPath)\Interop\Unix\System.Native\Interop.Threading.cs">
<Link>Interop\Unix\System.Native\Interop.Threading.cs</Link>
</Compile>
</ItemGroup>
<ItemGroup>
<Compile Include="$(CompilerCommonPath)\System\Collections\Generic\ArrayBuilder.cs">
Expand Down
24 changes: 12 additions & 12 deletions ...eaot/System.Private.CoreLib/src/System/Runtime/CompilerServices/ClassConstructorRunner.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public static unsafe void EnsureClassConstructorRun(StaticClassConstructionConte
cctors[cctorIndex].HoldingThread = ManagedThreadIdNone;
NoisyLog("Releasing cctor lock, context={0}, thread={1}", pContext, currentManagedThreadId);

cctorLock.Release();
cctorLock.Exit();
}
}
else
Expand Down Expand Up @@ -142,10 +142,10 @@ private static unsafe bool DeadlockAwareAcquire(CctorHandle cctor, StaticClassCo
int cctorIndex = cctor.Index;
Cctor[] cctors = cctor.Array;
Lock lck = cctors[cctorIndex].Lock;
if (lck.IsAcquired)
if (lck.IsHeldByCurrentThread)
return false; // Thread recursively triggered the same cctor.

if (lck.TryAcquire(waitIntervalInMS))
if (lck.TryEnter(waitIntervalInMS))
return true;

// We couldn't acquire the lock. See if this .cctor is involved in a cross-thread deadlock. If so, break
Expand All @@ -164,7 +164,7 @@ private static unsafe bool DeadlockAwareAcquire(CctorHandle cctor, StaticClassCo
// deadlock themselves, then that's a bug in user code.
for (;;)
{
using (LockHolder.Hold(s_cctorGlobalLock))
using (s_cctorGlobalLock.EnterScope())
{
// Ask the guy who holds the cctor lock we're trying to acquire who he's waiting for. Keep
// walking down that chain until we either discover a cycle or reach a non-blocking state. Note
Expand Down Expand Up @@ -233,7 +233,7 @@ private static unsafe bool DeadlockAwareAcquire(CctorHandle cctor, StaticClassCo
waitIntervalInMS *= 2;

// We didn't find a cycle yet, try to take the lock again.
if (lck.TryAcquire(waitIntervalInMS))
if (lck.TryEnter(waitIntervalInMS))
return true;
} // infinite loop
}
Expand Down Expand Up @@ -283,7 +283,7 @@ public static CctorHandle GetCctor(StaticClassConstructionContext* pContext)
}
#endif // TARGET_WASM

using (LockHolder.Hold(s_cctorGlobalLock))
using (s_cctorGlobalLock.EnterScope())
{
Cctor[]? resultArray = null;
int resultIndex = -1;
Expand Down Expand Up @@ -355,14 +355,14 @@ public static int Count
{
get
{
Debug.Assert(s_cctorGlobalLock.IsAcquired);
Debug.Assert(s_cctorGlobalLock.IsHeldByCurrentThread);
return s_count;
}
}

public static void Release(CctorHandle cctor)
{
using (LockHolder.Hold(s_cctorGlobalLock))
using (s_cctorGlobalLock.EnterScope())
{
Cctor[] cctors = cctor.Array;
int cctorIndex = cctor.Index;
Expand Down Expand Up @@ -419,7 +419,7 @@ public static int MarkThreadAsBlocked(int managedThreadId, CctorHandle blockedOn
#else
const int Grow = 10;
#endif
using (LockHolder.Hold(s_cctorGlobalLock))
using (s_cctorGlobalLock.EnterScope())
{
s_blockingRecords ??= new BlockingRecord[Grow];
int found;
Expand Down Expand Up @@ -450,14 +450,14 @@ public static int MarkThreadAsBlocked(int managedThreadId, CctorHandle blockedOn
public static void UnmarkThreadAsBlocked(int blockRecordIndex)
{
// This method must never throw
s_cctorGlobalLock.Acquire();
s_cctorGlobalLock.Enter();
s_blockingRecords[blockRecordIndex].BlockedOn = new CctorHandle(null, 0);
s_cctorGlobalLock.Release();
s_cctorGlobalLock.Exit();
}

public static CctorHandle GetCctorThatThreadIsBlockedOn(int managedThreadId)
{
Debug.Assert(s_cctorGlobalLock.IsAcquired);
Debug.Assert(s_cctorGlobalLock.IsHeldByCurrentThread);
for (int i = 0; i < s_nextBlockingRecordIndex; i++)
{
if (s_blockingRecords[i].ManagedThreadId == managedThreadId)
Expand Down
Loading