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 #34387

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>

diff --git a/src/net/http/transport.go b/src/net/http/transport.go
index 209414d85b179a767d55323378fd2111516f91db..102e61f6001b0f6f3df95866feb95da18fa8c0ed 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 b456e5823f38731bd307d95344267dbacbd0e700..e94ade5f7fd012e665ed0dada9b64dba5dec9d04 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.