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 <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Nicholas Husin <husin@google.com>

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

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

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.

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():