From 139891e81517231647ad48bade57604021b3fd1d Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Thu, 31 Mar 2016 00:28:10 -0700 Subject: [PATCH] net/http/httptest: clean up unnecessary goroutine Finishes cleanup which was too late to do when discovered during the Go 1.6 cycle. Fixes #14291 Change-Id: Idc69fadbba10baf246318a22b366709eff088a75 Reviewed-on: https://go-review.googlesource.com/21360 Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot Reviewed-by: Andrew Gerrand --- src/net/http/httptest/server.go | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/src/net/http/httptest/server.go b/src/net/http/httptest/server.go index 8655426eafe..8608077bd1e 100644 --- a/src/net/http/httptest/server.go +++ b/src/net/http/httptest/server.go @@ -202,12 +202,10 @@ func (s *Server) logCloseHangDebugInfo() { // CloseClientConnections closes any open HTTP connections to the test Server. func (s *Server) CloseClientConnections() { - var conns int - ch := make(chan bool) - s.mu.Lock() + nconn := len(s.conns) + ch := make(chan struct{}, nconn) for c := range s.conns { - conns++ s.closeConnChan(c, ch) } s.mu.Unlock() @@ -220,7 +218,7 @@ func (s *Server) CloseClientConnections() { // in tests. timer := time.NewTimer(5 * time.Second) defer timer.Stop() - for i := 0; i < conns; i++ { + for i := 0; i < nconn; i++ { select { case <-ch: case <-timer.C: @@ -294,7 +292,7 @@ func (s *Server) closeConn(c net.Conn) { s.closeConnChan(c, nil) } // closeConnChan is like closeConn, but takes an optional channel to receive a value // when the goroutine closing c is done. -func (s *Server) closeConnChan(c net.Conn, done chan<- bool) { +func (s *Server) closeConnChan(c net.Conn, done chan<- struct{}) { if runtime.GOOS == "plan9" { // Go's Plan 9 net package isn't great at unblocking reads when // their underlying TCP connections are closed. Don't trust @@ -304,20 +302,10 @@ func (s *Server) closeConnChan(c net.Conn, done chan<- bool) { s.forgetConn(c) } - // Somewhere in the chaos of https://golang.org/cl/15151 we found that - // some types of conns were blocking in Close too long (or deadlocking?) - // and we had to call Close in a goroutine. I (bradfitz) forget what - // that was at this point, but I suspect it was *tls.Conns, which - // were later fixed in https://golang.org/cl/18572, so this goroutine - // is _probably_ unnecessary now. But it's too late in Go 1.6 too remove - // it with confidence. - // TODO(bradfitz): try to remove it for Go 1.7. (golang.org/issue/14291) - go func() { - c.Close() - if done != nil { - done <- true - } - }() + c.Close() + if done != nil { + done <- struct{}{} + } } // forgetConn removes c from the set of tracked conns and decrements it from the