diff --git a/src/mscorlib/shared/System/Threading/ReaderWriterLockSlim.cs b/src/mscorlib/shared/System/Threading/ReaderWriterLockSlim.cs index 1136fed64d57..f0029818dc9c 100644 --- a/src/mscorlib/shared/System/Threading/ReaderWriterLockSlim.cs +++ b/src/mscorlib/shared/System/Threading/ReaderWriterLockSlim.cs @@ -170,6 +170,7 @@ private static bool IsRWEntryEmpty(ReaderWriterCount rwc) private bool IsRwHashEntryChanged(ReaderWriterCount lrwc) { + Debug.Assert(lrwc != null); return lrwc.lockID != _lockID; } @@ -295,74 +296,89 @@ private bool TryEnterReadLockCore(TimeoutTracker timeout) ReaderWriterCount lrwc = null; int id = Environment.CurrentManagedThreadId; - if (!_fIsReentrant) + if (!_fIsReentrant && id == _writeLockOwnerId) { - if (id == _writeLockOwnerId) - { - //Check for AW->AR - throw new LockRecursionException(SR.LockRecursionException_ReadAfterWriteNotAllowed); - } - - EnterMyLockForEnterAnyRead(); - - lrwc = GetThreadRWCount(false); + //Check for AW->AR + throw new LockRecursionException(SR.LockRecursionException_ReadAfterWriteNotAllowed); + } - //Check if the reader lock is already acquired. Note, we could - //check the presence of a reader by not allocating rwc (But that - //would lead to two lookups in the common case. It's better to keep - //a count in the structure). - if (lrwc.readercount > 0) - { - ExitMyLock(); - throw new LockRecursionException(SR.LockRecursionException_RecursiveReadNotAllowed); - } - else if (id == _upgradeLockOwnerId) - { - //The upgrade lock is already held. - //Update the global read counts and exit. + if (!TryEnterMyLockForEnterAnyRead(timeout)) + { + goto TimeoutAfterAnyWait; + } - lrwc.readercount++; - _owners++; - ExitMyLock(); - return true; - } + Debug.Assert(lrwc == null); + try + { + lrwc = GetThreadRWCount(dontAllocate: false); // allocation may throw } - else + finally { - EnterMyLockForEnterAnyRead(); - lrwc = GetThreadRWCount(false); - if (lrwc.readercount > 0) - { - lrwc.readercount++; - ExitMyLock(); - return true; - } - else if (id == _upgradeLockOwnerId) - { - //The upgrade lock is already held. - //Update the global read counts and exit. - lrwc.readercount++; - _owners++; - ExitMyLock(); - _fUpgradeThreadHoldingRead = true; - return true; - } - else if (id == _writeLockOwnerId) + if (lrwc == null) { - //The write lock is already held. - //Update global read counts here, - lrwc.readercount++; - _owners++; ExitMyLock(); - return true; } } - bool retVal = true; int spinCount = 0; for (; ;) { + // The following has to be done in every iteration of this spin loop because the RW lock state could have + // changed due to reentrancy by message pumping + + // The following are checked before the loop and before going around the loop + Debug.Assert(_fIsReentrant || id != _writeLockOwnerId); + Debug.Assert(!IsRwHashEntryChanged(lrwc)); + + if (!_fIsReentrant) + { + //Check if the reader lock is already acquired + if (lrwc.readercount > 0) + { + ExitMyLock(); + throw new LockRecursionException(SR.LockRecursionException_RecursiveReadNotAllowed); + } + else if (id == _upgradeLockOwnerId) + { + //The upgrade lock is already held. + //Update the global read counts and exit. + + lrwc.readercount++; + _owners++; + ExitMyLock(); + return true; + } + } + else + { + if (lrwc.readercount > 0) + { + lrwc.readercount++; + ExitMyLock(); + return true; + } + else if (id == _upgradeLockOwnerId) + { + //The upgrade lock is already held. + //Update the global read counts and exit. + lrwc.readercount++; + _owners++; + ExitMyLock(); + _fUpgradeThreadHoldingRead = true; + return true; + } + else if (id == _writeLockOwnerId) + { + //The write lock is already held. + //Update global read counts here, + lrwc.readercount++; + _owners++; + ExitMyLock(); + return true; + } + } + // We can enter a read lock if there are only read-locks have been given out // and a writer is not trying to get in. @@ -371,7 +387,8 @@ private bool TryEnterReadLockCore(TimeoutTracker timeout) // Good case, there is no contention, we are basically done _owners++; // Indicate we have another reader lrwc.readercount++; - break; + ExitMyLock(); + return true; } if (timeout.IsExpired) @@ -385,33 +402,86 @@ private bool TryEnterReadLockCore(TimeoutTracker timeout) ExitMyLock(); spinCount++; SpinWait(spinCount); - EnterMyLockForEnterAnyRead(); - //The per-thread structure may have been recycled as the lock is acquired (due to message pumping), load again. - if (IsRwHashEntryChanged(lrwc)) - lrwc = GetThreadRWCount(false); - continue; + + if (TryEnterMyLockForEnterAnyRead(timeout)) + { + goto TryAgain; + } + goto TimeoutAfterAnyWait; } // Drat, we need to wait. Mark that we have waiters and wait. if (_readEvent == null) // Create the needed event { - LazyCreateEvent(ref _readEvent, EnterLockType.Read); - if (IsRwHashEntryChanged(lrwc)) - lrwc = GetThreadRWCount(false); - continue; // since we left the lock, start over. + if (TryLazyCreateEvent(ref _readEvent, timeout, EnterLockType.Read)) + { + goto TryAgain; // since we left the lock, start over. + } + goto TimeoutAfterAnyWait; } - retVal = WaitOnEvent(_readEvent, ref _numReadWaiters, timeout, EnterLockType.Read); - if (!retVal) + if (!WaitOnEvent(_readEvent, ref _numReadWaiters, timeout, EnterLockType.Read)) { - return false; + goto TimeoutAfterAnyWait; } + +TryAgain: + ; // fix incorrect autoformat indenting for the next comment + + // The following has to be done in every iteration of this spin loop because the RW lock state could have + // changed due to reentrancy by message pumping + + if (!_fIsReentrant && id == _writeLockOwnerId) + { + //Check for AW->AR + ExitMyLock(); + throw new LockRecursionException(SR.LockRecursionException_ReadAfterWriteNotAllowed); + } + if (IsRwHashEntryChanged(lrwc)) - lrwc = GetThreadRWCount(false); + { + lrwc = null; + try + { + lrwc = GetThreadRWCount(dontAllocate: false); // allocation may throw + } + finally + { + if (lrwc == null) + { + ExitMyLock(); + } + } + } } - ExitMyLock(); - return retVal; +TimeoutAfterAnyWait: + Debug.Assert(timeout.IsExpired); + + // Message pumping during a previous spin wait may have changed the RW lock state. Check if we should throw instead + // of returning false. + + if (_fIsReentrant) + { + return false; + } + + if (id == _writeLockOwnerId) + { + // AW->AR + throw new LockRecursionException(SR.LockRecursionException_ReadAfterWriteNotAllowed); + } + + if (lrwc == null || IsRwHashEntryChanged(lrwc)) + { + lrwc = GetThreadRWCount(dontAllocate: true); + } + if (lrwc != null && lrwc.readercount > 0) + { + // AR->AR + throw new LockRecursionException(SR.LockRecursionException_RecursiveReadNotAllowed); + } + return false; } public void EnterWriteLock() @@ -440,8 +510,7 @@ private bool TryEnterWriteLockCore(TimeoutTracker timeout) throw new ObjectDisposedException(null); int id = Environment.CurrentManagedThreadId; - ReaderWriterCount lrwc; - bool upgradingToWrite = false; + ReaderWriterCount lrwc = null; if (!_fIsReentrant) { @@ -453,22 +522,20 @@ private bool TryEnterWriteLockCore(TimeoutTracker timeout) else if (id == _upgradeLockOwnerId) { //AU->AW case is allowed once. - upgradingToWrite = true; - EnterMyLockForUpgradeToWrite(); + if (!TryEnterMyLockForUpgradeToWrite(timeout)) + { + goto TimeoutAfterAnyWait; + } } else { - EnterMyLockForEnterWrite(); + if (!TryEnterMyLockForEnterWrite(timeout)) + { + goto TimeoutAfterAnyWait; + } } - lrwc = GetThreadRWCount(true); - - //Can't acquire write lock with reader lock held. - if (lrwc != null && lrwc.readercount > 0) - { - ExitMyLock(); - throw new LockRecursionException(SR.LockRecursionException_WriteAfterReadNotAllowed); - } + lrwc = GetThreadRWCount(dontAllocate: true); } else { @@ -478,40 +545,84 @@ private bool TryEnterWriteLockCore(TimeoutTracker timeout) } else if (id == _upgradeLockOwnerId) { - EnterMyLockForUpgradeToWrite(); + if (!TryEnterMyLockForUpgradeToWrite(timeout)) + { + goto TimeoutAfterAnyWait; + } } else { - EnterMyLockForEnterWrite(); + if (!TryEnterMyLockForEnterWrite(timeout)) + { + goto TimeoutAfterAnyWait; + } } - lrwc = GetThreadRWCount(false); - - if (id == _writeLockOwnerId) - { - lrwc.writercount++; - ExitMyLock(); - return true; - } - else if (id == _upgradeLockOwnerId) + Debug.Assert(lrwc == null); + try { - upgradingToWrite = true; + lrwc = GetThreadRWCount(dontAllocate: false); // allocation may throw } - else if (lrwc.readercount > 0) + finally { - //Write locks may not be acquired if only read locks have been - //acquired. - ExitMyLock(); - throw new LockRecursionException(SR.LockRecursionException_WriteAfterReadNotAllowed); + if (lrwc == null) + { + ExitMyLock(); + } } } - bool retVal = true; + bool waiterSignaled = false; int spinCount = 0; for (; ;) { - if (IsWriterAcquired()) + // The following has to be done in every iteration of this spin loop because the RW lock state could have + // changed due to reentrancy by message pumping + bool upgradingToWrite = false; + if (!_fIsReentrant) + { + // The following are checked before the loop and before going around the loop + Debug.Assert(id != _writeLockOwnerId); + Debug.Assert(lrwc == null || !IsRwHashEntryChanged(lrwc)); + + //Can't acquire write lock with reader lock held. + if (lrwc != null && lrwc.readercount > 0) + { + ExitMyLock(waiterSignaled); + throw new LockRecursionException(SR.LockRecursionException_WriteAfterReadNotAllowed); + } + + if (id == _upgradeLockOwnerId) + { + upgradingToWrite = true; + } + } + else + { + // The following are checked before the loop and before going around the loop + Debug.Assert(!IsRwHashEntryChanged(lrwc)); + + if (id == _writeLockOwnerId) + { + lrwc.writercount++; + ExitMyLock(waiterSignaled); + return true; + } + else if (id == _upgradeLockOwnerId) + { + upgradingToWrite = true; + } + else if (lrwc.readercount > 0) + { + //Write locks may not be acquired if only read locks have been + //acquired. + ExitMyLock(waiterSignaled); + throw new LockRecursionException(SR.LockRecursionException_WriteAfterReadNotAllowed); + } + } + + if (IsWriterAcquirable()) { // Good case, there is no contention, we are basically done SetWriterAcquired(); @@ -533,24 +644,15 @@ private bool TryEnterWriteLockCore(TimeoutTracker timeout) SetWriterAcquired(); // indicate we have a writer. break; } - else if (readercount == 2) + else if (readercount == 2 && lrwc != null && lrwc.readercount > 0) { - if (lrwc != null) - { - if (IsRwHashEntryChanged(lrwc)) - lrwc = GetThreadRWCount(false); - - if (lrwc.readercount > 0) - { - //This check is needed for EU->ER->EW case, as the owner count will be two. - Debug.Assert(_fIsReentrant); - Debug.Assert(_fUpgradeThreadHoldingRead); + //This check is needed for EU->ER->EW case, as the owner count will be two. + Debug.Assert(_fIsReentrant); + Debug.Assert(_fUpgradeThreadHoldingRead); - //Good case again, there is just one upgrader, and no readers. - SetWriterAcquired(); // indicate we have a writer. - break; - } - } + //Good case again, there is just one upgrader, and no readers. + SetWriterAcquired(); // indicate we have a writer. + break; } } @@ -562,49 +664,114 @@ private bool TryEnterWriteLockCore(TimeoutTracker timeout) if (spinCount < MaxSpinCount && ShouldSpinForEnterAnyWrite(upgradingToWrite)) { - ExitMyLock(); + ExitMyLock(waiterSignaled); spinCount++; SpinWait(spinCount); + + bool acquiredMyLock; if (upgradingToWrite) { - EnterMyLockForUpgradeToWrite(); + acquiredMyLock = TryEnterMyLockForUpgradeToWrite(timeout); } else { - EnterMyLockForEnterWrite(); + acquiredMyLock = TryEnterMyLockForEnterWrite(timeout); + } + + if (acquiredMyLock) + { + waiterSignaled = false; + goto TryAgain; } - continue; + goto TimeoutAfterAnyWait; } if (upgradingToWrite) { if (_waitUpgradeEvent == null) // Create the needed event { - LazyCreateEvent(ref _waitUpgradeEvent, EnterLockType.UpgradeToWrite); - continue; // since we left the lock, start over. + if (TryLazyCreateEvent(ref _waitUpgradeEvent, timeout, EnterLockType.UpgradeToWrite, waiterSignaled)) + { + waiterSignaled = false; + goto TryAgain; // since we left the lock, start over. + } + goto TimeoutAfterAnyWait; } Debug.Assert(_numWriteUpgradeWaiters == 0, "There can be at most one thread with the upgrade lock held."); - retVal = WaitOnEvent(_waitUpgradeEvent, ref _numWriteUpgradeWaiters, timeout, EnterLockType.UpgradeToWrite); - - //The lock is not held in case of failure. - if (!retVal) - return false; + waiterSignaled = + WaitOnEvent( + _waitUpgradeEvent, + ref _numWriteUpgradeWaiters, + timeout, + EnterLockType.UpgradeToWrite, + waiterSignaled); } else { // Drat, we need to wait. Mark that we have waiters and wait. if (_writeEvent == null) // create the needed event. { - LazyCreateEvent(ref _writeEvent, EnterLockType.Write); - continue; // since we left the lock, start over. + if (TryLazyCreateEvent(ref _writeEvent, timeout, EnterLockType.Write, waiterSignaled)) + { + waiterSignaled = false; + goto TryAgain; // since we left the lock, start over. + } + goto TimeoutAfterAnyWait; } - retVal = WaitOnEvent(_writeEvent, ref _numWriteWaiters, timeout, EnterLockType.Write); - //The lock is not held in case of failure. - if (!retVal) - return false; + waiterSignaled = + WaitOnEvent( + _writeEvent, + ref _numWriteWaiters, + timeout, + EnterLockType.Write, + waiterSignaled); + } + + //The lock is not held in case of failure. + if (!waiterSignaled) + { + goto TimeoutAfterAnyWait; + } + +TryAgain: + ; // fix incorrect autoformat indenting for the next comment + + // The following has to be done in every iteration of this spin loop because the RW lock state could have + // changed due to reentrancy by message pumping + if (!_fIsReentrant) + { + if (id == _writeLockOwnerId) + { + //Check for AW->AW + ExitMyLock(waiterSignaled); + throw new LockRecursionException(SR.LockRecursionException_RecursiveWriteNotAllowed); + } + + if (lrwc == null || IsRwHashEntryChanged(lrwc)) + { + lrwc = GetThreadRWCount(dontAllocate: true); + } + } + else + { + if (IsRwHashEntryChanged(lrwc)) + { + lrwc = null; + try + { + lrwc = GetThreadRWCount(dontAllocate: false); // allocation may throw + } + finally + { + if (lrwc == null) + { + ExitMyLock(waiterSignaled); + } + } + } } } @@ -612,8 +779,6 @@ private bool TryEnterWriteLockCore(TimeoutTracker timeout) if (_fIsReentrant) { - if (IsRwHashEntryChanged(lrwc)) - lrwc = GetThreadRWCount(false); lrwc.writercount++; } @@ -622,6 +787,36 @@ private bool TryEnterWriteLockCore(TimeoutTracker timeout) _writeLockOwnerId = id; return true; + +TimeoutAfterAnyWait: + Debug.Assert(timeout.IsExpired); + + // Message pumping during a previous spin wait may have changed the RW lock state. Check if we should throw + // instead of returning false. + + if (!_fIsReentrant) + { + if (id == _writeLockOwnerId) + { + // AW->AW + throw new LockRecursionException(SR.LockRecursionException_RecursiveWriteNotAllowed); + } + } + else if (id == _writeLockOwnerId || id == _upgradeLockOwnerId) + { + return false; + } + + if (lrwc == null || IsRwHashEntryChanged(lrwc)) + { + lrwc = GetThreadRWCount(dontAllocate: true); + } + if (lrwc != null && lrwc.readercount > 0) + { + // Write locks may not be acquired if only read locks have been acquired + throw new LockRecursionException(SR.LockRecursionException_WriteAfterReadNotAllowed); + } + return false; } public void EnterUpgradeableReadLock() @@ -650,7 +845,7 @@ private bool TryEnterUpgradeableReadLockCore(TimeoutTracker timeout) throw new ObjectDisposedException(null); int id = Environment.CurrentManagedThreadId; - ReaderWriterCount lrwc; + ReaderWriterCount lrwc = null; if (!_fIsReentrant) { @@ -665,53 +860,87 @@ private bool TryEnterUpgradeableReadLockCore(TimeoutTracker timeout) throw new LockRecursionException(SR.LockRecursionException_UpgradeAfterWriteNotAllowed); } - EnterMyLockForEnterAnyRead(); - lrwc = GetThreadRWCount(true); - //Can't acquire upgrade lock with reader lock held. - if (lrwc != null && lrwc.readercount > 0) + if (!TryEnterMyLockForEnterAnyRead(timeout)) { - ExitMyLock(); - throw new LockRecursionException(SR.LockRecursionException_UpgradeAfterReadNotAllowed); + goto TimeoutAfterAnyWait; } + + lrwc = GetThreadRWCount(dontAllocate: true); } else { - EnterMyLockForEnterAnyRead(); - lrwc = GetThreadRWCount(false); - - if (id == _upgradeLockOwnerId) + if (!TryEnterMyLockForEnterAnyRead(timeout)) { - lrwc.upgradecount++; - ExitMyLock(); - return true; + goto TimeoutAfterAnyWait; } - else if (id == _writeLockOwnerId) + + Debug.Assert(lrwc == null); + try { - //Write lock is already held, Just update the global state - //to show presence of upgrader. - Debug.Assert((_owners & WRITER_HELD) > 0); - _owners++; - _upgradeLockOwnerId = id; - lrwc.upgradecount++; - if (lrwc.readercount > 0) - _fUpgradeThreadHoldingRead = true; - ExitMyLock(); - return true; + lrwc = GetThreadRWCount(dontAllocate: false); // allocation may throw } - else if (lrwc.readercount > 0) + finally { - //Upgrade locks may not be acquired if only read locks have been - //acquired. - ExitMyLock(); - throw new LockRecursionException(SR.LockRecursionException_UpgradeAfterReadNotAllowed); + if (lrwc == null) + { + ExitMyLock(); + } } } - bool retVal = true; int spinCount = 0; for (; ;) { + // The following has to be done in every iteration of this spin loop because the RW lock state could have + // changed due to reentrancy by message pumping + if (!_fIsReentrant) + { + // The following are checked before the loop and before going around the loop + Debug.Assert(id != _upgradeLockOwnerId); + Debug.Assert(id != _writeLockOwnerId); + Debug.Assert(lrwc == null || !IsRwHashEntryChanged(lrwc)); + + //Can't acquire upgrade lock with reader lock held. + if (lrwc != null && lrwc.readercount > 0) + { + ExitMyLock(); + throw new LockRecursionException(SR.LockRecursionException_UpgradeAfterReadNotAllowed); + } + } + else + { + // The following are checked before the loop and before going around the loop + Debug.Assert(!IsRwHashEntryChanged(lrwc)); + + if (id == _upgradeLockOwnerId) + { + lrwc.upgradecount++; + ExitMyLock(); + return true; + } + else if (id == _writeLockOwnerId) + { + //Write lock is already held, Just update the global state + //to show presence of upgrader. + Debug.Assert((_owners & WRITER_HELD) > 0); + _owners++; + _upgradeLockOwnerId = id; + lrwc.upgradecount++; + if (lrwc.readercount > 0) + _fUpgradeThreadHoldingRead = true; + ExitMyLock(); + return true; + } + else if (lrwc.readercount > 0) + { + //Upgrade locks may not be acquired if only read locks have been + //acquired. + ExitMyLock(); + throw new LockRecursionException(SR.LockRecursionException_UpgradeAfterReadNotAllowed); + } + } + //Once an upgrade lock is taken, it's like having a reader lock held //until upgrade or downgrade operations are performed. @@ -719,7 +948,12 @@ private bool TryEnterUpgradeableReadLockCore(TimeoutTracker timeout) { _owners++; _upgradeLockOwnerId = id; - break; + if (_fIsReentrant) + { + lrwc.upgradecount++; + } + ExitMyLock(); + return true; } if (timeout.IsExpired) @@ -733,35 +967,109 @@ private bool TryEnterUpgradeableReadLockCore(TimeoutTracker timeout) ExitMyLock(); spinCount++; SpinWait(spinCount); - EnterMyLockForEnterAnyRead(); - continue; + + if (TryEnterMyLockForEnterAnyRead(timeout)) + { + goto TryAgain; + } + goto TimeoutAfterAnyWait; } // Drat, we need to wait. Mark that we have waiters and wait. if (_upgradeEvent == null) // Create the needed event { - LazyCreateEvent(ref _upgradeEvent, EnterLockType.UpgradeableRead); - continue; // since we left the lock, start over. + if (TryLazyCreateEvent(ref _upgradeEvent, timeout, EnterLockType.UpgradeableRead)) + { + goto TryAgain; // since we left the lock, start over. + } + goto TimeoutAfterAnyWait; } //Only one thread with the upgrade lock held can proceed. - retVal = WaitOnEvent(_upgradeEvent, ref _numUpgradeWaiters, timeout, EnterLockType.UpgradeableRead); - if (!retVal) - return false; + if (!WaitOnEvent(_upgradeEvent, ref _numUpgradeWaiters, timeout, EnterLockType.UpgradeableRead)) + { + goto TimeoutAfterAnyWait; + } + +TryAgain: + ; // fix incorrect autoformat indenting for the next comment + + // The following has to be done in every iteration of this spin loop because the RW lock state could have + // changed due to reentrancy by message pumping + if (!_fIsReentrant) + { + if (id == _upgradeLockOwnerId) + { + //Check for AU->AU + ExitMyLock(); + throw new LockRecursionException(SR.LockRecursionException_RecursiveUpgradeNotAllowed); + } + else if (id == _writeLockOwnerId) + { + //Check for AU->AW + ExitMyLock(); + throw new LockRecursionException(SR.LockRecursionException_UpgradeAfterWriteNotAllowed); + } + + if (lrwc == null || IsRwHashEntryChanged(lrwc)) + { + lrwc = GetThreadRWCount(dontAllocate: true); + } + } + else + { + if (IsRwHashEntryChanged(lrwc)) + { + lrwc = null; + try + { + lrwc = GetThreadRWCount(dontAllocate: false); // allocation may throw + } + finally + { + if (lrwc == null) + { + ExitMyLock(); + } + } + } + } } - if (_fIsReentrant) +TimeoutAfterAnyWait: + Debug.Assert(timeout.IsExpired); + + // Message pumping during a previous spin wait may have changed the RW lock state. Check if we should throw + // instead of returning false. + + if (!_fIsReentrant) { - //The lock may have been dropped getting here, so make a quick check to see whether some other - //thread did not grab the entry. - if (IsRwHashEntryChanged(lrwc)) - lrwc = GetThreadRWCount(false); - lrwc.upgradecount++; + if (id == _upgradeLockOwnerId) + { + // AU->AU + throw new LockRecursionException(SR.LockRecursionException_RecursiveUpgradeNotAllowed); + } + else if (id == _writeLockOwnerId) + { + // AU->AW + throw new LockRecursionException(SR.LockRecursionException_UpgradeAfterWriteNotAllowed); + } + } + else if (id == _upgradeLockOwnerId || id == _writeLockOwnerId) + { + return false; } - ExitMyLock(); - - return true; + if (lrwc == null || IsRwHashEntryChanged(lrwc)) + { + lrwc = GetThreadRWCount(dontAllocate: true); + } + if (lrwc != null && lrwc.readercount > 0) + { + // Upgrade locks may not be acquired if only read locks have been acquired + throw new LockRecursionException(SR.LockRecursionException_WriteAfterReadNotAllowed); + } + return false; } public void ExitReadLock() @@ -770,7 +1078,7 @@ public void ExitReadLock() EnterMyLockForExitAnyRead(); - lrwc = GetThreadRWCount(true); + lrwc = GetThreadRWCount(dontAllocate: true); if (lrwc == null || lrwc.readercount < 1) { @@ -819,7 +1127,7 @@ public void ExitWriteLock() else { EnterMyLockForEnterRecursiveWriteOrExitWrite(); - lrwc = GetThreadRWCount(false); + lrwc = GetThreadRWCount(dontAllocate: false); if (lrwc == null) { @@ -866,7 +1174,7 @@ public void ExitUpgradeableReadLock() else { EnterMyLockForExitAnyRead(); - lrwc = GetThreadRWCount(true); + lrwc = GetThreadRWCount(dontAllocate: true); if (lrwc == null) { @@ -903,41 +1211,52 @@ public void ExitUpgradeableReadLock() /// while holding a spin lock). If all goes well, reenter the lock and /// set 'waitEvent' /// - private void LazyCreateEvent(ref EventWaitHandle waitEvent, EnterLockType enterLockType) + private bool TryLazyCreateEvent( + ref EventWaitHandle waitEvent, + TimeoutTracker timeout, + EnterLockType enterLockType, + bool writeWaiterSignaled = false) { #if DEBUG Debug.Assert(MyLockHeld); Debug.Assert(waitEvent == null); #endif - ExitMyLock(); + ExitMyLock(writeWaiterSignaled); var newEvent = new EventWaitHandle( false, enterLockType == EnterLockType.Read ? EventResetMode.ManualReset : EventResetMode.AutoReset); + bool acquiredMyLock; switch (enterLockType) { case EnterLockType.Read: case EnterLockType.UpgradeableRead: - EnterMyLockForEnterAnyRead(isForWait: true); + acquiredMyLock = TryEnterMyLockForEnterAnyRead(timeout, isForWait: true); break; case EnterLockType.Write: - EnterMyLockForEnterWrite(isForWait: true); + acquiredMyLock = TryEnterMyLockForEnterWrite(timeout, isForWait: true); break; default: Debug.Assert(enterLockType == EnterLockType.UpgradeToWrite); - EnterMyLockForUpgradeToWrite(isForWait: true); + acquiredMyLock = TryEnterMyLockForUpgradeToWrite(timeout, isForWait: true); break; } + if (!acquiredMyLock) + { + return false; + } + if (waitEvent == null) // maybe someone snuck in. waitEvent = newEvent; else newEvent.Dispose(); + return true; } /// @@ -948,7 +1267,8 @@ private bool WaitOnEvent( EventWaitHandle waitEvent, ref uint numWaiters, TimeoutTracker timeout, - EnterLockType enterLockType) + EnterLockType enterLockType, + bool writeWaiterSignaled = false) { #if DEBUG Debug.Assert(MyLockHeld); @@ -963,31 +1283,34 @@ private bool WaitOnEvent( if (_numWriteUpgradeWaiters == 1) SetUpgraderWaiting(); - bool waitSuccessful = false; - ExitMyLock(); // Do the wait outside of any lock + ExitMyLock(writeWaiterSignaled); // Do the wait outside of any lock + bool waitSuccessful = false; try { waitSuccessful = waitEvent.WaitOne(timeout.RemainingMilliseconds); } finally { + var infiniteTimeout = new TimeoutTracker(Timeout.Infinite); + bool acquiredMyLock; switch (enterLockType) { case EnterLockType.Read: case EnterLockType.UpgradeableRead: - EnterMyLockForEnterAnyRead(); + acquiredMyLock = TryEnterMyLockForEnterAnyRead(infiniteTimeout); break; case EnterLockType.Write: - EnterMyLockForEnterWrite(); + acquiredMyLock = TryEnterMyLockForEnterWrite(infiniteTimeout); break; default: Debug.Assert(enterLockType == EnterLockType.UpgradeToWrite); - EnterMyLockForUpgradeToWrite(); + acquiredMyLock = TryEnterMyLockForUpgradeToWrite(infiniteTimeout); break; } + Debug.Assert(acquiredMyLock); --numWaiters; @@ -1001,14 +1324,7 @@ private bool WaitOnEvent( if (!waitSuccessful) // We may also be about to throw for some reason. Exit myLock. { - if (enterLockType >= EnterLockType.Write) - { - // Write waiters block read waiters from acquiring the lock. Since this was the last write waiter, try - // to wake up the appropriate read waiters. - ExitAndWakeUpAppropriateReadWaiters(); - } - else - ExitMyLock(); + ExitMyLock(enterLockType >= EnterLockType.Write); } } return waitSuccessful; @@ -1093,7 +1409,7 @@ private void ExitAndWakeUpAppropriateReadWaiters() _upgradeEvent.Set(); //release one upgrader. } - private bool IsWriterAcquired() + private bool IsWriterAcquirable() { return (_owners & ~WAITING_WRITERS) == 0; } @@ -1178,15 +1494,12 @@ private bool TryEnterMyLock() } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void EnterMyLockForEnterAnyRead(bool isForWait = false) + private bool TryEnterMyLockForEnterAnyRead(TimeoutTracker timeout, bool isForWait = false) { - if (!TryEnterMyLock()) - { - EnterMyLockSpinForEnterAnyRead(isForWait); - } + return TryEnterMyLock() || TryEnterMyLockSpinForEnterAnyRead(timeout, isForWait); } - private void EnterMyLockSpinForEnterAnyRead(bool isForWait) + private bool TryEnterMyLockSpinForEnterAnyRead(TimeoutTracker timeout, bool isForWait) { if (!isForWait) { @@ -1200,11 +1513,18 @@ private void EnterMyLockSpinForEnterAnyRead(bool isForWait) { if (_myLock == 0 && TryEnterMyLock()) { - return; + return true; } continue; } + // When deprioritized, the operation would not make progress on the RW lock, so if the timeout has expired, + // don't bother spinning for _myLock + if (timeout.IsExpired) + { + return false; + } + // It's possible for an EnterMyLock* thread to be deprioritized for an extended duration. It's undesirable // for a deprioritized thread to keep waking up to spin despite a Sleep(1) when a large number of such // threads are involved. After a threshold of Sleep(1)s, ignore the deprioritization and enter _myLock to @@ -1216,7 +1536,7 @@ private void EnterMyLockSpinForEnterAnyRead(bool isForWait) } } - EnterMyLockSpinForWait(); + return TryEnterMyLockSpinForWait(timeout); } [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -1250,15 +1570,12 @@ private void EnterMyLockSpinForExitAnyRead() } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void EnterMyLockForEnterWrite(bool isForWait = false) + private bool TryEnterMyLockForEnterWrite(TimeoutTracker timeout, bool isForWait = false) { - if (!TryEnterMyLock()) - { - EnterMyLockSpinForEnterWrite(isForWait); - } + return TryEnterMyLock() || TryEnterMyLockSpinForEnterWrite(timeout, isForWait); } - private void EnterMyLockSpinForEnterWrite(bool isForWait) + private bool TryEnterMyLockSpinForEnterWrite(TimeoutTracker timeout, bool isForWait) { // Writers are typically much less frequent and much less in number than readers. Waiting writers take precedence // over new read attempts in order to let current readers release their lock and allow a writer to obtain the lock. @@ -1272,6 +1589,7 @@ private void EnterMyLockSpinForEnterWrite(bool isForWait) // to readers. Interlocked.Add(ref _enterMyLockDeprioritizationState, DeprioritizeEnterReadIncrement); + bool acquiredMyLock; if (!isForWait) { int pc = ProcessorCount; @@ -1284,11 +1602,20 @@ private void EnterMyLockSpinForEnterWrite(bool isForWait) { if (_myLock == 0 && TryEnterMyLock()) { - goto Entered; + acquiredMyLock = true; + goto Exit; } continue; } + // When deprioritized, the operation would not make progress on the RW lock, so if the timeout has expired, + // don't bother spinning for _myLock + if (timeout.IsExpired) + { + acquiredMyLock = false; + goto Exit; + } + // It's possible for an EnterMyLock* thread to be deprioritized for an extended duration. It's undesirable // for a deprioritized thread to keep waking up to spin despite a Sleep(1) when a large number of such // threads are involved. After a threshold of Sleep(1)s, ignore the deprioritization and enter _myLock to @@ -1300,22 +1627,20 @@ private void EnterMyLockSpinForEnterWrite(bool isForWait) } } - EnterMyLockSpinForWait(); + acquiredMyLock = TryEnterMyLockSpinForWait(timeout); -Entered: +Exit: Interlocked.Add(ref _enterMyLockDeprioritizationState, -DeprioritizeEnterReadIncrement); + return acquiredMyLock; } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void EnterMyLockForUpgradeToWrite(bool isForWait = false) + private bool TryEnterMyLockForUpgradeToWrite(TimeoutTracker timeout, bool isForWait = false) { - if (!TryEnterMyLock()) - { - EnterMyLockSpinForUpgradeToWrite(isForWait); - } + return TryEnterMyLock() || TryEnterMyLockSpinForUpgradeToWrite(timeout, isForWait); } - private void EnterMyLockSpinForUpgradeToWrite(bool isForWait) + private bool TryEnterMyLockSpinForUpgradeToWrite(TimeoutTracker timeout, bool isForWait) { // A read lock is held and an exit-read is not nearby, so deprioritize enter-write threads as they will not be able // to make progress. This thread also intends to enter a write lock, so deprioritize enter-read threads as well, see @@ -1324,6 +1649,7 @@ private void EnterMyLockSpinForUpgradeToWrite(bool isForWait) ref _enterMyLockDeprioritizationState, DeprioritizeEnterReadIncrement + DeprioritizeEnterWriteIncrement); + bool acquiredMyLock; if (!isForWait) { int pc = ProcessorCount; @@ -1336,11 +1662,20 @@ private void EnterMyLockSpinForUpgradeToWrite(bool isForWait) { if (_myLock == 0 && TryEnterMyLock()) { - goto Entered; + acquiredMyLock = true; + goto Exit; } continue; } + // When deprioritized, the operation would not make progress on the RW lock, so if the timeout has expired, + // don't bother spinning for _myLock + if (timeout.IsExpired) + { + acquiredMyLock = false; + goto Exit; + } + // It's possible for an EnterMyLock* thread to be deprioritized for an extended duration. It's undesirable // for a deprioritized thread to keep waking up to spin despite a Sleep(1) when a large number of such // threads are involved. After a threshold of Sleep(1)s, ignore the deprioritization and enter _myLock to @@ -1352,12 +1687,13 @@ private void EnterMyLockSpinForUpgradeToWrite(bool isForWait) } } - EnterMyLockSpinForWait(); + acquiredMyLock = TryEnterMyLockSpinForWait(timeout); -Entered: +Exit: Interlocked.Add( ref _enterMyLockDeprioritizationState, -(DeprioritizeEnterReadIncrement + DeprioritizeEnterWriteIncrement)); + return acquiredMyLock; } [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -1393,7 +1729,7 @@ private void EnterMyLockSpinForEnterRecursiveWriteOrExitWrite() -(DeprioritizeEnterReadIncrement + DeprioritizeEnterWriteIncrement)); } - private void EnterMyLockSpinForWait() + private bool TryEnterMyLockSpinForWait(TimeoutTracker timeout) { // It is likely that this thread will enter a wait state after entering _myLock, so ignore deprioritizations to let // this thread acquire _myLock and hopefully stop spinning on the RW lock @@ -1404,7 +1740,12 @@ private void EnterMyLockSpinForWait() if (_myLock == 0 && TryEnterMyLock()) { - break; + return true; + } + + if (timeout.IsExpired) + { + return false; } } } @@ -1415,6 +1756,25 @@ private void ExitMyLock() Volatile.Write(ref _myLock, 0); } + private void ExitMyLock(bool wakeUpReadWaiters) + { + if (wakeUpReadWaiters) + { + // This parameter should be true when exiting _myLock after entering it for the purpose of entering a write lock + // and it was not possible to acquire the write lock, in the following cases: + // - The writer was waiting and the wait was not successful (timeout or exception terminated the wait) + // - The writer was waiting and the wait was successful (the writer was signaled) + // + // Write waiters block read waiters from acquiring the lock. Since the write lock was not acquired, if this was + // the last write waiter, it must wake up the appropriate read waiters. + ExitAndWakeUpAppropriateReadWaiters(); + } + else + { + ExitMyLock(); + } + } + #if DEBUG private bool MyLockHeld { get { return _myLock != 0; } } #endif @@ -1548,7 +1908,7 @@ public int RecursiveReadCount get { int count = 0; - ReaderWriterCount lrwc = GetThreadRWCount(true); + ReaderWriterCount lrwc = GetThreadRWCount(dontAllocate: true); if (lrwc != null) count = lrwc.readercount; @@ -1564,7 +1924,7 @@ public int RecursiveUpgradeCount { int count = 0; - ReaderWriterCount lrwc = GetThreadRWCount(true); + ReaderWriterCount lrwc = GetThreadRWCount(dontAllocate: true); if (lrwc != null) count = lrwc.upgradecount; @@ -1588,7 +1948,7 @@ public int RecursiveWriteCount { int count = 0; - ReaderWriterCount lrwc = GetThreadRWCount(true); + ReaderWriterCount lrwc = GetThreadRWCount(dontAllocate: true); if (lrwc != null) count = lrwc.writercount;