net/http: fix memory leak in Transport

Fixes #5794

R=golang-dev, r
CC=golang-dev
https://golang.org/cl/10747044

diff --git a/src/pkg/net/http/export_test.go b/src/pkg/net/http/export_test.go
index 3fc2453267697bee2178e03fbc7c8878e12c4e3b..271ff4df9c3f0eb5eb46e314f0b4dd7d41ebef72 100644 (file)
--- a/src/pkg/net/http/export_test.go
+++ b/src/pkg/net/http/export_test.go
@@ -48,6 +48,12 @@ func (t *Transport) IdleConnCountForTesting(cacheKey string) int {
        return len(conns)
 }
 
+func (t *Transport) IdleConnChMapSizeForTesting() int {
+       t.idleMu.Lock()
+       defer t.idleMu.Unlock()
+       return len(t.idleConnCh)
+}
+
 func NewTestTimeoutHandler(handler Handler, ch <-chan time.Time) Handler {
        f := func() <-chan time.Time {
                return ch

diff --git a/src/pkg/net/http/transport.go b/src/pkg/net/http/transport.go
index bd2106593b4dc81af03a3c5754e9c466a7195ef9..3f650ddb48e1fa0b995aedffcef4459e0c177d0e 100644 (file)
--- a/src/pkg/net/http/transport.go
+++ b/src/pkg/net/http/transport.go
@@ -217,6 +217,7 @@ func (t *Transport) CloseIdleConnections() {
        t.idleMu.Lock()
        m := t.idleConn
        t.idleConn = nil
+       t.idleConnCh = nil
        t.idleMu.Unlock()
        if m == nil {
                return
@@ -295,8 +296,10 @@ func (t *Transport) putIdleConn(pconn *persistConn) bool {
                max = DefaultMaxIdleConnsPerHost
        }
        t.idleMu.Lock()
+
+       waitingDialer := t.idleConnCh[key]
        select {
-       case t.idleConnCh[key] <- pconn:
+       case waitingDialer <- pconn:
                // We're done with this pconn and somebody else is
                // currently waiting for a conn of this type (they're
                // actively dialing, but this conn is ready
@@ -305,6 +308,11 @@ func (t *Transport) putIdleConn(pconn *persistConn) bool {
                t.idleMu.Unlock()
                return true
        default:
+               if waitingDialer != nil {
+                       // They had populated this, but their dial won
+                       // first, so we can clean up this map entry.
+                       delete(t.idleConnCh, key)
+               }
        }
        if t.idleConn == nil {
                t.idleConn = make(map[string][]*persistConn)
@@ -324,7 +332,13 @@ func (t *Transport) putIdleConn(pconn *persistConn) bool {
        return true
 }
 
+// getIdleConnCh returns a channel to receive and return idle
+// persistent connection for the given connectMethod.
+// It may return nil, if persistent connections are not being used.
 func (t *Transport) getIdleConnCh(cm *connectMethod) chan *persistConn {
+       if t.DisableKeepAlives {
+               return nil
+       }
        key := cm.key()
        t.idleMu.Lock()
        defer t.idleMu.Unlock()

diff --git a/src/pkg/net/http/transport_test.go b/src/pkg/net/http/transport_test.go
index 2d24b83189412571e073040e53b0a8a0733ab2c0..a34760a089914641f8f70d7bc5a7466e71ddbf90 100644 (file)
--- a/src/pkg/net/http/transport_test.go
+++ b/src/pkg/net/http/transport_test.go
@@ -1575,6 +1575,41 @@ func TestProxyFromEnvironment(t *testing.T) {
        }
 }
 
+func TestIdleConnChannelLeak(t *testing.T) {
+       var mu sync.Mutex
+       var n int
+
+       ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) {
+               mu.Lock()
+               n++
+               mu.Unlock()
+       }))
+       defer ts.Close()
+
+       tr := &Transport{
+               Dial: func(netw, addr string) (net.Conn, error) {
+                       return net.Dial(netw, ts.Listener.Addr().String())
+               },
+       }
+       defer tr.CloseIdleConnections()
+
+       c := &Client{Transport: tr}
+
+       // First, without keep-alives.
+       for _, disableKeep := range []bool{true, false} {
+               tr.DisableKeepAlives = disableKeep
+               for i := 0; i < 5; i++ {
+                       _, err := c.Get(fmt.Sprintf("http://foo-host-%d.tld/", i))
+                       if err != nil {
+                               t.Fatal(err)
+                       }
+               }
+               if got := tr.IdleConnChMapSizeForTesting(); got != 0 {
+                       t.Fatalf("ForDisableKeepAlives = %v, map size = %d; want 0", disableKeep, got)
+               }
+       }
+}
+
 // rgz is a gzip quine that uncompresses to itself.
 var rgz = []byte{
        0x1f, 0x8b, 0x08, 0x08, 0x00, 0x00, 0x00, 0x00,