http2: improve handling of slow-closing net.Conns
Closing a *tls.Conn can write to the network, blocking indefinitely.
Don't hold cc.mu while closing Conns. If closing a *tls.Conn takes too
long (arbitrarily set at 250ms), try to acquire the underlying net.Conn
(Go 1.18+) and close it instead.
Change-Id: Ib4e4a26595108eba9cd767231c727fd9412658b5
Reviewed-on: https://go-review.googlesource.com/c/net/+/359636
Trust: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Trust: Brad Fitzpatrick <bradfitz@golang.org>
diff --git a/http2/go118.go b/http2/go118.go
new file mode 100644
index 0000000..aca4b2b
--- /dev/null
+++ b/http2/go118.go
@@ -0,0 +1,17 @@
+// Copyright 2021 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build go1.18
+// +build go1.18
+
+package http2
+
+import (
+ "crypto/tls"
+ "net"
+)
+
+func tlsUnderlyingConn(tc *tls.Conn) net.Conn {
+ return tc.NetConn()
+}
diff --git a/http2/not_go118.go b/http2/not_go118.go
new file mode 100644
index 0000000..eab532c
--- /dev/null
+++ b/http2/not_go118.go
@@ -0,0 +1,17 @@
+// Copyright 2021 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build !go1.18
+// +build !go1.18
+
+package http2
+
+import (
+ "crypto/tls"
+ "net"
+)
+
+func tlsUnderlyingConn(tc *tls.Conn) net.Conn {
+ return nil
+}
diff --git a/http2/transport.go b/http2/transport.go
index f135b0f..4f09897 100644
--- a/http2/transport.go
+++ b/http2/transport.go
@@ -735,7 +735,6 @@
err := cc.Ping(ctx)
if err != nil {
cc.closeForLostPing()
- cc.t.connPool().MarkDead(cc)
return
}
}
@@ -907,6 +906,24 @@
cc.closeIfIdle()
}
+func (cc *ClientConn) closeConn() error {
+ t := time.AfterFunc(250*time.Millisecond, cc.forceCloseConn)
+ defer t.Stop()
+ return cc.tconn.Close()
+}
+
+// A tls.Conn.Close can hang for a long time if the peer is unresponsive.
+// Try to shut it down more aggressively.
+func (cc *ClientConn) forceCloseConn() {
+ tc, ok := cc.tconn.(*tls.Conn)
+ if !ok {
+ return
+ }
+ if nc := tlsUnderlyingConn(tc); nc != nil {
+ nc.Close()
+ }
+}
+
func (cc *ClientConn) closeIfIdle() {
cc.mu.Lock()
if len(cc.streams) > 0 || cc.streamsReserved > 0 {
@@ -921,7 +938,7 @@
if VerboseLogs {
cc.vlogf("http2: Transport closing idle conn %p (forSingleUse=%v, maxStream=%v)", cc, cc.singleUse, nextID-2)
}
- cc.tconn.Close()
+ cc.closeConn()
}
func (cc *ClientConn) isDoNotReuseAndIdle() bool {
@@ -938,7 +955,7 @@
return err
}
// Wait for all in-flight streams to complete or connection to close
- done := make(chan error, 1)
+ done := make(chan struct{})
cancelled := false // guarded by cc.mu
go func() {
cc.mu.Lock()
@@ -946,7 +963,7 @@
for {
if len(cc.streams) == 0 || cc.closed {
cc.closed = true
- done <- cc.tconn.Close()
+ close(done)
break
}
if cancelled {
@@ -957,8 +974,8 @@
}()
shutdownEnterWaitStateHook()
select {
- case err := <-done:
- return err
+ case <-done:
+ return cc.closeConn()
case <-ctx.Done():
cc.mu.Lock()
// Free the goroutine above
@@ -1001,9 +1018,9 @@
for _, cs := range cc.streams {
cs.abortStreamLocked(err)
}
- defer cc.cond.Broadcast()
- defer cc.mu.Unlock()
- return cc.tconn.Close()
+ cc.cond.Broadcast()
+ cc.mu.Unlock()
+ return cc.closeConn()
}
// Close closes the client connection immediately.
@@ -1978,7 +1995,7 @@
cc.vlogf("http2: Transport closing idle conn %p (forSingleUse=%v, maxStream=%v)", cc, cc.singleUse, cc.nextStreamID-2)
}
cc.closed = true
- defer cc.tconn.Close()
+ defer cc.closeConn()
}
cc.mu.Unlock()
@@ -2025,8 +2042,8 @@
func (rl *clientConnReadLoop) cleanup() {
cc := rl.cc
- defer cc.tconn.Close()
- defer cc.t.connPool().MarkDead(cc)
+ cc.t.connPool().MarkDead(cc)
+ defer cc.closeConn()
defer close(cc.readerDone)
if cc.idleTimer != nil {
