query: fix deadlock in endpointset (thanos-io#4795)

Avoid RLock()ing twice as described here:
thanos-io#4766 (comment)
(due to
https://stackoverflow.com/questions/30547916/goroutine-blocks-when-calling-rwmutex-rlock-twice-after-an-rwmutex-unlock/30549188).
Fix it by removing HasClients() and simply changing it with `er.clients != nil`.

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>

pkg/query/endpointset.go

@@ -688,46 +688,39 @@ func (er *endpointRef) ComponentType() component.Component {
 	return component.FromString(er.metadata.ComponentType)
 }
 
-func (er *endpointRef) HasClients() bool {
-	er.mtx.RLock()
-	defer er.mtx.RUnlock()
-
-	return er.clients != nil
-}
-
 func (er *endpointRef) HasStoreAPI() bool {
 	er.mtx.RLock()
 	defer er.mtx.RUnlock()
 
-	return er.HasClients() && er.clients.store != nil
+	return er.clients != nil && er.clients.store != nil
 }
 
 func (er *endpointRef) HasRulesAPI() bool {
 	er.mtx.RLock()
 	defer er.mtx.RUnlock()
 
-	return er.HasClients() && er.clients.rule != nil
+	return er.clients != nil && er.clients.rule != nil
 }
 
 func (er *endpointRef) HasTargetsAPI() bool {
 	er.mtx.RLock()
 	defer er.mtx.RUnlock()
 
-	return er.HasClients() && er.clients.target != nil
+	return er.clients != nil && er.clients.target != nil
 }
 
 func (er *endpointRef) HasMetricMetadataAPI() bool {
 	er.mtx.RLock()
 	defer er.mtx.RUnlock()
 
-	return er.HasClients() && er.clients.metricMetadata != nil
+	return er.clients != nil && er.clients.metricMetadata != nil
 }
 
 func (er *endpointRef) HasExemplarsAPI() bool {
 	er.mtx.RLock()
 	defer er.mtx.RUnlock()
 
-	return er.HasClients() && er.clients.exemplar != nil
+	return er.clients != nil && er.clients.exemplar != nil
 }
 
 func (er *endpointRef) LabelSets() []labels.Labels {

pkg/query/endpointset_test.go

@@ -12,6 +12,7 @@ import (
 	"testing"
 	"time"
 
+	"golang.org/x/sync/errgroup"
 	"google.golang.org/grpc"
 
 	"github.com/pkg/errors"
@@ -1185,3 +1186,47 @@ func assertRegisteredAPIs(t *testing.T, expectedAPIs *APIs, er *endpointRef) {
 	testutil.Equals(t, expectedAPIs.metricMetadata, er.HasMetricMetadataAPI())
 	testutil.Equals(t, expectedAPIs.exemplars, er.HasExemplarsAPI())
 }
+
+// Regression test for: https://github.com/thanos-io/thanos/issues/4766.
+func TestDeadlockLocking(t *testing.T) {
+	t.Parallel()
+
+	mockEndpointRef := &endpointRef{
+		addr: "mockedStore",
+		metadata: &endpointMetadata{
+			&infopb.InfoResponse{},
+		},
+		clients: &endpointClients{},
+	}
+
+	g := &errgroup.Group{}
+	deadline := time.Now().Add(3 * time.Second)
+
+	g.Go(func() error {
+		for {
+			if time.Now().After(deadline) {
+				break
+			}
+			mockEndpointRef.Update(&endpointMetadata{
+				InfoResponse: &infopb.InfoResponse{},
+			})
+		}
+		return nil
+	})
+
+	g.Go(func() error {
+		for {
+			if time.Now().After(deadline) {
+				break
+			}
+			mockEndpointRef.HasStoreAPI()
+			mockEndpointRef.HasExemplarsAPI()
+			mockEndpointRef.HasMetricMetadataAPI()
+			mockEndpointRef.HasRulesAPI()
+			mockEndpointRef.HasTargetsAPI()
+		}
+		return nil
+	})
+
+	testutil.Ok(t, g.Wait())
+}