http2: detect write-blocked PING frames
We start the PingTimeout timer before writing a PING frame.
However, writing the frame can block indefinitely (either
acquiring cc.wmu or writing to the conn), and the timer is
not checked until after the frame is written.
Move PING writes into a separate goroutine, so we can detect
write-blocked connections.
Fixes golang/go#48810.
Change-Id: Ifd67fa23799e750d02754e1fe5d32719f60faed4
Reviewed-on: https://go-review.googlesource.com/c/net/+/354389
Trust: Damien Neil <dneil@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
diff --git a/http2/transport.go b/http2/transport.go
index e6aede6..653a1a0 100644
--- a/http2/transport.go
+++ b/http2/transport.go
@@ -2637,19 +2637,24 @@
}
cc.mu.Unlock()
}
- cc.wmu.Lock()
- if err := cc.fr.WritePing(false, p); err != nil {
- cc.wmu.Unlock()
- return err
- }
- if err := cc.bw.Flush(); err != nil {
- cc.wmu.Unlock()
- return err
- }
- cc.wmu.Unlock()
+ errc := make(chan error, 1)
+ go func() {
+ cc.wmu.Lock()
+ defer cc.wmu.Unlock()
+ if err := cc.fr.WritePing(false, p); err != nil {
+ errc <- err
+ return
+ }
+ if err := cc.bw.Flush(); err != nil {
+ errc <- err
+ return
+ }
+ }()
select {
case <-c:
return nil
+ case err := <-errc:
+ return err
case <-ctx.Done():
return ctx.Err()
case <-cc.readerDone:
diff --git a/http2/transport_test.go b/http2/transport_test.go
index 234a2f8..3c02695 100644
--- a/http2/transport_test.go
+++ b/http2/transport_test.go
@@ -3494,6 +3494,36 @@
ct.run()
}
+func TestTransportPingWriteBlocks(t *testing.T) {
+ st := newServerTester(t,
+ func(w http.ResponseWriter, r *http.Request) {},
+ optOnlyServer,
+ )
+ defer st.Close()
+ tr := &Transport{
+ TLSClientConfig: tlsConfigInsecure,
+ DialTLS: func(network, addr string, cfg *tls.Config) (net.Conn, error) {
+ s, c := net.Pipe() // unbuffered, unlike a TCP conn
+ go func() {
+ // Read initial handshake frames.
+ // Without this, we block indefinitely in newClientConn,
+ // and never get to the point of sending a PING.
+ var buf [1024]byte
+ s.Read(buf[:])
+ }()
+ return c, nil
+ },
+ PingTimeout: 1 * time.Millisecond,
+ ReadIdleTimeout: 1 * time.Millisecond,
+ }
+ defer tr.CloseIdleConnections()
+ c := &http.Client{Transport: tr}
+ _, err := c.Get(st.ts.URL)
+ if err == nil {
+ t.Fatalf("Get = nil, want error")
+ }
+}
+
func TestTransportPingWhenReading(t *testing.T) {
testCases := []struct {
name string
