Conversation
sylvesterkaczmarek
left a comment
There was a problem hiding this comment.
One retry hole still looks open here: _closed is set before _close_connections(closing). If one of those connection closes raises, a second aclose() returns immediately and that cleanup is never retried. Could the flag move after _close_connections(), or otherwise keep those pending closes retryable?
|
Fixed in dc2fb71. The response stream now retains detached connections until each close succeeds, then marks itself closed. Retries skip the already-completed request removal and resume pending cleanup. The regression now covers both an unread response and a fully read response evicted with Validation: 97 related tests passed, 6 existing xfails xpassed; Ruff, Mypy and CI then stopped at package validation: its pinned Twine rejects the metadata 2.5 emitted by the current Hatchling. Commit 6d5f158 explicitly selects metadata 2.4 for both distribution targets, retaining the existing Python support and tooling versions. The wheel and sdist both build and pass |
Summary
Fixes #1115.
A response close interrupted by cancellation used to leave both byte-stream wrappers marked closed before their cleanup completed. Any later close then returned immediately, leaving the pool request and its HTTP/1.1 connection counted as active. This is the connection leak reported in encode/httpx#3782.
The wrappers now serialize close calls and set
_closedonly after the inner close and pool bookkeeping succeed. An interrupted close therefore remains retryable. The generated synchronous implementations are kept in step with the async sources, as required by the repository.The regression test interrupts the network close with
CancelledError, confirms the pool is still occupied, retriesresponse.aclose(), and verifies that the request and connection are removed. The test fails againstmasterwith one active request and connection.Checklist
Validation:
scripts/unasync.py --checkReview follow-up: detached connection cleanup now remains retryable after pool bookkeeping. The regression covers both unread and fully read responses. On Windows, 97 related tests pass and 6 existing xfails xpass; 3 immediate-expiry failures also reproduce on the previous commit. Ruff, Mypy and unasync checks pass. Wheel and sdist builds pass Twine 6.1.0 validation with explicit metadata 2.4, avoiding the CI failure caused by the newer Hatchling default.