Skip to content

Retry response cleanup after cancellation - #1116

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

pentaoa wants to merge 2 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

@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?

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