Skip to content

Commit b320e50

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 7366720 commit b320e50

File tree

2 files changed

+59
-0
lines changed

2 files changed

+59
-0
lines changed

src/net/http/transport.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2254,6 +2254,7 @@ func (pc *persistConn) readLoop() {
22542254
pc.t.cancelRequest(rc.cancelKey, rc.req.Context().Err())
22552255
case <-pc.closech:
22562256
alive = false
2257+
pc.t.setReqCanceler(rc.cancelKey, nil)
22572258
}
22582259

22592260
testHookReadLoopBeforeNextRead()

src/net/http/transport_test.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6750,3 +6750,61 @@ func testRequestSanitization(t *testing.T, mode testMode) {
67506750
resp.Body.Close()
67516751
}
67526752
}
6753+
6754+
// Issue 61708
6755+
func TestTransportAndServerSharedBodyReqCancelerCleanupOnConnectionClose(t *testing.T) {
6756+
run(t, testTransportAndServerSharedBodyReqCancelerCleanupOnConnectionClose, []testMode{http1Mode})
6757+
}
6758+
func testTransportAndServerSharedBodyReqCancelerCleanupOnConnectionClose(t *testing.T, mode testMode) {
6759+
const bodySize = 1 << 20
6760+
6761+
backend := newClientServerTest(t, mode, HandlerFunc(func(rw ResponseWriter, req *Request) {
6762+
io.Copy(rw, req.Body)
6763+
}))
6764+
t.Logf("Backend address: %s", backend.ts.Listener.Addr().String())
6765+
6766+
var proxy *clientServerTest
6767+
proxy = newClientServerTest(t, mode, HandlerFunc(func(rw ResponseWriter, req *Request) {
6768+
breq, _ := NewRequest("POST", backend.ts.URL, req.Body)
6769+
6770+
bresp, err := backend.c.Do(breq)
6771+
if err != nil {
6772+
t.Fatalf("Unexpected proxy outbound request error: %v", err)
6773+
return
6774+
}
6775+
defer bresp.Body.Close()
6776+
6777+
_, err = io.Copy(rw, bresp.Body)
6778+
if err == nil {
6779+
t.Fatalf("Expected proxy copy error")
6780+
return
6781+
}
6782+
t.Logf("Proxy copy error: %v", err)
6783+
// Could be one of:
6784+
// read tcp backend.c->backend.ts: use of closed network connection
6785+
// write tcp proxy.ts->proxy.c: write: broken pipe
6786+
// write tcp proxy.ts->proxy.c: write: connection reset by peer
6787+
}))
6788+
t.Logf("Proxy address: %s", proxy.ts.Listener.Addr().String())
6789+
6790+
req, _ := NewRequest("POST", proxy.ts.URL, io.LimitReader(neverEnding('a'), bodySize))
6791+
res, err := proxy.c.Do(req)
6792+
if err != nil {
6793+
t.Fatalf("Original request: %v", err)
6794+
}
6795+
// Close body without reading to trigger proxy copy error
6796+
res.Body.Close()
6797+
6798+
// Verify no outstanding requests after readLoop/writeLoop
6799+
// goroutines shut down.
6800+
waitCondition(t, 10*time.Millisecond, func(d time.Duration) bool {
6801+
n := backend.tr.NumPendingRequestsForTesting()
6802+
if n > 0 {
6803+
if d > 0 {
6804+
t.Logf("pending requests = %d after %v (want 0)", n, d)
6805+
}
6806+
return false
6807+
}
6808+
return true
6809+
})
6810+
}

0 commit comments

Comments
 (0)