x/net/http2: setting Request.Close doesn't close TCP connections

[tatsuhiro-t]

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

$ go version
go version go1.16.10 linux/amd64

Does this issue reproduce with the latest release?

go1.16.10, which is the latest release of go1.16, is affected.

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

go env Output

$ go env
GOHOSTARCH="amd64"
GOHOSTOS="linux"

What did you do?

go run the source code below:

package main

import (
        "context"
        "fmt"
        "io"
        "net"
        "net/http"
)

func main() {
        tr := http.DefaultTransport.(*http.Transport).Clone()

        dialContext := tr.DialContext

        tr.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) {
                conn, err := dialContext(ctx, network, addr)
                if err != nil {
                        return conn, err
                }

                fmt.Printf("%v -> %v\n", addr, conn.RemoteAddr())

                return conn, nil
        }

        c := http.Client{
                Transport: tr,
        }

        for i := 0; i < 2; i++ {
                req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, "https://example.com", nil)
                if err != nil {
                        panic(err)
                }

                req.Close = true

                resp, err := c.Do(req)
                if err != nil {
                        panic(err)
                }
                defer resp.Body.Close()

                io.ReadAll(resp.Body)

                fmt.Println("body read")
        }
}

With go1.16.9, it outputs:

example.com:443 -> 93.184.216.34:443
body read
example.com:443 -> 93.184.216.34:443
body read

IP address lookup is done per request and connection is not reused. Note that connection is HTTP/2.

With go1.16.10, it outputs:

example.com:443 -> 93.184.216.34:443
body read
body read

Notice that IP address lookup is done once and the connection is reused for the second request.
This contradicts the documentation of Request.Close:

	// Close indicates whether to close the connection after
	// replying to this request (for servers) or after sending this
	// request and reading its response (for clients).
	//
	// For server requests, the HTTP server handles this automatically
	// and this field is not needed by Handlers.
	//
	// For client requests, setting this field prevents re-use of
	// TCP connections between requests to the same hosts, as if
	// Transport.DisableKeepAlives were set.
	Close bool

If Transport.DisableKeepAlives is set to true, go1.16.10 does not reuse connection.

So go1.16.10 has a different behaviour between Transport.DisableKeepAlives and Request.Close.

What did you expect to see?

go1.16.10 should not reuse connection if Request.Close is set to true.

What did you see instead?

go1.16.10 reuses connection even if Request.Close is set to true.

[seankhliao]

cc @neild @tombergan

[neild]

Missed because our HTTP/2 tests for connection reuse call ("http2".Transport).RoundTrip directly, and don't exercise the more common path where net/http manages creating new connections.

Sorry about that.

[neild]

I think there's a preexisting bug in here as well.

Request.Close states that setting it prevents re-use of TCP connections. However, for HTTP/2 connections (even prior to the introduction of this bug), a request with Close set can be sent on a connection with in-flight requests on it.

[gopherbot]

Change https://golang.org/cl/361498 mentions this issue: http2: close conns after use when req.Close is set

[neild]

@gopherbot Please open backport issues for 1.16 and 1.17

[gopherbot]

Backport issue(s) opened: #49560 (for 1.16), #49561 (for 1.17).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases.

[gopherbot]

Change https://golang.org/cl/368375 mentions this issue: [internal-branch.go1.17-vendor] http2: close conns after use when req.Close is set

[gopherbot]

Change https://golang.org/cl/368374 mentions this issue: [internal-branch.go1.16-vendor] http2: close conns after use when req.Close is set