From b8767037bd342c7702373611c039169738a9bce4 Mon Sep 17 00:00:00 2001 From: Chris Lundquist Date: Sat, 16 May 2026 17:52:52 +0000 Subject: [PATCH 1/6] net/http/internal/http2: reuse WindowUpdateFrame across ReadFrame calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirror DataFrame's existing reuse pattern for WindowUpdateFrame: add the struct as a sibling field on frameCache, expose a nil-safe getWindowUpdateFrame, and have parseWindowUpdateFrame populate the cached struct in place. Reuse is opt-in via Framer.SetReuseFrames, matching DataFrame's existing API contract; without SetReuseFrames, parseWindowUpdateFrame returns a freshly-allocated *WindowUpdateFrame and behavior is identical to before this CL. When SetReuseFrames is in effect, both in-tree consumers extract the StreamID and Increment fields synchronously before the next ReadFrame call: * On the server, (*serverConn).processWindowUpdate runs on the serve goroutine. The readFrames goroutine that calls ReadFrame blocks on res.readMore() until the previous frame has been fully processed. * On the client, (*clientConnReadLoop).processWindowUpdate runs inline within readLoop's frame-handling switch and returns before the next ReadFrame call. WindowUpdateFrame parses dominate the heap-allocation profile for large transfers: the receiver sends one WINDOW_UPDATE per ~16 KiB of data at both the stream and connection levels. BenchmarkParseWindowUpdateFrame (added in a later CL in this stack) measures the cache's contribution on linux/amd64: variant B/op allocs/op ns/op Default 16 1 ~95 Reused 0 0 ~77 (-1 alloc, -19%) Four tests guard the change: TestReadFrameReusesWindowUpdate: same pointer across consecutive WINDOW_UPDATE parses when SetReuseFrames is in effect. TestReadFrameWindowUpdateNoAllocsWhenReused: locks in zero allocations per parse with SetReuseFrames. TestReadFrameWindowUpdateOverwrites: defensive — poisons the cached struct and confirms every field is re-assigned by the next parse. TestReadFrameWindowUpdateDistinctWithoutReuse: pre-SetReuseFrames API contract — distinct pointers per parse, no mutation across calls. Change-Id: Ibcef724f7bc2ee2bd1a74cb2577d61e3117a872e --- src/net/http/internal/http2/frame.go | 33 ++- src/net/http/internal/http2/frame_test.go | 251 ++++++++++++++++++++++ 2 files changed, 278 insertions(+), 6 deletions(-) diff --git a/src/net/http/internal/http2/frame.go b/src/net/http/internal/http2/frame.go index a2de8c271904f0..2941bdb2443d5d 100644 --- a/src/net/http/internal/http2/frame.go +++ b/src/net/http/internal/http2/frame.go @@ -433,7 +433,8 @@ func (fr *Framer) SetReuseFrames() { } type frameCache struct { - dataFrame DataFrame + dataFrame DataFrame + windowUpdateFrame WindowUpdateFrame } func (fc *frameCache) getDataFrame() *DataFrame { @@ -443,6 +444,13 @@ func (fc *frameCache) getDataFrame() *DataFrame { return &fc.dataFrame } +func (fc *frameCache) getWindowUpdateFrame() *WindowUpdateFrame { + if fc == nil { + return &WindowUpdateFrame{} + } + return &fc.windowUpdateFrame +} + // 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 +1004,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 +1042,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. diff --git a/src/net/http/internal/http2/frame_test.go b/src/net/http/internal/http2/frame_test.go index 43cd66bfc3bdc0..841725098f03d9 100644 --- a/src/net/http/internal/http2/frame_test.go +++ b/src/net/http/internal/http2/frame_test.go @@ -764,6 +764,257 @@ 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) + } +} + func TestWritePing(t *testing.T) { testWritePing(t, false) } func TestWritePingAck(t *testing.T) { testWritePing(t, true) } From f368cb722218c5011b810b64bfdd0649147a30a6 Mon Sep 17 00:00:00 2001 From: Chris Lundquist Date: Sun, 17 May 2026 19:09:48 +0000 Subject: [PATCH 2/6] net/http/internal/http2: reuse HeadersFrame across ReadFrame calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirror DataFrame's existing reuse pattern for HeadersFrame: add it as a sibling field on frameCache, expose a nil-safe getHeadersFrame, and have parseHeadersFrame populate the cached struct in place. Reuse is opt-in via Framer.SetReuseFrames, matching DataFrame and the prior WindowUpdateFrame CL on this branch; without SetReuseFrames, parseHeadersFrame allocates a fresh *HeadersFrame and behavior is identical to before this CL. The explicit `*hf = HeadersFrame{FrameHeader: fh}` reset at the top of parseHeadersFrame zeros every field so values from a previous parse (Priority, headerFragBuf, Flags carried by FrameHeader) cannot leak into the next caller. The later flag-gated assignments fill Priority only when FlagHeadersPriority is set. Reuse safety, when SetReuseFrames is in effect: * In net/http's internal use of this package, the *HeadersFrame returned by parseHeadersFrame is wrapped into a MetaHeadersFrame by readMetaFrame within the same ReadFrame call. The MetaHeadersFrame's embedded *HeadersFrame is then invalidated (Header().valid = false) before the next ReadFrame. * The headerFragBuf slice already aliases the framer's read buffer in both the cached and fresh variants, matching the existing DataFrame contract. Every request on every HTTP/2 connection triggers exactly one parseHeadersFrame call on each side, so when SetReuseFrames is in effect this allocation is removed from the hottest of hot paths. BenchmarkParseHeadersFrame (added in a later CL in this stack) measures the cache's contribution on linux/amd64: variant B/op allocs/op ns/op Default 48 1 ~125 Reused 0 0 ~86 (-1 alloc, -31%) Three tests guard the change: TestReadFrameReusesHeadersFrame: same *HeadersFrame pointer across consecutive HEADERS parses when SetReuseFrames is in effect. TestReadFrameHeadersOverwrites: defensive — parses a HEADERS frame with Priority + padding then a HEADERS frame with neither, asserts Priority and Flags do not bleed. TestReadFrameHeadersDistinctWithoutReuse: pre-SetReuseFrames API contract — distinct *HeadersFrame pointers per parse, no mutation across calls. Change-Id: Ieacc0f4cb14454169d87a533b9ab06e01142f809 --- src/net/http/internal/http2/frame.go | 26 +++- src/net/http/internal/http2/frame_test.go | 163 ++++++++++++++++++++++ 2 files changed, 187 insertions(+), 2 deletions(-) diff --git a/src/net/http/internal/http2/frame.go b/src/net/http/internal/http2/frame.go index 2941bdb2443d5d..d95bcbb325ba27 100644 --- a/src/net/http/internal/http2/frame.go +++ b/src/net/http/internal/http2/frame.go @@ -435,6 +435,7 @@ func (fr *Framer) SetReuseFrames() { type frameCache struct { dataFrame DataFrame windowUpdateFrame WindowUpdateFrame + headersFrame HeadersFrame } func (fc *frameCache) getDataFrame() *DataFrame { @@ -451,6 +452,13 @@ func (fc *frameCache) getWindowUpdateFrame() *WindowUpdateFrame { return &fc.windowUpdateFrame } +func (fc *frameCache) getHeadersFrame() *HeadersFrame { + if fc == nil { + return &HeadersFrame{} + } + return &fc.headersFrame +} + // 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{ @@ -1064,6 +1072,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 @@ -1090,8 +1104,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 { diff --git a/src/net/http/internal/http2/frame_test.go b/src/net/http/internal/http2/frame_test.go index 841725098f03d9..f132f950556b0a 100644 --- a/src/net/http/internal/http2/frame_test.go +++ b/src/net/http/internal/http2/frame_test.go @@ -1015,6 +1015,169 @@ func TestReadFrameWindowUpdateDistinctWithoutReuse(t *testing.T) { } } +// 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) + } +} + func TestWritePing(t *testing.T) { testWritePing(t, false) } func TestWritePingAck(t *testing.T) { testWritePing(t, true) } From 8a639684c8159f53a1377618eef8945818bd7145 Mon Sep 17 00:00:00 2001 From: Chris Lundquist Date: Sun, 17 May 2026 19:11:24 +0000 Subject: [PATCH 3/6] net/http/internal/http2: reuse MetaHeadersFrame across ReadFrame calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirror DataFrame's existing reuse pattern for the MetaHeadersFrame wrapper produced by readMetaFrame: store it as a sibling field on frameCache, expose a nil-safe getMetaHeadersFrame, and have readMetaFrame populate the cached struct in place. Reuse is opt-in via Framer.SetReuseFrames, matching DataFrame, WindowUpdateFrame, and HeadersFrame on this branch; without SetReuseFrames, readMetaFrame allocates a fresh *MetaHeadersFrame and behavior is identical to before this CL. The explicit `*mh = MetaHeadersFrame{HeadersFrame: hf}` reset at the top of readMetaFrame ensures Fields, Truncated, and any future-added field are zeroed so values from a previous (possibly aborted) parse cannot leak into the next caller. Reuse safety, when SetReuseFrames is in effect: * The two consumers ((*serverConn).processHeaders and (*clientConnReadLoop).processHeaders) extract PseudoValue, RegularFields, and Truncated synchronously and never retain the *MetaHeadersFrame past the next ReadFrame. * The embedded HeadersFrame is the cached one from the prior CL on this branch; the *MetaHeadersFrame wrapper is one more level of the same sharing, with the same single-Framer-goroutine safety. BenchmarkReadMetaFrame (added in a later CL in this stack) measures the combined HF+MHF cache contribution on linux/amd64: variant B/op allocs/op ns/op Default 472 9 ~1062 Reused 376 7 ~1004 (-2 allocs, -5%) The -2 allocs reflect the *HeadersFrame (saved by the HF CL) and the *MetaHeadersFrame (saved here). The remaining 7 allocations come from HPACK Fields-slice growth and the SetEmitFunc closure, which are addressed by separate CLs not part of this branch. Three tests guard the change: TestReadMetaFrameReusesMetaHeadersFrame: same *MetaHeadersFrame pointer across consecutive parses when SetReuseFrames is in effect. TestReadMetaFrameMetaHeadersOverwrites: defensive — first parse triggers Truncated, second parse fits; Truncated does not leak. TestReadMetaFrameDistinctWithoutReuse: pre-SetReuseFrames API contract — distinct *MetaHeadersFrame pointers per parse. Change-Id: I93e6916075b1110137eada96b8017bc38b700165 --- src/net/http/internal/http2/frame.go | 18 ++- src/net/http/internal/http2/frame_test.go | 152 ++++++++++++++++++++++ 2 files changed, 169 insertions(+), 1 deletion(-) diff --git a/src/net/http/internal/http2/frame.go b/src/net/http/internal/http2/frame.go index d95bcbb325ba27..980d54bee32c71 100644 --- a/src/net/http/internal/http2/frame.go +++ b/src/net/http/internal/http2/frame.go @@ -436,6 +436,7 @@ type frameCache struct { dataFrame DataFrame windowUpdateFrame WindowUpdateFrame headersFrame HeadersFrame + metaHeadersFrame MetaHeadersFrame } func (fc *frameCache) getDataFrame() *DataFrame { @@ -459,6 +460,13 @@ func (fc *frameCache) getHeadersFrame() *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{ @@ -1636,6 +1644,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 @@ -1753,7 +1766,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_test.go b/src/net/http/internal/http2/frame_test.go index f132f950556b0a..6da56b6d10e8b3 100644 --- a/src/net/http/internal/http2/frame_test.go +++ b/src/net/http/internal/http2/frame_test.go @@ -1178,6 +1178,158 @@ func TestReadFrameHeadersDistinctWithoutReuse(t *testing.T) { } } +// 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) } From 61099b6596c68e9e6427199258d7a7984ab72762 Mon Sep 17 00:00:00 2001 From: Chris Lundquist Date: Mon, 18 May 2026 21:15:45 +0000 Subject: [PATCH 4/6] net/http/internal/http2: add microbenchmarks for SetReuseFrames path Add direct parser microbenchmarks that exercise the SetReuseFrames- enabled paths so the reuse savings introduced earlier in this stack are observable per-cache. Each has Default and Reused subtests so benchstat can attribute the delta to the cache rather than to other parts of the parse path: * BenchmarkParseDataFrame (preexisting DataFrame cache) * BenchmarkParseWindowUpdateFrame (new on this branch) * BenchmarkParseHeadersFrame (new on this branch) * BenchmarkReadMetaFrame (new HeadersFrame + MetaHeadersFrame caches) BenchmarkParseDataFrame is included so the contribution of the preexisting DataFrame cache can be compared directly against the caches added in this stack; without it, a reviewer has no reference point. Microbench, linux/amd64: bench Default Reused ParseDataFrame 48 B, 1 alloc 0 B, 0 alloc ParseWindowUpdateFrame 16 B, 1 alloc 0 B, 0 alloc ParseHeadersFrame 48 B, 1 alloc 0 B, 0 alloc ReadMetaFrame 472 B, 9 allocs 376 B, 7 allocs Each parse-path cache saves the same shape (1 alloc per parse). The ReadMetaFrame Reused result reflects both the HeadersFrame cache (saving 1 alloc) and the MetaHeadersFrame cache (saving 1 alloc). The remaining 7 allocations come from HPACK Fields-slice growth and the SetEmitFunc closure; those are out of scope for this branch. The package's existing end-to-end download benchmarks do not exercise the SetReuseFrames path because no in-tree caller opts in. Wiring SetReuseFrames into transport.go/server.go (whether via a test-only hook or a public knob) is a separate change and is deliberately not part of this branch. Change-Id: Ic3651e52baae0417e40d2e31bdd5bdee6b4e6ce9 --- src/net/http/internal/http2/frame_test.go | 146 ++++++++++++++++++++++ 1 file changed, 146 insertions(+) diff --git a/src/net/http/internal/http2/frame_test.go b/src/net/http/internal/http2/frame_test.go index 6da56b6d10e8b3..12084fac22bd13 100644 --- a/src/net/http/internal/http2/frame_test.go +++ b/src/net/http/internal/http2/frame_test.go @@ -2171,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) }) +} From b2834db68072e462110f09412d4894c38abe3b62 Mon Sep 17 00:00:00 2001 From: Chris Lundquist Date: Tue, 19 May 2026 02:38:16 +0000 Subject: [PATCH 5/6] net/http/internal/http2: add race-detector tests for SetReuseFrames retention contract The SetReuseFrames contract is that frames 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 retained data while the reader goroutine writes into the same backing memory on the next ReadFrame. The existing reuse tests assert pointer/slice-header stability across ReadFrame calls but do not exercise the concurrent reader/consumer shape that the server uses (serverConn.readFrames feeding the serve goroutine over a channel gate) and do not reach the production process* paths at all. Add three complementary tests covering both levels. frame_reuse_race_test.go (package http2): TestFrameReuseRaceCorrect (positive control) Models the gated handoff: a reader goroutine calls ReadFrame and delivers the result over a channel, the consumer fully consumes the frame's payload (DataFrame.Data, HeadersFrame.HeaderBlockFragment, MetaHeadersFrame.Fields, WindowUpdateFrame.Increment) and only then signals the per-frame gate so the reader may proceed. Drives 800 frames across DATA, WINDOW_UPDATE, HEADERS, and HEADERS+CONTINUATION (the meta-headers path). Under -race must pass; future changes that eagerly mutate the cached frame or its backing buffer before the consumer signals done would fail this test. TestFrameReuseRaceAdversarial (negative control) Same plumbing, but the consumer deliberately leaks the retained slice into a sidecar goroutine and closes the gate immediately. All payloads are size-stable so the framer's readBuf stops growing and the retained slice keeps aliasing the buffer the next ReadFrame writes into. Guarded behind H2_REUSE_RACE_NEGATIVE=1 so default go test skips it; the assertion is the race detector itself. The DataFrame.Data and HeadersFrame.HeaderBlockFragment cases fire the detector under -race; the MetaHeadersFrame case is retained for contract documentation but does not race because hpack allocates independent strings (see consumeHeaderFieldWork docstring). frame_reuse_e2e_test.go (package http2_test): TestFrameReuseEndToEndStress Drives concurrent multiplexed requests through the real net/http HTTP/2 server and Transport (both of which call SetReuseFrames on their 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). 8 workers x 25 requests, each exercising headers, body, and trailers in both directions. 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 cannot reach the production process* code paths; this one does. Without -race, TestFrameReuseRaceCorrect remains a useful integration test of the gated reader/consumer pattern, and TestFrameReuseEndToEndStress remains a smoke test of the server + Transport multiplexed path. No production-code behavior change; tests only. Change-Id: I8a86eb2440618c2f85f3d823ceabcf789cbb596c --- .../internal/http2/frame_reuse_e2e_test.go | 114 ++++++ .../internal/http2/frame_reuse_race_test.go | 385 ++++++++++++++++++ 2 files changed, 499 insertions(+) create mode 100644 src/net/http/internal/http2/frame_reuse_e2e_test.go create mode 100644 src/net/http/internal/http2/frame_reuse_race_test.go 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()) +} From 4382c906d19c3486eb846e48cb728ae4f44e9826 Mon Sep 17 00:00:00 2001 From: Chris Lundquist Date: Tue, 19 May 2026 02:38:54 +0000 Subject: [PATCH 6/6] net/http/internal/http2: enable SetReuseFrames in server and Transport Call Framer.SetReuseFrames once on the per-connection Framer in both serverConn (serveConn) and Transport (newClientConn) so the parsed *DataFrame, *WindowUpdateFrame, *HeadersFrame, and *MetaHeadersFrame structs returned by ReadFrame are reused across calls instead of being heap-allocated each time. SetReuseFrames has shipped as an opt-in Framer method since CL 34812 (golang/net, 2017) but no in-tree caller previously opted in, so the cache was effectively dead code from the standard library's perspective; this CL turns it on. A new GODEBUG setting, http2reuseframes, is registered as a compat escape hatch in line with the existing http2client / http2server / http2debug settings: - default (or http2reuseframes=1): reuse on, this CL's behavior. - GODEBUG=http2reuseframes=0: reuse disabled, pre-CL behavior. If a deployment encounters an issue the static audit and -race coverage didn't catch, operators can flip the setting without recompiling. Per-frame allocation savings, microbench numbers (linux/amd64, from the BenchmarkParse* and BenchmarkReadMetaFrame benchmarks added earlier in this stack): bench Default Reused ParseDataFrame 48 B, 1 alloc 0 B, 0 alloc ParseWindowUpdateFrame 16 B, 1 alloc 0 B, 0 alloc ParseHeadersFrame 48 B, 1 alloc 0 B, 0 alloc ReadMetaFrame 472 B, 9 allocs 376 B, 7 allocs End-to-end allocation reductions on the package's existing read-path benchmarks (-count=10, benchstat master vs branch): bench allocs/op delta p ClientGzip -59.20% 0.000 DownloadFrameSize/16k -46.73% 0.000 DownloadFrameSize/64k -49.00% 0.000 DownloadFrameSize/128k -49.55% 0.000 DownloadFrameSize/256k -49.87% 0.000 DownloadFrameSize/512k -49.98% 0.000 ClientRequestHeaders/0 -8.00% 0.000 ClientResponseHeaders/0 -8.00% 0.000 ClientRequestHeaders/10 -5.41% 0.000 ClientResponseHeaders/10 -3.48% 0.000 geomean (allocs/op) -19.46% Write-side benchmarks (WriteScheduler*, WriteQueue) are unchanged, as expected; those paths do not exercise ReadFrame. ClientGzip latency also improves by -2.74% (p=0.023). The remaining allocations in ReadMetaFrame come from HPACK Fields-slice growth and the SetEmitFunc closure, which this CL does not address. Reuse safety, by frame type: DataFrame serverConn.processData and clientConnReadLoop.processData read Length, StreamID, StreamEnded, and the Data() slice synchronously before returning. The bytes from Data() flow through {server,client}Conn.body / cs.bufPipe -> dataBuffer.Write, which copies into a pool-allocated chunk; no slice retained past ReadFrame. WindowUpdateFrame serverConn.processWindowUpdate and clientConnReadLoop.processWindowUpdate read only the scalar StreamID and Increment fields. The struct type has no slice fields, so there is nothing to alias the read buffer. HeadersFrame Both server and Transport set Framer.ReadMetaHeaders during init, so a bare *HeadersFrame is never delivered to consumer code; the Framer always returns *MetaHeadersFrame on a HEADERS frame. readMetaFrame clears MetaHeadersFrame.HeadersFrame.headerFragBuf and calls invalidate() on the embedded *HeadersFrame before returning. The aliased frag buf is therefore not exposed past readMetaFrame. MetaHeadersFrame The Fields slice is freshly allocated per parse: readMetaFrame does *mh = MetaHeadersFrame{HeadersFrame: hf} (Fields zeroed to nil), then the HPACK emit callback grows it via append, so each returned MetaHeadersFrame has its own backing array. HPACK Name/Value strings are independently allocated by the decoder (hpack.decodeString returns string(u.b) / buf.String(), both of which copy), so no string aliases the read buffer. serverConn.processHeaders / processTrailerHeaders and clientConnReadLoop.processHeaders / handleResponse / processTrailers iterate Fields synchronously on the read-loop / serve goroutine and copy strings into a fresh http.Header before returning. No code stores *MetaHeadersFrame past the dispatch. Server-side gating: readFrames -> readFrameCh -> serve calls processFrameFromReader synchronously and only then invokes readMore(), which unblocks readFrames for the next ReadFrame. So even though the server consumes frames on a different goroutine from the one calling ReadFrame, every frame is fully consumed before the cache is overwritten. Transport-side gating: clientConnReadLoop.run consumes each frame synchronously on the read-loop goroutine before the next ReadFrame, and what escapes to the RoundTrip / response-body goroutines is either copied (DataFrame -> bufPipe) or composed of immutable, independently-allocated Go strings (Header maps). While here, defensively clear GoAwayFrame.debugData after copying it to cc.goAwayDebug in processGoAway. GoAwayFrame is not in frameCache today so the retained cc.goAway pointer is safe, but this Transport already stores a *GoAwayFrame across ReadFrame calls; clearing the only field that aliases the read buffer prevents a future addition of GoAwayFrame to the reuse cache from silently turning cc.goAway.DebugData() into a use-after-overwrite. Verification: net/http/internal/http2: go test -race -count=30 PASS (8m13s) net/http/internal/http2: go test -race -count=10 -cpu=1,2,4,8 PASS (6m17s) net/http: go test -race -count=5 PASS (2m15s) TestFrameReuseRaceCorrect -race -count=200 PASS (5s) TestFrameReuseRaceAdversarial under H2_REUSE_RACE_NEGATIVE=1 -race -count=10 RACE (10/10) TestFrameReuseEndToEndStress -race PASS GODEBUG=http2reuseframes=0 -race (TestFrameReuseEndToEndStress, TestFrameReuseRaceCorrect) PASS The adversarial test deliberately violates the reuse contract and asserts the race detector fires; the other runs validate the production code paths both with reuse on (the default) and with reuse disabled via GODEBUG. Change-Id: I9d6d0c2e761b0901314c25f362c99062260b31a7 --- doc/godebug.md | 8 ++++++++ src/internal/godebugs/table.go | 1 + src/net/http/internal/http2/frame.go | 7 +++++++ src/net/http/internal/http2/server.go | 5 +++++ src/net/http/internal/http2/transport.go | 12 ++++++++++++ src/runtime/metrics/doc.go | 5 +++++ 6 files changed, 38 insertions(+) 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 980d54bee32c71..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 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.