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 <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>

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
  }