Skip to content

Commit eb4e5de

Browse files
fraenkelbradfitz
authored andcommitted
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>
1 parent c4fbaee commit eb4e5de

File tree

2 files changed

+39
-0
lines changed

2 files changed

+39
-0
lines changed

src/net/http/transport.go

+1
Original file line numberDiff line numberDiff line change
@@ -539,6 +539,7 @@ func (t *Transport) roundTrip(req *Request) (*Response, error) {
539539
}
540540
if http2isNoCachedConnError(err) {
541541
t.removeIdleConn(pconn)
542+
t.decConnsPerHost(pconn.cacheKey)
542543
} else if !pconn.shouldRetryRequest(req, err) {
543544
// Issue 16465: return underlying net.Conn.Read error from peek,
544545
// as we've historically done.

src/net/http/transport_test.go

+38
Original file line numberDiff line numberDiff line change
@@ -3594,6 +3594,44 @@ func TestTransportTraceGotConnH2IdleConns(t *testing.T) {
35943594
wantIdle("after round trip", 1)
35953595
}
35963596

3597+
func TestTransportRemovesH2ConnsAfterIdle(t *testing.T) {
3598+
if testing.Short() {
3599+
t.Skip("skipping in short mode")
3600+
}
3601+
3602+
trFunc := func(tr *Transport) {
3603+
tr.MaxConnsPerHost = 1
3604+
tr.MaxIdleConnsPerHost = 1
3605+
tr.IdleConnTimeout = 10 * time.Millisecond
3606+
}
3607+
cst := newClientServerTest(t, h2Mode, HandlerFunc(func(w ResponseWriter, r *Request) {}), trFunc)
3608+
defer cst.close()
3609+
3610+
if _, err := cst.c.Get(cst.ts.URL); err != nil {
3611+
t.Fatalf("got error: %s", err)
3612+
}
3613+
3614+
time.Sleep(100 * time.Millisecond)
3615+
got := make(chan error)
3616+
go func() {
3617+
if _, err := cst.c.Get(cst.ts.URL); err != nil {
3618+
got <- err
3619+
}
3620+
close(got)
3621+
}()
3622+
3623+
timeout := time.NewTimer(5 * time.Second)
3624+
defer timeout.Stop()
3625+
select {
3626+
case err := <-got:
3627+
if err != nil {
3628+
t.Fatalf("got error: %s", err)
3629+
}
3630+
case <-timeout.C:
3631+
t.Fatal("request never completed")
3632+
}
3633+
}
3634+
35973635
// This tests that an client requesting a content range won't also
35983636
// implicitly ask for gzip support. If they want that, they need to do it
35993637
// on their own.

0 commit comments

Comments
 (0)