diff --git a/src/encoding/json/bench_test.go b/src/encoding/json/bench_test.go index 95609140b0..133084976b 100644 --- a/src/encoding/json/bench_test.go +++ b/src/encoding/json/bench_test.go @@ -99,6 +99,36 @@ func BenchmarkCodeEncoder(b *testing.B) { b.SetBytes(int64(len(codeJSON))) } +func BenchmarkCodeEncoderError(b *testing.B) { + b.ReportAllocs() + if codeJSON == nil { + b.StopTimer() + codeInit() + b.StartTimer() + } + + // Trigger an error in Marshal with cyclic data. + type Dummy struct { + Name string + Next *Dummy + } + dummy := Dummy{Name: "Dummy"} + dummy.Next = &dummy + + b.RunParallel(func(pb *testing.PB) { + enc := NewEncoder(io.Discard) + for pb.Next() { + if err := enc.Encode(&codeStruct); err != nil { + b.Fatal("Encode:", err) + } + if _, err := Marshal(dummy); err == nil { + b.Fatal("expect an error here") + } + } + }) + b.SetBytes(int64(len(codeJSON))) +} + func BenchmarkCodeMarshal(b *testing.B) { b.ReportAllocs() if codeJSON == nil { @@ -116,6 +146,35 @@ func BenchmarkCodeMarshal(b *testing.B) { b.SetBytes(int64(len(codeJSON))) } +func BenchmarkCodeMarshalError(b *testing.B) { + b.ReportAllocs() + if codeJSON == nil { + b.StopTimer() + codeInit() + b.StartTimer() + } + + // Trigger an error in Marshal with cyclic data. + type Dummy struct { + Name string + Next *Dummy + } + dummy := Dummy{Name: "Dummy"} + dummy.Next = &dummy + + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + if _, err := Marshal(&codeStruct); err != nil { + b.Fatal("Marshal:", err) + } + if _, err := Marshal(dummy); err == nil { + b.Fatal("expect an error here") + } + } + }) + b.SetBytes(int64(len(codeJSON))) +} + func benchMarshalBytes(n int) func(*testing.B) { sample := []byte("hello world") // Use a struct pointer, to avoid an allocation when passing it as an @@ -134,6 +193,36 @@ func benchMarshalBytes(n int) func(*testing.B) { } } +func benchMarshalBytesError(n int) func(*testing.B) { + sample := []byte("hello world") + // Use a struct pointer, to avoid an allocation when passing it as an + // interface parameter to Marshal. + v := &struct { + Bytes []byte + }{ + bytes.Repeat(sample, (n/len(sample))+1)[:n], + } + + // Trigger an error in Marshal with cyclic data. + type Dummy struct { + Name string + Next *Dummy + } + dummy := Dummy{Name: "Dummy"} + dummy.Next = &dummy + + return func(b *testing.B) { + for i := 0; i < b.N; i++ { + if _, err := Marshal(v); err != nil { + b.Fatal("Marshal:", err) + } + if _, err := Marshal(dummy); err == nil { + b.Fatal("expect an error here") + } + } + } +} + func BenchmarkMarshalBytes(b *testing.B) { b.ReportAllocs() // 32 fits within encodeState.scratch. @@ -145,6 +234,17 @@ func BenchmarkMarshalBytes(b *testing.B) { b.Run("4096", benchMarshalBytes(4096)) } +func BenchmarkMarshalBytesError(b *testing.B) { + b.ReportAllocs() + // 32 fits within encodeState.scratch. + b.Run("32", benchMarshalBytesError(32)) + // 256 doesn't fit in encodeState.scratch, but is small enough to + // allocate and avoid the slower base64.NewEncoder. + b.Run("256", benchMarshalBytesError(256)) + // 4096 is large enough that we want to avoid allocating for it. + b.Run("4096", benchMarshalBytesError(4096)) +} + func BenchmarkCodeDecoder(b *testing.B) { b.ReportAllocs() if codeJSON == nil { diff --git a/src/encoding/json/encode.go b/src/encoding/json/encode.go index 5b67251fbb..9d59b0ff2b 100644 --- a/src/encoding/json/encode.go +++ b/src/encoding/json/encode.go @@ -156,6 +156,7 @@ import ( // an error. func Marshal(v any) ([]byte, error) { e := newEncodeState() + defer encodeStatePool.Put(e) err := e.marshal(v, encOpts{escapeHTML: true}) if err != nil { @@ -163,8 +164,6 @@ func Marshal(v any) ([]byte, error) { } buf := append([]byte(nil), e.Bytes()...) - encodeStatePool.Put(e) - return buf, nil } diff --git a/src/encoding/json/encode_test.go b/src/encoding/json/encode_test.go index 0b021f0074..c1b9ed2676 100644 --- a/src/encoding/json/encode_test.go +++ b/src/encoding/json/encode_test.go @@ -12,6 +12,7 @@ import ( "math" "reflect" "regexp" + "runtime/debug" "strconv" "testing" "unicode" @@ -760,6 +761,41 @@ func TestIssue10281(t *testing.T) { } } +func TestMarshalErrorAndReuseEncodeState(t *testing.T) { + // Disable the GC temporarily to prevent encodeState's in Pool being cleaned away during the test. + percent := debug.SetGCPercent(-1) + defer debug.SetGCPercent(percent) + + // Trigger an error in Marshal with cyclic data. + type Dummy struct { + Name string + Next *Dummy + } + dummy := Dummy{Name: "Dummy"} + dummy.Next = &dummy + if b, err := Marshal(dummy); err == nil { + t.Errorf("Marshal(dummy) = %#q; want error", b) + } + + type Data struct { + A string + I int + } + data := Data{A: "a", I: 1} + b, err := Marshal(data) + if err != nil { + t.Errorf("Marshal(%v) = %v", data, err) + } + + var data2 Data + if err := Unmarshal(b, &data2); err != nil { + t.Errorf("Unmarshal(%v) = %v", data2, err) + } + if data2 != data { + t.Errorf("expect: %v, but get: %v", data, data2) + } +} + func TestHTMLEscape(t *testing.T) { var b, want bytes.Buffer m := `{"M":"foo &` + "\xe2\x80\xa8 \xe2\x80\xa9" + `"}` diff --git a/src/encoding/json/stream.go b/src/encoding/json/stream.go index b278ee4013..1442ef29ef 100644 --- a/src/encoding/json/stream.go +++ b/src/encoding/json/stream.go @@ -202,7 +202,10 @@ func (enc *Encoder) Encode(v any) error { if enc.err != nil { return enc.err } + e := newEncodeState() + defer encodeStatePool.Put(e) + err := e.marshal(v, encOpts{escapeHTML: enc.escapeHTML}) if err != nil { return err @@ -231,7 +234,6 @@ func (enc *Encoder) Encode(v any) error { if _, err = enc.w.Write(b); err != nil { enc.err = err } - encodeStatePool.Put(e) return err } diff --git a/src/encoding/json/stream_test.go b/src/encoding/json/stream_test.go index 0e156d98e9..1f40c79670 100644 --- a/src/encoding/json/stream_test.go +++ b/src/encoding/json/stream_test.go @@ -12,6 +12,7 @@ import ( "net/http" "net/http/httptest" "reflect" + "runtime/debug" "strings" "testing" ) @@ -59,6 +60,43 @@ func TestEncoder(t *testing.T) { } } +func TestEncoderErrorAndReuseEncodeState(t *testing.T) { + // Disable the GC temporarily to prevent encodeState's in Pool being cleaned away during the test. + percent := debug.SetGCPercent(-1) + defer debug.SetGCPercent(percent) + + // Trigger an error in Marshal with cyclic data. + type Dummy struct { + Name string + Next *Dummy + } + dummy := Dummy{Name: "Dummy"} + dummy.Next = &dummy + + var buf bytes.Buffer + enc := NewEncoder(&buf) + if err := enc.Encode(dummy); err == nil { + t.Errorf("Encode(dummy) == nil; want error") + } + + type Data struct { + A string + I int + } + data := Data{A: "a", I: 1} + if err := enc.Encode(data); err != nil { + t.Errorf("Marshal(%v) = %v", data, err) + } + + var data2 Data + if err := Unmarshal(buf.Bytes(), &data2); err != nil { + t.Errorf("Unmarshal(%v) = %v", data2, err) + } + if data2 != data { + t.Errorf("expect: %v, but get: %v", data, data2) + } +} + var streamEncodedIndent = `0.1 "hello" null