From 8085c49b03cb7e6722fdbe89dcd366bb5bbb10b5 Mon Sep 17 00:00:00 2001 From: Timofey Koolin Date: Thu, 18 Apr 2024 13:55:30 +0300 Subject: [PATCH] Fixed data race in logger ydb/log/WithNames --- CHANGELOG.md | 2 ++ log/context.go | 12 +++++++++--- log/context_test.go | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 03957b2fc..608699669 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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` diff --git a/log/context.go b/log/context.go index 81231df01..b724e5ec3 100644 --- a/log/context.go +++ b/log/context.go @@ -1,6 +1,8 @@ package log -import "context" +import ( + "context" +) type ( ctxLevelKey struct{} @@ -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 { @@ -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 { diff --git a/log/context_test.go b/log/context_test.go index 3d8c05067..a3bfb6250 100644 --- a/log/context_test.go +++ b/log/context_test.go @@ -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) { @@ -54,3 +58,34 @@ 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) { + 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])) + } + }) +}