diff options
author | Julian Tibble <tibbes@github.com> | 2024-02-14 13:24:52 +0000 |
---|---|---|
committer | Carlos Amedee <carlos@golang.org> | 2024-02-28 20:05:29 +0000 |
commit | 16830ab48abb0cbc702298cafaa69839d0c9f4f9 (patch) | |
tree | 3b094f723345296988e9737254304825337c8d25 | |
parent | 056b0edcb8c152152021eebf4cf42adbfbe77992 (diff) | |
download | go-16830ab48abb0cbc702298cafaa69839d0c9f4f9.tar.gz |
[release-branch.go1.22] net/http: add missing call to decConnsPerHost
A recent change to Transport.dialConnFor introduced an early return that
skipped dialing. This path did not call decConnsPerHost, which can cause
subsequent HTTP calls to hang if Transport.MaxConnsPerHost is set.
For #65705
Fixes #65759
Change-Id: I157591114b02a3a66488d3ead7f1e6dbd374a41c
Reviewed-on: https://go-review.googlesource.com/c/go/+/564036
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Damien Neil <dneil@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
(cherry picked from commit 098a87fb1930b9ef99d394fe1bca75f1bd74ce8d)
Reviewed-on: https://go-review.googlesource.com/c/go/+/566536
Reviewed-by: Carlos Amedee <carlos@golang.org>
-rw-r--r-- | src/net/http/transport.go | 1 | ||||
-rw-r--r-- | src/net/http/transport_test.go | 50 |
2 files changed, 51 insertions, 0 deletions
diff --git a/src/net/http/transport.go b/src/net/http/transport.go index 57c70e72f9..17067ac07c 100644 --- a/src/net/http/transport.go +++ b/src/net/http/transport.go @@ -1478,6 +1478,7 @@ func (t *Transport) dialConnFor(w *wantConn) { defer w.afterDial() ctx := w.getCtxForDial() if ctx == nil { + t.decConnsPerHost(w.key) return } diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go index 3057024b76..3fb5624664 100644 --- a/src/net/http/transport_test.go +++ b/src/net/http/transport_test.go @@ -730,6 +730,56 @@ func testTransportMaxConnsPerHost(t *testing.T, mode testMode) { } } +func TestTransportMaxConnsPerHostDialCancellation(t *testing.T) { + run(t, testTransportMaxConnsPerHostDialCancellation, + testNotParallel, // because test uses SetPendingDialHooks + []testMode{http1Mode, https1Mode, http2Mode}, + ) +} + +func testTransportMaxConnsPerHostDialCancellation(t *testing.T, mode testMode) { + CondSkipHTTP2(t) + + h := HandlerFunc(func(w ResponseWriter, r *Request) { + _, err := w.Write([]byte("foo")) + if err != nil { + t.Fatalf("Write: %v", err) + } + }) + + cst := newClientServerTest(t, mode, h) + defer cst.close() + ts := cst.ts + c := ts.Client() + tr := c.Transport.(*Transport) + tr.MaxConnsPerHost = 1 + + // This request is cancelled when dial is queued, which preempts dialing. + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + SetPendingDialHooks(cancel, nil) + defer SetPendingDialHooks(nil, nil) + + req, _ := NewRequestWithContext(ctx, "GET", ts.URL, nil) + _, err := c.Do(req) + if !errors.Is(err, context.Canceled) { + t.Errorf("expected error %v, got %v", context.Canceled, err) + } + + // This request should succeed. + SetPendingDialHooks(nil, nil) + req, _ = NewRequest("GET", ts.URL, nil) + resp, err := c.Do(req) + if err != nil { + t.Fatalf("request failed: %v", err) + } + defer resp.Body.Close() + _, err = io.ReadAll(resp.Body) + if err != nil { + t.Fatalf("read body failed: %v", err) + } +} + func TestTransportRemovesDeadIdleConnections(t *testing.T) { run(t, testTransportRemovesDeadIdleConnections, []testMode{http1Mode}) } |