[go] net/http/internal/http2: let Transport's request-write goroutine exit early

1 view

Skip to first unread message

Brad Fitzpatrick (Gerrit)

unread,

Aug 6, 2026, 1:11:44 AM (4 days ago) Aug 6

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

Brad Fitzpatrick submitted the change with unreviewed changes

Unreviewed changes

1 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: src/net/http/internal/http2/transport_test.go
Insertions: 38, Deletions: 9.

@@ -5665,6 +5665,11 @@
 	tc := newTestClientConn(t)
 	tc.greet()
 
+	// Count the goroutines net/http has parked (the connection's read
+	// loop, etc.) before any request is in flight.
+	synctest.Wait()
+	base := bubbleNetHTTPGoroutines(t)
+
 	req, _ := http.NewRequest("GET", "https://dummy.tld/", nil)
 	rt := tc.roundTrip(req)
 
@@ -5678,10 +5683,11 @@
 	rt.wantStatus(200)
 
 	// The request is fully sent and the response is streaming with no
-	// end in sight. The request-writing goroutine should be gone.
+	// end in sight. The request-writing goroutine should be gone,
+	// leaving only the goroutines that predate the request.
 	synctest.Wait()
-	if n := requestWriteGoroutines(); n != 0 {
-		t.Errorf("got %d request-writing goroutines parked during long-lived response stream; want 0", n)
+	if n := bubbleNetHTTPGoroutines(t); n != base {
+		t.Errorf("got %d net/http goroutines parked during long-lived response stream; want %d (the pre-request baseline)", n, base)
 	}
 
 	// The stream still works and still cleans up at END_STREAM.
@@ -5690,14 +5696,32 @@
 	rt.wantBody([]byte("hello, world"))
 }
 
-// requestWriteGoroutines returns the number of goroutines in
-// clientStream.doRequest or clientStream.writeRequest.
-func requestWriteGoroutines() int {
+// bubbleNetHTTPGoroutines returns the number of goroutines in the calling
+// test's synctest bubble that were created by non-test functions under
+// net/http. The caller must be running in a synctest bubble.
+func bubbleNetHTTPGoroutines(t *testing.T) int {
 	buf := make([]byte, 1<<20)
 	buf = buf[:runtime.Stack(buf, true)]
+	// The first record is the calling goroutine, whose header names the
+	// test's bubble: "goroutine 8 [running, synctest bubble 3]:".
+	head, _, _ := strings.Cut(string(buf), "\n")
+	_, id, ok := strings.Cut(head, ", synctest bubble ")
+	if !ok {
+		t.Fatalf("calling goroutine is not in a synctest bubble: %s", head)
+	}
+	bubble := ", synctest bubble " + strings.TrimSuffix(id, "]:") + "]:"
 	n := 0
 	for g := range strings.SplitSeq(string(buf), "\n\n") {
-		if strings.Contains(g, ").writeRequest(") || strings.Contains(g, ").doRequest(") {
+		header, _, _ := strings.Cut(g, "\n")
+		if !strings.HasSuffix(header, bubble) {
+			continue
+		}
+		i := strings.LastIndex(g, "\ncreated by ")
+		if i < 0 {
+			continue
+		}
+		fn, loc, _ := strings.Cut(g[i+len("\ncreated by "):], "\n")
+		if strings.HasPrefix(fn, "net/http") && !strings.Contains(loc, "_test.go:") {
 			n++
 		}
 	}
@@ -5719,6 +5743,11 @@
 	})
 	tc.greet()
 
+	// Count the goroutines net/http has parked (the connection's read
+	// loop, etc.) before any request is in flight.
+	synctest.Wait()
+	base := bubbleNetHTTPGoroutines(t)
+
 	req, _ := http.NewRequest("GET", "https://dummy.tld/", nil)
 	rt := tc.roundTrip(req)
 
@@ -5727,8 +5756,8 @@
 	// The request-writing goroutine should be gone even before response
 	// headers arrive; the response header timeout is enforced by a timer.
 	synctest.Wait()
-	if n := requestWriteGoroutines(); n != 0 {
-		t.Errorf("got %d request-writing goroutines parked awaiting response headers; want 0", n)
+	if n := bubbleNetHTTPGoroutines(t); n != base {
+		t.Errorf("got %d net/http goroutines parked awaiting response headers; want %d (the pre-request baseline)", n, base)
 	}
 
 	// Response headers arrive within the timeout.
```
```
The name of the file: src/net/http/internal/http2/transport.go
Insertions: 3, Deletions: 1.

@@ -1284,6 +1284,8 @@
 			// Headers already arrived; nothing to enforce.
 		default:
 			cs.respHeaderTimeoutTimer = time.AfterFunc(respHeaderTimeout, func() {
+				cc.mu.Lock()
+				defer cc.mu.Unlock()
 				select {
 				case <-cs.respHeaderRecv:
 					// Headers arrived after all; we lost a race
@@ -1292,7 +1294,7 @@
 					return
 				default:
 				}
-				cs.abortStream(errTimeout)
+				cs.abortStreamLocked(errTimeout)
 			})
 		}
 	}
```

Change information

Commit message:

net/http/internal/http2: let Transport's request-write goroutine exit early

The goroutine spawned per Transport.RoundTrip to write the request
previously parked until the stream ended, even after the request was
fully sent, just to wait for the stream-end events and run
cleanupWriteRequest. For clients with many concurrent long-lived
response streams (long polls, event streams), that's a parked
goroutine and its stack per stream doing nothing, which adds up to a
large fraction of such a client's memory use (as seen in a production
load balancer).

Once the request is fully sent, detach: the goroutine exits, and
cleanupWriteRequest instead runs (on a short-lived goroutine) from
whichever stream-ending event fires first.

The only case where the goroutine stays running is if the caller uses
the deprecated Request.Cancel channel, but basically nobody uses that
anymore.

Updates #80735

Change-Id: I5500c691194457195b0869c1612bf93718ca96c1

Reviewed-on: https://go-review.googlesource.com/c/go/+/810780

Reviewed-by: Damien Neil <dn...@google.com>

Reviewed-by: Nicholas Husin <hu...@google.com>

Reviewed-by: Nicholas Husin <n...@golang.org>

TryBot-Bypass: Brad Fitzpatrick <brad...@golang.org>

Files:

M src/net/http/internal/http2/transport.go
M src/net/http/internal/http2/transport_test.go

Change size: L

Delta: 2 files changed, 261 insertions(+), 1 deletion(-)

Branch: refs/heads/master

Submit Requirements:

Code-Review: +2 by Damien Neil, +1 by Nicholas Husin
TryBots-Pass: LUCI-TryBot-Result-1 by golang...@luci-project-accounts.iam.gserviceaccount.com, TryBot-Bypass+1 by Brad Fitzpatrick

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

open

diffy

satisfied_requirement

Reply all

Reply to author

Forward

0 new messages