http2: make Transport honor Request.Close more aggressively
The old support for Request.Close was optimistic, but there were still
windows where two connections should share the same
connection. Instead, support it earlier and more explicitly.
This fixes a flaky test on FreeBSD which was probably only flaky due
to unfortunate scheduling/timing.
Change-Id: I2eed9e59116356257e1b7df7a034b1c5d5f6533a
Reviewed-on: https://go-review.googlesource.com/24810
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
diff --git a/http2/client_conn_pool.go b/http2/client_conn_pool.go
index 8cb3eaa..547e238 100644
--- a/http2/client_conn_pool.go
+++ b/http2/client_conn_pool.go
@@ -52,7 +52,16 @@
noDialOnMiss = false
)
-func (p *clientConnPool) getClientConn(_ *http.Request, addr string, dialOnMiss bool) (*ClientConn, error) {
+func (p *clientConnPool) getClientConn(req *http.Request, addr string, dialOnMiss bool) (*ClientConn, error) {
+ if req.Close && dialOnMiss {
+ // It gets its own connection.
+ cc, err := p.t.dialClientConn(addr)
+ if err != nil {
+ return nil, err
+ }
+ cc.singleUse = true
+ return cc, nil
+ }
p.mu.Lock()
for _, cc := range p.conns[addr] {
if cc.CanTakeNewRequest() {
diff --git a/http2/server_test.go b/http2/server_test.go
index 61a7f9d..a45905f 100644
--- a/http2/server_test.go
+++ b/http2/server_test.go
@@ -104,6 +104,8 @@
case optQuiet:
quiet = true
}
+ case func(net.Conn, http.ConnState):
+ ts.Config.ConnState = v
default:
t.Fatalf("unknown newServerTester option type %T", v)
}
diff --git a/http2/transport.go b/http2/transport.go
index ad65ab6..fb8dd99 100644
--- a/http2/transport.go
+++ b/http2/transport.go
@@ -139,9 +139,10 @@
// ClientConn is the state of a single HTTP/2 client connection to an
// HTTP/2 server.
type ClientConn struct {
- t *Transport
- tconn net.Conn // usually *tls.Conn, except specialized impls
- tlsState *tls.ConnectionState // nil only for specialized impls
+ t *Transport
+ tconn net.Conn // usually *tls.Conn, except specialized impls
+ tlsState *tls.ConnectionState // nil only for specialized impls
+ singleUse bool // whether being used for a single http.Request
// readLoop goroutine fields:
readerDone chan struct{} // closed on error
@@ -515,6 +516,9 @@
}
func (cc *ClientConn) canTakeNewRequestLocked() bool {
+ if cc.singleUse && cc.nextStreamID > 1 {
+ return false
+ }
return cc.goAway == nil && !cc.closed &&
int64(len(cc.streams)+1) < int64(cc.maxConcurrentStreams) &&
cc.nextStreamID < 2147483647
@@ -1223,7 +1227,7 @@
func (rl *clientConnReadLoop) run() error {
cc := rl.cc
- rl.closeWhenIdle = cc.t.disableKeepAlives()
+ rl.closeWhenIdle = cc.t.disableKeepAlives() || cc.singleUse
gotReply := false // ever saw a reply
for {
f, err := cc.fr.ReadFrame()
diff --git a/http2/transport_test.go b/http2/transport_test.go
index ec7d7e0..6009cef 100644
--- a/http2/transport_test.go
+++ b/http2/transport_test.go
@@ -150,7 +150,9 @@
func onSameConn(t *testing.T, modReq func(*http.Request)) bool {
st := newServerTester(t, func(w http.ResponseWriter, r *http.Request) {
io.WriteString(w, r.RemoteAddr)
- }, optOnlyServer)
+ }, optOnlyServer, func(c net.Conn, st http.ConnState) {
+ t.Logf("conn %v is now state %v", c.RemoteAddr(), st)
+ })
defer st.Close()
tr := &Transport{TLSClientConfig: tlsConfigInsecure}
defer tr.CloseIdleConnections()