Skip to content

Commit c21837a

Browse files
authored
Merge pull request #1203 from ydb-platform/fix-race-on-with-names
Fixed data race using `log.WithNames`
2 parents 35a861a + 7f832c8 commit c21837a

File tree

3 files changed

+47
-3
lines changed

3 files changed

+47
-3
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
* Fixed data race using `log.WithNames`
2+
13
## v3.65.1
24
* Updated dependency `ydb-go-genproto`
35
* Added processing of `Ydb.StatusIds_EXTERNAL_ERROR` in `retry.Retry`

log/context.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package log
22

3-
import "context"
3+
import (
4+
"context"
5+
)
46

57
type (
68
ctxLevelKey struct{}
@@ -18,7 +20,11 @@ func LevelFromContext(ctx context.Context) Level {
1820
}
1921

2022
func WithNames(ctx context.Context, names ...string) context.Context {
21-
return context.WithValue(ctx, ctxNamesKey{}, append(NamesFromContext(ctx), names...))
23+
// trim capacity for force allocate new memory while append and prevent data race
24+
oldNames := NamesFromContext(ctx)
25+
oldNames = oldNames[:len(oldNames):len(oldNames)]
26+
27+
return context.WithValue(ctx, ctxNamesKey{}, append(oldNames, names...))
2228
}
2329

2430
func NamesFromContext(ctx context.Context) []string {
@@ -27,7 +33,7 @@ func NamesFromContext(ctx context.Context) []string {
2733
return []string{}
2834
}
2935

30-
return v
36+
return v[:len(v):len(v)] // prevent re
3137
}
3238

3339
func with(ctx context.Context, lvl Level, names ...string) context.Context {

log/context_test.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,13 @@ package log
22

33
import (
44
"context"
5+
"strconv"
56
"testing"
7+
"time"
68

79
"github.com/stretchr/testify/require"
10+
11+
"github.com/ydb-platform/ydb-go-sdk/v3/internal/xtest"
812
)
913

1014
func TestLevelFromContext(t *testing.T) {
@@ -54,3 +58,35 @@ func TestNamesFromContext(t *testing.T) {
5458
})
5559
}
5660
}
61+
62+
func TestWithNamesRaceRegression(t *testing.T) {
63+
count := 100
64+
xtest.TestManyTimes(t, func(t testing.TB) {
65+
ctx := WithNames(context.Background(), "test")
66+
ctx = WithNames(ctx, "test")
67+
ctx = WithNames(ctx, "test")
68+
res := make([]context.Context, count)
69+
70+
start := make(chan bool)
71+
finished := make(chan bool)
72+
for i := 0; i < count; i++ {
73+
go func(index int) {
74+
<-start
75+
res[index] = WithNames(ctx, strconv.Itoa(index))
76+
finished <- true
77+
}(i)
78+
}
79+
80+
time.Sleep(time.Microsecond)
81+
close(start)
82+
83+
for i := 0; i < count; i++ {
84+
<-finished
85+
}
86+
87+
for i := 0; i < count; i++ {
88+
expected := []string{"test", "test", "test", strconv.Itoa(i)}
89+
require.Equal(t, expected, NamesFromContext(res[i]))
90+
}
91+
})
92+
}

0 commit comments

Comments
 (0)