From 01c190206fbdffa42f334f4b2bf2220f50e64920 Mon Sep 17 00:00:00 2001
From: Tom Bergan <tombergan@google.com>
Date: Thu, 2 Nov 2017 11:53:09 -0700
Subject: [PATCH] http2: fix transport data race on reused *http.Request
 objects

Based on golang/go#19653, it should be possible to reuse an http.Request
object after the outstanding request has completed. This CL fixes a race
in the http/2 library that occurs when a caller tries to reuse an
http.Request just after the request completed.

The new test failed with -race before this CL and passes after this CL.
Verified with -count 10000.

Updates golang/go#21316

Change-Id: I014cf9cefd0dd21f6f41763ba554d23ddc7fca40
Reviewed-on: https://go-review.googlesource.com/75530
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
---
 http2/transport.go      |  4 ++--
 http2/transport_test.go | 28 ++++++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/http2/transport.go b/http2/transport.go
index c3e8737ade..4392a09f83 100644
--- a/http2/transport.go
+++ b/http2/transport.go
@@ -1911,11 +1911,11 @@ func (rl *clientConnReadLoop) endStreamError(cs *clientStream, err error) {
 		err = io.EOF
 		code = cs.copyTrailers
 	}
-	cs.bufPipe.closeWithErrorAndCode(err, code)
-	delete(rl.activeRes, cs.ID)
 	if isConnectionCloseRequest(cs.req) {
 		rl.closeWhenIdle = true
 	}
+	cs.bufPipe.closeWithErrorAndCode(err, code)
+	delete(rl.activeRes, cs.ID)
 
 	select {
 	case cs.resc <- resAndError{err: err}:
diff --git a/http2/transport_test.go b/http2/transport_test.go
index 3925c0b1a5..30d7b5de0f 100644
--- a/http2/transport_test.go
+++ b/http2/transport_test.go
@@ -3097,6 +3097,34 @@ func TestTransportCancelDataResponseRace(t *testing.T) {
 	}
 }
 
+// Issue 21316: It should be safe to reuse an http.Request after the
+// request has completed.
+func TestTransportNoRaceOnRequestObjectAfterRequestComplete(t *testing.T) {
+	st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(200)
+		io.WriteString(w, "body")
+	}, optOnlyServer)
+	defer st.Close()
+
+	tr := &Transport{TLSClientConfig: tlsConfigInsecure}
+	defer tr.CloseIdleConnections()
+
+	req, _ := http.NewRequest("GET", st.ts.URL, nil)
+	resp, err := tr.RoundTrip(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err = io.Copy(ioutil.Discard, resp.Body); err != nil {
+		t.Fatalf("error reading response body: %v", err)
+	}
+	if err := resp.Body.Close(); err != nil {
+		t.Fatalf("error closing response body: %v", err)
+	}
+
+	// This access of req.Header should not race with code in the transport.
+	req.Header = http.Header{}
+}
+
 func TestTransportRetryAfterGOAWAY(t *testing.T) {
 	var dialer struct {
 		sync.Mutex