log/slog: don't panic on aliased Record

If the shared slice in a copied is modified, make a copy of it
and insert an attribute that warns of the bug.

Previously, we panicked, and panics in logging code should be avoided.

Change-Id: I24e9b0bf5c8cd09cf733e7dae8a82d025ef214e5
Reviewed-on: https://go-review.googlesource.com/c/go/+/513760
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

diff --git a/src/log/slog/record.go b/src/log/slog/record.go
index 67b76f34e17fa5a11d14ba10c1892c7351ac3645..82acc7ac7b1c026d3093cf2330f72ff7444495be 100644 (file)
--- a/src/log/slog/record.go
+++ b/src/log/slog/record.go
@@ -109,7 +109,9 @@ func (r *Record) AddAttrs(attrs ...Attr) {
        if cap(r.back) > len(r.back) {
                end := r.back[:len(r.back)+1][len(r.back)]
                if !end.isEmpty() {
-                       panic("copies of a slog.Record were both modified")
+                       // Don't panic; copy and muddle through.
+                       r.back = slices.Clip(r.back)
+                       r.back = append(r.back, String("!BUG", "AddAttrs unsafely called on copy of Record made without using Record.Clone"))
                }
        }
        ne := countEmptyGroups(attrs[i:])

diff --git a/src/log/slog/record_test.go b/src/log/slog/record_test.go
index 15d9330a855e5a70e7da0773b382b9cae2f4661b..931ab660418db0bf470aacff46f2294f2370c57d 100644 (file)
--- a/src/log/slog/record_test.go
+++ b/src/log/slog/record_test.go
@@ -96,12 +96,15 @@ func TestAliasingAndClone(t *testing.T) {
        r1.back = b
        // Make a copy that shares state.
        r2 := r1
-       // Adding to both should panic.
+       // Adding to both should insert a special Attr in the second.
+       r1AttrsBefore := attrsSlice(r1)
        r1.AddAttrs(Int("p", 0))
-       if !panics(func() { r2.AddAttrs(Int("p", 1)) }) {
-               t.Error("expected panic")
-       }
+       r2.AddAttrs(Int("p", 1))
+       check(r1, append(slices.Clip(r1AttrsBefore), Int("p", 0)))
        r1Attrs := attrsSlice(r1)
+       check(r2, append(slices.Clip(r1AttrsBefore),
+               String("!BUG", "AddAttrs unsafely called on copy of Record made without using Record.Clone"), Int("p", 1)))
+
        // Adding to a clone is fine.
        r2 = r1.Clone()
        check(r2, r1Attrs)