net/http: synchronize tests that use reqNum counters

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 <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>

diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go
index 9fe99a37a06b83869c38f64d6622f276351998dd..ebf685bcae41885f3d3fdb5b9d0e2b3d846c9aae 100644 (file)
--- 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()