diff --git a/src/net/http/transport.go b/src/net/http/transport.go index 5fe3e6ebb49ac2..6b59fc08122145 100644 --- a/src/net/http/transport.go +++ b/src/net/http/transport.go @@ -1226,6 +1226,18 @@ func (w *wantConn) tryDeliver(pc *persistConn, err error) bool { return true } +// cleanContext removes the context if the wantConn is not waiting anymore. +// It mitigates memory leak as the wantConn can be stuck in the wantConnQueue and +// this context can be big (it comes from the http.Request). +func (w *wantConn) cleanContext() { + if !w.waiting() { + w.mu.Lock() + defer w.mu.Unlock() + + w.ctx = nil + } +} + // cancel marks w as no longer wanting a result (for example, due to cancellation). // If a connection has been delivered already, cancel returns it with t.putOrCloseIdleConn. func (w *wantConn) cancel(t *Transport, err error) { @@ -1341,10 +1353,12 @@ func (t *Transport) getConn(treq *transportRequest, cm connectMethod) (pc *persi beforeDial: testHookPrePendingDial, afterDial: testHookPostPendingDial, } + defer func() { if err != nil { w.cancel(t, err) } + w.cleanContext() }() // Queue for idle connection. @@ -1362,11 +1376,46 @@ func (t *Transport) getConn(treq *transportRequest, cm connectMethod) (pc *persi return pc, nil } + defer func() { + t.idleMu.Lock() + defer t.idleMu.Unlock() + + if err != nil { + w.cancel(t, err) + } + + q, ok := t.idleConnWait[w.key] + if ok { + q.cleanFront() + t.idleConnWait[w.key] = q + if q.len() == 0 { + delete(t.idleConnWait, w.key) + } + } + }() + cancelc := make(chan error, 1) t.setReqCanceler(treq.cancelKey, func(err error) { cancelc <- err }) // Queue for permission to dial. t.queueForDial(w) + defer func() { + t.connsPerHostMu.Lock() + defer t.connsPerHostMu.Unlock() + + if err != nil { + w.cancel(t, err) + } + + q, ok := t.connsPerHostWait[w.key] + if ok { + q.cleanFront() + t.connsPerHostWait[w.key] = q + if q.len() == 0 { + delete(t.connsPerHostWait, w.key) + } + } + }() // Wait for completion or cancellation. select { diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go index e5d60afb1bb51f..da5c2e8882b057 100644 --- a/src/net/http/transport_test.go +++ b/src/net/http/transport_test.go @@ -3152,6 +3152,45 @@ func TestIdleConnChannelLeak(t *testing.T) { } } +func TestIdleConnWaitWantConnLeak(t *testing.T) { + // Not parallel: uses global test hooks. + var mu sync.Mutex + var n int + + ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { + mu.Lock() + n++ + mu.Unlock() + w.Write([]byte("CONTENT")) + })) + defer ts.Close() + + const nReqs = 5 + + c := ts.Client() + tr := c.Transport.(*Transport) + tr.Dial = func(netw, addr string) (net.Conn, error) { + return net.Dial(netw, ts.Listener.Addr().String()) + } + + // First, without keep-alives. + for _, disableKeep := range []bool{true, false} { + tr.DisableKeepAlives = disableKeep + + for i := 0; i < nReqs; i++ { + resp, err := c.Get(fmt.Sprintf("http://foo-host-%d.tld/", i)) + if err != nil { + t.Fatal(err) + } + resp.Body.Close() + } + + if got := tr.IdleConnWaitMapSizeForTesting(); got != 0 { + t.Fatalf("for DisableKeepAlives = %v, map size = %d; want 0", disableKeep, got) + } + } +} + // Verify the status quo: that the Client.Post function coerces its // body into a ReadCloser if it's a Closer, and that the Transport // then closes it.