diff --git a/src/mscorlib/shared/System/Threading/ReaderWriterLockSlim.cs b/src/mscorlib/shared/System/Threading/ReaderWriterLockSlim.cs index 1136fed64d57..536c3d4debf8 100644 --- a/src/mscorlib/shared/System/Threading/ReaderWriterLockSlim.cs +++ b/src/mscorlib/shared/System/Threading/ReaderWriterLockSlim.cs @@ -56,7 +56,7 @@ public class ReaderWriterLockSlim : IDisposable private static readonly int ProcessorCount = Environment.ProcessorCount; //Specifying if locked can be reacquired recursively. - private bool _fIsReentrant; + private readonly bool _fIsReentrant; // Lock specification for myLock: This lock protects exactly the local fields associated with this // instance of ReaderWriterLockSlim. It does NOT protect the memory associated with @@ -96,7 +96,7 @@ public class ReaderWriterLockSlim : IDisposable // Every lock instance has a unique ID, which is used by ReaderWriterCount to associate itself with the lock // without holding a reference to it. private static long s_nextLockID; - private long _lockID; + private readonly long _lockID; // See comments on ReaderWriterCount. [ThreadStatic] @@ -168,8 +168,9 @@ private static bool IsRWEntryEmpty(ReaderWriterCount rwc) return false; } - private bool IsRwHashEntryChanged(ReaderWriterCount lrwc) + private bool HasRwHashEntryChanged(ReaderWriterCount lrwc) { + Debug.Assert(lrwc != null); return lrwc.lockID != _lockID; } @@ -211,67 +212,76 @@ private ReaderWriterCount GetThreadRWCount(bool dontAllocate) return empty; } - public void EnterReadLock() - { - TryEnterReadLock(-1); - } - // // Common timeout support // private struct TimeoutTracker { - private int _total; - private int _start; + private readonly int _total; + private readonly int _start; + [MethodImpl(MethodImplOptions.AggressiveInlining)] public TimeoutTracker(TimeSpan timeout) { long ltm = (long)timeout.TotalMilliseconds; if (ltm < -1 || ltm > (long)Int32.MaxValue) - throw new ArgumentOutOfRangeException(nameof(timeout)); + ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.timeout); _total = (int)ltm; - if (_total != -1 && _total != 0) + if (_total > 0) _start = Environment.TickCount; else _start = 0; } + [MethodImpl(MethodImplOptions.AggressiveInlining)] public TimeoutTracker(int millisecondsTimeout) { if (millisecondsTimeout < -1) - throw new ArgumentOutOfRangeException(nameof(millisecondsTimeout)); + ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.millisecondsTimeout); _total = millisecondsTimeout; - if (_total != -1 && _total != 0) + if (_total > 0) _start = Environment.TickCount; else _start = 0; } + [MethodImpl(MethodImplOptions.NoInlining)] + private int CalculateRemainingMilliseconds() + { + Debug.Assert(_total > 0); + + int elapsed = Environment.TickCount - _start; + // elapsed may be negative if TickCount has overflowed by 2^31 milliseconds. + if (elapsed < 0 || elapsed >= _total) + return 0; + + return _total - elapsed; + } + public int RemainingMilliseconds { + [MethodImpl(MethodImplOptions.AggressiveInlining)] get { - if (_total == -1 || _total == 0) - return _total; - - int elapsed = Environment.TickCount - _start; - // elapsed may be negative if TickCount has overflowed by 2^31 milliseconds. - if (elapsed < 0 || elapsed >= _total) - return 0; - - return _total - elapsed; + return _total <= 0 ? _total : CalculateRemainingMilliseconds(); } } public bool IsExpired { + [MethodImpl(MethodImplOptions.AggressiveInlining)] get { - return RemainingMilliseconds == 0; + return _total >= 0 && (_total == 0 || CalculateRemainingMilliseconds() == 0); } } } + public void EnterReadLock() + { + TryEnterReadLock(-1); + } + public bool TryEnterReadLock(TimeSpan timeout) { return TryEnterReadLock(new TimeoutTracker(timeout)); @@ -289,80 +299,97 @@ private bool TryEnterReadLock(TimeoutTracker timeout) private bool TryEnterReadLockCore(TimeoutTracker timeout) { - if (_fDisposed) - throw new ObjectDisposedException(null); - ReaderWriterCount lrwc = null; int id = Environment.CurrentManagedThreadId; - if (!_fIsReentrant) + if (!TryEnterMyLockForEnterAnyRead(timeout)) { - if (id == _writeLockOwnerId) - { - //Check for AW->AR - throw new LockRecursionException(SR.LockRecursionException_ReadAfterWriteNotAllowed); - } - - EnterMyLockForEnterAnyRead(); - - lrwc = GetThreadRWCount(false); + goto TimeoutAfterAnyWait; + } - //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) + Debug.Assert(lrwc == null); + try + { + lrwc = GetThreadRWCount(dontAllocate: false); // allocation may throw + } + finally + { + if (lrwc == null) { - //The upgrade lock is already held. - //Update the global read counts and exit. - - lrwc.readercount++; - _owners++; ExitMyLock(); - return true; } } - else + + int spinCount = 0; + + for (; ;) { - EnterMyLockForEnterAnyRead(); - lrwc = GetThreadRWCount(false); - if (lrwc.readercount > 0) + // 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 + + Debug.Assert(!HasRwHashEntryChanged(lrwc)); + + if (_fDisposed) { - lrwc.readercount++; ExitMyLock(); - return true; + ThrowHelper.ThrowObjectDisposedException(); } - else if (id == _upgradeLockOwnerId) + + if (!_fIsReentrant) { - //The upgrade lock is already held. - //Update the global read counts and exit. - lrwc.readercount++; - _owners++; - ExitMyLock(); - _fUpgradeThreadHoldingRead = true; - return true; + if (id == _writeLockOwnerId) + { + //Check for AW->AR + ExitMyLock(); + ThrowHelper.ThrowLockRecursionException(ExceptionResource.LockRecursionException_ReadAfterWriteNotAllowed); + } + + //Check if the reader lock is already acquired + if (lrwc.readercount > 0) + { + ExitMyLock(); + ThrowHelper.ThrowLockRecursionException(ExceptionResource.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 (id == _writeLockOwnerId) + else { - //The write lock is already held. - //Update global read counts here, - lrwc.readercount++; - _owners++; - ExitMyLock(); - return true; + 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; + } } - } - bool retVal = true; - int spinCount = 0; - - for (; ;) - { // 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 +398,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 +413,83 @@ 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; + } + break; } // 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. + } + break; } - retVal = WaitOnEvent(_readEvent, ref _numReadWaiters, timeout, EnterLockType.Read); - if (!retVal) + if (!WaitOnEvent(_readEvent, ref _numReadWaiters, timeout, EnterLockType.Read)) { - return false; + break; + } + +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 (HasRwHashEntryChanged(lrwc)) + { + lrwc = null; + try + { + lrwc = GetThreadRWCount(dontAllocate: false); // allocation may throw + } + finally + { + if (lrwc == null) + { + ExitMyLock(); + } + } } - if (IsRwHashEntryChanged(lrwc)) - lrwc = GetThreadRWCount(false); } - 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 (_fDisposed) + { + ThrowHelper.ThrowObjectDisposedException(); + } + + if (_fIsReentrant) + { + return false; + } + + if (id == _writeLockOwnerId) + { + // AW->AR + ThrowHelper.ThrowLockRecursionException(ExceptionResource.LockRecursionException_ReadAfterWriteNotAllowed); + } + + if (lrwc == null || HasRwHashEntryChanged(lrwc)) + { + lrwc = GetThreadRWCount(dontAllocate: true); + } + if (lrwc != null && lrwc.readercount > 0) + { + // AR->AR + ThrowHelper.ThrowLockRecursionException(ExceptionResource.LockRecursionException_RecursiveReadNotAllowed); + } + return false; } public void EnterWriteLock() @@ -436,39 +514,28 @@ private bool TryEnterWriteLock(TimeoutTracker timeout) private bool TryEnterWriteLockCore(TimeoutTracker timeout) { - if (_fDisposed) - throw new ObjectDisposedException(null); - int id = Environment.CurrentManagedThreadId; - ReaderWriterCount lrwc; - bool upgradingToWrite = false; + ReaderWriterCount lrwc = null; if (!_fIsReentrant) { - if (id == _writeLockOwnerId) - { - //Check for AW->AW - throw new LockRecursionException(SR.LockRecursionException_RecursiveWriteNotAllowed); - } - else if (id == _upgradeLockOwnerId) + 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,81 +545,143 @@ 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) + Debug.Assert(lrwc == null); + try { - lrwc.writercount++; - ExitMyLock(); - return true; - } - else if (id == _upgradeLockOwnerId) - { - 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 + + if (_fDisposed) { - // Good case, there is no contention, we are basically done - SetWriterAcquired(); - break; + ExitMyLock(waiterSignaled); + ThrowHelper.ThrowObjectDisposedException(); } - //Check if there is just one upgrader, and no readers. - //Assumption: Only one thread can have the upgrade lock, so the - //following check will fail for all other threads that may sneak in - //when the upgrading thread is waiting. + bool upgradingToWrite = false; + if (!_fIsReentrant) + { + Debug.Assert(lrwc == null || !HasRwHashEntryChanged(lrwc)); - if (upgradingToWrite) + if (id == _writeLockOwnerId) + { + //Check for AW->AW + ExitMyLock(waiterSignaled); + ThrowHelper.ThrowLockRecursionException(ExceptionResource.LockRecursionException_RecursiveWriteNotAllowed); + } + else if (id == _upgradeLockOwnerId) + { + upgradingToWrite = true; + } + + //Can't acquire write lock with reader lock held. + if (lrwc != null && lrwc.readercount > 0) + { + ExitMyLock(waiterSignaled); + ThrowHelper.ThrowLockRecursionException(ExceptionResource.LockRecursionException_WriteAfterReadNotAllowed); + } + } + else { - uint readercount = GetNumReaders(); + Debug.Assert(!HasRwHashEntryChanged(lrwc)); - if (readercount == 1) + if (id == _writeLockOwnerId) { - //Good case again, there is just one upgrader, and no readers. - SetWriterAcquired(); // indicate we have a writer. - break; + lrwc.writercount++; + ExitMyLock(waiterSignaled); + return true; + } + else if (id == _upgradeLockOwnerId) + { + upgradingToWrite = true; } - else if (readercount == 2) + else if (lrwc.readercount > 0) { - if (lrwc != null) + //Write locks may not be acquired if only read locks have been + //acquired. + ExitMyLock(waiterSignaled); + ThrowHelper.ThrowLockRecursionException(ExceptionResource.LockRecursionException_WriteAfterReadNotAllowed); + } + } + + do + { + if (IsWriterAcquirable()) + { + // Good case, there is no contention, we are basically done + } + else + { + //Check if there is just one upgrader, and no readers. + //Assumption: Only one thread can have the upgrade lock, so the + //following check will fail for all other threads that may sneak in + //when the upgrading thread is waiting. + + if (!upgradingToWrite) { - if (IsRwHashEntryChanged(lrwc)) - lrwc = GetThreadRWCount(false); + break; + } - 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); + uint readercount = GetNumReaders(); - //Good case again, there is just one upgrader, and no readers. - SetWriterAcquired(); // indicate we have a writer. - break; - } + if (readercount == 1) + { + //Good case again, there is just one upgrader, and no readers. + } + else if (readercount == 2 && lrwc != null && 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); + + //Good case again, there is just one upgrader, and no readers. + } + else + { + break; } } - } + + SetWriterAcquired(); + + if (_fIsReentrant) + { + lrwc.writercount++; + } + + ExitMyLock(); + + _writeLockOwnerId = id; + return true; + } while (false); if (timeout.IsExpired) { @@ -562,66 +691,144 @@ 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; + break; } 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. + } + break; } 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. + } + break; } - 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) + { + break; + } + +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 (lrwc == null || HasRwHashEntryChanged(lrwc)) + { + lrwc = GetThreadRWCount(dontAllocate: true); + } + } + else + { + if (HasRwHashEntryChanged(lrwc)) + { + lrwc = null; + try + { + lrwc = GetThreadRWCount(dontAllocate: false); // allocation may throw + } + finally + { + if (lrwc == null) + { + ExitMyLock(waiterSignaled); + } + } + } } } - Debug.Assert((_owners & WRITER_HELD) > 0); +TimeoutAfterAnyWait: + Debug.Assert(timeout.IsExpired); - if (_fIsReentrant) + // Message pumping during a previous spin wait may have changed the RW lock state. Check if we should throw + // instead of returning false. + + if (_fDisposed) { - if (IsRwHashEntryChanged(lrwc)) - lrwc = GetThreadRWCount(false); - lrwc.writercount++; + ThrowHelper.ThrowObjectDisposedException(); } - ExitMyLock(); - - _writeLockOwnerId = id; + if (!_fIsReentrant) + { + if (id == _writeLockOwnerId) + { + // AW->AW + ThrowHelper.ThrowLockRecursionException(ExceptionResource.LockRecursionException_RecursiveWriteNotAllowed); + } + } + else if (id == _writeLockOwnerId || id == _upgradeLockOwnerId) + { + return false; + } - return true; + if (lrwc == null || HasRwHashEntryChanged(lrwc)) + { + lrwc = GetThreadRWCount(dontAllocate: true); + } + if (lrwc != null && lrwc.readercount > 0) + { + // Write locks may not be acquired if only read locks have been acquired + ThrowHelper.ThrowLockRecursionException(ExceptionResource.LockRecursionException_WriteAfterReadNotAllowed); + } + return false; } public void EnterUpgradeableReadLock() @@ -646,72 +853,109 @@ private bool TryEnterUpgradeableReadLock(TimeoutTracker timeout) private bool TryEnterUpgradeableReadLockCore(TimeoutTracker timeout) { - if (_fDisposed) - throw new ObjectDisposedException(null); - int id = Environment.CurrentManagedThreadId; - ReaderWriterCount lrwc; + ReaderWriterCount lrwc = null; if (!_fIsReentrant) { - if (id == _upgradeLockOwnerId) + if (!TryEnterMyLockForEnterAnyRead(timeout)) { - //Check for AU->AU - throw new LockRecursionException(SR.LockRecursionException_RecursiveUpgradeNotAllowed); - } - else if (id == _writeLockOwnerId) - { - //Check for AU->AW - throw new LockRecursionException(SR.LockRecursionException_UpgradeAfterWriteNotAllowed); + goto TimeoutAfterAnyWait; } - EnterMyLockForEnterAnyRead(); - lrwc = GetThreadRWCount(true); - //Can't acquire upgrade lock with reader lock held. - if (lrwc != null && lrwc.readercount > 0) - { - ExitMyLock(); - throw new LockRecursionException(SR.LockRecursionException_UpgradeAfterReadNotAllowed); - } + 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 (_fDisposed) + { + ExitMyLock(); + ThrowHelper.ThrowObjectDisposedException(); + } + + if (!_fIsReentrant) + { + Debug.Assert(lrwc == null || !HasRwHashEntryChanged(lrwc)); + + if (id == _upgradeLockOwnerId) + { + //Check for AU->AU + ExitMyLock(); + ThrowHelper.ThrowLockRecursionException(ExceptionResource.LockRecursionException_RecursiveUpgradeNotAllowed); + } + + if (id == _writeLockOwnerId) + { + //Check for AU->AW + ExitMyLock(); + ThrowHelper.ThrowLockRecursionException(ExceptionResource.LockRecursionException_UpgradeAfterWriteNotAllowed); + } + + //Can't acquire upgrade lock with reader lock held. + if (lrwc != null && lrwc.readercount > 0) + { + ExitMyLock(); + ThrowHelper.ThrowLockRecursionException(ExceptionResource.LockRecursionException_UpgradeAfterReadNotAllowed); + } + } + else + { + Debug.Assert(!HasRwHashEntryChanged(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(); + ThrowHelper.ThrowLockRecursionException(ExceptionResource.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 +963,12 @@ private bool TryEnterUpgradeableReadLockCore(TimeoutTracker timeout) { _owners++; _upgradeLockOwnerId = id; - break; + if (_fIsReentrant) + { + lrwc.upgradecount++; + } + ExitMyLock(); + return true; } if (timeout.IsExpired) @@ -733,35 +982,101 @@ private bool TryEnterUpgradeableReadLockCore(TimeoutTracker timeout) ExitMyLock(); spinCount++; SpinWait(spinCount); - EnterMyLockForEnterAnyRead(); - continue; + + if (TryEnterMyLockForEnterAnyRead(timeout)) + { + goto TryAgain; + } + break; } // 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. + } + break; } //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)) + { + break; + } + +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 (lrwc == null || HasRwHashEntryChanged(lrwc)) + { + lrwc = GetThreadRWCount(dontAllocate: true); + } + } + else + { + if (HasRwHashEntryChanged(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 (_fDisposed) { - //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++; + ThrowHelper.ThrowObjectDisposedException(); } - ExitMyLock(); + if (!_fIsReentrant) + { + if (id == _upgradeLockOwnerId) + { + // AU->AU + ThrowHelper.ThrowLockRecursionException(ExceptionResource.LockRecursionException_RecursiveUpgradeNotAllowed); + } + else if (id == _writeLockOwnerId) + { + // AU->AW + ThrowHelper.ThrowLockRecursionException(ExceptionResource.LockRecursionException_UpgradeAfterWriteNotAllowed); + } + } + else if (id == _upgradeLockOwnerId || id == _writeLockOwnerId) + { + return false; + } - return true; + if (lrwc == null || HasRwHashEntryChanged(lrwc)) + { + lrwc = GetThreadRWCount(dontAllocate: true); + } + if (lrwc != null && lrwc.readercount > 0) + { + // Upgrade locks may not be acquired if only read locks have been acquired + ThrowHelper.ThrowLockRecursionException(ExceptionResource.LockRecursionException_WriteAfterReadNotAllowed); + } + return false; } public void ExitReadLock() @@ -770,13 +1085,13 @@ public void ExitReadLock() EnterMyLockForExitAnyRead(); - lrwc = GetThreadRWCount(true); + lrwc = GetThreadRWCount(dontAllocate: true); if (lrwc == null || lrwc.readercount < 1) { //You have to be holding the read lock to make this call. ExitMyLock(); - throw new SynchronizationLockException(SR.SynchronizationLockException_MisMatchedRead); + ThrowHelper.ThrowSynchronizationLockException(ExceptionResource.SynchronizationLockException_MisMatchedRead); } if (_fIsReentrant) @@ -812,25 +1127,25 @@ public void ExitWriteLock() if (Environment.CurrentManagedThreadId != _writeLockOwnerId) { //You have to be holding the write lock to make this call. - throw new SynchronizationLockException(SR.SynchronizationLockException_MisMatchedWrite); + ThrowHelper.ThrowSynchronizationLockException(ExceptionResource.SynchronizationLockException_MisMatchedWrite); } EnterMyLockForEnterRecursiveWriteOrExitWrite(); } else { EnterMyLockForEnterRecursiveWriteOrExitWrite(); - lrwc = GetThreadRWCount(false); + lrwc = GetThreadRWCount(dontAllocate: false); if (lrwc == null) { ExitMyLock(); - throw new SynchronizationLockException(SR.SynchronizationLockException_MisMatchedWrite); + ThrowHelper.ThrowSynchronizationLockException(ExceptionResource.SynchronizationLockException_MisMatchedWrite); } if (lrwc.writercount < 1) { ExitMyLock(); - throw new SynchronizationLockException(SR.SynchronizationLockException_MisMatchedWrite); + ThrowHelper.ThrowSynchronizationLockException(ExceptionResource.SynchronizationLockException_MisMatchedWrite); } lrwc.writercount--; @@ -859,25 +1174,25 @@ public void ExitUpgradeableReadLock() if (Environment.CurrentManagedThreadId != _upgradeLockOwnerId) { //You have to be holding the upgrade lock to make this call. - throw new SynchronizationLockException(SR.SynchronizationLockException_MisMatchedUpgrade); + ThrowHelper.ThrowSynchronizationLockException(ExceptionResource.SynchronizationLockException_MisMatchedUpgrade); } EnterMyLockForExitAnyRead(); } else { EnterMyLockForExitAnyRead(); - lrwc = GetThreadRWCount(true); + lrwc = GetThreadRWCount(dontAllocate: true); if (lrwc == null) { ExitMyLock(); - throw new SynchronizationLockException(SR.SynchronizationLockException_MisMatchedUpgrade); + ThrowHelper.ThrowSynchronizationLockException(ExceptionResource.SynchronizationLockException_MisMatchedUpgrade); } if (lrwc.upgradecount < 1) { ExitMyLock(); - throw new SynchronizationLockException(SR.SynchronizationLockException_MisMatchedUpgrade); + ThrowHelper.ThrowSynchronizationLockException(ExceptionResource.SynchronizationLockException_MisMatchedUpgrade); } lrwc.upgradecount--; @@ -903,41 +1218,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 +1274,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 +1290,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 +1331,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 +1416,7 @@ private void ExitAndWakeUpAppropriateReadWaiters() _upgradeEvent.Set(); //release one upgrader. } - private bool IsWriterAcquired() + private bool IsWriterAcquirable() { return (_owners & ~WAITING_WRITERS) == 0; } @@ -1178,15 +1501,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 +1520,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 +1543,7 @@ private void EnterMyLockSpinForEnterAnyRead(bool isForWait) } } - EnterMyLockSpinForWait(); + return TryEnterMyLockSpinForWait(timeout); } [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -1250,15 +1577,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 +1596,7 @@ private void EnterMyLockSpinForEnterWrite(bool isForWait) // to readers. Interlocked.Add(ref _enterMyLockDeprioritizationState, DeprioritizeEnterReadIncrement); + bool acquiredMyLock; if (!isForWait) { int pc = ProcessorCount; @@ -1284,11 +1609,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 +1634,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 +1656,7 @@ private void EnterMyLockSpinForUpgradeToWrite(bool isForWait) ref _enterMyLockDeprioritizationState, DeprioritizeEnterReadIncrement + DeprioritizeEnterWriteIncrement); + bool acquiredMyLock; if (!isForWait) { int pc = ProcessorCount; @@ -1336,11 +1669,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 +1694,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 +1736,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 +1747,12 @@ private void EnterMyLockSpinForWait() if (_myLock == 0 && TryEnterMyLock()) { - break; + return true; + } + + if (timeout.IsExpired) + { + return false; } } } @@ -1415,6 +1763,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 @@ -1448,10 +1815,10 @@ private void Dispose(bool disposing) if (disposing && !_fDisposed) { if (WaitingReadCount > 0 || WaitingUpgradeCount > 0 || WaitingWriteCount > 0) - throw new SynchronizationLockException(SR.SynchronizationLockException_IncorrectDispose); + ThrowHelper.ThrowSynchronizationLockException(ExceptionResource.SynchronizationLockException_IncorrectDispose); if (IsReadLockHeld || IsUpgradeableReadLockHeld || IsWriteLockHeld) - throw new SynchronizationLockException(SR.SynchronizationLockException_IncorrectDispose); + ThrowHelper.ThrowSynchronizationLockException(ExceptionResource.SynchronizationLockException_IncorrectDispose); if (_writeEvent != null) { @@ -1548,7 +1915,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 +1931,7 @@ public int RecursiveUpgradeCount { int count = 0; - ReaderWriterCount lrwc = GetThreadRWCount(true); + ReaderWriterCount lrwc = GetThreadRWCount(dontAllocate: true); if (lrwc != null) count = lrwc.upgradecount; @@ -1588,7 +1955,7 @@ public int RecursiveWriteCount { int count = 0; - ReaderWriterCount lrwc = GetThreadRWCount(true); + ReaderWriterCount lrwc = GetThreadRWCount(dontAllocate: true); if (lrwc != null) count = lrwc.writercount; diff --git a/src/mscorlib/src/System/ThrowHelper.cs b/src/mscorlib/src/System/ThrowHelper.cs index b4fc5a8df04b..269b6dc97958 100644 --- a/src/mscorlib/src/System/ThrowHelper.cs +++ b/src/mscorlib/src/System/ThrowHelper.cs @@ -40,6 +40,7 @@ using System.Runtime.Serialization; using System.Diagnostics; using System.Diagnostics.Contracts; +using System.Threading; namespace System { @@ -156,6 +157,7 @@ internal static void ThrowArgumentNullException(ExceptionArgument argument, Exce throw new ArgumentNullException(GetArgumentName(argument), GetResourceString(resource)); } + [MethodImpl(MethodImplOptions.NoInlining)] internal static void ThrowArgumentOutOfRangeException(ExceptionArgument argument) { throw new ArgumentOutOfRangeException(GetArgumentName(argument)); @@ -216,6 +218,11 @@ internal static void ThrowObjectDisposedException(ExceptionResource resource) throw new ObjectDisposedException(null, GetResourceString(resource)); } + internal static void ThrowObjectDisposedException() + { + throw new ObjectDisposedException(null); + } + internal static void ThrowNotSupportedException() { throw new NotSupportedException(); @@ -226,6 +233,18 @@ internal static void ThrowAggregateException(List exceptions) throw new AggregateException(exceptions); } + [MethodImpl(MethodImplOptions.NoInlining)] + internal static void ThrowLockRecursionException(ExceptionResource resource) + { + throw new LockRecursionException(GetResourceString(resource)); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + internal static void ThrowSynchronizationLockException(ExceptionResource resource) + { + throw new SynchronizationLockException(GetResourceString(resource)); + } + internal static void ThrowArgumentException_Argument_InvalidArrayType() { throw GetArgumentException(ExceptionResource.Argument_InvalidArrayType); @@ -425,6 +444,9 @@ internal enum ExceptionArgument // internal enum ExceptionResource { + None, + ObjectDisposed, + Argument_ImplementIComparable, Argument_InvalidType, Argument_InvalidArgumentForComparison, @@ -526,6 +548,19 @@ internal enum ExceptionResource InvalidOperation_HandleIsNotInitialized, AsyncMethodBuilder_InstanceNotInitialized, ArgumentNull_SafeHandle, + + LockRecursionException_ReadAfterWriteNotAllowed, + LockRecursionException_RecursiveReadNotAllowed, + LockRecursionException_RecursiveWriteNotAllowed, + LockRecursionException_WriteAfterReadNotAllowed, + LockRecursionException_RecursiveUpgradeNotAllowed, + LockRecursionException_UpgradeAfterWriteNotAllowed, + LockRecursionException_UpgradeAfterReadNotAllowed, + + SynchronizationLockException_MisMatchedRead, + SynchronizationLockException_MisMatchedWrite, + SynchronizationLockException_MisMatchedUpgrade, + SynchronizationLockException_IncorrectDispose, } }