Brad Fitzpatrick (Gerrit)

unread,

Aug 6, 2026, 1:41:44 PM (yesterday) Aug 6

to goph...@pubsubhelper.golang.org, Brad Fitzpatrick, golang-co...@googlegroups.com

Brad Fitzpatrick has uploaded the change for review

Commit message

net/http/internal/http2: release handler write buffers when idle

Each in-flight server response held a 4KB bufio.Writer for the
lifetime of its handler, including handlers that sit idle
mid-response for a very long time between writes, as when streaming
long polls or server-sent events. For servers with many such
concurrent streams, that's 4KB of dead weight per stream. (e.g. 40 GB
of RAM at 10M streaming conns)

Instead of tying the buffer to the responseWriterState for the whole
response, acquire it from a pool on the first buffered write and
return it to the pool whenever a Flush leaves it empty. Handlers that
never flush keep the buffer until the handler completes, as before.

Updates #80735

Change-Id: Icf7cdb5c21abb1126f6571cb00bbe2f00bf23b3c

Change diff

diff --git a/src/net/http/internal/http2/export_test.go b/src/net/http/internal/http2/export_test.go
index 570fd3f..742cb05 100644
--- a/src/net/http/internal/http2/export_test.go
+++ b/src/net/http/internal/http2/export_test.go
@@ -225,3 +225,15 @@
 func EncodeRequestHeaders(req *ClientRequest, addGzipHeader bool, peerMaxHeaderListSize uint64, headerf func(name, value string)) (httpcommon.EncodeHeadersResult, error) {
 	return encodeRequestHeaders(req, addGzipHeader, peerMaxHeaderListSize, headerf)
 }
+
+func (w *responseWriter) hasWriteBuffer() bool {
+	return w.rws.bw != nil
+}
+
+// ResponseWriterHasWriteBufferForTesting reports whether w (which must
+// be or embed this package's responseWriter) currently holds a write
+// buffer. It is for testing that Flush releases the buffer while a
+// handler is parked mid-response.
+func ResponseWriterHasWriteBufferForTesting(w any) bool {
+	return w.(interface{ hasWriteBuffer() bool }).hasWriteBuffer()
+}
diff --git a/src/net/http/internal/http2/server.go b/src/net/http/internal/http2/server.go
index aed43b2..e3285c8 100644
--- a/src/net/http/internal/http2/server.go
+++ b/src/net/http/internal/http2/server.go
@@ -75,9 +75,20 @@
 
 var responseWriterStatePool = sync.Pool{
 	New: func() any {
-		rws := &responseWriterState{}
-		rws.bw = bufio.NewWriterSize(chunkWriter{rws}, handlerChunkWriteSize)
-		return rws
+		return &responseWriterState{}
+	},
+}
+
+// handlerWriterPool is a pool of the bufio.Writers used by
+// responseWriterState (rws.bw) to buffer handler response writes.
+//
+// The buffers are acquired from the pool lazily on the first buffered
+// write and, notably, are returned to it by Flush when empty, so that a
+// handler that's parked mid-response for a long time (e.g. streaming a
+// long poll) doesn't pin a buffer per stream.
+var handlerWriterPool = sync.Pool{
+	New: func() any {
+		return bufio.NewWriterSize(nil, handlerChunkWriteSize)
 	},
 }
 
@@ -2246,11 +2257,8 @@
 
 func (sc *serverConn) newResponseWriter(st *stream) *responseWriter {
 	rws := responseWriterStatePool.Get().(*responseWriterState)
-	bwSave := rws.bw
 	*rws = responseWriterState{} // zero all the fields
 	rws.conn = sc
-	rws.bw = bwSave
-	rws.bw.Reset(chunkWriter{rws})
 	rws.stream = st
 	return &responseWriter{rws: rws}
 }
@@ -2501,7 +2509,13 @@
 	conn   *serverConn
 
 	// TODO: adjust buffer writing sizes based on server config, frame size updates from peer, etc
-	bw *bufio.Writer // writing to a chunkWriter{this *responseWriterState}
+	//
+	// bw buffers handler writes, writing to a chunkWriter{this
+	// *responseWriterState}. It is nil until the first buffered write
+	// (see responseWriter.write) and is returned to handlerWriterPool
+	// (and set nil again) whenever a Flush leaves it empty, so a
+	// handler parked mid-response doesn't pin a buffer.
+	bw *bufio.Writer
 
 	// mutated by http.Handler goroutine:
 	handlerHeader Header   // nil until called
@@ -2786,8 +2800,11 @@
 		panic("Header called after Handler finished")
 	}
 	var err error
-	if rws.bw.Buffered() > 0 {
+	if rws.bw != nil && rws.bw.Buffered() > 0 {
 		err = rws.bw.Flush()
+		if err == nil {
+			rws.releaseWriteBuffer()
+		}
 	} else {
 		// The bufio.Writer won't call chunkWriter.Write
 		// (writeChunk with zero bytes), so we have to do it
@@ -2942,6 +2959,10 @@
 		return 0, errors.New("http2: handler wrote more than declared Content-Length")
 	}
 
+	if rws.bw == nil {
+		rws.bw = handlerWriterPool.Get().(*bufio.Writer)
+		rws.bw.Reset(chunkWriter{rws})
+	}
 	if dataB != nil {
 		return rws.bw.Write(dataB)
 	} else {
@@ -2949,10 +2970,24 @@
 	}
 }
 
