Skip to content

Commit 0a4d352

Browse files
fraenkelbradfitz
authored andcommitted
net/http: fix TestTransportMaxConnsPerHost flakes
The testcase created a race between the close of the current connection and the client grabbing a connection for the next request. The client may receive the current connection which may be closed during its use. We can have the trasnport close all idle connections thereby forcing the client to receive a new connection. Closing idle connections did not handle cleaning up host connection counts for http/2. We will now decrement the host connection count for http/2 connections. Fixes #31784 Change-Id: Iefc0d0d7ed9fa3acd8b4f42004f1579fc1de63fd Reviewed-on: https://go-review.googlesource.com/c/go/+/174950 Reviewed-by: Brad Fitzpatrick <[email protected]> Run-TryBot: Brad Fitzpatrick <[email protected]> TryBot-Result: Gobot Gobot <[email protected]>
1 parent 2c67cdf commit 0a4d352

File tree

2 files changed

+5
-10
lines changed

2 files changed

+5
-10
lines changed

src/net/http/transport.go

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1416,7 +1416,7 @@ func (t *Transport) dialConn(ctx context.Context, cm connectMethod) (*persistCon
14161416

14171417
if s := pconn.tlsState; s != nil && s.NegotiatedProtocolIsMutual && s.NegotiatedProtocol != "" {
14181418
if next, ok := t.TLSNextProto[s.NegotiatedProtocol]; ok {
1419-
return &persistConn{cacheKey: pconn.cacheKey, alt: next(cm.targetAddr, pconn.conn.(*tls.Conn))}, nil
1419+
return &persistConn{t: t, cacheKey: pconn.cacheKey, alt: next(cm.targetAddr, pconn.conn.(*tls.Conn))}, nil
14201420
}
14211421
}
14221422

@@ -2344,13 +2344,8 @@ func (pc *persistConn) closeLocked(err error) {
23442344
if pc.closed == nil {
23452345
pc.closed = err
23462346
if pc.alt != nil {
2347-
// Do nothing; can only get here via getConn's
2348-
// handlePendingDial's putOrCloseIdleConn when
2349-
// it turns out the abandoned connection in
2350-
// flight ended up negotiating an alternate
2351-
// protocol. We don't use the connection
2352-
// freelist for http2. That's done by the
2353-
// alternate protocol's RoundTripper.
2347+
// Clean up any host connection counting.
2348+
pc.t.decHostConnCount(pc.cacheKey)
23542349
} else {
23552350
if err != errCallerOwnsConn {
23562351
pc.conn.Close()

src/net/http/transport_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import (
2222
"fmt"
2323
"go/token"
2424
"internal/nettrace"
25-
"internal/testenv"
2625
"io"
2726
"io/ioutil"
2827
"log"
@@ -592,7 +591,7 @@ func TestTransportMaxConnsPerHostIncludeDialInProgress(t *testing.T) {
592591

593592
func TestTransportMaxConnsPerHost(t *testing.T) {
594593
defer afterTest(t)
595-
testenv.SkipFlaky(t, 31784)
594+
596595
h := HandlerFunc(func(w ResponseWriter, r *Request) {
597596
_, err := w.Write([]byte("foo"))
598597
if err != nil {
@@ -666,6 +665,7 @@ func TestTransportMaxConnsPerHost(t *testing.T) {
666665
}
667666

668667
(<-connCh).Close()
668+
tr.CloseIdleConnections()
669669

670670
doReq()
671671
expected++

0 commit comments

Comments
 (0)