Skip to content

Commit 891f3d3

Browse files
committed
net/http: fix memory leak in http.Transport
The wantConnQueue keeps reference on wantConn which keeps reference on the request context until the next request for the same key. This can be a problem if you never do another request with the same key. So now, we force the clean of the idleConnWait/connsPerHostWait after returning the response or error for a getConn.
1 parent 16d6a52 commit 891f3d3

File tree

2 files changed

+65
-0
lines changed

2 files changed

+65
-0
lines changed

src/net/http/transport.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1361,12 +1361,38 @@ func (t *Transport) getConn(treq *transportRequest, cm connectMethod) (pc *persi
13611361
t.setReqCanceler(treq.cancelKey, func(error) {})
13621362
return pc, nil
13631363
}
1364+
defer func() {
1365+
t.idleMu.Lock()
1366+
defer t.idleMu.Unlock()
1367+
1368+
q, ok := t.idleConnWait[w.key]
1369+
if ok {
1370+
q.cleanFront()
1371+
t.idleConnWait[w.key] = q
1372+
if q.len() == 0 {
1373+
delete(t.idleConnWait, w.key)
1374+
}
1375+
}
1376+
}()
13641377

13651378
cancelc := make(chan error, 1)
13661379
t.setReqCanceler(treq.cancelKey, func(err error) { cancelc <- err })
13671380

13681381
// Queue for permission to dial.
13691382
t.queueForDial(w)
1383+
defer func() {
1384+
t.connsPerHostMu.Lock()
1385+
defer t.connsPerHostMu.Unlock()
1386+
1387+
q, ok := t.connsPerHostWait[w.key]
1388+
if ok {
1389+
q.cleanFront()
1390+
t.connsPerHostWait[w.key] = q
1391+
if q.len() == 0 {
1392+
delete(t.connsPerHostWait, w.key)
1393+
}
1394+
}
1395+
}()
13701396

13711397
// Wait for completion or cancellation.
13721398
select {

src/net/http/transport_test.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3152,6 +3152,45 @@ func TestIdleConnChannelLeak(t *testing.T) {
31523152
}
31533153
}
31543154

3155+
func TestIdleConnWaitWantConnLeak(t *testing.T) {
3156+
// Not parallel: uses global test hooks.
3157+
var mu sync.Mutex
3158+
var n int
3159+
3160+
ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) {
3161+
mu.Lock()
3162+
n++
3163+
mu.Unlock()
3164+
w.Write([]byte("CONTENT"))
3165+
}))
3166+
defer ts.Close()
3167+
3168+
const nReqs = 5
3169+
3170+
c := ts.Client()
3171+
tr := c.Transport.(*Transport)
3172+
tr.Dial = func(netw, addr string) (net.Conn, error) {
3173+
return net.Dial(netw, ts.Listener.Addr().String())
3174+
}
3175+
3176+
// First, without keep-alives.
3177+
for _, disableKeep := range []bool{true, false} {
3178+
tr.DisableKeepAlives = disableKeep
3179+
3180+
for i := 0; i < nReqs; i++ {
3181+
resp, err := c.Get(fmt.Sprintf("http://foo-host-%d.tld/", i))
3182+
if err != nil {
3183+
t.Fatal(err)
3184+
}
3185+
resp.Body.Close()
3186+
}
3187+
3188+
if got := tr.IdleConnWaitMapSizeForTesting(); got != 0 {
3189+
t.Fatalf("for DisableKeepAlives = %v, map size = %d; want 0", disableKeep, got)
3190+
}
3191+
}
3192+
}
3193+
31553194
// Verify the status quo: that the Client.Post function coerces its
31563195
// body into a ReadCloser if it's a Closer, and that the Transport
31573196
// then closes it.

0 commit comments

Comments
 (0)