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]>

Author: neild

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
 	}