http2: avoid race between processing END_STREAM and closing Response.Body

When the client gets an END_STREAM from the peer, it causes the
response body to return io.EOF and closes cs.peerClosed.

It is possible for the caller of RoundTrip to read io.EOF from the
response body and end the request by calling Response.Body.Close
before we close cs.peerClosed. In this case, we send a spurious
RST_STREAM to the peer.

Closing cs.peerClosed first reverses the race: This can cause the
response body to return "request canceled" rather than io.EOF.

Close both streams with the connection mutex held.

Fixes golang/go#49314

Change-Id: I75557670497c96dd6ed1b566bb4f0f3106a0c9f9
Reviewed-on: https://go-review.googlesource.com/c/net/+/361267
Trust: Damien Neil <[email protected]>
Run-TryBot: Damien Neil <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Brad Fitzpatrick <[email protected]>

Author: neild

http2/transport.go

@@ -2576,6 +2576,12 @@ func (rl *clientConnReadLoop) endStream(cs *clientStream) {
 	// server.go's (*stream).endStream method.
 	if !cs.readClosed {
 		cs.readClosed = true
+		// Close cs.bufPipe and cs.peerClosed with cc.mu held to avoid a
+		// race condition: The caller can read io.EOF from Response.Body
+		// and close the body before we close cs.peerClosed, causing
+		// cleanupWriteRequest to send a RST_STREAM.
+		rl.cc.mu.Lock()
+		defer rl.cc.mu.Unlock()
 		cs.bufPipe.closeWithErrorAndCode(io.EOF, cs.copyTrailers)
 		close(cs.peerClosed)
 	}