Skip to content

Commit

Permalink
[release-branch.go1.22] net/http: add missing call to decConnsPerHost
Browse files Browse the repository at this point in the history
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 golang#65705
Fixes golang#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 098a87f)
Reviewed-on: https://go-review.googlesource.com/c/go/+/566536
Reviewed-by: Carlos Amedee <carlos@golang.org>
  • Loading branch information
tibbes authored and bradfitz committed Mar 5, 2024
1 parent 0249524 commit 729e510
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 0 deletions.
1 change: 1 addition & 0 deletions src/net/http/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -1483,6 +1483,7 @@ func (t *Transport) dialConnFor(w *wantConn) {
defer w.afterDial()
ctx := w.getCtxForDial()
if ctx == nil {
t.decConnsPerHost(w.key)
return
}

Expand Down
50 changes: 50 additions & 0 deletions src/net/http/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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})
}
Expand Down

0 comments on commit 729e510

Please sign in to comment.