From 437362a7a789a803b970238fce46e2cdc20319e2 Mon Sep 17 00:00:00 2001
From: TJ Hoplock <t.hoplock@gmail.com>
Date: Tue, 10 Dec 2024 02:24:20 -0500
Subject: [PATCH] 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>
---
 util/logging/dedupe.go      |  5 ++++-
 util/logging/dedupe_test.go | 24 ++++++++++++++++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/util/logging/dedupe.go b/util/logging/dedupe.go
index e7dff20f7..8137f4f22 100644
--- a/util/logging/dedupe.go
+++ b/util/logging/dedupe.go
@@ -33,7 +33,7 @@ type Deduper struct {
 	next   *slog.Logger
 	repeat time.Duration
 	quit   chan struct{}
-	mtx    sync.RWMutex
+	mtx    *sync.RWMutex
 	seen   map[string]time.Time
 }
 
@@ -43,6 +43,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()
@@ -88,6 +89,7 @@ func (d *Deduper) WithAttrs(attrs []slog.Attr) slog.Handler {
 		repeat: d.repeat,
 		quit:   d.quit,
 		seen:   d.seen,
+		mtx:    d.mtx,
 	}
 }
 
@@ -103,6 +105,7 @@ func (d *Deduper) WithGroup(name string) slog.Handler {
 		repeat: d.repeat,
 		quit:   d.quit,
 		seen:   d.seen,
+		mtx:    d.mtx,
 	}
 }
 
diff --git a/util/logging/dedupe_test.go b/util/logging/dedupe_test.go
index 5baa90b03..a8774aefd 100644
--- a/util/logging/dedupe_test.go
+++ b/util/logging/dedupe_test.go
@@ -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() })
+}