fix(deduper): use ptr to sync.RWMutex, fix panic during concurrent use

Resolves: #15559

As accurately noted in the issue description, the map is shared among
child loggers that get created when `WithAttr()`/`WithGroup()` are
called on the underlying handler, which happens via `log.With()` and
`log.WithGroup()` respectively.

The RW mutex was a value in the previous implementation that used
go-kit/log, and I should've updated it to use a pointer when I converted
the deduper.

Also adds a test.

Signed-off-by: TJ Hoplock <t.hoplock@gmail.com>
This commit is contained in:
TJ Hoplock 2024-12-10 02:24:20 -05:00
parent 1f56e8492c
commit 03f2a65bc8
2 changed files with 28 additions and 1 deletions

View file

@ -31,7 +31,7 @@ type Deduper struct {
next *slog.Logger
repeat time.Duration
quit chan struct{}
mtx sync.RWMutex
mtx *sync.RWMutex
seen map[string]time.Time
}
@ -41,6 +41,7 @@ func Dedupe(next *slog.Logger, repeat time.Duration) *Deduper {
next: next,
repeat: repeat,
quit: make(chan struct{}),
mtx: new(sync.RWMutex),
seen: map[string]time.Time{},
}
go d.run()
@ -86,6 +87,7 @@ func (d *Deduper) WithAttrs(attrs []slog.Attr) slog.Handler {
repeat: d.repeat,
quit: d.quit,
seen: d.seen,
mtx: d.mtx,
}
}
@ -101,6 +103,7 @@ func (d *Deduper) WithGroup(name string) slog.Handler {
repeat: d.repeat,
quit: d.quit,
seen: d.seen,
mtx: d.mtx,
}
}

View file

@ -56,3 +56,27 @@ func TestDedupe(t *testing.T) {
}
require.Len(t, lines, 2)
}
func TestDedupeConcurrent(t *testing.T) {
d := Dedupe(promslog.New(&promslog.Config{}), 250*time.Millisecond)
dlog := slog.New(d)
defer d.Stop()
concurrentWriteFunc := func() {
go func() {
dlog1 := dlog.With("writer", 1)
for i := 0; i < 10; i++ {
dlog1.With("foo", "bar").Info("test", "hello", "world")
}
}()
go func() {
dlog2 := dlog.With("writer", 2)
for i := 0; i < 10; i++ {
dlog2.With("foo", "bar").Info("test", "hello", "world")
}
}()
}
require.NotPanics(t, func() { concurrentWriteFunc() })
}