net/http: make Transport.CloseIdleConnections close non-bundled http2.Transport

Previously Transport.CloseIdleConnections only closed the HTTP/2
Transport's idle connections if the HTTP/2 transport was configured
automatically via the bundled copy (in h2_bundle.go).

This makes it also work if the user called http2.ConfigureTransport
themselves using golang.org/x/net/http2 instead of the bundled copy.

No tests because we have no current way to run such cross-repo tests,
at least in any efficient or non-flaky way.

Tested by hand that:

    package main

    import (
        "net/http"

        "golang.org/x/net/http2"
    )

    func main() {
        tr := &http.Transport{}
        http2.ConfigureTransport(tr)
        tr.CloseIdleConnections()
    }

... now works and calls the x/net/http2.Transport.CloseIdleConnections
code. (I threw in a print statement locally)

Fixes #22891 once CL 123656 is also in.

Change-Id: Id697fd3e7877c3a988bc3c3368b88940ba56cfd0
Reviewed-on: https://go-review.googlesource.com/123657
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

diff --git a/src/net/http/export_test.go b/src/net/http/export_test.go
index 7cdb51b05b0eb7c633682aab238694b055306336..2c606a45a3ff3f2130ba5cf67417910247c857ef 100644 (file)
--- a/src/net/http/export_test.go
+++ b/src/net/http/export_test.go
@@ -117,7 +117,7 @@ func (t *Transport) IdleConnStrsForTesting() []string {
 
 func (t *Transport) IdleConnStrsForTesting_h2() []string {
        var ret []string
-       noDialPool := t.h2transport.ConnPool.(http2noDialClientConnPool)
+       noDialPool := t.h2transport.(*http2Transport).ConnPool.(http2noDialClientConnPool)
        pool := noDialPool.http2clientConnPool
 
        pool.mu.Lock()

diff --git a/src/net/http/transport.go b/src/net/http/transport.go
index 10b961219b36ed5c30fccd1fd810d615aff4aa6c..4e2dd3beb5737f89c7fb877552501dc52ddabf3f 100644 (file)
--- a/src/net/http/transport.go
+++ b/src/net/http/transport.go
@@ -24,6 +24,7 @@ import (
        "net/textproto"
        "net/url"
        "os"
+       "reflect"
        "strings"
        "sync"
        "sync/atomic"
@@ -255,7 +256,17 @@ type Transport struct {
        // nextProtoOnce guards initialization of TLSNextProto and
        // h2transport (via onceSetNextProtoDefaults)
        nextProtoOnce sync.Once
-       h2transport   *http2Transport // non-nil if http2 wired up
+       h2transport   h2Transport // non-nil if http2 wired up
+}
+
+// h2Transport is the interface we expect to be able to call from
+// net/http against an *http2.Transport that's either bundled into
+// h2_bundle.go or supplied by the user via x/net/http2.
+//
+// We name it with the "h2" prefix to stay out of the "http2" prefix
+// namespace used by x/tools/cmd/bundle for h2_bundle.go.
+type h2Transport interface {
+       CloseIdleConnections()
 }
 
 // onceSetNextProtoDefaults initializes TLSNextProto.
@@ -264,6 +275,21 @@ func (t *Transport) onceSetNextProtoDefaults() {
        if strings.Contains(os.Getenv("GODEBUG"), "http2client=0") {
                return
        }
+
+       // If they've already configured http2 with
+       // golang.org/x/net/http2 instead of the bundled copy, try to
+       // get at its http2.Transport value (via the the "https"
+       // altproto map) so we can call CloseIdleConnections on it if
+       // requested. (Issue 22891)
+       altProto, _ := t.altProto.Load().(map[string]RoundTripper)
+       if rv := reflect.ValueOf(altProto["https"]); rv.IsValid() && rv.Type().Kind() == reflect.Struct && rv.Type().NumField() == 1 {
+               if v := rv.Field(0); v.CanInterface() {
+                       if h2i, ok := v.Interface().(h2Transport); ok {
+                               t.h2transport = h2i
+                       }
+               }
+       }
+
        if t.TLSNextProto != nil {
                // This is the documented way to disable http2 on a
                // Transport.