fix(deps-dev): fixup mutex for make check-locks (#2920)

internal/blobtesting/faulty.go

@@ -25,13 +25,14 @@ const (
 type FaultyStorage struct {
 	base blob.Storage
 
-	fault.Set
+	*fault.Set
 }
 
 // NewFaultyStorage creates new Storage with fault injection.
 func NewFaultyStorage(base blob.Storage) *FaultyStorage {
 	return &FaultyStorage{
 		base: base,
+		Set:  fault.NewSet(),
 	}
 }
 

internal/fault/fault_set.go

@@ -16,16 +16,21 @@ type Method int
 
 // Set encapsulates a set of faults.
 type Set struct {
-	mu          sync.Mutex
-	faults      map[Method][]*Fault
+	mu sync.Locker
+	// +checklocks:mu
+	faults map[Method][]*Fault
+	// +checklocks:mu
 	callCounter map[Method]int
 }
 
-func (s *Set) ensureInitialized() {
-	if s.faults == nil {
-		s.faults = map[Method][]*Fault{}
-		s.callCounter = map[Method]int{}
+func NewSet() *Set {
+	q := &Set{
+		mu:          &sync.Mutex{},
+		faults:      map[Method][]*Fault{},
+		callCounter: map[Method]int{},
 	}
+
+	return q
 }
 
 // AddFault adds a new fault for a given method.
@@ -33,8 +38,6 @@ func (s *Set) AddFault(method Method) *Fault {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
-	s.ensureInitialized()
-
 	f := New()
 	s.faults[method] = append(s.faults[method], f)
 
@@ -46,8 +49,6 @@ func (s *Set) AddFaults(method Method, faults ...*Fault) {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
-	s.ensureInitialized()
-
 	s.faults[method] = append(s.faults[method], faults...)
 }
 
@@ -73,10 +74,10 @@ func (s *Set) VerifyAllFaultsExercised(t *testing.T) {
 
 // GetNextFault returns the error message to return on next fault.
 func (s *Set) GetNextFault(ctx context.Context, method Method, args ...interface{}) (bool, error) {
+	// Lock set for map accesses.  Call counters will be updated for the fault-set, and the fault for the fault-set method
+	// will be gotten.
 	s.mu.Lock()
 
-	s.ensureInitialized()
-
 	s.callCounter[method]++
 
 	faults := s.faults[method]
@@ -86,31 +87,53 @@ func (s *Set) GetNextFault(ctx context.Context, method Method, args ...interface
 		return false, nil
 	}
 
+	// Access the "next" fault.  The fault at the end of the queue
 	f := faults[0]
+	// `fault` comes from `s.faults` so nested locks held at this point.
+	f.mu.Lock()
+
+	// Count down repeat count in fault.
 	if f.repeatCount > 0 {
 		f.repeatCount--
 		log(ctx).Debugf("will repeat %v more times the fault for %v %v", f.repeatCount, method, args)
 	} else {
+		// `repeatCount` == 0.  Remove the fault if there are faults remaining in the queue ...
 		if remaining := faults[1:]; len(remaining) > 0 {
 			s.faults[method] = remaining
 		} else {
+			// ... otherwise delete the map entry for the method
 			delete(s.faults, method)
 		}
 	}
 
+	delay := f.sleep
+
+	// Two locks are held, so unlock both before waiting.
+	f.mu.Unlock()
+
 	s.mu.Unlock()
 
-	if f.sleep > 0 {
-		log(ctx).Debugf("sleeping for %v in %v %v", f.sleep, method, args)
-		time.Sleep(f.sleep)
+	if delay > 0 {
+		// sleep for a while
+		log(ctx).Debugf("sleeping for %v in %v %v", delay, method, args)
+		time.Sleep(delay)
 	}
 
-	if f.callback != nil {
-		f.callback()
+	// Re-acquire fault lock to get callback functions.  Callbacks will be called without lock.
+	f.mu.Lock()
+
+	cb := f.callback
+	errCb := f.errCallback
+
+	f.mu.Unlock()
+
+	// No more references to `f`.  Perform callbacks inline.
+	if cb != nil {
+		cb()
 	}
 
-	if f.errCallback != nil {
-		err := f.errCallback()
+	if errCb != nil {
+		err := errCb()
 		log(ctx).Debugf("returning %v for %v %v", err, method, args)
 
 		return true, err