net/context/ctxhttp: fix case where Body could leak and not be closed
Fixes golang/go#14065
Change-Id: Ic19a0f740cddced8fb782f65cea14da383b047b1
Reviewed-on: https://go-review.googlesource.com/18802
Reviewed-by: Olivier Poitrey <rs@rhapsodyk.net>
Reviewed-by: Daniel Morsing <daniel.morsing@gmail.com>
Reviewed-by: Chris Broadfoot <cbro@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
diff --git a/context/ctxhttp/ctxhttp.go b/context/ctxhttp/ctxhttp.go
index f434f44..62620d4 100644
--- a/context/ctxhttp/ctxhttp.go
+++ b/context/ctxhttp/ctxhttp.go
@@ -14,6 +14,14 @@
"golang.org/x/net/context"
)
+func nop() {}
+
+var (
+ testHookContextDoneBeforeHeaders = nop
+ testHookDoReturned = nop
+ testHookDidBodyClose = nop
+)
+
// Do sends an HTTP request with the provided http.Client and returns an HTTP response.
// If the client is nil, http.DefaultClient is used.
// If the context is canceled or times out, ctx.Err() will be returned.
@@ -33,6 +41,7 @@
go func() {
resp, err := client.Do(req)
+ testHookDoReturned()
result <- responseAndError{resp, err}
}()
@@ -40,7 +49,15 @@
select {
case <-ctx.Done():
+ testHookContextDoneBeforeHeaders()
cancel()
+ // Clean up after the goroutine calling client.Do:
+ go func() {
+ if r := <-result; r.resp != nil {
+ testHookDidBodyClose()
+ r.resp.Body.Close()
+ }
+ }()
return nil, ctx.Err()
case r := <-result:
var err error
diff --git a/context/ctxhttp/ctxhttp_test.go b/context/ctxhttp/ctxhttp_test.go
index 139fed4..77c25ba 100644
--- a/context/ctxhttp/ctxhttp_test.go
+++ b/context/ctxhttp/ctxhttp_test.go
@@ -8,8 +8,10 @@
import (
"io/ioutil"
+ "net"
"net/http"
"net/http/httptest"
+ "sync"
"testing"
"time"
@@ -111,3 +113,64 @@
return Get(ctx, nil, serv.URL)
}
+
+// golang.org/issue/14065
+func TestClosesResponseBodyOnCancel(t *testing.T) {
+ defer func() { testHookContextDoneBeforeHeaders = nop }()
+ defer func() { testHookDoReturned = nop }()
+ defer func() { testHookDidBodyClose = nop }()
+
+ ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}))
+ defer ts.Close()
+
+ ctx, cancel := context.WithCancel(context.Background())
+
+ // closed when Do enters select case <-ctx.Done()
+ enteredDonePath := make(chan struct{})
+
+ testHookContextDoneBeforeHeaders = func() {
+ close(enteredDonePath)
+ }
+
+ testHookDoReturned = func() {
+ // We now have the result (the Flush'd headers) at least,
+ // so we can cancel the request.
+ cancel()
+
+ // But block the client.Do goroutine from sending
+ // until Do enters into the <-ctx.Done() path, since
+ // otherwise if both channels are readable, select
+ // picks a random one.
+ <-enteredDonePath
+ }
+
+ sawBodyClose := make(chan struct{})
+ testHookDidBodyClose = func() { close(sawBodyClose) }
+
+ tr := &http.Transport{}
+ defer tr.CloseIdleConnections()
+ c := &http.Client{Transport: tr}
+ req, _ := http.NewRequest("GET", ts.URL, nil)
+ _, doErr := Do(ctx, c, req)
+
+ select {
+ case <-sawBodyClose:
+ case <-time.After(5 * time.Second):
+ t.Fatal("timeout waiting for body to close")
+ }
+
+ if doErr != ctx.Err() {
+ t.Errorf("Do error = %v; want %v", doErr, ctx.Err())
+ }
+}
+
+type noteCloseConn struct {
+ net.Conn
+ onceClose sync.Once
+ closefn func()
+}
+
+func (c *noteCloseConn) Close() error {
+ c.onceClose.Do(c.closefn)
+ return c.Conn.Close()
+}
