Skip to content

Retry response cleanup after cancellation - #1116

Open
pentaoa wants to merge 4 commits into
encode:masterfrom
pentaoa:fix/retry-interrupted-response-close
Open

pentaoa wants to merge 4 commits into
encode:masterfrom
pentaoa:fix/retry-interrupted-response-close

Conversation

@pentaoa

@pentaoa pentaoa commented Sep 17, 2026

Copy link
Copy Markdown

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 _closed only 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, retries response.aclose(), and verifies that the request and connection are removed. The test fails against master with one active request and connection.

Checklist

  • I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • Documentation is unaffected because this restores the existing close contract.

Validation:

  • Relevant async and sync HTTP/1.1, connection-pool, and cancellation tests: 99 passed, 6 existing xfails xpassed
  • Repository-pinned Ruff formatting and lint checks
  • Mypy on the four changed implementation files
  • scripts/unasync.py --check

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

@sylvesterkaczmarek sylvesterkaczmarek left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@pentaoa

pentaoa commented Sep 20, 2026

Copy link
Copy Markdown
Author

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 max_keepalive_connections=0. The latter failed before this update because the network close was never retried; both pass now, including a third close that must be a no-op.

Validation: 97 related tests passed, 6 existing xfails xpassed; Ruff, Mypy and unasync --check passed. Three immediate-expiry tests also fail on the previous commit on this Windows host and were excluded from that passing run.

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 twine check with the repository-pinned Twine 6.1.0. CI is rerunning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Interrupted response close cannot be retried

2 participants