From 7c3577e48f629120604d232c7a3994cf40ae4cda Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 17 Dec 2012 12:01:00 -0800 Subject: [PATCH] net/http: fix goroutine leak in error case Fixes #4531 R=golang-dev, rsc CC=golang-dev https://golang.org/cl/6937069 --- src/pkg/net/http/transport.go | 1 + src/pkg/net/http/transport_test.go | 39 ++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/src/pkg/net/http/transport.go b/src/pkg/net/http/transport.go index 1dd5cc5308..d0505bf13f 100644 --- 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 4647d20fb3..c37ef13a41 100644 --- 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{}