From 6cf6067d4eb20dfb3d31c0a8ccdbfdf0bf304b72 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Tue, 26 Sep 2023 12:35:57 -0400 Subject: [PATCH] net/http: add extra synchronization for a Logf call in TestTransportAndServerSharedBodyRace 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 Auto-Submit: Bryan Mills Reviewed-by: Damien Neil --- src/net/http/serve_test.go | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go index 93503d6581..00230020e7 100644 --- 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) { -- 2.50.0