From: Nicholas S. Husin Date: Mon, 2 Feb 2026 21:38:01 +0000 (-0500) Subject: net/http: prevent blocking when draining response body after it has been closed X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=1179cfc9b490ce5a8c3adaccea84c79e69f711d7;p=gostls13.git net/http: prevent blocking when draining response body after it has been closed Previously, draining the response body after it has been closed causes Response.Body.Close to block for longer than it otherwise would. In a worst-case scenario, this means that we are incurring a 50 ms delay for each HTTP/1 request that we make. This CL makes sure that a response body is drained asynchronously and updates relevant documentations to reflect the current behavior. For #77370 Change-Id: I2486961bc1ea3d43d727d0aabc7a6ca7dfb166ee Reviewed-on: https://go-review.googlesource.com/c/go/+/741222 Reviewed-by: Damien Neil LUCI-TryBot-Result: Go LUCI Reviewed-by: Nicholas Husin --- diff --git a/src/net/http/client.go b/src/net/http/client.go index d6a8010735..f5c5d2e449 100644 --- a/src/net/http/client.go +++ b/src/net/http/client.go @@ -565,6 +565,9 @@ func urlErrorOp(method string) string { // read to EOF and closed, the [Client]'s underlying [RoundTripper] // (typically [Transport]) may not be able to re-use a persistent TCP // connection to the server for a subsequent "keep-alive" request. +// Note, however, that [Transport] will automatically try to read a +// [Response] Body to EOF asynchronously up to a conservative limit +// when a Body is closed. // // The request Body, if non-nil, will be closed by the underlying // Transport, even on errors. The Body may be closed asynchronously after diff --git a/src/net/http/clientconn_test.go b/src/net/http/clientconn_test.go index 03d47939aa..1027d75fed 100644 --- a/src/net/http/clientconn_test.go +++ b/src/net/http/clientconn_test.go @@ -13,6 +13,7 @@ import ( "sync/atomic" "testing" "testing/synctest" + "time" ) func TestTransportNewClientConnRoundTrip(t *testing.T) { run(t, testTransportNewClientConnRoundTrip) } @@ -283,6 +284,9 @@ func TestClientConnReserveAndConsume(t *testing.T) { } test.consume(t, cc, mode) + if mode == http1Mode || mode == https1Mode { + time.Sleep(http.MaxPostCloseReadTime) + } synctest.Wait() // State hook should be called, either to report the diff --git a/src/net/http/response.go b/src/net/http/response.go index 0c3d7f6d85..1ff83a1892 100644 --- a/src/net/http/response.go +++ b/src/net/http/response.go @@ -61,7 +61,10 @@ type Response struct { // a zero-length body. It is the caller's responsibility to // close Body. The default HTTP client's Transport may not // reuse HTTP/1.x "keep-alive" TCP connections if the Body is - // not read to completion and closed. + // not read to completion and closed; however, manually reading + // the body to completion should not be needed in most cases, + // as closing the body will also cause the body to be read to + // completion asynchronously, up to a conservative limit. // // The Body is automatically dechunked if the server replied // with a "chunked" Transfer-Encoding. diff --git a/src/net/http/transport.go b/src/net/http/transport.go index 1356d20e94..924e7cfcb0 100644 --- a/src/net/http/transport.go +++ b/src/net/http/transport.go @@ -2476,7 +2476,9 @@ func (pc *persistConn) readLoop() { // reading the response body. (or for cancellation or death) select { case bodyEOF := <-waitForBodyRead: - if !bodyEOF && resp.ContentLength <= maxPostCloseReadBytes { + tryDrain := !bodyEOF && resp.ContentLength <= maxPostCloseReadBytes + if tryDrain { + eofc <- struct{}{} bodyEOF = maybeDrainBody(body.body) } alive = alive && @@ -2484,7 +2486,7 @@ func (pc *persistConn) readLoop() { !pc.sawEOF && pc.wroteRequest() && tryPutIdleConn(rc.treq) - if bodyEOF { + if !tryDrain && bodyEOF { eofc <- struct{}{} } case <-rc.treq.ctx.Done():