From 09300d89e94da7b1f32742e80768acc711b5c590 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Thu, 14 Sep 2023 09:03:37 -0400 Subject: [PATCH] net/http: synchronize tests that use reqNum counters MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit This suppresses the race reported in #62638. I am not 100% certain how that race happens, but here is my theory. The increment of reqNum happens before the server writes the response headers, and the server necessarily writes the headers before the client receives them. However, that write/read pair occurs through I/O syscalls rather than Go synchronization primitives, so it doesn't necessarily create a “happens before” relationship as defined by the Go memory model: although we can establish a sequence of events, that sequence is not visible to the race detector, nor to the compiler. Fixes #62638. Change-Id: I90d66ec3fc32b9b8e1f9bbf0bc2eb289b964b99b Reviewed-on: https://go-review.googlesource.com/c/go/+/528475 LUCI-TryBot-Result: Go LUCI Reviewed-by: Damien Neil Auto-Submit: Bryan Mills --- src/net/http/serve_test.go | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go index 9fe99a37a0..ebf685bcae 100644 --- a/src/net/http/serve_test.go +++ b/src/net/http/serve_test.go @@ -658,10 +658,9 @@ func testServerTimeouts(t *testing.T, mode testMode) { } func testServerTimeoutsWithTimeout(t *testing.T, timeout time.Duration, mode testMode) error { - reqNum := 0 + var reqNum atomic.Int32 ts := newClientServerTest(t, mode, HandlerFunc(func(res ResponseWriter, req *Request) { - reqNum++ - fmt.Fprintf(res, "req=%d", reqNum) + fmt.Fprintf(res, "req=%d", reqNum.Add(1)) }), func(ts *httptest.Server) { ts.Config.ReadTimeout = timeout ts.Config.WriteTimeout = timeout @@ -861,13 +860,15 @@ func TestWriteDeadlineEnforcedPerStream(t *testing.T) { } func testWriteDeadlineEnforcedPerStream(t *testing.T, mode testMode, timeout time.Duration) error { - reqNum := 0 + firstRequest := make(chan bool, 1) ts := newClientServerTest(t, mode, HandlerFunc(func(res ResponseWriter, req *Request) { - reqNum++ - if reqNum == 1 { - return // first request succeeds + select { + case firstRequest <- true: + // first request succeeds + default: + // second request times out + time.Sleep(timeout) } - time.Sleep(timeout) // second request times out }), func(ts *httptest.Server) { ts.Config.WriteTimeout = timeout / 2 }).ts @@ -917,13 +918,15 @@ func TestNoWriteDeadline(t *testing.T) { } func testNoWriteDeadline(t *testing.T, mode testMode, timeout time.Duration) error { - reqNum := 0 + firstRequest := make(chan bool, 1) ts := newClientServerTest(t, mode, HandlerFunc(func(res ResponseWriter, req *Request) { - reqNum++ - if reqNum == 1 { - return // first request succeeds + select { + case firstRequest <- true: + // first request succeeds + default: + // second request times out + time.Sleep(timeout) } - time.Sleep(timeout) // second request timesout })).ts c := ts.Client() -- 2.50.0