Stowage/go
2
0
Fork 0
mirror of https://github.com/golang/go.git synced 2026-06-27 03:11:23 +00:00
Code Issues Projects Releases Packages Wiki Activity

net/http: resolve data race in TestTransportReadToEndReusesConn

Browse source 
In HTTP/3, race detector gets tripped off due to concurrent addrSeen
access: when the goroutine is reading / reinitializing addrSeen, it is
possible that the server handler goroutine (which writes to addrSeen) is
still alive since connection closure is asynchronous.

Logically, actual data race should be impossible. When the main
goroutine finishes calling io.ReadAll, it should guarantee that the
server handler has written the response body and will no longer be
writing addrSeen.

Regardless, modify the test to pass the RemoteAddr as a header instead
to make the race detector happy. Also get rid of an obsolete comment
related to afterTest that is no longer used in the test.

For #70914

Change-Id: I7aa1f17a8017090a0047833d9015725f6a6a6964
Reviewed-on: https://go-review.googlesource.com/c/go/+/771100
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Nicholas Husin <husin@google.com>
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This commit is contained in:
Nicholas S. Husin 2026-04-27 15:04:28 -04:00 • committed by Nicholas Husin
parent 5fb2392a6f
commit 65d5c5f6dd
1 changed files with 5 additions and 14 deletions
Download patch file Download diff file Expand all files Collapse all files

19
src/net/http/transport_test.go
View file

@ -428,16 +428,11 @@ func testTransportIdleCacheKeys(t *testing.T, mode testMode) {
// Tests that the HTTP transport re-uses connections when a client
// reads to the end of a response Body without closing it.
func TestTransportReadToEndReusesConn(t *testing.T) {
// HTTP/3 trips off race detector.
run(t, testTransportReadToEndReusesConn, http3SkippedMode)
}
func TestTransportReadToEndReusesConn(t *testing.T) { run(t, testTransportReadToEndReusesConn) }
func testTransportReadToEndReusesConn(t *testing.T, mode testMode) {
const msg = "foobar"
var addrSeen map[string]int
ts := newClientServerTest(t, mode, HandlerFunc(func(w ResponseWriter, r *Request) {
addrSeen[r.RemoteAddr]++
w.Header().Set("Remote-Addr", r.RemoteAddr)
if r.URL.Path == "/chunked/" {
w.WriteHeader(200)
w.(Flusher).Flush()
@ -450,18 +445,13 @@ func testTransportReadToEndReusesConn(t *testing.T, mode testMode) {
for pi, path := range []string{"/content-length/", "/chunked/"} {
wantLen := []int{len(msg), -1}[pi]
addrSeen = make(map[string]int)
for i := 0; i < 3; i++ {
addrSeen := make(map[string]int)
for range 3 {
res, err := ts.Client().Get(ts.URL + path)
if err != nil {
t.Errorf("Get %s: %v", path, err)
continue
}
// We want to close this body eventually (before the
// defer afterTest at top runs), but not before the
// len(addrSeen) check at the bottom of this test,
// since Closing this early in the loop would risk
// making connections be re-used for the wrong reason.
defer res.Body.Close()
if res.ContentLength != int64(wantLen) {
@ -471,6 +461,7 @@ func testTransportReadToEndReusesConn(t *testing.T, mode testMode) {
if string(got) != msg || err != nil {
t.Errorf("%s ReadAll(Body) = %q, %v; want %q, nil", path, string(got), err, msg)
}
addrSeen[res.Header.Get("Remote-Addr")]++
}
if len(addrSeen) != 1 {
t.Errorf("for %s, server saw %d distinct client addresses; want 1", path, len(addrSeen))