Harden pkg/locks with GCNamedMutex and fix unlock ordering

Author: adkerr

pkg/locks/gc_named_locks.go

@@ -82,13 +82,12 @@ func (g *GCNamedMutex) Unlock(name string) {
 		g.m.Unlock()
 		return
 	}
+	resourceMutex.m.Unlock()
 	resourceMutex.c--
 	if resourceMutex.c == 0 {
 		delete(g.mutexes, name)
 	}
 	g.m.Unlock()
-
-	resourceMutex.m.Unlock()
 }
 
 func (g *GCNamedMutex) RLock(name string) {
@@ -111,13 +110,12 @@ func (g *GCNamedMutex) RUnlock(name string) {
 		g.m.Unlock()
 		return
 	}
+	resourceMutex.m.RUnlock()
 	resourceMutex.c--
 	if resourceMutex.c == 0 {
 		delete(g.mutexes, name)
 	}
 	g.m.Unlock()
-
-	resourceMutex.m.RUnlock()
 }
 
 // LockWithGuard acquires a write lock and returns a wrapper for convenient unlock handling.

pkg/locks/locks.go

@@ -4,79 +4,26 @@ package locks
 
 import (
 	"context"
-	"sync"
-	"sync/atomic"
 
-	. "github.com/netapp/trident/logging"
+	"github.com/netapp/trident/logging"
 )
 
-// sync.Map is like a Go map[interface{}]interface{} but is safe for concurrent use by multiple
-// goroutines without additional locking or coordination.
-var sharedLocks, waitQueue sync.Map
+// sharedLocks provides garbage-collected named locks for package-level Lock/Unlock.
+var sharedLocks = NewGCNamedMutex()
 
-// getLock returns a mutex with the specified ID.  If the lock does not exist, one is created.
-// This method uses sync.Map primitive (concurrency safe map) to defend against race conditions where multiple
-// callers try to get a lock at the same time.
-func getLock(ctx context.Context, lockID string) *sync.Mutex {
-	newLock := &sync.Mutex{}
-	storedLock, loaded := sharedLocks.LoadOrStore(lockID, newLock)
-	if !loaded {
-		Logc(ctx).WithField("lock", lockID).Debug("Created shared lock.")
-	}
-	return storedLock.(*sync.Mutex)
-}
-
-// Lock acquires a mutex with the specified ID.  The mutex does not need to exist before
-// calling this method.  The semantics of this method are intentionally identical to sync.Mutex.Lock().
+// Lock acquires a mutex with the specified ID. The mutex does not need to exist
+// before calling this method. Semantics match sync.Mutex.Lock().
+//
+// ctx is used for logging only; acquisition is not cancelled when ctx is done.
 func Lock(ctx context.Context, lockContext, lockID string) {
-	IncrementQueueSize(lockID)
-	Logc(ctx).WithField("lockContext", lockContext).Debugf("Attempting to acquire shared lock (%s); %d position in the queue.",
-		lockID, WaitQueueSize(lockID))
-
-	getLock(ctx, lockID).Lock()
-
-	DecrementQueueSize(lockID)
-	Logc(ctx).WithField("lockContext", lockContext).Debugf("Acquired shared lock (%s).", lockID)
+	logging.Logc(ctx).WithField("lockContext", lockContext).Debugf(
+		"Attempting to acquire shared lock (%s).", lockID)
+	sharedLocks.Lock(lockID)
+	logging.Logc(ctx).WithField("lockContext", lockContext).Debugf("Acquired shared lock (%s).", lockID)
 }
 
-// Unlock releases a mutex with the specified ID.  The semantics of this method are intentionally
-// identical to sync.Mutex.Unlock().
+// Unlock releases a mutex with the specified ID. Semantics match sync.Mutex.Unlock().
 func Unlock(ctx context.Context, lockContext, lockID string) {
-	getLock(ctx, lockID).Unlock()
-	Logc(ctx).WithField("lockContext", lockContext).Debugf("Released shared lock (%s).", lockID)
-}
-
-// IncrementQueueSize increments the wait queue size by 1
-func IncrementQueueSize(lockID string) {
-	currentWait, ok := waitQueue.Load(lockID)
-	if !ok {
-		valPtr := new(uint32(1))
-		waitQueue.Store(lockID, valPtr)
-	} else {
-		if ptr, ok := currentWait.(*uint32); ok {
-			atomic.AddUint32(ptr, 1)
-		}
-	}
-}
-
-// DecrementQueueSize decrements the wait queue size by 1
-func DecrementQueueSize(lockID string) {
-	currentWait, ok := waitQueue.Load(lockID)
-	if ok {
-		if ptr, ok := currentWait.(*uint32); ok {
-			if atomic.LoadUint32(ptr) > 0 {
-				atomic.AddUint32(ptr, ^uint32(0))
-			}
-		}
-	}
-}
-
-// WaitQueueSize returns the wait queue size
-func WaitQueueSize(lockID string) uint32 {
-	currentWait, ok := waitQueue.Load(lockID)
-	if !ok {
-		return 0
-	}
-
-	return atomic.LoadUint32(currentWait.(*uint32))
+	sharedLocks.Unlock(lockID)
+	logging.Logc(ctx).WithField("lockContext", lockContext).Debugf("Released shared lock (%s).", lockID)
 }