From 198b78c5ed0973c75ac77578424a908f9f45ce1a Mon Sep 17 00:00:00 2001
From: Damien Neil <dneil@google.com>
Date: Wed, 6 Oct 2021 09:36:17 -0700
Subject: [PATCH] [internal-branch.go1.16-vendor] 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.

Updates golang/go#49076

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>
Reviewed-on: https://go-review.googlesource.com/c/net/+/357092
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
---
 http2/transport.go      | 25 +++++++++++++++----------
 http2/transport_test.go | 30 ++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 10 deletions(-)

diff --git a/http2/transport.go b/http2/transport.go
index ee5307e48..94fe9aa57 100644
--- a/http2/transport.go
+++ b/http2/transport.go
@@ -2594,19 +2594,24 @@ func (cc *ClientConn) Ping(ctx context.Context) error {
 		}
 		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 e394be3d3..1ed19897a 100644
--- a/http2/transport_test.go
+++ b/http2/transport_test.go
@@ -3494,6 +3494,36 @@ func TestTransportCloseAfterLostPing(t *testing.T) {
 	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