diff options
| author | Jonathan Amsterdam <jba@google.com> | 2023-07-28 10:35:10 -0400 |
|---|---|---|
| committer | Jonathan Amsterdam <jba@google.com> | 2023-08-07 15:10:55 +0000 |
| commit | cf39736f87613195aa8a2d7304ee20d7d9eeaa47 (patch) | |
| tree | deb5b0a6e09993bdd8020d54a568343cc30343a0 /src/log | |
| parent | 8a6acecfab2879ba4e26487c99f204ede8b3d935 (diff) | |
| download | go-cf39736f87613195aa8a2d7304ee20d7d9eeaa47.tar.gz go-cf39736f87613195aa8a2d7304ee20d7d9eeaa47.zip | |
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>
Diffstat (limited to 'src/log')
| -rw-r--r-- | src/log/slog/record.go | 4 | ||||
| -rw-r--r-- | src/log/slog/record_test.go | 11 |
2 files changed, 10 insertions, 5 deletions
diff --git a/src/log/slog/record.go b/src/log/slog/record.go index 67b76f34e1..82acc7ac7b 100644 --- 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 15d9330a85..931ab66041 100644 --- 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) |