net/http: proper way to close idle connections in http.DefaultTransport

[georgysavva]

What version of Go are you using (go version)?

$ go version
go version go1.14 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/georgysavva/Library/Caches/go-build"
GOENV="/Users/georgysavva/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/georgysavva/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/gd/lq1x2k352p3d74_5zby2jjbc0000gn/T/go-build124744337=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I have a simple program that uses the default http transport: http.DefautlTransport and initializes two different http.Client (with different timeout settings for example) on top of it:
https://play.golang.org/p/-AQxYXiG0El
I know that http transport keeps opened tcp connections in the pool and I want to close them before my program finishes because I don't need them anymore and it's a good practice to clean up resources.

I can call firstHTTPClient.CloseIdleConnections() and secondHTTPClient.CloseIdleConnections() it will close the underlying http.DefaultTransport connections, but it feels a bit as a side effect behavior. It will call .CloseIdleConnections() on the same transport twice, it's not a problem, but again, feels a bit unoptimized.

I would better instead close the transport directly and only one. But the problem is that http.DefaultTranport is an interface type http.RoundTripper, not http.Transport and it doesn't have .CloseIdleConnections() method. In order to call it you need to assert the type:

	if tr, ok := httpTransport.(interface{ CloseIdleConnections() }); ok {
		tr.CloseIdleConnections()
	}

This is that you basically do in the http.Client.CloseIdleConnections() implementation. But It's inconvenient for the user code to do the type assertion on the default http transport every time. Wouldn't it be better to create a separate function and expose it to the user:

func CloseIdleConnections(transport http.RoundTripper) {
	if tr, ok := transport.(interface{ CloseIdleConnections() }); ok {
		tr.CloseIdleConnections()
	}
}

You could actually reuse that function in the http.Client.CloseIdleConnections() implementation.

[networkimprov]

cc @bradfitz @fraenkel @neild

Seems overlooked by triage?

[fraenkel]

Only connections that are idle are closed so it really doesn't matter what does it, a Client or a Transport.
What exactly are you trying to optimize or more importantly, why?

[georgysavva]

@fraenkel Hi! Thanks for your message.
So you suggest to just call .CloseIdleConnections() on all clients that I have; i.e. firstHTTPClient and secondHTTPClient in example here https://play.golang.org/p/-AQxYXiG0El?
Yeah, I understand that the result is the same whether I call it on client or transport.
It just feels strange that I can call .CloseIdleConnections() on a custom http.Trasnport, but with the http.DefaultTransport I should do it on a client.

[fraenkel]

You can call it on the client's transport if you type assert it to a http.Transport or an interface that defines the CloseIdleConnections().
Most people will never use the default transport because of how its configured. Once you define your own Transport that is used by Client(s), you have the option of calling the method on the Transport or the Client.

[georgysavva]

So a better approach would be to use a custom Transport in production code and don’t bother with the default one at all?

[fraenkel]

Its a judgement call. If the default values suit you, you can leverage it. If you want more control and guarantee that you aren't sharing the Transport with other bits of code, creating your own is safer.

[georgysavva]

Okay, I understand. Thanks for helping me to figure this out!