+// releaseWriteBuffer returns rws.bw, which must be empty, to
+// handlerWriterPool.
+func (rws *responseWriterState) releaseWriteBuffer() {
+	bw := rws.bw
+	rws.bw = nil
+	bw.Reset(nil) // don't retain the chunkWriter's rws pointer in the pool
+	handlerWriterPool.Put(bw)
+}
+
 func (w *responseWriter) handlerDone() {
 	rws := w.rws
 	rws.handlerDone = true
 	w.Flush()
+	if rws.bw != nil {
+		// A failed Flush left data (and a sticky error) behind;
+		// discard both and recycle the buffer.
+		rws.releaseWriteBuffer()
+	}
 	w.rws = nil
 	responseWriterStatePool.Put(rws)
 }
diff --git a/src/net/http/internal/http2/server_test.go b/src/net/http/internal/http2/server_test.go
index 3d42cd4..09551ab 100644
--- a/src/net/http/internal/http2/server_test.go
+++ b/src/net/http/internal/http2/server_test.go
@@ -2368,6 +2368,52 @@
 	})
 }
 
+// TestServer_Response_FlushReleasesWriteBuffer verifies that a handler's
+// write buffer is released back to the pool by an empty-leaving Flush and
+// lazily reacquired by the next write, so that handlers parked mid-response
+// (long polls) don't pin a 4KB buffer per stream.
+func TestServer_Response_FlushReleasesWriteBuffer(t *testing.T) {
+	synctest.Test(t, testServer_Response_FlushReleasesWriteBuffer)
+}
+func testServer_Response_FlushReleasesWriteBuffer(t *testing.T) {
+	const msg = "hello, "
+	const msg2 = "world"
+	testServerResponse(t, func(w http.ResponseWriter, r *http.Request) error {
+		if ResponseWriterHasWriteBufferForTesting(w) {
+			return fmt.Errorf("write buffer allocated before first write")
+		}
+		io.WriteString(w, msg)
+		if !ResponseWriterHasWriteBufferForTesting(w) {
+			return fmt.Errorf("write buffer not allocated after buffered write")
+		}
+		w.(http.Flusher).Flush()
+		if ResponseWriterHasWriteBufferForTesting(w) {
+			return fmt.Errorf("write buffer not released by Flush")
+		}
+		io.WriteString(w, msg2)
+		if !ResponseWriterHasWriteBufferForTesting(w) {
+			return fmt.Errorf("write buffer not reacquired by write after Flush")
+		}
+		return nil
+	}, func(st *serverTester) {
+		getSlash(st)
+		st.wantHeaders(wantHeader{
+			streamID:  1,
+			endStream: false,
+		})
+		st.wantData(wantData{
+			streamID:  1,
+			endStream: false,
+			data:      []byte(msg),
+		})
+		st.wantData(wantData{
+			streamID:  1,
+			endStream: true,
+			data:      []byte(msg2),
+		})
+	})
+}
+
 func TestServer_Response_Automatic100Continue(t *testing.T) {
 	synctest.Test(t, testServer_Response_Automatic100Continue)
 }

Change information

Files:

M src/net/http/internal/http2/export_test.go
M src/net/http/internal/http2/server.go
M src/net/http/internal/http2/server_test.go

Change size: M

Delta: 3 files changed, 101 insertions(+), 8 deletions(-)

Open in Gerrit

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

unsatisfied_requirement

satisfied_requirement

open

diffy

Brad Fitzpatrick (Gerrit)

unread,

Aug 6, 2026, 1:44:31 PM (yesterday) Aug 6

to Brad Fitzpatrick, goph...@pubsubhelper.golang.org, Damien Neil, Nicholas Husin, golang-co...@googlegroups.com

Attention needed from Damien Neil and Nicholas Husin

Brad Fitzpatrick voted Commit-Queue+1

Commit-Queue

+1

Open in Gerrit

Related details

Attention is currently required from:

Damien Neil
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.

Gerrit

unsatisfied_requirement

satisfied_requirement

open

diffy

Damien Neil (Gerrit)

unread,

6:35 PM (4 hours ago) 6:35 PM

to Brad Fitzpatrick, goph...@pubsubhelper.golang.org, golang...@luci-project-accounts.iam.gserviceaccount.com, Nicholas Husin, golang-co...@googlegroups.com

Attention needed from Brad Fitzpatrick and Nicholas Husin

Damien Neil added 1 comment

File src/net/http/internal/http2/server.go

Line 2806, Patchset 1 (Latest): rws.releaseWriteBuffer()

Damien Neil . unresolved

Release the buffer even when rws.bw.Buffered() == 0? It's possible there was a write that caused a bufio.Writer to be pulled from the pool, but which was large enough to bypass the buffer.

Open in Gerrit

Related details

Attention is currently required from:

Brad Fitzpatrick
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.

Gerrit

unsatisfied_requirement

satisfied_requirement

open

diffy

Reply all

Reply to author

Forward

[go] net/http/internal/http2: release handler write buffers when idle

Brad Fitzpatrick (Gerrit)

Brad Fitzpatrick has uploaded the change for review

Commit message

Change diff

Change information

Related details

Brad Fitzpatrick (Gerrit)

Brad Fitzpatrick voted Commit-Queue+1

Related details

Damien Neil (Gerrit)

Damien Neil added 1 comment

Related details