Skip to content

Commit

Permalink
fix: deep copy the fields to avoid data race (#128)
Browse files Browse the repository at this point in the history
* fix(test): use sublogger generated from With() in race detection

The new sublogger generated from With() are never used, which may cause
optimization of the sublogger, and no race condition is detected.

Use a for loop to spawn a lots of goroutines to ensure the race
condition would happen in CI.

We also need to replace the bytes.Buffer to io.Discard, since the
bytes.Buffer is also not thread-safe.

Signed-off-by: yuguorui <[email protected]>

* fix: deep copy the fields to avoid data race

sl.fields and l.fields reference the the same object, when we create
multiple goroutines and bump loggers with the With() method, the golang
race detector will complain about this.

Signed-off-by: yuguorui <[email protected]>

---------

Signed-off-by: yuguorui <[email protected]>
  • Loading branch information
yuguorui authored May 14, 2024
1 parent 77a8113 commit 81cf0ff
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 13 deletions.
3 changes: 2 additions & 1 deletion logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,8 @@ func (l *Logger) With(keyvals ...interface{}) *Logger {
sl.b = bytes.Buffer{}
sl.mu = &sync.RWMutex{}
sl.helpers = &sync.Map{}
sl.fields = append(l.fields, keyvals...)
sl.fields = append(make([]interface{}, 0, len(l.fields)+len(keyvals)), l.fields...)
sl.fields = append(sl.fields, keyvals...)
sl.styles = &st
return &sl
}
Expand Down
34 changes: 22 additions & 12 deletions logger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package log

import (
"bytes"
"fmt"
"io"
"sync"
"testing"
Expand Down Expand Up @@ -204,7 +205,7 @@ func TestLogWithPrefix(t *testing.T) {
}

func TestLogWithRaceCondition(t *testing.T) {
var buf bytes.Buffer
w := io.Discard
cases := []struct {
name string
}{
Expand All @@ -214,21 +215,30 @@ func TestLogWithRaceCondition(t *testing.T) {
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
buf.Reset()
l := New(&buf)
l := New(w)

var done sync.WaitGroup
done.Add(2)

go func() {
l.With("arg1", "val1", "arg2", "val2")
done.Done()
}()
longArgs := make([]interface{}, 0, 1000)
for i := 0; i < 1000; i++ {
longArgs = append(longArgs, fmt.Sprintf("arg%d", i), fmt.Sprintf("val%d", i))
}
l = l.With(longArgs...)

for i := 0; i < 100; i++ {
done.Add(1)
go func() {
ll := l.With("arg1", "val1", "arg2", "val2")
ll.Info("kinda long long log message")
done.Done()
}()

go func() {
l.Info("kinda long log message")
done.Done()
}()
done.Add(1)
go func() {
l.Info("kinda long log message")
done.Done()
}()
}
done.Wait()
})
}
Expand Down

0 comments on commit 81cf0ff

Please sign in to comment.