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 <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>

diff --git a/src/net/http/httptest/server.go b/src/net/http/httptest/server.go
index 8655426eafebbae569449c1c0cdf1c3de8a748f2..8608077bd1eb9b49fa9070fefd5484eca188e330 100644 (file)
--- 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