Skip to content

Commit 6b31f98

Browse files
net/http: fix request canceler leak on connection close
Due to a race condition persistConn could be closed without removing request canceler. Note that without the fix test occasionally passes and to demonstrate the issue it has to be run multiple times, e.g. using -count=10. Fixes #61708
1 parent 56b3b24 commit 6b31f98

File tree

2 files changed

+53
-0
lines changed

2 files changed

+53
-0
lines changed

src/net/http/transport.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2267,6 +2267,7 @@ func (pc *persistConn) readLoop() {
22672267
pc.t.cancelRequest(rc.cancelKey, rc.req.Context().Err())
22682268
case <-pc.closech:
22692269
alive = false
2270+
pc.t.setReqCanceler(rc.cancelKey, nil)
22702271
}
22712272

22722273
testHookReadLoopBeforeNextRead()

src/net/http/transport_test.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6810,3 +6810,55 @@ func testRequestSanitization(t *testing.T, mode testMode) {
68106810
resp.Body.Close()
68116811
}
68126812
}
6813+
6814+
// Issue 61708
6815+
func TestTransportAndServerSharedBodyReqCancelerCleanupOnConnectionClose(t *testing.T) {
6816+
run(t, testTransportAndServerSharedBodyReqCancelerCleanupOnConnectionClose, []testMode{http1Mode})
6817+
}
6818+
func testTransportAndServerSharedBodyReqCancelerCleanupOnConnectionClose(t *testing.T, mode testMode) {
6819+
const bodySize = 1 << 20
6820+
6821+
backend := newClientServerTest(t, mode, HandlerFunc(func(rw ResponseWriter, req *Request) {
6822+
io.Copy(rw, req.Body)
6823+
}))
6824+
t.Logf("Backend address: %s", backend.ts.Listener.Addr().String())
6825+
6826+
var proxy *clientServerTest
6827+
proxy = newClientServerTest(t, mode, HandlerFunc(func(rw ResponseWriter, req *Request) {
6828+
breq, _ := NewRequest("POST", backend.ts.URL, req.Body)
6829+
6830+
bresp, err := backend.c.Do(breq)
6831+
if err != nil {
6832+
t.Fatalf("Unexpected proxy outbound request error: %v", err)
6833+
}
6834+
defer bresp.Body.Close()
6835+
6836+
_, err = io.Copy(rw, bresp.Body)
6837+
if err == nil {
6838+
t.Fatalf("Expected proxy copy error")
6839+
}
6840+
t.Logf("Proxy copy error: %v", err)
6841+
}))
6842+
t.Logf("Proxy address: %s", proxy.ts.Listener.Addr().String())
6843+
6844+
req, _ := NewRequest("POST", proxy.ts.URL, io.LimitReader(neverEnding('a'), bodySize))
6845+
res, err := proxy.c.Do(req)
6846+
if err != nil {
6847+
t.Fatalf("Original request: %v", err)
6848+
}
6849+
// Close body without reading to trigger proxy copy error
6850+
res.Body.Close()
6851+
6852+
// Verify no outstanding requests after readLoop/writeLoop
6853+
// goroutines shut down.
6854+
waitCondition(t, 10*time.Millisecond, func(d time.Duration) bool {
6855+
n := backend.tr.NumPendingRequestsForTesting()
6856+
if n > 0 {
6857+
if d > 0 {
6858+
t.Logf("pending requests = %d after %v (want 0)", n, d)
6859+
}
6860+
return false
6861+
}
6862+
return true
6863+
})
6864+
}

0 commit comments

Comments
 (0)