Skip to content

Commit 1faa6d8

Browse files
committed
internal/http3: avoid potential race when aborting RoundTrip
TestRoundTripRequestBodyErrors/read_error currently has a small chance of failing due to a race condition. Most of the time, within writeBodyAndTrailer which runs concurrently, when a request body returns an error on Read, rt.abort will be called with the Read error. Since rt.abort is idempotent, the Read error will then be propagated and the test would pass as expected. However, before rt.abort is called with the Read error, rt.reqBodyWriter.Close is first called, which will close the write direction of the QUIC stream. This creates a small chance where the main goroutine which runs RoundTrip can encounter an EOF error due to the closed stream, and subsequently call rt.abort with a different error than expected, causing test flakiness. To fix this, this change makes sure that rt.abort is immediately called when a request body Read returns an error. This should be a no-op, since a body Read error has always been prioritized over a potential error from rt.reqBodyWriter.Close anyways. Fixes golang/go#77782 Change-Id: I9ec5acf82dd95ac91963ce5c8ddbf08b49af9508 Reviewed-on: https://go-review.googlesource.com/c/net/+/751280 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Nicholas Husin <husin@google.com> Reviewed-by: Damien Neil <dneil@google.com>
1 parent 8d297f1 commit 1faa6d8

1 file changed

Lines changed: 4 additions & 6 deletions

File tree

internal/http3/roundtrip.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,9 @@ func (cc *ClientConn) writeBodyAndTrailer(rt *roundTripState, req *http.Request)
222222
rt.reqBody = http.NoBody
223223
}
224224

225-
_, err := io.Copy(&rt.reqBodyWriter, rt.reqBody)
225+
if _, err := io.Copy(&rt.reqBodyWriter, rt.reqBody); err != nil {
226+
rt.abort(err)
227+
}
226228
// Get rid of any trailer that was not declared beforehand, before we
227229
// close the request body which will cause the trailer headers to be
228230
// written.
@@ -231,11 +233,7 @@ func (cc *ClientConn) writeBodyAndTrailer(rt *roundTripState, req *http.Request)
231233
delete(req.Trailer, name)
232234
}
233235
}
234-
if closeErr := rt.reqBodyWriter.Close(); err == nil {
235-
err = closeErr
236-
}
237-
// Something went wrong writing the body.
238-
if err != nil {
236+
if err := rt.reqBodyWriter.Close(); err != nil {
239237
rt.abort(err)
240238
}
241239
}

0 commit comments

Comments
 (0)