net/http: fix WaitGroup race in TestTransportNoReuseAfterEarlyResponse

The remaining race reported in go.dev/issue/66519 is that it's possible
for copying.Wait to start running before all copying.Add calls complete.
It happens infrequently as is, but padding both Wait and Add calls with
a 100 ms sleep makes it highly reproducible.

Arranging the Add call to happen before responding "foo" to the POST
request should be enough to guarantee that Wait doesn't start running
until all Add calls have already happened.

While here, delete a blank line that gets more in the way of reading
error handling code than it helps.

For #64252.
Fixes #66519 (optimistically).

Change-Id: Ibf264d8cc5ffc2495e8ae8e66a15591310c65e71
Reviewed-on: https://go-review.googlesource.com/c/go/+/739060
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>

diff --git a/src/net/http/client_test.go b/src/net/http/client_test.go
index d184f720319ce4fb9f4c9103564dcd577a1c9df9..9952ed52c61245d28e11e746dd7c66d0151edcbf 100644 (file)
--- a/src/net/http/client_test.go
+++ b/src/net/http/client_test.go
@@ -438,7 +438,6 @@ func testRedirectsByMethod(t *testing.T, mode testMode, method string, table []r
                req, _ := NewRequest(method, ts.URL+tt.suffix, strings.NewReader(content))
                req.GetBody = func() (io.ReadCloser, error) { return io.NopCloser(strings.NewReader(content)), nil }
                res, err := c.Do(req)
-
                if err != nil {
                        t.Fatal(err)
                }

diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go
index 37e6cbb8e3cf2d5bccbf4dcf94a358840b8cff82..799384e5acc6e741a21f57e86e14f0ee719902fd 100644 (file)
--- a/src/net/http/transport_test.go
+++ b/src/net/http/transport_test.go
@@ -3877,7 +3877,7 @@ func testTransportNoReuseAfterEarlyResponse(t *testing.T, mode testMode) {
                c net.Conn
        }
        var getOkay bool
-       var copying sync.WaitGroup
+       var willCopy sync.WaitGroup
        closeConn := func() {
                sconn.Lock()
                defer sconn.Unlock()
@@ -3891,7 +3891,7 @@ func testTransportNoReuseAfterEarlyResponse(t *testing.T, mode testMode) {
        }
        defer func() {
                closeConn()
-               copying.Wait()
+               willCopy.Wait()
        }()
 
        ts := newClientServerTest(t, mode, HandlerFunc(func(w ResponseWriter, r *Request) {
@@ -3903,12 +3903,12 @@ func testTransportNoReuseAfterEarlyResponse(t *testing.T, mode testMode) {
                sconn.Lock()
                sconn.c = conn
                sconn.Unlock()
-               conn.Write([]byte("HTTP/1.1 200 OK\r\nContent-Length: 3\r\n\r\nfoo")) // keep-alive
 
-               copying.Add(1)
+               willCopy.Add(1)
+               conn.Write([]byte("HTTP/1.1 200 OK\r\nContent-Length: 3\r\n\r\nfoo")) // keep-alive
                go func() {
                        io.Copy(io.Discard, conn)
-                       copying.Done()
+                       willCopy.Done()
                }()
        })).ts
        c := ts.Client()