Skip to content

Commit

Permalink
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.

Fixes: #65705

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>
  • Loading branch information
tibbes authored and gopherbot committed Feb 20, 2024
1 parent d42cd45 commit 098a87f
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 @@ -1478,6 +1478,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 098a87f

Please sign in to comment.