[release-branch.go1.13] net/http: remove http2 connections when no longer cached

When the http2 transport returns a NoCachedConnError, the connection
must be removed from the idle list as well as the connections per host.

Fixes #34498

Change-Id: I7875c9c95e694a37a339bb04385243b49f9b20d3
Reviewed-on: https://go-review.googlesource.com/c/go/+/196665
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/197377
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>

2 files changed, 39 insertions, 0 deletions

diff --git a/src/net/http/transport.go b/src/net/http/transport.go
index 9c62f0f779..caa4bdc36f 100644
--- a/src/net/http/transport.go
+++ b/src/net/http/transport.go
@@ -539,6 +539,7 @@ func (t *Transport) roundTrip(req *Request) (*Response, error) {
 		}
 		if http2isNoCachedConnError(err) {
 			t.removeIdleConn(pconn)
+			t.decConnsPerHost(pconn.cacheKey)
 		} else if !pconn.shouldRetryRequest(req, err) {
 			// Issue 16465: return underlying net.Conn.Read error from peek,
 			// as we've historically done.
diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go
index 746c4c530e..4fabddd367 100644
--- a/src/net/http/transport_test.go
+++ b/src/net/http/transport_test.go
@@ -3594,6 +3594,44 @@ func TestTransportTraceGotConnH2IdleConns(t *testing.T) {
 	wantIdle("after round trip", 1)
 }
 
+func TestTransportRemovesH2ConnsAfterIdle(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping in short mode")
+	}
+
+	trFunc := func(tr *Transport) {
+		tr.MaxConnsPerHost = 1
+		tr.MaxIdleConnsPerHost = 1
+		tr.IdleConnTimeout = 10 * time.Millisecond
+	}
+	cst := newClientServerTest(t, h2Mode, HandlerFunc(func(w ResponseWriter, r *Request) {}), trFunc)
+	defer cst.close()
+
+	if _, err := cst.c.Get(cst.ts.URL); err != nil {
+		t.Fatalf("got error: %s", err)
+	}
+
+	time.Sleep(100 * time.Millisecond)
+	got := make(chan error)
+	go func() {
+		if _, err := cst.c.Get(cst.ts.URL); err != nil {
+			got <- err
+		}
+		close(got)
+	}()
+
+	timeout := time.NewTimer(5 * time.Second)
+	defer timeout.Stop()
+	select {
+	case err := <-got:
+		if err != nil {
+			t.Fatalf("got error: %s", err)
+		}
+	case <-timeout.C:
+		t.Fatal("request never completed")
+	}
+}
+
 // This tests that an client requesting a content range won't also
 // implicitly ask for gzip support. If they want that, they need to do it
 // on their own.