net/http: fix request canceler leak on connection close

writeLoop goroutine closes persistConn closech in case of request body
write error which in turn finishes readLoop without removing request canceler.

Fixes #61708

Change-Id: Ib7c832a91b49bc7888a35a4fd2bd692236c04f86
GitHub-Last-Rev: b74b9055e87121d4dc5d97a3f3ef1afe545bc92d
GitHub-Pull-Request: golang/go#62305
Reviewed-on: https://go-review.googlesource.com/c/go/+/523296
Reviewed-by: Carlos Amedee <carlos@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>

diff --git a/src/net/http/transport.go b/src/net/http/transport.go
index cc590f1b374ccbb56be0e9e5b62a159fb9602de6..44d5515705f2f87b91e9feb60d677e9273bc2b9c 100644 (file)
--- a/src/net/http/transport.go
+++ b/src/net/http/transport.go
@@ -2277,6 +2277,7 @@ func (pc *persistConn) readLoop() {
                        pc.t.cancelRequest(rc.cancelKey, rc.req.Context().Err())
                case <-pc.closech:
                        alive = false
+                       pc.t.setReqCanceler(rc.cancelKey, nil)
                }
 
                testHookReadLoopBeforeNextRead()

diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go
index e407d1768af3f42111d62b6293aeeaaf9cd2692b..d3f43cfd9ab98187c1bcdfbd3989f1e89b050ae3 100644 (file)
--- a/src/net/http/transport_test.go
+++ b/src/net/http/transport_test.go
@@ -6969,3 +6969,65 @@ func testProxyAuthHeader(t *testing.T, mode testMode) {
        }
        resp.Body.Close()
 }
+
+// Issue 61708
+func TestTransportReqCancelerCleanupOnRequestBodyWriteError(t *testing.T) {
+       ln := newLocalListener(t)
+       addr := ln.Addr().String()
+
+       done := make(chan struct{})
+       go func() {
+               conn, err := ln.Accept()
+               if err != nil {
+                       t.Errorf("ln.Accept: %v", err)
+                       return
+               }
+               // Start reading request before sending response to avoid
+               // "Unsolicited response received on idle HTTP channel" RoundTrip error.
+               if _, err := io.ReadFull(conn, make([]byte, 1)); err != nil {
+                       t.Errorf("conn.Read: %v", err)
+                       return
+               }
+               io.WriteString(conn, "HTTP/1.1 200\r\nContent-Length: 3\r\n\r\nfoo")
+               <-done
+               conn.Close()
+       }()
+
+       didRead := make(chan bool)
+       SetReadLoopBeforeNextReadHook(func() { didRead <- true })
+       defer SetReadLoopBeforeNextReadHook(nil)
+
+       tr := &Transport{}
+
+       // Send a request with a body guaranteed to fail on write.
+       req, err := NewRequest("POST", "http://"+addr, io.LimitReader(neverEnding('x'), 1<<30))
+       if err != nil {
+               t.Fatalf("NewRequest: %v", err)
+       }
+
+       resp, err := tr.RoundTrip(req)
+       if err != nil {
+               t.Fatalf("tr.RoundTrip: %v", err)
+       }
+
+       close(done)
+
+       // Before closing response body wait for readLoopDone goroutine
+       // to complete due to closed connection by writeLoop.
+       <-didRead
+
+       resp.Body.Close()
+
+       // Verify no outstanding requests after readLoop/writeLoop
+       // goroutines shut down.
+       waitCondition(t, 10*time.Millisecond, func(d time.Duration) bool {
+               n := tr.NumPendingRequestsForTesting()
+               if n > 0 {
+                       if d > 0 {
+                               t.Logf("pending requests = %d after %v (want 0)", n, d)
+                       }
+                       return false
+               }
+               return true
+       })
+}