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 <[email protected]>
Reviewed-by: Than McIntosh <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Bryan Mills <[email protected]>
Reviewed-by: Damien Neil <[email protected]>

Author: Bryan C. Mills

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
 	})
 }