[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>

diff --git a/src/net/http/transport.go b/src/net/http/transport.go
index 9c62f0f7797c8d5acde233584c9c2c335907620a..caa4bdc36ff01c2b9d84b62759b7cd1c0ba31455 100644 (file)
--- 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 746c4c530e30873c5da331084dbd5c9436ab3321..4fabddd36786a8cc6a12c1374d1226290dd1d125 100644 (file)
--- 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.