encoding/json/jsontext: preserve buffer capacity in Decoder.Reset

The Decoder.Reset method is not preserving the internal buffer between
resets, causing buffer capacity to be lost and resulting in unnecessary
allocations when reusing decoders. This is particularly problematic when
decoding many small messages.

This commit fixes the Reset method to preserve the internal buffer. It
makes sure aliasing is removed if the buffer currently points to an
internal byte slice of a bytes.Buffer. It adds a TestDecoderReset test
structured into subtests to better validate the different scenarios.

Change-Id: Ia685bff47034598224489173bb7f2ffd48e89da5
GitHub-Last-Rev: 462ddc936409ee3da6d75def7d9de59eb3c65f8d
GitHub-Pull-Request: golang/go#74120
Reviewed-on: https://go-review.googlesource.com/c/go/+/681177
Reviewed-by: Sean Liao <sean@liao.dev>
Reviewed-by: Joseph Tsai <joetsai@digital-static.net>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Joseph Tsai <joetsai@digital-static.net>

diff --git a/src/encoding/json/jsontext/decode.go b/src/encoding/json/jsontext/decode.go
index 784ae4709ae5b67c683e7c7b489e4d8948371830..44b436686fb06566c1889b14949eda58d8f4ca52 100644 (file)
--- a/src/encoding/json/jsontext/decode.go
+++ b/src/encoding/json/jsontext/decode.go
@@ -138,7 +138,14 @@ func (d *Decoder) Reset(r io.Reader, opts ...Options) {
        case d.s.Flags.Get(jsonflags.WithinArshalCall):
                panic("jsontext: cannot reset Decoder passed to json.UnmarshalerFrom")
        }
-       d.s.reset(nil, r, opts...)
+       // If the decoder was previously aliasing a bytes.Buffer,
+       // invalidate the alias to avoid writing into the bytes.Buffer's
+       // internal buffer.
+       b := d.s.buf[:0]
+       if _, ok := d.s.rd.(*bytes.Buffer); ok {
+               b = nil // avoid reusing b since it aliases the previous bytes.Buffer.
+       }
+       d.s.reset(b, r, opts...)
 }
 
 func (d *decoderState) reset(b []byte, r io.Reader, opts ...Options) {

diff --git a/src/encoding/json/jsontext/decode_test.go b/src/encoding/json/jsontext/decode_test.go
index 67580e6f4f0eabcc8d44f902006aa5b9d21ce89b..78296bb5900c04b85eb8f3b39fc937c204dfd52f 100644 (file)
--- a/src/encoding/json/jsontext/decode_test.go
+++ b/src/encoding/json/jsontext/decode_test.go
@@ -1265,3 +1265,86 @@ func TestPeekableDecoder(t *testing.T) {
                }
        }
 }
+
+// TestDecoderReset tests that the decoder preserves its internal
+// buffer between Reset calls to avoid frequent allocations when reusing the decoder.
+// It ensures that the buffer capacity is maintained while avoiding aliasing
+// issues with bytes.Buffer.
+func TestDecoderReset(t *testing.T) {
+       // Create a decoder with a reasonably large JSON input to ensure buffer growth
+       largeJSON := `{"key1":"value1","key2":"value2","key3":"value3","key4":"value4","key5":"value5"}`
+       dec := NewDecoder(strings.NewReader(largeJSON))
+
+       t.Run("Test capacity preservation", func(t *testing.T) {
+               // Read the first JSON value to grow the internal buffer
+               val1, err := dec.ReadValue()
+               if err != nil {
+                       t.Fatalf("first ReadValue failed: %v", err)
+               }
+               if string(val1) != largeJSON {
+                       t.Fatalf("first ReadValue = %q, want %q", val1, largeJSON)
+               }
+
+               // Get the buffer capacity after first use
+               initialCapacity := cap(dec.s.buf)
+               if initialCapacity == 0 {
+                       t.Fatalf("expected non-zero buffer capacity after first use")
+               }
+
+               // Reset with a new reader - this should preserve the buffer capacity
+               dec.Reset(strings.NewReader(largeJSON))
+
+               // Verify the buffer capacity is preserved (or at least not smaller)
+               preservedCapacity := cap(dec.s.buf)
+               if preservedCapacity < initialCapacity {
+                       t.Fatalf("buffer capacity reduced after Reset: got %d, want at least %d", preservedCapacity, initialCapacity)
+               }
+
+               // Read the second JSON value to ensure the decoder still works correctly
+               val2, err := dec.ReadValue()
+               if err != nil {
+                       t.Fatalf("second ReadValue failed: %v", err)
+               }
+               if string(val2) != largeJSON {
+                       t.Fatalf("second ReadValue = %q, want %q", val2, largeJSON)
+               }
+       })
+
+       var bbBuf []byte
+       t.Run("Test aliasing with bytes.Buffer", func(t *testing.T) {
+               // Test with bytes.Buffer to verify proper aliasing behavior.
+               bb := bytes.NewBufferString(largeJSON)
+               dec.Reset(bb)
+               bbBuf = bb.Bytes()
+
+               // Read the third JSON value to ensure functionality with bytes.Buffer
+               val3, err := dec.ReadValue()
+               if err != nil {
+                       t.Fatalf("fourth ReadValue failed: %v", err)
+               }
+               if string(val3) != largeJSON {
+                       t.Fatalf("fourth ReadValue = %q, want %q", val3, largeJSON)
+               }
+               // The decoder buffer should alias bytes.Buffer's internal buffer.
+               if len(dec.s.buf) == 0 || len(bbBuf) == 0 || &dec.s.buf[0] != &bbBuf[0] {
+                       t.Fatalf("decoder buffer does not alias bytes.Buffer")
+               }
+       })
+
+       t.Run("Test aliasing removed after Reset", func(t *testing.T) {
+               // Reset with a new reader and verify the buffer is not aliased.
+               dec.Reset(strings.NewReader(largeJSON))
+               val4, err := dec.ReadValue()
+               if err != nil {
+                       t.Fatalf("fifth ReadValue failed: %v", err)
+               }
+               if string(val4) != largeJSON {
+                       t.Fatalf("fourth ReadValue = %q, want %q", val4, largeJSON)
+               }
+
+               // The decoder buffer should not alias the bytes.Buffer's internal buffer.
+               if len(dec.s.buf) == 0 || len(bbBuf) == 0 || &dec.s.buf[0] == &bbBuf[0] {
+                       t.Fatalf("decoder buffer aliases bytes.Buffer")
+               }
+       })
+}