Skip to content

Commit

Permalink
Fixed data race in logger ydb/log/WithNames
Browse files Browse the repository at this point in the history
  • Loading branch information
rekby committed Apr 18, 2024
1 parent 35a861a commit 0567481
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 3 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
* Fixed data race in logger ydb/log/WithNames

## v3.65.1
* Updated dependency `ydb-go-genproto`
* Added processing of `Ydb.StatusIds_EXTERNAL_ERROR` in `retry.Retry`
Expand Down
12 changes: 9 additions & 3 deletions log/context.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package log

import "context"
import (
"context"
)

type (
ctxLevelKey struct{}
Expand All @@ -18,7 +20,11 @@ func LevelFromContext(ctx context.Context) Level {
}

func WithNames(ctx context.Context, names ...string) context.Context {
return context.WithValue(ctx, ctxNamesKey{}, append(NamesFromContext(ctx), names...))
// trim capacity for force allocate new memory while append and prevent data race
oldNames := NamesFromContext(ctx)
oldNames = oldNames[:len(oldNames):len(oldNames)]

return context.WithValue(ctx, ctxNamesKey{}, append(oldNames, names...))
}

func NamesFromContext(ctx context.Context) []string {
Expand All @@ -27,7 +33,7 @@ func NamesFromContext(ctx context.Context) []string {
return []string{}
}

return v
return v[:len(v):len(v)] // prevent re
}

func with(ctx context.Context, lvl Level, names ...string) context.Context {
Expand Down
36 changes: 36 additions & 0 deletions log/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,13 @@ package log

import (
"context"
"strconv"
"testing"
"time"

"github.com/stretchr/testify/require"

"github.com/ydb-platform/ydb-go-sdk/v3/internal/xtest"
)

func TestLevelFromContext(t *testing.T) {
Expand Down Expand Up @@ -54,3 +58,35 @@ func TestNamesFromContext(t *testing.T) {
})
}
}

func TestWithNamesRaceRegression(t *testing.T) {
count := 100
xtest.TestManyTimes(t, func(t testing.TB) {
ctx := WithNames(context.Background(), "test")
ctx = WithNames(ctx, "test")
ctx = WithNames(ctx, "test")
res := make([]context.Context, count)

start := make(chan bool)
finished := make(chan bool)
for i := 0; i < count; i++ {
go func(index int) {
<-start
res[index] = WithNames(ctx, strconv.Itoa(index))
finished <- true
}(i)
}

time.Sleep(time.Microsecond)
close(start)

for i := 0; i < count; i++ {
<-finished
}

for i := 0; i < count; i++ {
expected := []string{"test", "test", "test", strconv.Itoa(i)}
require.Equal(t, expected, NamesFromContext(res[i]))
}
})
}

0 comments on commit 0567481

Please sign in to comment.