From 3e222d87711275c132f5ffd5618c2e5e7641b10a Mon Sep 17 00:00:00 2001
From: Matthew Jaffee <jaffee@pilosa.com>
Date: Tue, 14 Sep 2021 17:00:00 -0500
Subject: [PATCH] tweak to locking which should avoid stall/deadlock w/ mutex
 check

The view.go change is straightforward and fairly obviously more
correct.

The field.go change avoids holding the field read lock for the
duration of the mutex check request. The thinking was that while the
read lock was held something else was attempting to get a write lock,
which blocked all other read locks and something was getting into a
loop. Seebs might have a more detailed explanation, but that's as far
as my understanding goes at the moment. I believe this change is safe
though as we don't read/modify any field level data structures after
grabbing the standard view.
---
 field.go | 8 +++++++-
 view.go  | 4 ++--
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/field.go b/field.go
index 793371272..a6c4b5e75 100644
--- a/field.go
+++ b/field.go
@@ -1098,9 +1098,15 @@ func (f *Field) MutexCheck(ctx context.Context, qcx *Qcx, details bool, limit in
 	if f.Type() != FieldTypeMutex {
 		return nil, errors.New("mutex check only valid for mutex fields")
 	}
+
+	// Rather than deferring the unlock, we grab the standard view
+	// from the field's viewMap and unlock immediately. This avoids
+	// holding the rlock for a potentially long time which blocks any
+	// write lock, and pending write locks block other read locks.
 	f.mu.RLock()
-	defer f.mu.RUnlock()
 	standard := f.viewMap[viewStandard]
+	f.mu.RUnlock()
+
 	if standard == nil {
 		// no standard view present means we've never needed to create it,
 		// so it has no bits set, so it has no extra bits set.
diff --git a/view.go b/view.go
index bb0eb3a91..ac94f64d8 100644
--- a/view.go
+++ b/view.go
@@ -300,8 +300,8 @@ func (v *view) Fragment(shard uint64) *fragment {
 
 // allFragments returns a list of all fragments in the view.
 func (v *view) allFragments() []*fragment {
-	v.mu.Lock()
-	defer v.mu.Unlock()
+	v.mu.RLock()
+	defer v.mu.RUnlock()
 
 	other := make([]*fragment, 0, len(v.fragments))
 	for _, fragment := range v.fragments {