From b74b9055e87121d4dc5d97a3f3ef1afe545bc92d Mon Sep 17 00:00:00 2001 From: Alexander Yastrebov Date: Sun, 27 Aug 2023 01:45:32 +0200 Subject: [PATCH] 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 --- src/net/http/transport.go | 1 + src/net/http/transport_test.go | 62 ++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/src/net/http/transport.go b/src/net/http/transport.go index cc590f1b374ccb..44d5515705f2f8 100644 --- 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 e407d1768af3f4..d3f43cfd9ab981 100644 --- 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 + }) +}