From: Brad Fitzpatrick Date: Wed, 27 Feb 2013 01:12:50 +0000 (-0800) Subject: net/http: fix a bunch of test leaks X-Git-Tag: go1.1rc2~824 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=65fcb39dc7685c89a40ce97fdc167ac067c79df9;p=gostls13.git net/http: fix a bunch of test leaks And one real leak in TimeoutHandler. Fixes #4821 R=golang-dev, adg CC=golang-dev https://golang.org/cl/7369056 --- diff --git a/src/pkg/net/http/client_test.go b/src/pkg/net/http/client_test.go index 9514a4b961..88649bb167 100644 --- a/src/pkg/net/http/client_test.go +++ b/src/pkg/net/http/client_test.go @@ -55,6 +55,7 @@ func pedanticReadAll(r io.Reader) (b []byte, err error) { } func TestClient(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewServer(robotsTxtHandler) defer ts.Close() @@ -72,6 +73,7 @@ func TestClient(t *testing.T) { } func TestClientHead(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewServer(robotsTxtHandler) defer ts.Close() @@ -94,6 +96,7 @@ func (t *recordingTransport) RoundTrip(req *Request) (resp *Response, err error) } func TestGetRequestFormat(t *testing.T) { + defer checkLeakedTransports(t) tr := &recordingTransport{} client := &Client{Transport: tr} url := "http://dummy.faketld/" @@ -110,6 +113,7 @@ func TestGetRequestFormat(t *testing.T) { } func TestPostRequestFormat(t *testing.T) { + defer checkLeakedTransports(t) tr := &recordingTransport{} client := &Client{Transport: tr} @@ -136,6 +140,7 @@ func TestPostRequestFormat(t *testing.T) { } func TestPostFormRequestFormat(t *testing.T) { + defer checkLeakedTransports(t) tr := &recordingTransport{} client := &Client{Transport: tr} @@ -177,6 +182,7 @@ func TestPostFormRequestFormat(t *testing.T) { } func TestRedirects(t *testing.T) { + defer checkLeakedTransports(t) var ts *httptest.Server ts = httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { n, _ := strconv.Atoi(r.FormValue("n")) @@ -223,6 +229,7 @@ func TestRedirects(t *testing.T) { if err != nil { t.Fatalf("Get error: %v", err) } + res.Body.Close() finalUrl := res.Request.URL.String() if e, g := "", fmt.Sprintf("%v", err); e != g { t.Errorf("with custom client, expected error %q, got %q", e, g) @@ -242,12 +249,14 @@ func TestRedirects(t *testing.T) { if res == nil { t.Fatalf("Expected a non-nil Response on CheckRedirect failure (http://golang.org/issue/3795)") } + res.Body.Close() if res.Header.Get("Location") == "" { t.Errorf("no Location header in Response") } } func TestPostRedirects(t *testing.T) { + defer checkLeakedTransports(t) var log struct { sync.Mutex bytes.Buffer @@ -265,6 +274,7 @@ func TestPostRedirects(t *testing.T) { w.WriteHeader(code) } })) + defer ts.Close() tests := []struct { suffix string want int // response code @@ -364,6 +374,7 @@ func (j *TestJar) Cookies(u *url.URL) []*Cookie { } func TestRedirectCookiesOnRequest(t *testing.T) { + defer checkLeakedTransports(t) var ts *httptest.Server ts = httptest.NewServer(echoCookiesRedirectHandler) defer ts.Close() @@ -381,6 +392,7 @@ func TestRedirectCookiesOnRequest(t *testing.T) { } func TestRedirectCookiesJar(t *testing.T) { + defer checkLeakedTransports(t) var ts *httptest.Server ts = httptest.NewServer(echoCookiesRedirectHandler) defer ts.Close() @@ -393,6 +405,7 @@ func TestRedirectCookiesJar(t *testing.T) { if err != nil { t.Fatalf("Get: %v", err) } + resp.Body.Close() matchReturnedCookies(t, expectedCookies, resp.Cookies()) } @@ -416,6 +429,7 @@ func matchReturnedCookies(t *testing.T, expected, given []*Cookie) { } func TestJarCalls(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { pathSuffix := r.RequestURI[1:] if r.RequestURI == "/nosetcookie" { @@ -479,6 +493,7 @@ func (j *RecordingJar) logf(format string, args ...interface{}) { } func TestStreamingGet(t *testing.T) { + defer checkLeakedTransports(t) say := make(chan string) ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { w.(Flusher).Flush() @@ -529,6 +544,7 @@ func (c *writeCountingConn) Write(p []byte) (int, error) { // TestClientWrites verifies that client requests are buffered and we // don't send a TCP packet per line of the http request + body. func TestClientWrites(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { })) defer ts.Close() @@ -562,6 +578,7 @@ func TestClientWrites(t *testing.T) { } func TestClientInsecureTransport(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewTLSServer(HandlerFunc(func(w ResponseWriter, r *Request) { w.Write([]byte("Hello")) })) @@ -576,15 +593,20 @@ func TestClientInsecureTransport(t *testing.T) { InsecureSkipVerify: insecure, }, } + defer tr.CloseIdleConnections() c := &Client{Transport: tr} - _, err := c.Get(ts.URL) + res, err := c.Get(ts.URL) if (err == nil) != insecure { t.Errorf("insecure=%v: got unexpected err=%v", insecure, err) } + if res != nil { + res.Body.Close() + } } } func TestClientErrorWithRequestURI(t *testing.T) { + defer checkLeakedTransports(t) req, _ := NewRequest("GET", "http://localhost:1234/", nil) req.RequestURI = "/this/field/is/illegal/and/should/error/" _, err := DefaultClient.Do(req) @@ -613,6 +635,7 @@ func newTLSTransport(t *testing.T, ts *httptest.Server) *Transport { } func TestClientWithCorrectTLSServerName(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewTLSServer(HandlerFunc(func(w ResponseWriter, r *Request) { if r.TLS.ServerName != "127.0.0.1" { t.Errorf("expected client to set ServerName 127.0.0.1, got: %q", r.TLS.ServerName) @@ -627,6 +650,7 @@ func TestClientWithCorrectTLSServerName(t *testing.T) { } func TestClientWithIncorrectTLSServerName(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewTLSServer(HandlerFunc(func(w ResponseWriter, r *Request) {})) defer ts.Close() @@ -644,6 +668,7 @@ func TestClientWithIncorrectTLSServerName(t *testing.T) { // Verify Response.ContentLength is populated. http://golang.org/issue/4126 func TestClientHeadContentLength(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { if v := r.FormValue("cl"); v != "" { w.Header().Set("Content-Length", v) diff --git a/src/pkg/net/http/filetransport_test.go b/src/pkg/net/http/filetransport_test.go index cf68045d2e..6f1a537e2e 100644 --- a/src/pkg/net/http/filetransport_test.go +++ b/src/pkg/net/http/filetransport_test.go @@ -61,4 +61,5 @@ func TestFileTransport(t *testing.T) { if res.StatusCode != 404 { t.Errorf("for %s, StatusCode = %d, want 404", badURL, res.StatusCode) } + res.Body.Close() } diff --git a/src/pkg/net/http/fs_test.go b/src/pkg/net/http/fs_test.go index a0a5628809..0dd6d0df9e 100644 --- a/src/pkg/net/http/fs_test.go +++ b/src/pkg/net/http/fs_test.go @@ -54,6 +54,7 @@ var ServeFileRangeTests = []struct { } func TestServeFile(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { ServeFile(w, r, "testdata/file") })) @@ -169,6 +170,7 @@ var fsRedirectTestData = []struct { } func TestFSRedirect(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewServer(StripPrefix("/test", FileServer(Dir(".")))) defer ts.Close() @@ -193,6 +195,7 @@ func (fs *testFileSystem) Open(name string) (File, error) { } func TestFileServerCleans(t *testing.T) { + defer checkLeakedTransports(t) ch := make(chan string, 1) fs := FileServer(&testFileSystem{func(name string) (File, error) { ch <- name @@ -224,6 +227,7 @@ func mustRemoveAll(dir string) { } func TestFileServerImplicitLeadingSlash(t *testing.T) { + defer checkLeakedTransports(t) tempDir, err := ioutil.TempDir("", "") if err != nil { t.Fatalf("TempDir: %v", err) @@ -302,6 +306,7 @@ func TestEmptyDirOpenCWD(t *testing.T) { } func TestServeFileContentType(t *testing.T) { + defer checkLeakedTransports(t) const ctype = "icecream/chocolate" ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { if r.FormValue("override") == "1" { @@ -318,12 +323,14 @@ func TestServeFileContentType(t *testing.T) { if h := resp.Header.Get("Content-Type"); h != want { t.Errorf("Content-Type mismatch: got %q, want %q", h, want) } + resp.Body.Close() } get("0", "text/plain; charset=utf-8") get("1", ctype) } func TestServeFileMimeType(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { ServeFile(w, r, "testdata/style.css") })) @@ -332,6 +339,7 @@ func TestServeFileMimeType(t *testing.T) { if err != nil { t.Fatal(err) } + resp.Body.Close() want := "text/css; charset=utf-8" if h := resp.Header.Get("Content-Type"); h != want { t.Errorf("Content-Type mismatch: got %q, want %q", h, want) @@ -339,6 +347,7 @@ func TestServeFileMimeType(t *testing.T) { } func TestServeFileFromCWD(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { ServeFile(w, r, "fs_test.go") })) @@ -354,6 +363,7 @@ func TestServeFileFromCWD(t *testing.T) { } func TestServeFileWithContentEncoding(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { w.Header().Set("Content-Encoding", "foo") ServeFile(w, r, "testdata/file") @@ -363,12 +373,14 @@ func TestServeFileWithContentEncoding(t *testing.T) { if err != nil { t.Fatal(err) } + resp.Body.Close() if g, e := resp.ContentLength, int64(-1); g != e { t.Errorf("Content-Length mismatch: got %d, want %d", g, e) } } func TestServeIndexHtml(t *testing.T) { + defer checkLeakedTransports(t) const want = "index.html says hello\n" ts := httptest.NewServer(FileServer(Dir("."))) defer ts.Close() @@ -390,6 +402,7 @@ func TestServeIndexHtml(t *testing.T) { } func TestFileServerZeroByte(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewServer(FileServer(Dir("."))) defer ts.Close() @@ -458,6 +471,7 @@ func (fs fakeFS) Open(name string) (File, error) { } func TestDirectoryIfNotModified(t *testing.T) { + defer checkLeakedTransports(t) const indexContents = "I am a fake index.html file" fileMod := time.Unix(1000000000, 0).UTC() fileModStr := fileMod.Format(TimeFormat) @@ -531,6 +545,7 @@ func mustStat(t *testing.T, fileName string) os.FileInfo { } func TestServeContent(t *testing.T) { + defer checkLeakedTransports(t) type serveParam struct { name string modtime time.Time @@ -663,6 +678,7 @@ func TestServeContent(t *testing.T) { // verifies that sendfile is being used on Linux func TestLinuxSendfile(t *testing.T) { + defer checkLeakedTransports(t) if runtime.GOOS != "linux" { t.Skip("skipping; linux-only test") } diff --git a/src/pkg/net/http/serve_test.go b/src/pkg/net/http/serve_test.go index dc07a8969d..3300fef59b 100644 --- a/src/pkg/net/http/serve_test.go +++ b/src/pkg/net/http/serve_test.go @@ -184,6 +184,7 @@ var vtests = []struct { } func TestHostHandlers(t *testing.T) { + defer checkLeakedTransports(t) mux := NewServeMux() for _, h := range handlers { mux.Handle(h.pattern, stringHandler(h.msg)) @@ -256,6 +257,7 @@ func TestMuxRedirectLeadingSlashes(t *testing.T) { } func TestServerTimeouts(t *testing.T) { + defer checkLeakedTransports(t) reqNum := 0 ts := httptest.NewUnstartedServer(HandlerFunc(func(res ResponseWriter, req *Request) { reqNum++ @@ -268,6 +270,7 @@ func TestServerTimeouts(t *testing.T) { // Hit the HTTP server successfully. tr := &Transport{DisableKeepAlives: true} // they interfere with this test + defer tr.CloseIdleConnections() c := &Client{Transport: tr} r, err := c.Get(ts.URL) if err != nil { @@ -330,6 +333,7 @@ func TestServerTimeouts(t *testing.T) { // shouldn't cause a handler to block forever on reads (next HTTP // request) that will never happen. func TestOnlyWriteTimeout(t *testing.T) { + defer checkLeakedTransports(t) var conn net.Conn var afterTimeoutErrc = make(chan error, 1) ts := httptest.NewUnstartedServer(HandlerFunc(func(w ResponseWriter, req *Request) { @@ -388,6 +392,7 @@ func (l trackLastConnListener) Accept() (c net.Conn, err error) { // TestIdentityResponse verifies that a handler can unset func TestIdentityResponse(t *testing.T) { + defer checkLeakedTransports(t) handler := HandlerFunc(func(rw ResponseWriter, req *Request) { rw.Header().Set("Content-Length", "3") rw.Header().Set("Transfer-Encoding", req.FormValue("te")) @@ -433,10 +438,12 @@ func TestIdentityResponse(t *testing.T) { // Verify that ErrContentLength is returned url := ts.URL + "/?overwrite=1" - _, err := Get(url) + res, err := Get(url) if err != nil { t.Fatalf("error with Get of %s: %v", url, err) } + res.Body.Close() + // Verify that the connection is closed when the declared Content-Length // is larger than what the handler wrote. conn, err := net.Dial("tcp", ts.Listener.Addr().String()) @@ -461,6 +468,7 @@ func TestIdentityResponse(t *testing.T) { } func testTCPConnectionCloses(t *testing.T, req string, h Handler) { + defer checkLeakedTransports(t) s := httptest.NewServer(h) defer s.Close() @@ -531,6 +539,7 @@ func TestHandlersCanSetConnectionClose10(t *testing.T) { } func TestSetsRemoteAddr(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { fmt.Fprintf(w, "%s", r.RemoteAddr) })) @@ -551,6 +560,7 @@ func TestSetsRemoteAddr(t *testing.T) { } func TestChunkedResponseHeaders(t *testing.T) { + defer checkLeakedTransports(t) log.SetOutput(ioutil.Discard) // is noisy otherwise defer log.SetOutput(os.Stderr) @@ -565,6 +575,7 @@ func TestChunkedResponseHeaders(t *testing.T) { if err != nil { t.Fatalf("Get error: %v", err) } + defer res.Body.Close() if g, e := res.ContentLength, int64(-1); g != e { t.Errorf("expected ContentLength of %d; got %d", e, g) } @@ -580,6 +591,7 @@ func TestChunkedResponseHeaders(t *testing.T) { // chunking in their response headers and aren't allowed to produce // output. func Test304Responses(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { w.WriteHeader(StatusNotModified) _, err := w.Write([]byte("illegal body")) @@ -609,6 +621,7 @@ func Test304Responses(t *testing.T) { // allowed to produce output, and don't set a Content-Type since // the real type of the body data cannot be inferred. func TestHeadResponses(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { _, err := w.Write([]byte("Ignored body")) if err != ErrBodyNotAllowed { @@ -643,6 +656,7 @@ func TestHeadResponses(t *testing.T) { } func TestTLSHandshakeTimeout(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewUnstartedServer(HandlerFunc(func(w ResponseWriter, r *Request) {})) ts.Config.ReadTimeout = 250 * time.Millisecond ts.StartTLS() @@ -662,6 +676,7 @@ func TestTLSHandshakeTimeout(t *testing.T) { } func TestTLSServer(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewTLSServer(HandlerFunc(func(w ResponseWriter, r *Request) { if r.TLS != nil { w.Header().Set("X-TLS-Set", "true") @@ -744,6 +759,7 @@ var serverExpectTests = []serverExpectTest{ // Tests that the server responds to the "Expect" request header // correctly. func TestServerExpect(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { // Note using r.FormValue("readbody") because for POST // requests that would read from r.Body, which we only @@ -881,6 +897,7 @@ func TestServerUnreadRequestBodyLarge(t *testing.T) { } func TestTimeoutHandler(t *testing.T) { + defer checkLeakedTransports(t) sendHi := make(chan bool, 1) writeErrors := make(chan error, 1) sayHi := HandlerFunc(func(w ResponseWriter, r *Request) { @@ -955,6 +972,7 @@ func TestRedirectMunging(t *testing.T) { // the previous request's body, which is not optimal for zero-lengthed bodies, // as the client would then see http.ErrBodyReadAfterClose and not 0, io.EOF. func TestZeroLengthPostAndResponse(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewServer(HandlerFunc(func(rw ResponseWriter, r *Request) { all, err := ioutil.ReadAll(r.Body) if err != nil { @@ -1005,6 +1023,7 @@ func TestHandlerPanicWithHijack(t *testing.T) { } func testHandlerPanic(t *testing.T, withHijack bool, panicValue interface{}) { + defer checkLeakedTransports(t) // Unlike the other tests that set the log output to ioutil.Discard // to quiet the output, this test uses a pipe. The pipe serves three // purposes: @@ -1024,6 +1043,7 @@ func testHandlerPanic(t *testing.T, withHijack bool, panicValue interface{}) { pr, pw := io.Pipe() log.SetOutput(pw) defer log.SetOutput(os.Stderr) + defer pw.Close() ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { if withHijack { @@ -1045,7 +1065,7 @@ func testHandlerPanic(t *testing.T, withHijack bool, panicValue interface{}) { buf := make([]byte, 4<<10) _, err := pr.Read(buf) pr.Close() - if err != nil { + if err != nil && err != io.EOF { t.Error(err) } done <- true @@ -1069,6 +1089,7 @@ func testHandlerPanic(t *testing.T, withHijack bool, panicValue interface{}) { } func TestNoDate(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { w.Header()["Date"] = nil })) @@ -1084,6 +1105,7 @@ func TestNoDate(t *testing.T) { } func TestStripPrefix(t *testing.T) { + defer checkLeakedTransports(t) h := HandlerFunc(func(w ResponseWriter, r *Request) { w.Header().Set("X-Path", r.URL.Path) }) @@ -1097,6 +1119,7 @@ func TestStripPrefix(t *testing.T) { if g, e := res.Header.Get("X-Path"), "/bar"; g != e { t.Errorf("test 1: got %s, want %s", g, e) } + res.Body.Close() res, err = Get(ts.URL + "/bar") if err != nil { @@ -1105,9 +1128,11 @@ func TestStripPrefix(t *testing.T) { if g, e := res.StatusCode, 404; g != e { t.Errorf("test 2: got status %v, want %v", g, e) } + res.Body.Close() } func TestRequestLimit(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { t.Fatalf("didn't expect to get request in Handler") })) @@ -1124,6 +1149,7 @@ func TestRequestLimit(t *testing.T) { // we do support it (at least currently), so we expect a response below. t.Fatalf("Do: %v", err) } + defer res.Body.Close() if res.StatusCode != 413 { t.Fatalf("expected 413 response status; got: %d %s", res.StatusCode, res.Status) } @@ -1150,6 +1176,7 @@ func (cr countReader) Read(p []byte) (n int, err error) { } func TestRequestBodyLimit(t *testing.T) { + defer checkLeakedTransports(t) const limit = 1 << 20 ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { r.Body = MaxBytesReader(w, r.Body, limit) @@ -1186,6 +1213,7 @@ func TestRequestBodyLimit(t *testing.T) { // TestClientWriteShutdown tests that if the client shuts down the write // side of their TCP connection, the server doesn't send a 400 Bad Request. func TestClientWriteShutdown(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) {})) defer ts.Close() conn, err := net.Dial("tcp", ts.Listener.Addr().String()) @@ -1240,6 +1268,7 @@ func TestServerBufferedChunking(t *testing.T) { // closing the TCP connection, causing the client to get a RST. // See http://golang.org/issue/3595 func TestServerGracefulClose(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { Error(w, "bye", StatusUnauthorized) })) @@ -1282,6 +1311,7 @@ func TestServerGracefulClose(t *testing.T) { } func TestCaseSensitiveMethod(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { if r.Method != "get" { t.Errorf(`Got method %q; want "get"`, r.Method) diff --git a/src/pkg/net/http/server.go b/src/pkg/net/http/server.go index a965a0e9f9..b6ab782286 100644 --- a/src/pkg/net/http/server.go +++ b/src/pkg/net/http/server.go @@ -1476,7 +1476,7 @@ func (h *timeoutHandler) errorBody() string { } func (h *timeoutHandler) ServeHTTP(w ResponseWriter, r *Request) { - done := make(chan bool) + done := make(chan bool, 1) tw := &timeoutWriter{w: w} go func() { h.handler.ServeHTTP(tw, r) diff --git a/src/pkg/net/http/transport.go b/src/pkg/net/http/transport.go index 5cd3997a26..a94579de6d 100644 --- a/src/pkg/net/http/transport.go +++ b/src/pkg/net/http/transport.go @@ -590,11 +590,11 @@ func (pc *persistConn) readLoop() { if err == nil { resp, err = ReadResponse(pc.br, rc.req) } + hasBody := resp != nil && rc.req.Method != "HEAD" && resp.ContentLength != 0 if err != nil { pc.close() } else { - hasBody := rc.req.Method != "HEAD" && resp.ContentLength != 0 if rc.addedGzip && hasBody && resp.Header.Get("Content-Encoding") == "gzip" { resp.Header.Del("Content-Encoding") resp.Header.Del("Content-Length") @@ -614,7 +614,6 @@ func (pc *persistConn) readLoop() { alive = false } - hasBody := resp != nil && rc.req.Method != "HEAD" && resp.ContentLength != 0 var waitForBodyRead chan bool if hasBody { lastbody = resp.Body diff --git a/src/pkg/net/http/transport_test.go b/src/pkg/net/http/transport_test.go index 6eb670dd08..a0ba91735c 100644 --- a/src/pkg/net/http/transport_test.go +++ b/src/pkg/net/http/transport_test.go @@ -102,11 +102,13 @@ func (tcs *testConnSet) check(t *testing.T) { // Two subsequent requests and verify their response is the same. // The response from the server is our own IP:port func TestTransportKeepAlives(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewServer(hostPortHandler) defer ts.Close() for _, disableKeepAlive := range []bool{false, true} { tr := &Transport{DisableKeepAlives: disableKeepAlive} + defer tr.CloseIdleConnections() c := &Client{Transport: tr} fetch := func(n int) string { @@ -133,6 +135,7 @@ func TestTransportKeepAlives(t *testing.T) { } func TestTransportConnectionCloseOnResponse(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewServer(hostPortHandler) defer ts.Close() @@ -183,6 +186,7 @@ func TestTransportConnectionCloseOnResponse(t *testing.T) { } func TestTransportConnectionCloseOnRequest(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewServer(hostPortHandler) defer ts.Close() @@ -233,6 +237,7 @@ func TestTransportConnectionCloseOnRequest(t *testing.T) { } func TestTransportIdleCacheKeys(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewServer(hostPortHandler) defer ts.Close() @@ -265,6 +270,7 @@ func TestTransportIdleCacheKeys(t *testing.T) { } func TestTransportMaxPerHostIdleConns(t *testing.T) { + defer checkLeakedTransports(t) resch := make(chan string) gotReq := make(chan bool) ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { @@ -333,6 +339,7 @@ func TestTransportMaxPerHostIdleConns(t *testing.T) { } func TestTransportServerClosingUnexpectedly(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewServer(hostPortHandler) defer ts.Close() @@ -389,6 +396,7 @@ func TestTransportServerClosingUnexpectedly(t *testing.T) { // Test for http://golang.org/issue/2616 (appropriate issue number) // This fails pretty reliably with GOMAXPROCS=100 or something high. func TestStressSurpriseServerCloses(t *testing.T) { + defer checkLeakedTransports(t) if testing.Short() { t.Skip("skipping test in short mode") } @@ -444,6 +452,7 @@ func TestStressSurpriseServerCloses(t *testing.T) { // TestTransportHeadResponses verifies that we deal with Content-Lengths // with no bodies properly func TestTransportHeadResponses(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { if r.Method != "HEAD" { panic("expected HEAD; got " + r.Method) @@ -472,6 +481,7 @@ func TestTransportHeadResponses(t *testing.T) { // TestTransportHeadChunkedResponse verifies that we ignore chunked transfer-encoding // on responses to HEAD requests. func TestTransportHeadChunkedResponse(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { if r.Method != "HEAD" { panic("expected HEAD; got " + r.Method) @@ -513,6 +523,7 @@ var roundTripTests = []struct { // Test that the modification made to the Request by the RoundTripper is cleaned up func TestRoundTripGzip(t *testing.T) { + defer checkLeakedTransports(t) const responseBody = "test response body" ts := httptest.NewServer(HandlerFunc(func(rw ResponseWriter, req *Request) { accept := req.Header.Get("Accept-Encoding") @@ -569,6 +580,7 @@ func TestRoundTripGzip(t *testing.T) { } func TestTransportGzip(t *testing.T) { + defer checkLeakedTransports(t) const testString = "The test string aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" const nRandBytes = 1024 * 1024 ts := httptest.NewServer(HandlerFunc(func(rw ResponseWriter, req *Request) { @@ -661,6 +673,7 @@ func TestTransportGzip(t *testing.T) { } func TestTransportProxy(t *testing.T) { + defer checkLeakedTransports(t) ch := make(chan string, 1) ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { ch <- "real server" @@ -689,6 +702,7 @@ func TestTransportProxy(t *testing.T) { // but checks that we don't recurse forever, and checks that // Content-Encoding is removed. func TestTransportGzipRecursive(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { w.Header().Set("Content-Encoding", "gzip") w.Write(rgz) @@ -715,6 +729,7 @@ func TestTransportGzipRecursive(t *testing.T) { // tests that persistent goroutine connections shut down when no longer desired. func TestTransportPersistConnLeak(t *testing.T) { + defer checkLeakedTransports(t) gotReqCh := make(chan bool) unblockCh := make(chan bool) ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { @@ -780,6 +795,7 @@ func TestTransportPersistConnLeak(t *testing.T) { // golang.org/issue/4531: Transport leaks goroutines when // request.ContentLength is explicitly short func TestTransportPersistConnLeakShortBody(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { })) defer ts.Close() @@ -818,6 +834,7 @@ func TestTransportPersistConnLeakShortBody(t *testing.T) { // This used to crash; http://golang.org/issue/3266 func TestTransportIdleConnCrash(t *testing.T) { + defer checkLeakedTransports(t) tr := &Transport{} c := &Client{Transport: tr} @@ -847,6 +864,7 @@ func TestTransportIdleConnCrash(t *testing.T) { // which sadly lacked a triggering test. The large response body made // the old race easier to trigger. func TestIssue3644(t *testing.T) { + defer checkLeakedTransports(t) const numFoos = 5000 ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { w.Header().Set("Connection", "close") @@ -874,6 +892,7 @@ func TestIssue3644(t *testing.T) { // Test that a client receives a server's reply, even if the server doesn't read // the entire request body. func TestIssue3595(t *testing.T) { + defer checkLeakedTransports(t) const deniedMsg = "sorry, denied." ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { Error(w, deniedMsg, StatusUnauthorized) @@ -898,6 +917,7 @@ func TestIssue3595(t *testing.T) { // From http://golang.org/issue/4454 , // "client fails to handle requests with no body and chunked encoding" func TestChunkedNoContent(t *testing.T) { + defer checkLeakedTransports(t) ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { w.WriteHeader(StatusNoContent) })) @@ -920,6 +940,7 @@ func TestChunkedNoContent(t *testing.T) { } func TestTransportConcurrency(t *testing.T) { + defer checkLeakedTransports(t) const maxProcs = 16 const numReqs = 500 defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(maxProcs)) @@ -964,6 +985,7 @@ func TestTransportConcurrency(t *testing.T) { } func TestIssue4191_InfiniteGetTimeout(t *testing.T) { + defer checkLeakedTransports(t) const debug = false mux := NewServeMux() mux.HandleFunc("/get", func(w ResponseWriter, r *Request) { @@ -1024,6 +1046,7 @@ func TestIssue4191_InfiniteGetTimeout(t *testing.T) { } func TestIssue4191_InfiniteGetToPutTimeout(t *testing.T) { + defer checkLeakedTransports(t) const debug = false mux := NewServeMux() mux.HandleFunc("/get", func(w ResponseWriter, r *Request) { @@ -1103,6 +1126,7 @@ func (fooProto) RoundTrip(req *Request) (*Response, error) { } func TestTransportAltProto(t *testing.T) { + defer checkLeakedTransports(t) tr := &Transport{} c := &Client{Transport: tr} tr.RegisterProtocol("foo", fooProto{}) @@ -1121,6 +1145,7 @@ func TestTransportAltProto(t *testing.T) { } func TestTransportNoHost(t *testing.T) { + defer checkLeakedTransports(t) tr := &Transport{} _, err := tr.RoundTrip(&Request{ Header: make(Header), diff --git a/src/pkg/net/http/z_last_test.go b/src/pkg/net/http/z_last_test.go new file mode 100644 index 0000000000..44095a8d9f --- /dev/null +++ b/src/pkg/net/http/z_last_test.go @@ -0,0 +1,60 @@ +// Copyright 2013 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package http_test + +import ( + "net/http" + "runtime" + "strings" + "testing" + "time" +) + +// Verify the other tests didn't leave any goroutines running. +// This is in a file named z_last_test.go so it sorts at the end. +func TestGoroutinesRunning(t *testing.T) { + n := runtime.NumGoroutine() + t.Logf("num goroutines = %d", n) + if n > 20 { + // Currently 14 on Linux (blocked in epoll_wait, + // waiting for on fds that are closed?), but give some + // slop for now. + buf := make([]byte, 1<<20) + buf = buf[:runtime.Stack(buf, true)] + t.Errorf("Too many goroutines:\n%s", buf) + } +} + +func checkLeakedTransports(t *testing.T) { + http.DefaultTransport.(*http.Transport).CloseIdleConnections() + if testing.Short() { + return + } + buf := make([]byte, 1<<20) + var stacks string + var bad string + badSubstring := map[string]string{ + ").readLoop(": "a Transport", + ").writeLoop(": "a Transport", + "created by net/http/httptest.(*Server).Start": "an httptest.Server", + "timeoutHandler": "a TimeoutHandler", + } + for i := 0; i < 4; i++ { + bad = "" + stacks = string(buf[:runtime.Stack(buf, true)]) + for substr, what := range badSubstring { + if strings.Contains(stacks, substr) { + bad = what + } + } + if bad == "" { + return + } + // Bad stuff found, but goroutines might just still be + // shutting down, so give it some time. + time.Sleep(250 * time.Millisecond) + } + t.Errorf("Test appears to have leaked %s:\n%s", bad, stacks) +}