http2: avoid racy access to clientStream.requestedGzip

clientStream.requestedGzip is set from clientStream.writeRequest,
and examined by clientConn.readLoop. I'm not sure if there's
any possible way for an actual data race to happen here in
practice, since the read loop should only examine the field
after the request is sent by writeRequest, but it's enough
for the race detector to complain.

Set the field in ClientConn.roundTrip instead, before
the clientStream has become accessible to any other goroutines.

No test, but a following CL has race detector failures without
this change.

Change-Id: Id30f1b95bcfcc35c213440e0e47cce3feaaff06d
Reviewed-on: https://go-review.googlesource.com/c/net/+/586245
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>

Author: neild

http2/transport.go

@@ -1301,6 +1301,26 @@ func (cc *ClientConn) roundTrip(req *http.Request, streamf func(*clientStream))
 		donec:                make(chan struct{}),
 	}
 
+	// TODO(bradfitz): this is a copy of the logic in net/http. Unify somewhere?
+	if !cc.t.disableCompression() &&
+		req.Header.Get("Accept-Encoding") == "" &&
+		req.Header.Get("Range") == "" &&
+		!cs.isHead {
+		// Request gzip only, not deflate. Deflate is ambiguous and
+		// not as universally supported anyway.
+		// See: https://zlib.net/zlib_faq.html#faq39
+		//
+		// Note that we don't request this for HEAD requests,
+		// due to a bug in nginx:
+		//   http://trac.nginx.org/nginx/ticket/358
+		//   https://golang.org/issue/5522
+		//
+		// We don't request gzip if the request is for a range, since
+		// auto-decoding a portion of a gzipped document will just fail
+		// anyway. See https://golang.org/issue/8923
+		cs.requestedGzip = true
+	}
+
 	go cs.doRequest(req, streamf)
 
 	waitDone := func() error {
@@ -1449,26 +1469,6 @@ func (cs *clientStream) writeRequest(req *http.Request, streamf func(*clientStre
 		streamf(cs)
 	}
 
-	// TODO(bradfitz): this is a copy of the logic in net/http. Unify somewhere?
-	if !cc.t.disableCompression() &&
-		req.Header.Get("Accept-Encoding") == "" &&
-		req.Header.Get("Range") == "" &&
-		!cs.isHead {
-		// Request gzip only, not deflate. Deflate is ambiguous and
-		// not as universally supported anyway.
-		// See: https://zlib.net/zlib_faq.html#faq39
-		//
-		// Note that we don't request this for HEAD requests,
-		// due to a bug in nginx:
-		//   http://trac.nginx.org/nginx/ticket/358
-		//   https://golang.org/issue/5522
-		//
-		// We don't request gzip if the request is for a range, since
-		// auto-decoding a portion of a gzipped document will just fail
-		// anyway. See https://golang.org/issue/8923
-		cs.requestedGzip = true
-	}
-
 	continueTimeout := cc.t.expectContinueTimeout()
 	if continueTimeout != 0 {
 		if !httpguts.HeaderValuesContainsToken(req.Header["Expect"], "100-continue") {

http2/transport_test.go

@@ -2895,6 +2895,7 @@ func TestRoundTripDoesntConsumeRequestBodyEarly(t *testing.T) {
 	cc := &ClientConn{
 		closed:      true,
 		reqHeaderMu: make(chan struct{}, 1),
+		t:           &Transport{},
 	}
 	_, err := cc.RoundTrip(req)
 	if err != errClientConnUnusable {