Skip to content

Commit

Permalink
Fix and re-enable RecordInfoLockTest; remove no-longer-needed re-entr…
Browse files Browse the repository at this point in the history
…ancy from RecordInfo.SpinLock() (#409)
  • Loading branch information
TedHartMS authored Feb 26, 2021
1 parent e163614 commit b2292b8
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 54 deletions.
38 changes: 1 addition & 37 deletions cs/src/core/Index/Common/RecordInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -144,45 +144,18 @@ public void Unpin()
throw new InvalidOperationException();
}
#endif
/// <summary>
/// The RecordInfo locked by this thread, if any.
/// </summary>
[ThreadStatic]
internal static RecordInfo* threadLockedRecord;

/// <summary>
/// The number of times the current thread has (re-)entered the lock.
/// </summary>
[ThreadStatic]
internal static int threadLockedRecordEntryCount;

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public void SpinLock()
{
// Check for a re-entrant lock.
if (threadLockedRecord == (RecordInfo*)Unsafe.AsPointer(ref this))
{
Debug.Assert(threadLockedRecordEntryCount > 0);
++threadLockedRecordEntryCount;
return;
}

// RecordInfo locking is intended for use in concurrent callbacks only (ConcurrentReader, ConcurrentWriter, InPlaceUpdater),
// so only the RecordInfo for that callback should be locked. A different RecordInfo being locked implies a missing Unlock.
Debug.Assert(threadLockedRecord == null);
Debug.Assert(threadLockedRecordEntryCount == 0);
while (true)
{
long expected_word = word;
if ((expected_word & kLatchBitMask) == 0)
{
var found_word = Interlocked.CompareExchange(ref word, expected_word | kLatchBitMask, expected_word);
if (found_word == expected_word)
{
threadLockedRecord = (RecordInfo*)Unsafe.AsPointer(ref this);
threadLockedRecordEntryCount = 1;
return;
}
}
Thread.Yield();
}
Expand All @@ -191,16 +164,7 @@ public void SpinLock()
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public void Unlock()
{
Debug.Assert(threadLockedRecord == (RecordInfo*)Unsafe.AsPointer(ref this));
if (threadLockedRecord == (RecordInfo*)Unsafe.AsPointer(ref this))
{
Debug.Assert(threadLockedRecordEntryCount > 0);
if (--threadLockedRecordEntryCount == 0)
{
word &= ~kLatchBitMask;
threadLockedRecord = null;
}
}
word &= ~kLatchBitMask;
}

public bool IsNull()
Expand Down
23 changes: 6 additions & 17 deletions cs/test/LockTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -63,27 +63,16 @@ public void TearDown()
log = null;
}

[Test]
public unsafe void RecordInfoLockTest()
{
// Re-entrancy check
static void checkLatch(RecordInfo* ptr, long count)
for (var ii = 0; ii < 5; ++ii)
{
Assert.IsTrue(RecordInfo.threadLockedRecord == ptr);
Assert.IsTrue(RecordInfo.threadLockedRecordEntryCount == count);
RecordInfo recordInfo = new RecordInfo();
RecordInfo* ri = &recordInfo;

XLockTest(() => ri->SpinLock(), () => ri->Unlock());
}
RecordInfo recordInfo = new RecordInfo();
RecordInfo* ri = (RecordInfo*)Unsafe.AsPointer(ref recordInfo);
checkLatch(null, 0);
recordInfo.SpinLock();
checkLatch(ri, 1);
recordInfo.SpinLock();
checkLatch(ri, 2);
recordInfo.Unlock();
checkLatch(ri, 1);
recordInfo.Unlock();
checkLatch(null, 0);

XLockTest(() => recordInfo.SpinLock(), () => recordInfo.Unlock());
}

private void XLockTest(Action locker, Action unlocker)
Expand Down

0 comments on commit b2292b8

Please sign in to comment.