http2: fix data race on cc.singleUse

NewClientConn starts a goroutine (which reads cc.singleUse at start),
so it's not safe to set cc.singleUse after calling NewClientConn.

Change-Id: I5c28db15e8e8fd8ec5ef0e04b7f81e61efbd873e
Reviewed-on: https://go-review.googlesource.com/25230
Reviewed-by: Andrew Gerrand <adg@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

http2/client_conn_pool.go

@@ -55,11 +55,11 @@ const (
 func (p *clientConnPool) getClientConn(req *http.Request, addr string, dialOnMiss bool) (*ClientConn, error) {
 	if isConnectionCloseRequest(req) && dialOnMiss {
 		// It gets its own connection.
-		cc, err := p.t.dialClientConn(addr)
+		const singleUse = true
+		cc, err := p.t.dialClientConn(addr, singleUse)
 		if err != nil {
 			return nil, err
 		}
-		cc.singleUse = true
 		return cc, nil
 	}
 	p.mu.Lock()
@@ -104,7 +104,8 @@ func (p *clientConnPool) getStartDialLocked(addr string) *dialCall {
 
 // run in its own goroutine.
 func (c *dialCall) dial(addr string) {
-	c.res, c.err = c.p.t.dialClientConn(addr)
+	const singleUse = false // shared conn
+	c.res, c.err = c.p.t.dialClientConn(addr, singleUse)
 	close(c.done)
 
 	c.p.mu.Lock()

http2/transport.go

@@ -339,7 +339,7 @@ func shouldRetryRequest(req *http.Request, err error) bool {
 	return err == errClientConnUnusable
 }
 
-func (t *Transport) dialClientConn(addr string) (*ClientConn, error) {
+func (t *Transport) dialClientConn(addr string, singleUse bool) (*ClientConn, error) {
 	host, _, err := net.SplitHostPort(addr)
 	if err != nil {
 		return nil, err
@@ -348,7 +348,7 @@ func (t *Transport) dialClientConn(addr string) (*ClientConn, error) {
 	if err != nil {
 		return nil, err
 	}
-	return t.NewClientConn(tconn)
+	return t.newClientConn(tconn, singleUse)
 }
 
 func (t *Transport) newTLSConfig(host string) *tls.Config {
@@ -409,6 +409,10 @@ func (t *Transport) expectContinueTimeout() time.Duration {
 }
 
 func (t *Transport) NewClientConn(c net.Conn) (*ClientConn, error) {
+	return t.newClientConn(c, false)
+}
+
+func (t *Transport) newClientConn(c net.Conn, singleUse bool) (*ClientConn, error) {
 	if VerboseLogs {
 		t.vlogf("http2: Transport creating client conn to %v", c.RemoteAddr())
 	}
@@ -426,6 +430,7 @@ func (t *Transport) NewClientConn(c net.Conn) (*ClientConn, error) {
 		initialWindowSize:    65535,    // spec default
 		maxConcurrentStreams: 1000,     // "infinite", per spec. 1000 seems good enough.
 		streams:              make(map[uint32]*clientStream),
+		singleUse:            singleUse,
 	}
 	cc.cond = sync.NewCond(&cc.mu)
 	cc.flow.add(int32(initialWindowSize))