From: Bryan C. Mills <bcmills@google.com>
Date: Thu, 14 Sep 2023 13:03:37 +0000 (-0400)
Subject: net/http: synchronize tests that use reqNum counters
X-Git-Tag: go1.22rc1~832
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=09300d89e94da7b1f32742e80768acc711b5c590;p=gostls13.git

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 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()