net/http: fix goroutine leak in error case

Fixes #4531

R=golang-dev, rsc
CC=golang-dev
https://golang.org/cl/6937069

diff --git a/src/pkg/net/http/transport.go b/src/pkg/net/http/transport.go
index 1dd5cc53085f8248f49c9e0fcda312289f73f368..d0505bf13f041918a2ebcb727ba54564aa3b27b3 100644 (file)
--- a/src/pkg/net/http/transport.go
+++ b/src/pkg/net/http/transport.go
@@ -742,6 +742,7 @@ WaitResponse:
                case err := <-writeErrCh:
                        if err != nil {
                                re = responseAndError{nil, err}
+                               pc.close()
                                break WaitResponse
                        }
                case <-pconnDeadCh:

diff --git a/src/pkg/net/http/transport_test.go b/src/pkg/net/http/transport_test.go
index 4647d20fb3cf22e23494bfac74d3c71f04f5ae6e..c37ef13a4163bb7ad8b26117ec33e977eb889df5 100644 (file)
--- a/src/pkg/net/http/transport_test.go
+++ b/src/pkg/net/http/transport_test.go
@@ -778,6 +778,45 @@ func TestTransportPersistConnLeak(t *testing.T) {
        }
 }
 
+// golang.org/issue/4531: Transport leaks goroutines when
+// request.ContentLength is explicitly short
+func TestTransportPersistConnLeakShortBody(t *testing.T) {
+       ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) {
+       }))
+       defer ts.Close()
+
+       tr := &Transport{}
+       c := &Client{Transport: tr}
+
+       n0 := runtime.NumGoroutine()
+       body := []byte("Hello")
+       for i := 0; i < 20; i++ {
+               req, err := NewRequest("POST", ts.URL, bytes.NewReader(body))
+               if err != nil {
+                       t.Fatal(err)
+               }
+               req.ContentLength = int64(len(body) - 2) // explicitly short
+               _, err = c.Do(req)
+               if err == nil {
+                       t.Fatal("Expect an error from writing too long of a body.")
+               }
+       }
+       nhigh := runtime.NumGoroutine()
+       tr.CloseIdleConnections()
+       time.Sleep(50 * time.Millisecond)
+       runtime.GC()
+       nfinal := runtime.NumGoroutine()
+
+       growth := nfinal - n0
+
+       // We expect 0 or 1 extra goroutine, empirically.  Allow up to 5.
+       // Previously we were leaking one per numReq.
+       t.Logf("goroutine growth: %d -> %d -> %d (delta: %d)", n0, nhigh, nfinal, growth)
+       if int(growth) > 5 {
+               t.Error("too many new goroutines")
+       }
+}
+
 // This used to crash; http://golang.org/issue/3266
 func TestTransportIdleConnCrash(t *testing.T) {
        tr := &Transport{}