[go] net/http/internal/http2: enable SetReuseFrames in server and Transport
4 views
Skip to first unread message
Gerrit Bot (Gerrit)
May 23, 2026, 2:41:58 PMMay 23
to goph...@pubsubhelper.golang.org, Hiro Protagonist, golang-co...@googlegroups.com
Gerrit Bot has uploaded the change for review![]( "Open in Gerrit")
Commit message
net/http/internal/http2: enable SetReuseFrames in server and Transport
This stack extends Framer.SetReuseFrames to cover *WindowUpdateFrame,
*HeadersFrame, and *MetaHeadersFrame (in addition to the existing
*DataFrame), then calls SetReuseFrames once on the per-connection
Framer in serverConn and Transport.newClientConn. Result: ~50% fewer
per-op allocations on the package's read-path benchmarks
(BenchmarkDownloadFrameSize, BenchmarkClientGzip), with no measurable
change on write-path benchmarks and no API change.
A new GODEBUG=http2reuseframes=0 setting is registered alongside
http2client / http2server / http2debug and reverts to the pre-CL
allocate-per-frame behavior.
Background
Framer.SetReuseFrames has shipped as an opt-in Framer method since
golang/net CL 34812 (commit bb807669, 2017-02-27, fixing #18502). It
was added for gRPC-Go, whose Framer profiling showed parseDataFrame
accounting for ~66% of allocations (1.4 GB of 2.2 GB) on a 64-conn /
100-stream / 900K RT/sec benchmark. gRPC-Go calls SetReuseFrames
directly on the Framer it owns.
No in-tree caller in the standard library has ever opted in, so the
cache has been effectively dead code from the stdlib's perspective
for nine years. This CL turns it on.
Prior attempts and reviewer concerns
This is the third attempt to enable frame reuse on the stdlib's
HTTP/2 server/Transport:
- Issue #68352 (still open, 2024-07-09): a request to improve
frame decoding performance. neild rejected a user-facing config
option ("There's no need for a user-configurable option here.
If there are performance improvements that can be made to frame
decoding in the HTTP/2 implementation, we can just make them.")
but left the door open for internal enablement.
- golang/net CL 597455 (PR golang/net#216, 2024-07-10): tried to
enable on both server and Transport unconditionally. neild's
blocking review: "this is not correct. The server read goroutine
(serverConn.readFrames) reads frames and delivers them to the
main server goroutine for processing," and "Changing the HTTP/2
server and/or transport to use the frame cache is likely to
require some significant refactoring." Stale; never landed.
- golang/net CL 676235 (server-only, 2025-05-25): neild Hold+1 on
2025-06-24: "this is not a safe change for the server.
SetReuseFrames causes each frame to be invalidated by the next
call to ReadFrame, but the server processes frames concurrently
with ReadFrame calls." Abandoned 2025-10-24.
The blocker on both prior CLs was correctness, specifically retention
of frame data past the next ReadFrame. This CL addresses that with a
static retention audit, a race-detector test that exercises the gated
handoff, an end-to-end stress test under -race, and a GODEBUG escape
hatch in case the analysis missed something.
Stack layout
Six commits, each independently reviewable:
1. reuse *WindowUpdateFrame across ReadFrame calls
2. reuse *HeadersFrame across ReadFrame calls
3. reuse *MetaHeadersFrame across ReadFrame calls
4. add microbenchmarks for the SetReuseFrames path
5. add race-detector tests (positive control, adversarial negative
control, end-to-end stress)
6. enable SetReuseFrames in server and Transport, register
http2reuseframes GODEBUG
Commits 1-3 mirror the existing *DataFrame cache pattern from CL
34812: wholesale "*p = T{...}" reset at the top of each parse
function, slot in frameCache, nil-safe get<Type>Frame getter. No
behavior change without SetReuseFrames; the caches are dormant unless
opted in.
Safety analysis
Static retention audit across every code path that consumes a
reusable *Frame, plus a separate concurrency audit. Per frame type:
*DataFrame
f.Data() is copied into dataBuffer.Write
(databuffer.go:126 copy(chunk[b.w:], p)) before readMore. The
pool-allocated chunk has independent backing.
*WindowUpdateFrame
Struct has only scalar fields. Safe by construction.
*HeadersFrame
Both server and Transport set Framer.ReadMetaHeaders during init
(server.go:322, transport.go:663), so a bare *HeadersFrame never
reaches consumer code. readMetaFrame (frame.go:1868-1869) clears
headerFragBuf=nil and invalidates the embedded *HeadersFrame
before returning.
*MetaHeadersFrame
readMetaFrame:1772 does "*mh = MetaHeadersFrame{HeadersFrame: hf}",
zeroing Fields and Truncated. Append grows a fresh backing array
per parse. hpack.decodeString (vendor/.../hpack.go:509-520)
returns string(u.b) or buf.String(), both of which copy, so
Name/Value strings are independently allocated and do not alias
readBuf. Server's processHeaders / newWriterAndRequest and
Transport's handleResponse / processTrailers iterate Fields
synchronously and copy into fresh http.Header maps before
returning. No code stores *MetaHeadersFrame past dispatch.
Concurrency:
Server: serverConn.readFrames (server.go:692-711) sends each
parsed frame on readFrameCh then blocks on gate. The serve
goroutine receives, calls processFrameFromReader synchronously,
then res.readMore() (server.go:854), which releases the gate.
The single readMore call site fires only after the synchronous
dispatch returns, so the cache is never overwritten while
consumer code still references it.
Transport: clientConnReadLoop.run (transport.go:2046-2090) reads
frames in a tight loop with no per-frame gate, so each process*
function must fully consume aliased data before returning.
Verified: every byte that escapes the loop is either copied
(DataFrame -> bufPipe) or composed of immutable,
independently-allocated Go strings (Header maps populated from
hpack-decoded strings).
Spawned goroutines: runHandler (server) only sees the fresh
*ServerRequest whose Header/Trailer/URL are independently
allocated. writeFrameAsync, body-close, Shutdown, ping writers
don't touch read-side cached frames.
Defensive cleanup: GoAwayFrame.debugData is cleared after copying it
to cc.goAwayDebug in processGoAway. GoAwayFrame is not in frameCache
today so the retained cc.goAway=f pointer is safe, but the 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.
Benchmarks
-count=10 -benchtime=1s -benchmem, linux/amd64 i7-6770HQ @ 2.60GHz,
master vs h2-frame-cache, via benchstat.
Allocations per op (the main win):
Benchmark master branch delta p
ClientGzip 174 71 -59.20% 0.000
DownloadFrameSize/16k 369.2k 196.7k -46.73% 0.000
DownloadFrameSize/64k 96.32k 49.12k -49.00% 0.000
DownloadFrameSize/128k 48.62k 24.53k -49.55% 0.000
DownloadFrameSize/256k 24.32k 12.19k -49.87% 0.000
DownloadFrameSize/512k 12,175 6,090 -49.98% 0.000
ClientRequestHeaders/0 50 46 -8.00% 0.000
ClientRequestHeaders/10 74 70 -5.41% 0.000
ClientResponseHeaders/0 50 46 -8.00% 0.000
ClientResponseHeaders/10 115 111 -3.48% 0.000
geomean (allocs/op) -19.46%
Write-side benchmarks (WriteScheduler*, WriteQueue) are unchanged
within noise; those paths don't touch ReadFrame.
Latency / throughput (sec/op):
Benchmark delta sec/op p
ClientGzip -2.74% 0.023
ClientRequestHeaders/0 -1.04% 0.029
ClientResponseHeaders/0 -1.12% 0.019
DownloadFrameSize/* sec/op ~ (10-54% RSD, socket noise)
Write/WriteQueue ~ (p >= 0.22)
Per-cache attribution from microbenchmarks added in commit 4:
Cache Default Reused
ParseDataFrame 115 ns / 48 B / 1 alloc 76 ns / 0 / 0
ParseWindowUpdateFrame 92 ns / 16 B / 1 alloc 74 ns / 0 / 0
ParseHeadersFrame 124 ns / 48 B / 1 alloc 82 ns / 0 / 0
ReadMetaFrame 1073 ns / 472 B / 9 951 ns / 376 / 7
The residual 7 allocations in ReadMetaFrame/Reused come from HPACK
Fields-slice growth and the SetEmitFunc closure; out of scope for
this stack.
Test plan
Tests added in commit 5 and verified against the enablement commit:
frame_reuse_race_test.go (package http2, synthetic Framer-pair tests):
- TestFrameReuseRaceCorrect: positive control. Models
serverConn.readFrames with a reader goroutine, a channel, a
per-frame gate, and a consumer that fully consumes payload
before signalling done. 800 frames across DATA, WINDOW_UPDATE,
HEADERS, HEADERS+CONTINUATION. Must PASS under -race.
- TestFrameReuseRaceAdversarial: negative control, gated behind
H2_REUSE_RACE_NEGATIVE=1. Same plumbing, but deliberately leaks
slices into a sidecar goroutine across the next ReadFrame. Must
RACE under -race to verify the detector functions for this
pattern.
frame_reuse_e2e_test.go (package http2_test, real server + Transport):
- TestFrameReuseEndToEndStress: 8 workers x 25 multiplexed
requests through the real httptest-based HTTP/2 server and
net/http.Transport, both running with SetReuseFrames enabled.
Handler and client touch every byte of every header name/value
and trailer in both directions, drain bodies. Regression test
against future refactors of processHeaders / handleResponse /
processTrailers / processData that fail to copy aliased data.
Validation runs (linux/amd64):
Run Result
net/http/internal/http2 -race -count=30 PASS 8m13s
net/http/internal/http2 -race -count=10
-cpu=1,2,4,8 PASS 6m17s
net/http -race -count=10 PASS ~5min
TestFrameReuseRaceCorrect -race -count=200 PASS 5s
TestFrameReuseRaceAdversarial under
H2_REUSE_RACE_NEGATIVE=1 -race -count=10 RACE 10/10
TestFrameReuseEndToEndStress -race PASS 0.5s
GODEBUG=http2reuseframes=0 + correct-path
tests -race PASS
GODEBUG escape hatch
Registered alongside the existing http2 GODEBUGs in
src/internal/godebugs/table.go:
Value Behavior
unset or http2reuseframes=1 reuse on (default)
GODEBUG=http2reuseframes=0 reuse off (pre-CL behavior)
increments
/godebug/non-default-behavior/
http2reuseframes:events
Documented in doc/godebug.md under the Go 1.27 section. The intent is
a compat hatch, not a tuning knob: operators can disable without
recompiling if the static analysis and race coverage missed
something. This addresses the safety concerns from prior CLs while
staying consistent with the preference against user-facing
performance knobs (this is purely a safety/compat escape).
Compatibility
- No API change.
- No change in public behavior; the read loop returns the same
parsed frame values, only the per-call heap allocation is
elided.
- internal/http2 is unimportable from user code; the Framer type
and SetReuseFrames method remain unreachable from outside the
stdlib.
- The escape hatch is registered without Changed/Old version
gating (mirroring http2client / http2server) since there is no
language-visible behavior change to anchor to a go directive
version.
Updates #18502
Updates #68352
Change-Id: I11014d78ca0350f8d12055102f2fb22f7ade672a
GitHub-Last-Rev: 4382c906d19c3486eb846e48cb728ae4f44e9826
GitHub-Pull-Request: golang/go#79634
Change diff
diff --git a/doc/godebug.md b/doc/godebug.md
index 9c5c01d..794d13e 100644
--- a/doc/godebug.md
+++ b/doc/godebug.md
@@ -188,6 +188,14 @@
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 10b3919..a011ec3 100644
--- a/src/internal/godebugs/table.go
+++ b/src/internal/godebugs/table.go
@@ -42,6 +42,7 @@
{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 a2de8c2..6de8733 100644
--- a/src/net/http/internal/http2/frame.go
+++ b/src/net/http/internal/http2/frame.go
@@ -9,6 +9,7 @@
"encoding/binary"
"errors"
"fmt"
+ "internal/godebug"
"io"
"log"
"slices"
@@ -22,6 +23,12 @@
"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 @@
}
type frameCache struct {
- dataFrame DataFrame
+ dataFrame DataFrame
+ windowUpdateFrame WindowUpdateFrame
+ headersFrame HeadersFrame
+ metaHeadersFrame MetaHeadersFrame
}
func (fc *frameCache) getDataFrame() *DataFrame {
@@ -443,6 +453,27 @@
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 @@
// 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 @@
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 @@
// 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 @@
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 @@
//
// 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 @@
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 0000000..fd190c5
--- /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 0000000..5988f85
--- /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 43cd66b..12084fa 100644
--- a/src/net/http/internal/http2/frame_test.go
+++ b/src/net/http/internal/http2/frame_test.go
@@ -764,6 +764,572 @@
}
}
+// 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 @@
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 37808dd..d77c055 100644
--- a/src/net/http/internal/http2/server.go
+++ b/src/net/http/internal/http2/server.go
@@ -322,6 +322,11 @@
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 ae3cf35..bf12d53 100644
--- a/src/net/http/internal/http2/transport.go
+++ b/src/net/http/internal/http2/transport.go
@@ -662,6 +662,11 @@
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 @@
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 22648f4..9024c73 100644
--- a/src/runtime/metrics/doc.go
+++ b/src/runtime/metrics/doc.go
@@ -311,6 +311,11 @@
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.
Change information
Files:
- M doc/godebug.md
- M src/internal/godebugs/table.go
- M src/net/http/internal/http2/frame.go
- A src/net/http/internal/http2/frame_reuse_e2e_test.go
- A src/net/http/internal/http2/frame_reuse_race_test.go
- M src/net/http/internal/http2/frame_test.go
- M src/net/http/internal/http2/server.go
- M src/net/http/internal/http2/transport.go
- M src/runtime/metrics/doc.go
Change size: XL
Delta: 9 files changed, 1317 insertions(+), 9 deletions(-)
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gopher Robot (Gerrit)
May 23, 2026, 2:44:13 PMMay 23
to Gerrit Bot, Hiro Protagonist, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Message from Gopher Robot![]( "Open in Gerrit")
Congratulations on opening your first change. Thank you for your contribution!
Next steps:
A maintainer will review your change and provide feedback. See
https://go.dev/doc/contribute#review for more info and tips to get your
patch through code review.
Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.
During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11 or adds a tag like "wait-release", it means that this CL will be
reviewed as part of the next development cycle. See https://go.dev/s/release
for more details.
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gerrit Bot (Gerrit)
Jun 9, 2026, 5:22:58 PM (12 hours ago) Jun 9
to Hiro Protagonist, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Damien Neil, Michael Pratt and Nicholas Husin
Gerrit Bot uploaded new patchset![]( "Open in Gerrit")
Gerrit Bot uploaded patch set #2 to this change.
Related details
Attention is currently required from:
- Damien Neil
- Michael Pratt
- Nicholas Husin
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages