Skip to content

Commit

Permalink
net/http: in the IdleConnStrsForTesting_h2 helper, omit conns that ca…
Browse files Browse the repository at this point in the history
…nnot be reused

In #59155, we observed that the IdleConnStrsForTesting_h2 helper
function sometimes reported extra connections after a
"client conn not usable" failure and retry. It turns out that that
state corresponds exactly to the
http2clientConnIdleState.canTakeNewRequest field, so (with a bit of
extra nethttpomithttp2 plumbing) we can use that field in the helper
to filter out the unusable connections.

Fixes #59155.

Change-Id: Ief6283c9c8c5ec47dd9f378beb0ddf720832484e
Reviewed-on: https://go-review.googlesource.com/c/go/+/477856
Reviewed-by: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
  • Loading branch information
Bryan C. Mills authored and gopherbot committed Mar 22, 2023
1 parent 4d9beb2 commit 11c40e3
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 6 deletions.
8 changes: 5 additions & 3 deletions src/net/http/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,11 @@ func (t *Transport) IdleConnStrsForTesting_h2() []string {
pool.mu.Lock()
defer pool.mu.Unlock()

for k, cc := range pool.conns {
for range cc {
ret = append(ret, k)
for k, ccs := range pool.conns {
for _, cc := range ccs {
if cc.idleState().canTakeNewRequest {
ret = append(ret, k)
}
}
}

Expand Down
10 changes: 9 additions & 1 deletion src/net/http/omithttp2.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,17 @@ type http2noDialClientConnPool struct {

type http2clientConnPool struct {
mu *sync.Mutex
conns map[string][]struct{}
conns map[string][]*http2clientConn
}

type http2clientConn struct{}

type http2clientConnIdleState struct {
canTakeNewRequest bool
}

func (cc *http2clientConn) idleState() http2clientConnIdleState { return http2clientConnIdleState{} }

func http2configureTransports(*Transport) (*http2Transport, error) { panic(noHTTP2) }

func http2isNoCachedConnError(err error) bool {
Expand Down
4 changes: 2 additions & 2 deletions src/net/http/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5047,7 +5047,7 @@ timeoutLoop:
}

var conn string
doReq := func(n int) (ok bool) {
doReq := func(n int) (timeoutOk bool) {
req, _ := NewRequest("GET", cst.ts.URL, nil)
req = req.WithContext(httptrace.WithClientTrace(context.Background(), &httptrace.ClientTrace{
PutIdleConn: func(err error) {
Expand Down Expand Up @@ -5094,7 +5094,7 @@ timeoutLoop:
waitCondition(t, timeout/2, func(d time.Duration) bool {
if got := idleConns(); len(got) != 0 {
if d >= timeout*3/2 {
t.Logf("after %d, idle conns = %q", d, got)
t.Logf("after %v, idle conns = %q", d, got)
}
return false
}
Expand Down

0 comments on commit 11c40e3

Please # to comment.