net/http: avoid leaking goroutines when TestServerGracefulClose retries

If the call to ReadString returns an error, the closure in
testServerGracefulClose will return an error and retry the test with a
longer timeout. If that happens, we need to wait for the conn.Write
goroutine to complete so that we don't leak connections across tests.

Updates #57084.
Fixes #62643.

Change-Id: Ia86c1bbd0a5e5d0aeccf4dfeb994c19d1fb10b00
Reviewed-on: https://go-review.googlesource.com/c/go/+/528398
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Damien Neil <dneil@google.com>

diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go
index ebf685bcae41885f3d3fdb5b9d0e2b3d846c9aae..cadadf48bcd0aceb529913a78111a0f8bb9418f5 100644 (file)
--- a/src/net/http/serve_test.go
+++ b/src/net/http/serve_test.go
@@ -3135,12 +3135,19 @@ func testServerGracefulClose(t *testing.T, mode testMode) {
                if err != nil {
                        return err
                }
-               defer conn.Close()
                writeErr := make(chan error)
                go func() {
                        _, err := conn.Write(req)
                        writeErr <- err
                }()
+               defer func() {
+                       conn.Close()
+                       // Wait for write to finish. This is a broken pipe on both
+                       // Darwin and Linux, but checking this isn't the point of
+                       // the test.
+                       <-writeErr
+               }()
+
                br := bufio.NewReader(conn)
                lineNum := 0
                for {
@@ -3156,10 +3163,6 @@ func testServerGracefulClose(t *testing.T, mode testMode) {
                                t.Errorf("Response line = %q; want a 401", line)
                        }
                }
-               // Wait for write to finish. This is a broken pipe on both
-               // Darwin and Linux, but checking this isn't the point of
-               // the test.
-               <-writeErr
                return nil
        })
 }