net/http: in the IdleConnStrsForTesting_h2 helper, omit conns that cannot 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>

diff --git a/src/net/http/export_test.go b/src/net/http/export_test.go
index fb5ab9396aa3e86be8b8f543f5dc07e6ba299297..8a61e651dcd61b021aff2218925d7d8fc27b96e5 100644 (file)
--- a/src/net/http/export_test.go
+++ b/src/net/http/export_test.go
@@ -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)
+                       }
                }
        }
 

diff --git a/src/net/http/omithttp2.go b/src/net/http/omithttp2.go
index 3316f55c6dcf5c5d496f5504d57068985991bf4a..ca08ddfad83a995ea11607f7c3872b0077dba743 100644 (file)
--- a/src/net/http/omithttp2.go
+++ b/src/net/http/omithttp2.go
@@ -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 {

diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go
index 1abb0aabd64db0ef9cb979e0e914a1ac7ca89885..b82c6156ac311e35264213232566d499b3e67cd6 100644 (file)
--- a/src/net/http/transport_test.go
+++ b/src/net/http/transport_test.go
@@ -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) {
@@ -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
                        }