Skip to content

Commit 4b50fd8

Browse files
authored
*: Fix data race between getting labels and setting labels in config (#45563)
close #45561
1 parent b74c572 commit 4b50fd8

File tree

2 files changed

+50
-5
lines changed

2 files changed

+50
-5
lines changed

server/http_handler.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2220,12 +2220,15 @@ func (h labelHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
22202220

22212221
if len(labels) > 0 {
22222222
cfg := *config.GetGlobalConfig()
2223-
if cfg.Labels == nil {
2224-
cfg.Labels = make(map[string]string, len(labels))
2225-
}
2226-
for k, v := range labels {
2227-
cfg.Labels[k] = v
2223+
// Be careful of data race. The key & value of cfg.Labels must not be changed.
2224+
if cfg.Labels != nil {
2225+
for k, v := range cfg.Labels {
2226+
if _, found := labels[k]; !found {
2227+
labels[k] = v
2228+
}
2229+
}
22282230
}
2231+
cfg.Labels = labels
22292232
config.StoreGlobalConfig(&cfg)
22302233
logutil.BgLogger().Info("update server labels", zap.Any("labels", cfg.Labels))
22312234
}

server/http_handler_test.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"encoding/json"
2525
"fmt"
2626
"io"
27+
"math/rand"
2728
"net"
2829
"net/http"
2930
"net/http/httptest"
@@ -1199,3 +1200,44 @@ func TestSetLabels(t *testing.T) {
11991200
// reset the global variable
12001201
config.GetGlobalConfig().Labels = map[string]string{}
12011202
}
1203+
1204+
func TestSetLabelsConcurrentWithGetLabel(t *testing.T) {
1205+
ts := createBasicHTTPHandlerTestSuite()
1206+
1207+
ts.startServer(t)
1208+
defer ts.stopServer(t)
1209+
1210+
testUpdateLabels := func() {
1211+
labels := map[string]string{}
1212+
labels["zone"] = fmt.Sprintf("z-%v", rand.Intn(100000))
1213+
buffer := bytes.NewBuffer([]byte{})
1214+
require.Nil(t, json.NewEncoder(buffer).Encode(labels))
1215+
resp, err := ts.postStatus("/labels", "application/json", buffer)
1216+
require.NoError(t, err)
1217+
require.NotNil(t, resp)
1218+
defer func() {
1219+
require.NoError(t, resp.Body.Close())
1220+
}()
1221+
require.Equal(t, http.StatusOK, resp.StatusCode)
1222+
newLabels := config.GetGlobalConfig().Labels
1223+
require.Equal(t, newLabels, labels)
1224+
}
1225+
done := make(chan struct{})
1226+
go func() {
1227+
for {
1228+
select {
1229+
case <-done:
1230+
return
1231+
default:
1232+
config.GetGlobalConfig().GetTiKVConfig()
1233+
}
1234+
}
1235+
}()
1236+
for i := 0; i < 100; i++ {
1237+
testUpdateLabels()
1238+
}
1239+
close(done)
1240+
1241+
// reset the global variable
1242+
config.GetGlobalConfig().Labels = map[string]string{}
1243+
}

0 commit comments

Comments
 (0)