From 08b79a30f6a48425cd2bc558f17e4e3d306dfe8b Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Thu, 19 Mar 2026 16:08:24 -0700 Subject: [PATCH] Fix lock table shared wait condition (#8707) * Fix lock table shared wait condition (#8696) * Refactor lock table waiter check * Add exclusive lock wait helper test --- weed/util/lock_table.go | 16 +++++++++++-- weed/util/lock_table_test.go | 44 ++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/weed/util/lock_table.go b/weed/util/lock_table.go index a932ae5b1..8f65aac06 100644 --- a/weed/util/lock_table.go +++ b/weed/util/lock_table.go @@ -52,6 +52,18 @@ func (lt *LockTable[T]) NewActiveLock(intention string, lockType LockType) *Acti return l } +func isNotFirstWaiter(lock *ActiveLock, entry *LockEntry) bool { + return len(entry.waiters) > 0 && lock.ID != entry.waiters[0].ID +} + +func shouldWaitForExclusiveLock(lock *ActiveLock, entry *LockEntry) bool { + return !lock.isDeleted && (isNotFirstWaiter(lock, entry) || entry.activeExclusiveLockOwnerCount > 0 || entry.activeSharedLockOwnerCount > 0) +} + +func shouldWaitForSharedLock(lock *ActiveLock, entry *LockEntry) bool { + return !lock.isDeleted && (isNotFirstWaiter(lock, entry) || entry.activeExclusiveLockOwnerCount > 0) +} + func (lt *LockTable[T]) AcquireLock(intention string, key T, lockType LockType) (lock *ActiveLock) { lt.mu.Lock() // Get or create the lock entry for the key @@ -81,11 +93,11 @@ func (lt *LockTable[T]) AcquireLock(intention string, key T, lockType LockType) } entry.waiters = append(entry.waiters, lock) if lockType == ExclusiveLock { - for !lock.isDeleted && ((len(entry.waiters) > 0 && lock.ID != entry.waiters[0].ID) || entry.activeExclusiveLockOwnerCount > 0 || entry.activeSharedLockOwnerCount > 0) { + for shouldWaitForExclusiveLock(lock, entry) { entry.cond.Wait() } } else { - for !lock.isDeleted && (len(entry.waiters) > 0 && lock.ID != entry.waiters[0].ID) || entry.activeExclusiveLockOwnerCount > 0 { + for shouldWaitForSharedLock(lock, entry) { entry.cond.Wait() } } diff --git a/weed/util/lock_table_test.go b/weed/util/lock_table_test.go index 001fa0bdf..eb5152d0b 100644 --- a/weed/util/lock_table_test.go +++ b/weed/util/lock_table_test.go @@ -40,3 +40,47 @@ func TestOrderedLock(t *testing.T) { wg.Wait() } + +func TestShouldWaitForSharedLock(t *testing.T) { + lock := &ActiveLock{ID: 2, lockType: SharedLock} + entry := &LockEntry{ + waiters: []*ActiveLock{{ID: 1}, lock}, + } + + if !shouldWaitForSharedLock(lock, entry) { + t.Fatal("shared lock should wait behind earlier waiters") + } + + entry.waiters = []*ActiveLock{lock} + entry.activeExclusiveLockOwnerCount = 1 + if !shouldWaitForSharedLock(lock, entry) { + t.Fatal("shared lock should wait while an exclusive owner is active") + } + + lock.isDeleted = true + if shouldWaitForSharedLock(lock, entry) { + t.Fatal("deleted shared lock should stop waiting even if an exclusive owner is active") + } +} + +func TestShouldWaitForExclusiveLock(t *testing.T) { + lock := &ActiveLock{ID: 2, lockType: ExclusiveLock} + entry := &LockEntry{ + waiters: []*ActiveLock{{ID: 1}, lock}, + } + + if !shouldWaitForExclusiveLock(lock, entry) { + t.Fatal("exclusive lock should wait behind earlier waiters") + } + + entry.waiters = []*ActiveLock{lock} + entry.activeSharedLockOwnerCount = 1 + if !shouldWaitForExclusiveLock(lock, entry) { + t.Fatal("exclusive lock should wait while shared owners are active") + } + + lock.isDeleted = true + if shouldWaitForExclusiveLock(lock, entry) { + t.Fatal("deleted exclusive lock should stop waiting even if shared owners are active") + } +}