net/http: add extra synchronization for a Logf call in TestTransportAndServerSharedBo...

This race was reported in
https://build.golang.org/log/6f043170946b665edb85b50804a62db68348c52f.

As best as I can tell, it is another instance of #38370. The deferred
call to backend.close() ought to be enough to ensure that the t.Logf
happens before the end of the test, but in practice it is not, and
with enough scheduling delay we can manage to trip the race detector
on a call to Logf after the test function has returned.

Updates #38370.

Change-Id: I5ee45df45c6bfad3239d665df65a138f1c4573a3
Reviewed-on: https://go-review.googlesource.com/c/go/+/531195
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: 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 93503d65813a74ff9c0097a67664e7ea461fbb96..00230020e7a2fef2b4460570cf6b8cf5784c7287 100644 (file)
--- a/src/net/http/serve_test.go
+++ b/src/net/http/serve_test.go
@@ -3978,14 +3978,29 @@ func testTransportAndServerSharedBodyRace(t *testing.T, mode testMode) {
 
                const bodySize = 1 << 20
 
+               var wg sync.WaitGroup
                backend := newClientServerTest(t, mode, HandlerFunc(func(rw ResponseWriter, req *Request) {
+                       // Work around https://go.dev/issue/38370: clientServerTest uses
+                       // an httptest.Server under the hood, and in HTTP/2 mode it does not always
+                       // “[block] until all outstanding requests on this server have completed”,
+                       // causing the call to Logf below to race with the end of the test.
+                       //
+                       // Since the client doesn't cancel the request until we have copied half
+                       // the body, this call to add happens before the test is cleaned up,
+                       // preventing the race.
+                       wg.Add(1)
+                       defer wg.Done()
+
                        n, err := io.CopyN(rw, req.Body, bodySize)
                        t.Logf("backend CopyN: %v, %v", n, err)
                        <-req.Context().Done()
                }))
                // We need to close explicitly here so that in-flight server
                // requests don't race with the call to SetRSTAvoidanceDelay for a retry.
-               defer backend.close()
+               defer func() {
+                       wg.Wait()
+                       backend.close()
+               }()
 
                var proxy *clientServerTest
                proxy = newClientServerTest(t, mode, HandlerFunc(func(rw ResponseWriter, req *Request) {