diff --git a/absl/base/internal/spinlock.cc b/absl/base/internal/spinlock.cc index a7d44f3e..35c0696a 100644 --- a/absl/base/internal/spinlock.cc +++ b/absl/base/internal/spinlock.cc @@ -125,8 +125,9 @@ void SpinLock::SlowLock() { // it as having a sleeper. if ((lock_value & kWaitTimeMask) == 0) { // Here, just "mark" that the thread is going to sleep. Don't store the - // lock wait time in the lock as that will cause the current lock - // owner to think it experienced contention. + // lock wait time in the lock -- the lock word stores the amount of time + // that the current holder waited before acquiring the lock, not the wait + // time of any thread currently waiting to acquire it. if (lockword_.compare_exchange_strong( lock_value, lock_value | kSpinLockSleeper, std::memory_order_relaxed, std::memory_order_relaxed)) { @@ -140,6 +141,14 @@ void SpinLock::SlowLock() { // this thread obtains the lock. lock_value = TryLockInternal(lock_value, wait_cycles); continue; // Skip the delay at the end of the loop. + } else if ((lock_value & kWaitTimeMask) == 0) { + // The lock is still held, without a waiter being marked, but something + // else about the lock word changed, causing our CAS to fail. For + // example, a new lock holder may have acquired the lock with + // kSpinLockDisabledScheduling set, whereas the previous holder had not + // set that flag. In this case, attempt again to mark ourselves as a + // waiter. + continue; } } diff --git a/absl/base/internal/spinlock.h b/absl/base/internal/spinlock.h index dce1c854..c73b5e09 100644 --- a/absl/base/internal/spinlock.h +++ b/absl/base/internal/spinlock.h @@ -137,8 +137,20 @@ class ABSL_LOCKABLE SpinLock { // // bit[0] encodes whether a lock is being held. // bit[1] encodes whether a lock uses cooperative scheduling. - // bit[2] encodes whether a lock disables scheduling. + // bit[2] encodes whether the current lock holder disabled scheduling when + // acquiring the lock. Only set when kSpinLockHeld is also set. // bit[3:31] encodes time a lock spent on waiting as a 29-bit unsigned int. + // This is set by the lock holder to indicate how long it waited on + // the lock before eventually acquiring it. The number of cycles is + // encoded as a 29-bit unsigned int, or in the case that the current + // holder did not wait but another waiter is queued, the LSB + // (kSpinLockSleeper) is set. The implementation does not explicitly + // track the number of queued waiters beyond this. It must always be + // assumed that waiters may exist if the current holder was required to + // queue. + // + // Invariant: if the lock is not held, the value is either 0 or + // kSpinLockCooperative. static constexpr uint32_t kSpinLockHeld = 1; static constexpr uint32_t kSpinLockCooperative = 2; static constexpr uint32_t kSpinLockDisabledScheduling = 4;