From 6826f5a7dbc43dc0bbeab569a7fc5a698f32e254 Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Thu, 18 May 2023 14:39:45 -0700 Subject: [PATCH] http2: close request bodies before RoundTrip error return When returning an error from RoundTrip, wait for the close of the request body to complete before returning. This avoids a race between the HTTP/2 transport closing the request body and the net/http retry loop examining the readTrackingBody to see if it has been closed. For golang/go#60041 Change-Id: I8be69ff5056806406716e01e02d1f631deeca088 Reviewed-on: https://go-review.googlesource.com/c/net/+/496335 Run-TryBot: Damien Neil TryBot-Result: Gopher Robot Reviewed-by: Bryan Mills --- http2/transport.go | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/http2/transport.go b/http2/transport.go index ff86a765e..4f08ccba9 100644 --- a/http2/transport.go +++ b/http2/transport.go @@ -1268,8 +1268,8 @@ func (cc *ClientConn) RoundTrip(req *http.Request) (*http.Response, error) { cancelRequest := func(cs *clientStream, err error) error { cs.cc.mu.Lock() - defer cs.cc.mu.Unlock() cs.abortStreamLocked(err) + bodyClosed := cs.reqBodyClosed if cs.ID != 0 { // This request may have failed because of a problem with the connection, // or for some unrelated reason. (For example, the user might have canceled @@ -1284,6 +1284,23 @@ func (cc *ClientConn) RoundTrip(req *http.Request) (*http.Response, error) { // will not help. cs.cc.doNotReuse = true } + cs.cc.mu.Unlock() + // Wait for the request body to be closed. + // + // If nothing closed the body before now, abortStreamLocked + // will have started a goroutine to close it. + // + // Closing the body before returning avoids a race condition + // with net/http checking its readTrackingBody to see if the + // body was read from or closed. See golang/go#60041. + // + // The body is closed in a separate goroutine without the + // connection mutex held, but dropping the mutex before waiting + // will keep us from holding it indefinitely if the body + // close is slow for some reason. + if bodyClosed != nil { + <-bodyClosed + } return err }