diff --git a/doc/godebug.md b/doc/godebug.md index 9c5c01d3b27670..794d13e587219b 100644 --- a/doc/godebug.md +++ b/doc/godebug.md @@ -188,6 +188,14 @@ Setting `x509sslcertoverrideplatform=0` disables this behavior in favor of using the platform certificate store instead of honoring the environment variables. We plan to remove this setting in Go 1.31. +Go 1.27 added a new `http2reuseframes` setting that controls whether the +net/http HTTP/2 server and Transport opt in to `Framer.SetReuseFrames`, +which reuses parsed `*DataFrame`, `*WindowUpdateFrame`, `*HeadersFrame`, +and `*MetaHeadersFrame` structs across `ReadFrame` calls to reduce +per-frame heap allocation. The default is reuse on. Setting +`http2reuseframes=0` reverts to allocating each parsed frame fresh, +matching the pre-Go 1.27 behavior. + ### Go 1.26 Go 1.26 added a new `httpcookiemaxnum` setting that controls the maximum number diff --git a/src/internal/godebugs/table.go b/src/internal/godebugs/table.go index 10b3919d51e10d..a011ec37759c88 100644 --- a/src/internal/godebugs/table.go +++ b/src/internal/godebugs/table.go @@ -42,6 +42,7 @@ var All = []Info{ {Name: "htmlmetacontenturlescape", Package: "html/template"}, {Name: "http2client", Package: "net/http"}, {Name: "http2debug", Package: "net/http", Opaque: true}, + {Name: "http2reuseframes", Package: "net/http"}, {Name: "http2server", Package: "net/http"}, {Name: "httpcookiemaxnum", Package: "net/http", Changed: 24, Old: "0"}, {Name: "httplaxcontentlength", Package: "net/http", Changed: 22, Old: "1"}, diff --git a/src/net/http/internal/http2/frame.go b/src/net/http/internal/http2/frame.go index a2de8c271904f0..6de87334adedac 100644 --- a/src/net/http/internal/http2/frame.go +++ b/src/net/http/internal/http2/frame.go @@ -9,6 +9,7 @@ import ( "encoding/binary" "errors" "fmt" + "internal/godebug" "io" "log" "slices" @@ -22,6 +23,12 @@ import ( "golang.org/x/net/http/httpguts" ) +// http2reuseframes controls whether the per-connection Framer in the +// stdlib server and Transport opts in to SetReuseFrames. Default is +// reuse on; GODEBUG=http2reuseframes=0 reverts to allocating each +// parsed frame fresh, matching the pre-CL behavior. +var http2reuseframes = godebug.New("http2reuseframes") + const frameHeaderLen = 9 var padZeros = make([]byte, 255) // zeros for padding @@ -433,7 +440,10 @@ func (fr *Framer) SetReuseFrames() { } type frameCache struct { - dataFrame DataFrame + dataFrame DataFrame + windowUpdateFrame WindowUpdateFrame + headersFrame HeadersFrame + metaHeadersFrame MetaHeadersFrame } func (fc *frameCache) getDataFrame() *DataFrame { @@ -443,6 +453,27 @@ func (fc *frameCache) getDataFrame() *DataFrame { return &fc.dataFrame } +func (fc *frameCache) getWindowUpdateFrame() *WindowUpdateFrame { + if fc == nil { + return &WindowUpdateFrame{} + } + return &fc.windowUpdateFrame +} + +func (fc *frameCache) getHeadersFrame() *HeadersFrame { + if fc == nil { + return &HeadersFrame{} + } + return &fc.headersFrame +} + +func (fc *frameCache) getMetaHeadersFrame() *MetaHeadersFrame { + if fc == nil { + return &MetaHeadersFrame{} + } + return &fc.metaHeadersFrame +} + // NewFramer returns a Framer that writes frames to w and reads them from r. func NewFramer(w io.Writer, r io.Reader) *Framer { fr := &Framer{ @@ -996,12 +1027,25 @@ func parseUnknownFrame(_ *frameCache, fh FrameHeader, countError func(string), p // A WindowUpdateFrame is used to implement flow control. // See https://httpwg.org/specs/rfc7540.html#rfc.section.6.9 +// +// When [Framer.SetReuseFrames] is in effect, the same *WindowUpdateFrame +// is returned by every (*Framer).ReadFrame call that parses a +// WINDOW_UPDATE and its fields are overwritten on each call, so callers +// must consume the StreamID and Increment fields before the next +// ReadFrame and must not retain the pointer. type WindowUpdateFrame struct { FrameHeader Increment uint32 // never read with high bit set } -func parseWindowUpdateFrame(_ *frameCache, fh FrameHeader, countError func(string), p []byte) (Frame, error) { +// parseWindowUpdateFrame populates the *WindowUpdateFrame returned by +// frameCache.getWindowUpdateFrame. When [Framer.SetReuseFrames] is in +// effect, that struct is reused across ReadFrame calls, so every field +// of WindowUpdateFrame must be assigned on the success path or stale +// data from a previous frame will be visible to the next caller. If a +// field is added to WindowUpdateFrame, update both this function and +// TestReadFrameWindowUpdateOverwrites. +func parseWindowUpdateFrame(fc *frameCache, fh FrameHeader, countError func(string), p []byte) (Frame, error) { if len(p) != 4 { countError("frame_windowupdate_bad_len") return nil, ConnectionError(ErrCodeFrameSize) @@ -1021,10 +1065,10 @@ func parseWindowUpdateFrame(_ *frameCache, fh FrameHeader, countError func(strin countError("frame_windowupdate_zero_inc_stream") return nil, streamError(fh.StreamID, ErrCodeProtocol) } - return &WindowUpdateFrame{ - FrameHeader: fh, - Increment: inc, - }, nil + wuf := fc.getWindowUpdateFrame() + wuf.FrameHeader = fh + wuf.Increment = inc + return wuf, nil } // WriteWindowUpdate writes a WINDOW_UPDATE frame. @@ -1043,6 +1087,12 @@ func (f *Framer) WriteWindowUpdate(streamID, incr uint32) error { // A HeadersFrame is used to open a stream and additionally carries a // header block fragment. +// +// When [Framer.SetReuseFrames] is in effect, the same *HeadersFrame +// is returned by every (*Framer).ReadFrame call that parses a HEADERS +// and its fields are overwritten on each call. The headerFragBuf +// slice always aliases the framer's read buffer and must not be +// retained past the next ReadFrame regardless of this setting. type HeadersFrame struct { FrameHeader @@ -1069,8 +1119,16 @@ func (f *HeadersFrame) HasPriority() bool { return f.FrameHeader.Flags.Has(FlagHeadersPriority) } -func parseHeadersFrame(_ *frameCache, fh FrameHeader, countError func(string), p []byte) (_ Frame, err error) { - hf := &HeadersFrame{ +// parseHeadersFrame populates the *HeadersFrame returned by +// frameCache.getHeadersFrame. When [Framer.SetReuseFrames] is in +// effect, that struct is reused across ReadFrame calls; the explicit +// `*hf = HeadersFrame{...}` reset below zeros every field so stale +// values (Priority, headerFragBuf) cannot leak from a prior frame. +// If a field is added to HeadersFrame, update the reset and +// TestReadFrameHeadersOverwrites. +func parseHeadersFrame(fc *frameCache, fh FrameHeader, countError func(string), p []byte) (_ Frame, err error) { + hf := fc.getHeadersFrame() + *hf = HeadersFrame{ FrameHeader: fh, } if fh.StreamID == 0 { @@ -1593,6 +1651,11 @@ type headersOrContinuation interface { // // This type of frame does not appear on the wire and is only returned // by the Framer when Framer.ReadMetaHeaders is set. +// +// When [Framer.SetReuseFrames] is in effect, the same *MetaHeadersFrame +// is returned by every ReadFrame call that produces one and its fields +// are overwritten on each call. Callers must consume Fields and +// Truncated before the next ReadFrame and must not retain the pointer. type MetaHeadersFrame struct { *HeadersFrame @@ -1710,7 +1773,10 @@ func (fr *Framer) readMetaFrame(hf *HeadersFrame) (Frame, error) { if fr.AllowIllegalReads { return nil, errors.New("illegal use of AllowIllegalReads with ReadMetaHeaders") } - mh := &MetaHeadersFrame{ + mh := fr.frameCache.getMetaHeadersFrame() + // Wholesale reset so values from a previous parse (Fields, + // Truncated, or any future-added field) cannot leak through. + *mh = MetaHeadersFrame{ HeadersFrame: hf, } var remainSize = fr.maxHeaderListSize() diff --git a/src/net/http/internal/http2/frame_reuse_e2e_test.go b/src/net/http/internal/http2/frame_reuse_e2e_test.go new file mode 100644 index 00000000000000..fd190c5f35e82d --- /dev/null +++ b/src/net/http/internal/http2/frame_reuse_e2e_test.go @@ -0,0 +1,114 @@ +// Copyright 2026 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package http2_test + +import ( + "io" + "net/http" + "strings" + "sync" + "testing" +) + +// TestFrameReuseEndToEndStress drives concurrent multiplexed requests +// through the real net/http HTTP/2 server and Transport (both of +// which call SetReuseFrames on the per-connection Framer) and touches +// every field reachable from a request/response in ways that would +// race against the read loop's next ReadFrame if anything still +// aliased the cached frame after the readMore gate (server) or +// read-loop iteration (Transport). +// +// Under -race this is a regression test against future refactors of +// processHeaders / handleResponse / processTrailers / processData +// that fail to copy aliased data. The synthetic Framer-pair tests in +// frame_reuse_race_test.go cannot reach the production process* code +// paths; this one does. +// +// The handler and client iterate Name/Value byte-by-byte so that any +// aliasing leak shows up as a concrete read concurrent with a write +// in bytes.(*Reader).Read inside ReadFrame. +func TestFrameReuseEndToEndStress(t *testing.T) { + if testing.Short() { + t.Skip("skipping stress test in short mode") + } + + const responseBody = "the quick brown fox jumps over the lazy dog" + + touchHeader := func(h http.Header) byte { + var sink byte + for k, vs := range h { + for i := 0; i < len(k); i++ { + sink ^= k[i] + } + for _, v := range vs { + for i := 0; i < len(v); i++ { + sink ^= v[i] + } + } + } + return sink + } + + ts := newTestServer(t, + func(w http.ResponseWriter, r *http.Request) { + _ = touchHeader(r.Header) + if _, err := io.Copy(io.Discard, r.Body); err != nil { + t.Errorf("server body copy: %v", err) + return + } + _ = touchHeader(r.Trailer) + w.Header().Set("Trailer", "X-Server-Trailer") + w.Header().Set("X-Response-Header", "response-value") + w.WriteHeader(http.StatusOK) + io.WriteString(w, responseBody) + w.Header().Set("X-Server-Trailer", "server-trailer-value") + }, + optQuiet, + ) + + tr := newTransport(t) + + const ( + workers = 8 + iterations = 25 + ) + + var wg sync.WaitGroup + for w := 0; w < workers; w++ { + wg.Add(1) + go func() { + defer wg.Done() + for i := 0; i < iterations; i++ { + body := strings.NewReader("client request body content") + req, err := http.NewRequest("POST", ts.URL, body) + if err != nil { + t.Errorf("NewRequest: %v", err) + return + } + req.Header.Set("X-Test-Header", "client-header-value") + req.Trailer = http.Header{ + "X-Client-Trailer": []string{"client-trailer-value"}, + } + res, err := tr.RoundTrip(req) + if err != nil { + t.Errorf("RoundTrip: %v", err) + return + } + _ = touchHeader(res.Header) + if _, err := io.Copy(io.Discard, res.Body); err != nil { + res.Body.Close() + t.Errorf("client body copy: %v", err) + return + } + if err := res.Body.Close(); err != nil { + t.Errorf("body close: %v", err) + return + } + _ = touchHeader(res.Trailer) + } + }() + } + wg.Wait() +} diff --git a/src/net/http/internal/http2/frame_reuse_race_test.go b/src/net/http/internal/http2/frame_reuse_race_test.go new file mode 100644 index 00000000000000..5988f85a00128f --- /dev/null +++ b/src/net/http/internal/http2/frame_reuse_race_test.go @@ -0,0 +1,385 @@ +// Copyright 2026 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package http2 + +import ( + "bytes" + "io" + "os" + "sync" + "sync/atomic" + "testing" + "time" + + "golang.org/x/net/http2/hpack" +) + +// The tests in this file exercise the Framer.SetReuseFrames contract +// from a concurrency angle. The reuse contract says the *Frame returned +// by ReadFrame, and any slice aliasing the Framer's read buffer +// reachable from that frame, are only valid until the next ReadFrame +// call. A retention bug under reuse manifests as one goroutine reading +// a retained slice while another (the reader) writes into the same +// backing memory on the next ReadFrame. +// +// These tests model the server's readFrames goroutine pattern (one +// reader goroutine, one consumer goroutine separated by a gate +// channel) and aim to be effective under "go test -race": +// +// - TestFrameReuseRaceCorrect: faithful gated handoff. Must NOT race +// under -race, regardless of how many frames are read. This is a +// positive control that asserts the gated pattern is safe with +// reuse on. +// +// - TestFrameReuseRaceAdversarial: deliberately leaks a slice past +// the gate. Must race under -race. Guarded behind the +// H2_REUSE_RACE_NEGATIVE=1 env var so normal `go test` and CI +// stay green; the assertion of the negative case is the race +// detector itself. +// +// A companion end-to-end stress test exists in +// frame_reuse_e2e_test.go (package http2_test) which drives the real +// net/http HTTP/2 server and Transport under -race. The synthetic +// tests in this file cannot reach the production process* code paths; +// the e2e test does. + +// preEncodeReuseRaceStream encodes a long sequence of frames +// exercising every frame type extended by SetReuseFrames: DATA, +// WINDOW_UPDATE, HEADERS, and HEADERS+CONTINUATION (so the meta +// header path runs). All HEADERS/DATA payloads are the same size so +// the framer's readBuf does not reallocate between frames; the +// retained slice in the adversarial subtest therefore continues to +// alias the same backing array that the next ReadFrame writes into. +func preEncodeReuseRaceStream(tb testing.TB, iters int) []byte { + tb.Helper() + var buf bytes.Buffer + fr := NewFramer(&buf, nil) + // Encode HPACK fields once; reuse to keep payload sizes stable. + hdrBlock := encodeHeaderRaw(tb, + ":method", "GET", + ":path", "/", + ":scheme", "http", + ":authority", "example.com", + ) + // Split hdrBlock so we can emit HEADERS+CONTINUATION for the + // meta path. + half := len(hdrBlock) / 2 + if half == 0 { + half = 1 + } + // Use a fixed-length data payload so the readBuf size stays + // constant once it grows on the first read. + dataPayload := bytes.Repeat([]byte{0xab}, 64) + + for i := 0; i < iters; i++ { + streamID := uint32(2*i + 1) + // DATA + if err := fr.WriteData(streamID, false, dataPayload); err != nil { + tb.Fatal(err) + } + // WINDOW_UPDATE + if err := fr.WriteWindowUpdate(streamID, uint32(1+i%4096)); err != nil { + tb.Fatal(err) + } + // HEADERS (single-frame, EndHeaders=true) + if err := fr.WriteHeaders(HeadersFrameParam{ + StreamID: streamID, + BlockFragment: hdrBlock, + EndHeaders: true, + }); err != nil { + tb.Fatal(err) + } + // HEADERS + CONTINUATION (drives the meta-headers path) + if err := fr.WriteHeaders(HeadersFrameParam{ + StreamID: streamID, + BlockFragment: hdrBlock[:half], + EndHeaders: false, + }); err != nil { + tb.Fatal(err) + } + if err := fr.WriteContinuation(streamID, true, hdrBlock[half:]); err != nil { + tb.Fatal(err) + } + } + return buf.Bytes() +} + +// runReadFramesGoroutine launches a goroutine that mimics +// serverConn.readFrames: reads a frame, sends it to ch, waits on the +// returned gate before reading the next. Returns the gate channel and +// a done channel that closes once the reader goroutine exits. +type framedRead struct { + frame Frame + err error + // done must be called once the consumer no longer retains frame + // or any slice aliasing the framer's read buffer. + done chan<- struct{} +} + +func runReadFramesGoroutine(tb testing.TB, fr *Framer, ch chan<- framedRead) (exited <-chan struct{}) { + tb.Helper() + exitc := make(chan struct{}) + go func() { + defer close(exitc) + for { + gate := make(chan struct{}) + f, err := fr.ReadFrame() + ch <- framedRead{frame: f, err: err, done: gate} + // Wait for the consumer to signal done before reading + // again. This re-creates the readFrames gate. + <-gate + if err != nil { + return + } + } + }() + return exitc +} + +// consumeSliceWork is a noinline helper that reads every byte of b +// once. Its only purpose is to ensure the race detector observes a +// read of the memory backing b. If a concurrent goroutine writes the +// same memory without a happens-before edge, -race will fire here. +// +//go:noinline +func consumeSliceWork(b []byte) byte { + var x byte + for _, v := range b { + x ^= v + } + return x +} + +// consumeHeaderFieldWork reads the name+value strings of every +// HeaderField. The string bytes themselves do not alias the framer's +// read buffer — hpack.decodeString allocates fresh strings via +// string(u.b) or bytes.Buffer.String() — so iterating Name/Value +// post-gate is not, in itself, a data race. The reuse-contract +// violation captured by the adversarial test below is retention of +// the Fields slice *header* (whose backing array is reachable from +// the cached *MetaHeadersFrame) past the next ReadFrame; the race +// detector cannot observe that for MetaHeadersFrame because hpack's +// string copies make the byte memory independent of the framer's +// read buffer. The function is kept for symmetry with consumeSliceWork +// and to document the contract for the MetaHeadersFrame case. +// +//go:noinline +func consumeHeaderFieldWork(fields []hpack.HeaderField) byte { + var x byte + for _, hf := range fields { + for i := 0; i < len(hf.Name); i++ { + x ^= hf.Name[i] + } + for i := 0; i < len(hf.Value); i++ { + x ^= hf.Value[i] + } + } + return x +} + +// TestFrameReuseRaceCorrect runs the full reader-consumer dance with +// SetReuseFrames enabled, consuming every frame's payload before +// signaling the gate. Under -race, this MUST NOT fire. +// +// What this catches: a reuse implementation that mutates the cached +// frame or its backing buffer before the consumer signals done (for +// example, by parsing the next frame eagerly on a background +// goroutine or by reusing the buffer for some other purpose). +func TestFrameReuseRaceCorrect(t *testing.T) { + const iters = 200 + + encoded := preEncodeReuseRaceStream(t, iters) + r := bytes.NewReader(encoded) + fr := NewFramer(io.Discard, r) + fr.SetReuseFrames() + fr.ReadMetaHeaders = hpack.NewDecoder(initialHeaderTableSize, nil) + + ch := make(chan framedRead) + exitc := runReadFramesGoroutine(t, fr, ch) + + // sink lets the compiler keep the work observable. + var sink atomic.Uint64 + + for { + select { + case res := <-ch: + if res.err == io.EOF { + close(res.done) + goto drain + } + if res.err != nil { + t.Fatalf("unexpected ReadFrame error: %v", res.err) + } + // Consume the slice/fields synchronously. Once we + // signal done, we forget the frame. + switch f := res.frame.(type) { + case *DataFrame: + sink.Add(uint64(consumeSliceWork(f.Data()))) + case *HeadersFrame: + sink.Add(uint64(consumeSliceWork(f.HeaderBlockFragment()))) + case *MetaHeadersFrame: + sink.Add(uint64(consumeHeaderFieldWork(f.Fields))) + case *WindowUpdateFrame: + sink.Add(uint64(f.Increment)) + default: + t.Fatalf("unexpected frame type %T", res.frame) + } + close(res.done) // gate: reader may proceed. + case <-time.After(30 * time.Second): + t.Fatal("timed out waiting for next frame") + } + } +drain: + <-exitc + t.Logf("consumed %d frames, sink=%d", iters*4, sink.Load()) +} + +// TestFrameReuseRaceAdversarial deliberately violates the reuse +// contract: the consumer hands a retained slice to a sidecar +// goroutine that keeps reading it indefinitely, while the gate is +// signaled immediately so the reader proceeds to overwrite the same +// memory. Under -race, this MUST fire. +// +// It is guarded behind H2_REUSE_RACE_NEGATIVE=1 because it is a +// negative control: failure (i.e., no race detected) is what we want +// to be loud about during development, but a passing test on stock +// CI is uninteresting. Run it as: +// +// H2_REUSE_RACE_NEGATIVE=1 go test -race -run TestFrameReuseRaceAdversarial +// +// What it would catch in production code: any handler/code path that +// holds onto HeaderBlockFragment, Data, or hpack.HeaderField name/value +// strings past the readMore call. +func TestFrameReuseRaceAdversarial(t *testing.T) { + if os.Getenv("H2_REUSE_RACE_NEGATIVE") != "1" { + t.Skip("skipping adversarial negative-control test; " + + "set H2_REUSE_RACE_NEGATIVE=1 to run under -race") + } + // 50 outer iterations -> 4*50 = 200 frames. That is far more + // than enough scheduler interleavings for the race detector to + // observe at least one concurrent read/write conflict. + const iters = 50 + const maxLiveAttackers = 8 // cap concurrent attacker goroutines + + encoded := preEncodeReuseRaceStream(t, iters) + r := bytes.NewReader(encoded) + fr := NewFramer(io.Discard, r) + fr.SetReuseFrames() + fr.ReadMetaHeaders = hpack.NewDecoder(initialHeaderTableSize, nil) + + ch := make(chan framedRead) + exitc := runReadFramesGoroutine(t, fr, ch) + + // stopAttackers is closed when the test is winding down to let + // any retained-slice readers exit. + stopAttackers := make(chan struct{}) + var attackers sync.WaitGroup + // Token bucket bounds the number of attacker goroutines alive + // at any time; without it the race detector slows to a crawl as + // many hundreds of goroutines all hammer the read buffer. + tokens := make(chan struct{}, maxLiveAttackers) + + // retainedRefs accumulates references we deliberately leak past + // the gate. Holding them in the test goroutine keeps them + // reachable for the GC's view, but the race detector still + // catches read/write conflicts on the underlying memory. + type retainedSlice struct { + b []byte + } + type retainedFields struct { + hf []hpack.HeaderField + } + var refs []any + + startAttacker := func(read func()) { + // Block briefly if too many attackers are already running. + // This will bound the runtime overhead without weakening the + // race detector signal (each retained reference still gets + // a goroutine that overlaps the next ReadFrame). + select { + case tokens <- struct{}{}: + case <-stopAttackers: + return + } + attackers.Add(1) + go func() { + defer attackers.Done() + defer func() { <-tokens }() + for i := 0; i < 200; i++ { + select { + case <-stopAttackers: + return + default: + } + read() + } + }() + } + + var sink atomic.Uint64 + + for { + select { + case res := <-ch: + if res.err == io.EOF { + close(res.done) + goto drain + } + if res.err != nil { + t.Fatalf("unexpected ReadFrame error: %v", res.err) + } + switch f := res.frame.(type) { + case *DataFrame: + retained := retainedSlice{b: f.Data()} + refs = append(refs, retained) + startAttacker(func() { + sink.Add(uint64(consumeSliceWork(retained.b))) + }) + case *HeadersFrame: + retained := retainedSlice{b: f.HeaderBlockFragment()} + refs = append(refs, retained) + startAttacker(func() { + sink.Add(uint64(consumeSliceWork(retained.b))) + }) + case *MetaHeadersFrame: + // Retaining the Fields slice header past the gate + // violates the reuse contract. The race detector + // will NOT fire for this frame type because hpack + // always allocates independent strings (see + // consumeHeaderFieldWork); the byte memory backing + // Name/Value is unrelated to the framer's read + // buffer. Included here to document contract intent; + // the DataFrame and HeadersFrame cases above are the + // ones that actually trigger -race. + retained := retainedFields{hf: f.Fields} + refs = append(refs, retained) + startAttacker(func() { + sink.Add(uint64(consumeHeaderFieldWork(retained.hf))) + }) + case *WindowUpdateFrame: + // No slice to retain on WindowUpdateFrame, just + // signal done. + default: + t.Fatalf("unexpected frame type %T", res.frame) + } + // Adversarial: signal done immediately, even though + // we still have outstanding readers of the frame's + // memory. + close(res.done) + case <-time.After(30 * time.Second): + t.Fatal("timed out waiting for next frame") + } + } +drain: + close(stopAttackers) + attackers.Wait() + <-exitc + // Force refs to outlive the loop above so the compiler does + // not eliminate the retentions. + if len(refs) == 0 { + t.Fatalf("expected retained references, got 0") + } + t.Logf("retained %d references, sink=%d", len(refs), sink.Load()) +} diff --git a/src/net/http/internal/http2/frame_test.go b/src/net/http/internal/http2/frame_test.go index 43cd66bfc3bdc0..12084fac22bd13 100644 --- a/src/net/http/internal/http2/frame_test.go +++ b/src/net/http/internal/http2/frame_test.go @@ -764,6 +764,572 @@ func TestWriteWindowUpdate(t *testing.T) { } } +// TestReadFrameReusesWindowUpdate verifies that ReadFrame returns the +// same *WindowUpdateFrame pointer for every WINDOW_UPDATE parsed when +// SetReuseFrames is in effect, so the parse path does not allocate a +// fresh struct each time. +func TestReadFrameReusesWindowUpdate(t *testing.T) { + fr, buf := testFramer() + fr.SetReuseFrames() + + // First read populates the cached struct. + if err := fr.WriteWindowUpdate(1, 5); err != nil { + t.Fatal(err) + } + first, err := fr.ReadFrame() + if err != nil { + t.Fatal(err) + } + firstWU, ok := first.(*WindowUpdateFrame) + if !ok { + t.Fatalf("first frame is %T, want *WindowUpdateFrame", first) + } + if firstWU.StreamID != 1 || firstWU.Increment != 5 { + t.Fatalf("first WINDOW_UPDATE = %+v; want StreamID=1 Increment=5", firstWU) + } + + // Subsequent WINDOW_UPDATEs return the same pointer with the new + // values. Because the pointer is reused, the originally-returned + // firstWU also reflects the latest fields after each ReadFrame. + cases := []struct { + streamID, increment uint32 + }{ + {streamID: 3, increment: 7}, + {streamID: 1, increment: 1024}, + {streamID: 1, increment: 1}, + } + for i, tc := range cases { + buf.Reset() + if err := fr.WriteWindowUpdate(tc.streamID, tc.increment); err != nil { + t.Fatal(err) + } + f, err := fr.ReadFrame() + if err != nil { + t.Fatal(err) + } + wu, ok := f.(*WindowUpdateFrame) + if !ok { + t.Fatalf("iter %d: frame is %T, want *WindowUpdateFrame", i, f) + } + if wu != firstWU { + t.Errorf("iter %d: pointer changed: have %p, want %p", i, wu, firstWU) + } + if wu.StreamID != tc.streamID || wu.Increment != tc.increment { + t.Errorf("iter %d: got %+v; want StreamID=%d Increment=%d", + i, wu, tc.streamID, tc.increment) + } + if firstWU.StreamID != tc.streamID || firstWU.Increment != tc.increment { + t.Errorf("iter %d: firstWU = %+v; want StreamID=%d Increment=%d (reuse contract violated)", + i, firstWU, tc.streamID, tc.increment) + } + } + + // Interleaving WINDOW_UPDATE with a different frame type does not + // disturb reuse: the cached struct is re-validated when the next + // WINDOW_UPDATE is parsed. + buf.Reset() + if err := fr.WritePing(false, [8]byte{1, 2, 3, 4, 5, 6, 7, 8}); err != nil { + t.Fatal(err) + } + if _, err := fr.ReadFrame(); err != nil { + t.Fatal(err) + } + buf.Reset() + if err := fr.WriteWindowUpdate(5, 9000); err != nil { + t.Fatal(err) + } + f, err := fr.ReadFrame() + if err != nil { + t.Fatal(err) + } + wu, ok := f.(*WindowUpdateFrame) + if !ok { + t.Fatalf("after PING, frame is %T, want *WindowUpdateFrame", f) + } + if wu != firstWU { + t.Errorf("after PING, pointer changed: have %p, want %p", wu, firstWU) + } + if wu.StreamID != 5 || wu.Increment != 9000 { + t.Errorf("after PING, got %+v; want StreamID=5 Increment=9000", wu) + } +} + +// TestReadFrameWindowUpdateNoAllocsWhenReused locks in the +// zero-allocation invariant for the WINDOW_UPDATE parse path when +// SetReuseFrames is in effect. If a regression makes +// parseWindowUpdateFrame allocate, this fails rather than only showing +// up in benchmarks. +func TestReadFrameWindowUpdateNoAllocsWhenReused(t *testing.T) { + // Pre-encode a WINDOW_UPDATE frame. + var enc bytes.Buffer + if err := NewFramer(&enc, nil).WriteWindowUpdate(1, 7); err != nil { + t.Fatal(err) + } + encoded := enc.Bytes() + + rbuf := bytes.NewReader(encoded) + fr := NewFramer(io.Discard, rbuf) + fr.SetReuseFrames() + + // Warm up the read buffer so its growth does not count toward the + // measurement. + rbuf.Reset(encoded) + if _, err := fr.ReadFrame(); err != nil { + t.Fatal(err) + } + + allocs := testing.AllocsPerRun(50, func() { + rbuf.Reset(encoded) + if _, err := fr.ReadFrame(); err != nil { + t.Fatal(err) + } + }) + if allocs != 0 { + t.Errorf("ReadFrame for WINDOW_UPDATE allocates %v objects/op; want 0", allocs) + } +} + +// TestReadFrameWindowUpdateOverwrites is a defensive test against +// future maintenance hazards. When SetReuseFrames is in effect, the +// cached *WindowUpdateFrame is reused across ReadFrame calls, so any +// field that parseWindowUpdateFrame forgets to assign would leak its +// value from the previous frame to the next consumer. +// +// The test poisons every byte of the cached struct, parses a fresh +// WINDOW_UPDATE, and then verifies every field reflects the new frame +// rather than the poison. If a field is added to WindowUpdateFrame in +// the future, the corresponding assertion (and parseWindowUpdateFrame) +// must be updated. +func TestReadFrameWindowUpdateOverwrites(t *testing.T) { + fr, buf := testFramer() + fr.SetReuseFrames() + + // First read to obtain the cached struct pointer. + if err := fr.WriteWindowUpdate(1, 5); err != nil { + t.Fatal(err) + } + first, err := fr.ReadFrame() + if err != nil { + t.Fatal(err) + } + wuf, ok := first.(*WindowUpdateFrame) + if !ok { + t.Fatalf("first frame is %T, want *WindowUpdateFrame", first) + } + + // Fill every byte of the cached struct with 0xFF. If any field is + // left unassigned by the next parse, it will keep this poison value. + // + // valid is a bool, and any non-zero byte reads as true; bulk 0xFF + // poison would therefore mask a parser that forgets to reassign + // valid. Set valid to false separately so the post-parse + // expectation (valid == true) genuinely tests a fresh write. + poison := unsafe.Slice((*byte)(unsafe.Pointer(wuf)), unsafe.Sizeof(*wuf)) + for i := range poison { + poison[i] = 0xFF + } + wuf.valid = false + + // Parse a fresh WINDOW_UPDATE. The poison must be fully gone. + buf.Reset() + const wantStreamID = 42 + const wantIncrement = 100 + if err := fr.WriteWindowUpdate(wantStreamID, wantIncrement); err != nil { + t.Fatal(err) + } + f, err := fr.ReadFrame() + if err != nil { + t.Fatal(err) + } + wu, ok := f.(*WindowUpdateFrame) + if !ok { + t.Fatalf("frame is %T, want *WindowUpdateFrame", f) + } + if wu != wuf { + t.Fatalf("pointer changed after poison: have %p, want %p", wu, wuf) + } + + // Check every field. Failure on any of these means + // parseWindowUpdateFrame left a field unassigned, which would leak + // data from a previous frame to the next consumer. + if wu.Type != FrameWindowUpdate { + t.Errorf("Type = %v (poison leak); want %v", wu.Type, FrameWindowUpdate) + } + if wu.Flags != 0 { + t.Errorf("Flags = %#x (poison leak); want 0", wu.Flags) + } + if wu.Length != 4 { + t.Errorf("Length = %d (poison leak); want 4", wu.Length) + } + if wu.StreamID != wantStreamID { + t.Errorf("StreamID = %d (poison leak); want %d", wu.StreamID, wantStreamID) + } + if !wu.valid { + t.Errorf("valid = false (poison leak); want true") + } + if wu.Increment != wantIncrement { + t.Errorf("Increment = %d (poison leak); want %d", wu.Increment, wantIncrement) + } +} + +// TestReadFrameWindowUpdateDistinctWithoutReuse asserts the +// pre-SetReuseFrames contract: without opting in, each parsed +// WINDOW_UPDATE returns a distinct *WindowUpdateFrame whose fields +// remain valid even after a subsequent ReadFrame. +func TestReadFrameWindowUpdateDistinctWithoutReuse(t *testing.T) { + fr, buf := testFramer() + + if err := fr.WriteWindowUpdate(1, 5); err != nil { + t.Fatal(err) + } + first, err := fr.ReadFrame() + if err != nil { + t.Fatal(err) + } + firstWU, ok := first.(*WindowUpdateFrame) + if !ok { + t.Fatalf("first frame is %T, want *WindowUpdateFrame", first) + } + + buf.Reset() + if err := fr.WriteWindowUpdate(3, 7); err != nil { + t.Fatal(err) + } + second, err := fr.ReadFrame() + if err != nil { + t.Fatal(err) + } + secondWU, ok := second.(*WindowUpdateFrame) + if !ok { + t.Fatalf("second frame is %T, want *WindowUpdateFrame", second) + } + + if firstWU == secondWU { + t.Errorf("without SetReuseFrames, expected distinct pointers; got same: %p", firstWU) + } + if firstWU.StreamID != 1 || firstWU.Increment != 5 { + t.Errorf("first WU mutated after second ReadFrame: %+v; want StreamID=1 Increment=5", firstWU) + } + if secondWU.StreamID != 3 || secondWU.Increment != 7 { + t.Errorf("second WU = %+v; want StreamID=3 Increment=7", secondWU) + } +} + +// TestReadFrameReusesHeadersFrame verifies that ReadFrame returns +// the same *HeadersFrame pointer for every HEADERS parsed when +// SetReuseFrames is in effect. +func TestReadFrameReusesHeadersFrame(t *testing.T) { + fr, buf := testFramer() + fr.SetReuseFrames() + + write := func(streamID uint32) { + t.Helper() + if err := fr.WriteHeaders(HeadersFrameParam{ + StreamID: streamID, + BlockFragment: []byte("abc"), + EndHeaders: true, + }); err != nil { + t.Fatal(err) + } + } + + write(1) + first, err := fr.ReadFrame() + if err != nil { + t.Fatal(err) + } + firstHF, ok := first.(*HeadersFrame) + if !ok { + t.Fatalf("first frame is %T, want *HeadersFrame", first) + } + + for i, streamID := range []uint32{3, 5, 7} { + buf.Reset() + write(streamID) + f, err := fr.ReadFrame() + if err != nil { + t.Fatal(err) + } + hf, ok := f.(*HeadersFrame) + if !ok { + t.Fatalf("iter %d: frame is %T, want *HeadersFrame", i, f) + } + if hf != firstHF { + t.Errorf("iter %d: pointer changed: have %p, want %p", i, hf, firstHF) + } + if hf.StreamID != streamID { + t.Errorf("iter %d: StreamID = %d, want %d", i, hf.StreamID, streamID) + } + } +} + +// TestReadFrameHeadersOverwrites is a defensive test against future +// maintenance hazards. When SetReuseFrames is in effect, the cached +// *HeadersFrame is reused across ReadFrame calls; any field that +// parseHeadersFrame forgets to assign would leak from the previous +// frame to the next caller. +// +// The test parses a HEADERS frame WITH Priority and padding to +// populate all fields, then parses a second frame WITHOUT either +// flag and asserts the previous Priority and headerFragBuf-related +// state do not bleed through. +func TestReadFrameHeadersOverwrites(t *testing.T) { + fr, buf := testFramer() + fr.SetReuseFrames() + + // First frame: priority + padding to populate every field. + if err := fr.WriteHeaders(HeadersFrameParam{ + StreamID: 9, + BlockFragment: []byte("xyz"), + EndHeaders: true, + PadLength: 3, + Priority: PriorityParam{ + StreamDep: 7, + Exclusive: true, + Weight: 100, + }, + }); err != nil { + t.Fatal(err) + } + first, err := fr.ReadFrame() + if err != nil { + t.Fatal(err) + } + hf1 := first.(*HeadersFrame) + if hf1.Priority.StreamDep != 7 || !hf1.Priority.Exclusive || hf1.Priority.Weight != 100 { + t.Fatalf("test setup: first frame priority = %+v; want StreamDep=7 Exclusive=true Weight=100", hf1.Priority) + } + + // Second frame: no priority, no padding. Priority must reset. + buf.Reset() + if err := fr.WriteHeaders(HeadersFrameParam{ + StreamID: 11, + BlockFragment: []byte("ab"), + EndHeaders: true, + }); err != nil { + t.Fatal(err) + } + second, err := fr.ReadFrame() + if err != nil { + t.Fatal(err) + } + hf2 := second.(*HeadersFrame) + if hf2 != hf1 { + t.Fatalf("expected same pointer (cached); have %p, want %p", hf2, hf1) + } + if (hf2.Priority != PriorityParam{}) { + t.Errorf("Priority leak: %+v (want zero)", hf2.Priority) + } + if hf2.Flags.Has(FlagHeadersPriority) || hf2.Flags.Has(FlagHeadersPadded) { + t.Errorf("Flags leak: %x", hf2.Flags) + } + if hf2.StreamID != 11 { + t.Errorf("StreamID = %d, want 11", hf2.StreamID) + } +} + +// TestReadFrameHeadersDistinctWithoutReuse asserts the +// pre-SetReuseFrames contract: without opting in, each parsed +// HEADERS returns a distinct *HeadersFrame whose Priority and +// FrameHeader remain stable after a subsequent ReadFrame. +func TestReadFrameHeadersDistinctWithoutReuse(t *testing.T) { + fr, buf := testFramer() + + if err := fr.WriteHeaders(HeadersFrameParam{ + StreamID: 1, + BlockFragment: []byte("abc"), + EndHeaders: true, + Priority: PriorityParam{ + StreamDep: 7, + Exclusive: true, + Weight: 100, + }, + }); err != nil { + t.Fatal(err) + } + first, err := fr.ReadFrame() + if err != nil { + t.Fatal(err) + } + hf1 := first.(*HeadersFrame) + + buf.Reset() + if err := fr.WriteHeaders(HeadersFrameParam{ + StreamID: 3, + BlockFragment: []byte("xy"), + EndHeaders: true, + }); err != nil { + t.Fatal(err) + } + second, err := fr.ReadFrame() + if err != nil { + t.Fatal(err) + } + hf2 := second.(*HeadersFrame) + + if hf1 == hf2 { + t.Errorf("without SetReuseFrames, expected distinct pointers; got same: %p", hf1) + } + if hf1.StreamID != 1 || hf1.Priority.StreamDep != 7 { + t.Errorf("first HF mutated after second ReadFrame: StreamID=%d Priority=%+v", hf1.StreamID, hf1.Priority) + } + if hf2.StreamID != 3 { + t.Errorf("second HF StreamID = %d, want 3", hf2.StreamID) + } +} + +// TestReadMetaFrameReusesMetaHeadersFrame verifies that every +// ReadFrame call returning a *MetaHeadersFrame returns the same +// cached pointer when SetReuseFrames is in effect. +func TestReadMetaFrameReusesMetaHeadersFrame(t *testing.T) { + fr, buf := testFramer() + fr.SetReuseFrames() + fr.ReadMetaHeaders = hpack.NewDecoder(initialHeaderTableSize, nil) + + writeMeta := func(streamID uint32, fields ...string) { + t.Helper() + block := encodeHeaderRaw(t, fields...) + if err := fr.WriteHeaders(HeadersFrameParam{ + StreamID: streamID, + BlockFragment: block, + EndHeaders: true, + }); err != nil { + t.Fatal(err) + } + } + + writeMeta(1, ":method", "GET", ":path", "/", ":scheme", "http", ":authority", "x") + first, err := fr.ReadFrame() + if err != nil { + t.Fatal(err) + } + firstMH, ok := first.(*MetaHeadersFrame) + if !ok { + t.Fatalf("first frame is %T, want *MetaHeadersFrame", first) + } + + for i, streamID := range []uint32{3, 5, 7} { + buf.Reset() + writeMeta(streamID, ":method", "POST", ":path", "/p", ":scheme", "http", ":authority", "y") + f, err := fr.ReadFrame() + if err != nil { + t.Fatal(err) + } + mh, ok := f.(*MetaHeadersFrame) + if !ok { + t.Fatalf("iter %d: frame is %T, want *MetaHeadersFrame", i, f) + } + if mh != firstMH { + t.Errorf("iter %d: *MetaHeadersFrame pointer changed: have %p, want %p", i, mh, firstMH) + } + if mh.StreamID != streamID { + t.Errorf("iter %d: StreamID = %d, want %d", i, mh.StreamID, streamID) + } + } +} + +// TestReadMetaFrameMetaHeadersOverwrites verifies the cached +// MetaHeadersFrame's Truncated flag (and any future-added field) +// does not leak from a prior parse: the explicit +// `*mh = MetaHeadersFrame{...}` reset at the start of readMetaFrame +// must zero every field. +func TestReadMetaFrameMetaHeadersOverwrites(t *testing.T) { + fr, buf := testFramer() + fr.SetReuseFrames() + fr.ReadMetaHeaders = hpack.NewDecoder(initialHeaderTableSize, nil) + // MaxHeaderListSize that fits the first two HPACK fields (each + // encodes as size = name + value + 32 = 42 for the pseudo-headers + // used below) but truncates from the third onward. The block of + // hpack-encoded bytes stays well under 2*MaxHeaderListSize so the + // early "header list too large" abort doesn't fire. + fr.MaxHeaderListSize = 100 + + writeMeta := func(streamID uint32, fields ...string) { + t.Helper() + block := encodeHeaderRaw(t, fields...) + if err := fr.WriteHeaders(HeadersFrameParam{ + StreamID: streamID, + BlockFragment: block, + EndHeaders: true, + }); err != nil { + t.Fatal(err) + } + } + + // First parse: too-many headers -> Truncated. + writeMeta(1, ":method", "GET", ":path", "/a", ":scheme", "http", ":authority", "x") + first, err := fr.ReadFrame() + if err != nil { + t.Fatal(err) + } + mh1 := first.(*MetaHeadersFrame) + if !mh1.Truncated { + t.Fatalf("test setup: first parse not marked Truncated as expected") + } + + // Raise the limit, parse a fitting frame, and confirm Truncated + // did not bleed into the second result. + fr.MaxHeaderListSize = 1 << 16 + buf.Reset() + writeMeta(3, ":method", "GET", ":path", "/b", ":scheme", "http", ":authority", "y") + second, err := fr.ReadFrame() + if err != nil { + t.Fatal(err) + } + mh2 := second.(*MetaHeadersFrame) + if mh2 != mh1 { + t.Fatalf("expected same cached *MetaHeadersFrame; have %p, want %p", mh2, mh1) + } + if mh2.Truncated { + t.Errorf("Truncated leaked from prior parse") + } +} + +// TestReadMetaFrameDistinctWithoutReuse asserts the pre-SetReuseFrames +// contract for the meta-headers path: without opting in, each +// readMetaFrame returns a distinct *MetaHeadersFrame. +func TestReadMetaFrameDistinctWithoutReuse(t *testing.T) { + fr, buf := testFramer() + fr.ReadMetaHeaders = hpack.NewDecoder(initialHeaderTableSize, nil) + + writeMeta := func(streamID uint32, fields ...string) { + t.Helper() + block := encodeHeaderRaw(t, fields...) + if err := fr.WriteHeaders(HeadersFrameParam{ + StreamID: streamID, + BlockFragment: block, + EndHeaders: true, + }); err != nil { + t.Fatal(err) + } + } + + writeMeta(1, ":method", "GET", ":path", "/", ":scheme", "http", ":authority", "x") + first, err := fr.ReadFrame() + if err != nil { + t.Fatal(err) + } + mh1 := first.(*MetaHeadersFrame) + + buf.Reset() + writeMeta(3, ":method", "POST", ":path", "/p", ":scheme", "http", ":authority", "y") + second, err := fr.ReadFrame() + if err != nil { + t.Fatal(err) + } + mh2 := second.(*MetaHeadersFrame) + + if mh1 == mh2 { + t.Errorf("without SetReuseFrames, expected distinct *MetaHeadersFrame pointers; got same: %p", mh1) + } + if mh1.StreamID != 1 { + t.Errorf("first MH mutated after second ReadFrame: StreamID=%d, want 1", mh1.StreamID) + } + if mh2.StreamID != 3 { + t.Errorf("second MH StreamID = %d, want 3", mh2.StreamID) + } +} + func TestWritePing(t *testing.T) { testWritePing(t, false) } func TestWritePingAck(t *testing.T) { testWritePing(t, true) } @@ -1605,3 +2171,149 @@ func TestTypeFrameParserHolePanic(t *testing.T) { t.Errorf("got %T; want *UnknownFrame", f) } } + +// BenchmarkParseDataFrame measures the per-parse cost of DATA +// frames with and without SetReuseFrames. The DataFrame cache is the +// preexisting one; this exists so its contribution can be compared +// directly against the WindowUpdate/Headers/MetaHeaders caches added +// in this stack. +func BenchmarkParseDataFrame(b *testing.B) { + var enc bytes.Buffer + if err := NewFramer(&enc, nil).WriteData(1, false, []byte("abc")); err != nil { + b.Fatal(err) + } + encoded := enc.Bytes() + + run := func(b *testing.B, reuse bool) { + rbuf := bytes.NewReader(encoded) + fr := NewFramer(io.Discard, rbuf) + if reuse { + fr.SetReuseFrames() + } + rbuf.Reset(encoded) + if _, err := fr.ReadFrame(); err != nil { + b.Fatal(err) + } + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + rbuf.Reset(encoded) + if _, err := fr.ReadFrame(); err != nil { + b.Fatal(err) + } + } + } + b.Run("Default", func(b *testing.B) { run(b, false) }) + b.Run("Reused", func(b *testing.B) { run(b, true) }) +} + +// BenchmarkParseWindowUpdateFrame measures the per-parse cost of +// WINDOW_UPDATE frames with and without SetReuseFrames. The Default +// case allocates a fresh *WindowUpdateFrame each call; Reused uses +// the cached one. +func BenchmarkParseWindowUpdateFrame(b *testing.B) { + var enc bytes.Buffer + if err := NewFramer(&enc, nil).WriteWindowUpdate(1, 7); err != nil { + b.Fatal(err) + } + encoded := enc.Bytes() + + run := func(b *testing.B, reuse bool) { + rbuf := bytes.NewReader(encoded) + fr := NewFramer(io.Discard, rbuf) + if reuse { + fr.SetReuseFrames() + } + // Warm up the read buffer so its growth doesn't count. + rbuf.Reset(encoded) + if _, err := fr.ReadFrame(); err != nil { + b.Fatal(err) + } + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + rbuf.Reset(encoded) + if _, err := fr.ReadFrame(); err != nil { + b.Fatal(err) + } + } + } + b.Run("Default", func(b *testing.B) { run(b, false) }) + b.Run("Reused", func(b *testing.B) { run(b, true) }) +} + +// BenchmarkParseHeadersFrame measures HEADERS parsing with and +// without SetReuseFrames. +func BenchmarkParseHeadersFrame(b *testing.B) { + var enc bytes.Buffer + if err := NewFramer(&enc, nil).WriteHeaders(HeadersFrameParam{ + StreamID: 1, + BlockFragment: []byte("abc"), + EndHeaders: true, + }); err != nil { + b.Fatal(err) + } + encoded := enc.Bytes() + + run := func(b *testing.B, reuse bool) { + rbuf := bytes.NewReader(encoded) + fr := NewFramer(io.Discard, rbuf) + if reuse { + fr.SetReuseFrames() + } + rbuf.Reset(encoded) + if _, err := fr.ReadFrame(); err != nil { + b.Fatal(err) + } + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + rbuf.Reset(encoded) + if _, err := fr.ReadFrame(); err != nil { + b.Fatal(err) + } + } + } + b.Run("Default", func(b *testing.B) { run(b, false) }) + b.Run("Reused", func(b *testing.B) { run(b, true) }) +} + +// BenchmarkReadMetaFrame measures HEADERS+HPACK decoding via +// readMetaFrame with and without SetReuseFrames. With reuse, the +// cached *HeadersFrame and *MetaHeadersFrame wrappers are both +// eliminated from the allocation count. +func BenchmarkReadMetaFrame(b *testing.B) { + block := encodeHeaderRaw(b, ":method", "GET", ":path", "/", ":scheme", "http", ":authority", "x") + var enc bytes.Buffer + if err := NewFramer(&enc, nil).WriteHeaders(HeadersFrameParam{ + StreamID: 1, + BlockFragment: block, + EndHeaders: true, + }); err != nil { + b.Fatal(err) + } + encoded := enc.Bytes() + + run := func(b *testing.B, reuse bool) { + rbuf := bytes.NewReader(encoded) + fr := NewFramer(io.Discard, rbuf) + fr.ReadMetaHeaders = hpack.NewDecoder(initialHeaderTableSize, nil) + if reuse { + fr.SetReuseFrames() + } + rbuf.Reset(encoded) + if _, err := fr.ReadFrame(); err != nil { + b.Fatal(err) + } + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + rbuf.Reset(encoded) + if _, err := fr.ReadFrame(); err != nil { + b.Fatal(err) + } + } + } + b.Run("Default", func(b *testing.B) { run(b, false) }) + b.Run("Reused", func(b *testing.B) { run(b, true) }) +} diff --git a/src/net/http/internal/http2/server.go b/src/net/http/internal/http2/server.go index 37808dd0d269a8..d77c055648cb6d 100644 --- a/src/net/http/internal/http2/server.go +++ b/src/net/http/internal/http2/server.go @@ -322,6 +322,11 @@ func (s *Server) serveConn(c net.Conn, opts *ServeConnOpts, newf func(*serverCon fr.ReadMetaHeaders = hpack.NewDecoder(uint32(conf.MaxDecoderHeaderTableSize), nil) fr.MaxHeaderListSize = sc.maxHeaderListSize() fr.SetMaxReadFrameSize(uint32(conf.MaxReadFrameSize)) + if http2reuseframes.Value() == "0" { + http2reuseframes.IncNonDefault() + } else { + fr.SetReuseFrames() + } sc.framer = fr if tc, ok := c.(connectionStater); ok { diff --git a/src/net/http/internal/http2/transport.go b/src/net/http/internal/http2/transport.go index ae3cf3571a631c..bf12d530f812fb 100644 --- a/src/net/http/internal/http2/transport.go +++ b/src/net/http/internal/http2/transport.go @@ -662,6 +662,11 @@ func (t *Transport) newClientConn(c net.Conn, singleUse bool, internalStateHook maxHeaderTableSize := uint32(conf.MaxDecoderHeaderTableSize) cc.fr.ReadMetaHeaders = hpack.NewDecoder(maxHeaderTableSize, nil) cc.fr.MaxHeaderListSize = t.maxHeaderListSize() + if http2reuseframes.Value() == "0" { + http2reuseframes.IncNonDefault() + } else { + cc.fr.SetReuseFrames() + } cc.henc = hpack.NewEncoder(&cc.hbuf) cc.henc.SetMaxDynamicTableSizeLimit(uint32(conf.MaxEncoderHeaderTableSize)) @@ -2626,6 +2631,13 @@ func (rl *clientConnReadLoop) processGoAway(f *GoAwayFrame) error { if cc.goAwayDebug == "" { cc.goAwayDebug = string(f.DebugData()) } + // debugData aliases the Framer's read buffer. SetReuseFrames does + // not cache GoAwayFrame today, so cc.goAway = f is safe, but we + // retain the *GoAwayFrame across ReadFrame calls; clear the only + // field that aliases the read buffer so a future addition of + // GoAwayFrame to frameCache cannot turn cc.goAway.DebugData() into + // a use-after-overwrite. + f.debugData = nil if old != nil && old.ErrCode != ErrCodeNo { cc.goAway.ErrCode = old.ErrCode } diff --git a/src/runtime/metrics/doc.go b/src/runtime/metrics/doc.go index 22648f4008c73f..9024c73c413be4 100644 --- a/src/runtime/metrics/doc.go +++ b/src/runtime/metrics/doc.go @@ -311,6 +311,11 @@ Below is the full list of supported metrics, ordered lexicographically. The number of non-default behaviors executed by the net/http package due to a non-default GODEBUG=http2client=... setting. + /godebug/non-default-behavior/http2reuseframes:events + The number of non-default behaviors executed by the net/http + package due to a non-default GODEBUG=http2reuseframes=... + setting. + /godebug/non-default-behavior/http2server:events The number of non-default behaviors executed by the net/http package due to a non-default GODEBUG=http2server=... setting.