fix issue thanos-io#4777: panic when warn is nil

Signed-off-by: ian woolf <btw515wolf2@gmail.com>
ianwoolf · Oct 28, 2021 · de6822d · de6822d
1 parent 24602c4
commit de6822d
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 2 deletions.
diff --git a/pkg/api/api.go b/pkg/api/api.go
@@ -211,7 +211,15 @@ func GetInstr(
 			if data, warnings, err := f(r); err != nil {
 				RespondError(w, err, data)
 			} else if data != nil {
-				Respond(w, data, warnings)
+				var _warnings []error
+				for i := range warnings {
+					if warnings[i] == nil {
+						level.Error(logger).Log("unexpected nil warning", "path", r.RequestURI, "warings", warnings)
+						continue
+					}
+					_warnings = append(_warnings, warnings[i])
+				}
+				respond(w, data, _warnings)
 			} else {
 				w.WriteHeader(http.StatusNoContent)
 			}
@@ -230,7 +238,7 @@ func GetInstr(
 	return instr
 }
 
-func Respond(w http.ResponseWriter, data interface{}, warnings []error) {
+func respond(w http.ResponseWriter, data interface{}, warnings []error) {
 	w.Header().Set("Content-Type", "application/json")
 	if len(warnings) > 0 {
 		w.Header().Set("Cache-Control", "no-store")
@@ -242,6 +250,9 @@ func Respond(w http.ResponseWriter, data interface{}, warnings []error) {
 		Data:   data,
 	}
 	for _, warn := range warnings {
+		if warn == nil {
+			continue
+		}
 		resp.Warnings = append(resp.Warnings, warn.Error())
 	}
 	_ = json.NewEncoder(w).Encode(resp)

diff --git a/pkg/exemplars/exemplars.go b/pkg/exemplars/exemplars.go
@@ -6,6 +6,7 @@ package exemplars
 import (
 	"context"
 	"sort"
+	"sync"
 
 	"github.com/pkg/errors"
 	"github.com/prometheus/prometheus/storage"
@@ -32,6 +33,7 @@ type GRPCClient struct {
 
 type exemplarsServer struct {
 	// This field just exist to pseudo-implement the unused methods of the interface.
+	sync.RWMutex
 	exemplarspb.Exemplars_ExemplarsServer
 	ctx context.Context
 
@@ -40,6 +42,8 @@ type exemplarsServer struct {
 }
 
 func (srv *exemplarsServer) Send(res *exemplarspb.ExemplarsResponse) error {
+	srv.Lock()
+	defer srv.Unlock()
 	if res.GetWarning() != "" {
 		srv.warnings = append(srv.warnings, errors.New(res.GetWarning()))
 		return nil

diff --git a/pkg/exemplars/exemplars_test.go b/pkg/exemplars/exemplars_test.go
@@ -6,6 +6,8 @@ package exemplars
 import (
 	"testing"
 
+	"github.com/pkg/errors"
+	"github.com/prometheus/prometheus/pkg/labels"
 	"github.com/thanos-io/thanos/pkg/exemplars/exemplarspb"
 	"github.com/thanos-io/thanos/pkg/store/labelpb"
 	"github.com/thanos-io/thanos/pkg/testutil"
@@ -15,6 +17,29 @@ func TestMain(m *testing.M) {
 	testutil.TolerantVerifyLeakMain(m)
 }
 
+func TestExemplarsServerSendDataRace(t *testing.T) {
+	es := &exemplarsServer{}
+
+	res := []*exemplarspb.ExemplarsResponse{
+		exemplarspb.NewExemplarsResponse(&exemplarspb.ExemplarData{
+			SeriesLabels: labelpb.ZLabelSet{Labels: labelpb.ZLabelsFromPromLabels(labels.FromMap(map[string]string{"__name__": "http_request_duration_bucket"}))},
+			Exemplars:    []*exemplarspb.Exemplar{{Value: 1}},
+		}),
+		exemplarspb.NewWarningExemplarsResponse(errors.New("warning from client1")), exemplarspb.NewExemplarsResponse(&exemplarspb.ExemplarData{
+			SeriesLabels: labelpb.ZLabelSet{Labels: labelpb.ZLabelsFromPromLabels(labels.FromMap(map[string]string{"__name__": "http_request_duration_bucket"}))},
+			Exemplars:    []*exemplarspb.Exemplar{{Value: 1}},
+		}),
+		exemplarspb.NewWarningExemplarsResponse(errors.New("warning from client2")), exemplarspb.NewExemplarsResponse(&exemplarspb.ExemplarData{
+			SeriesLabels: labelpb.ZLabelSet{Labels: labelpb.ZLabelsFromPromLabels(labels.FromMap(map[string]string{"__name__": "http_request_duration_bucket"}))},
+			Exemplars:    []*exemplarspb.Exemplar{{Value: 1}},
+		}),
+	}
+
+	for i := range res {
+		go es.Send(res[i])
+	}
+}
+
 func TestDedupExemplarsResponse(t *testing.T) {
 	for _, tc := range []struct {
 		name            string