Skip to content

Close request body async iterator when a write error occurs - #1119

Open
Dextheking1 wants to merge 5 commits into
encode:masterfrom
Dextheking1:fix/3597-writeerror-stream-aclose
Open

Dextheking1 wants to merge 5 commits into
encode:masterfrom
Dextheking1:fix/3597-writeerror-stream-aclose

Conversation

@Dextheking1

@Dextheking1 Dextheking1 commented Sep 23, 2026 •

Copy link
Copy Markdown

Closes encode/httpx#3597.

What

AsyncHTTP11Connection._send_request_body now closes the request body's async iterator in a finally block, so it is cleaned up on both the success and failure paths.

Why

When a WriteError is raised part-way through sending the request body, handle_async_request catches and suppresses it (to allow reading a pre-emptive error response from the server), but the request body's async iterator was left abandoned mid-iteration. It was then garbage collected without ever being exhausted, producing:

ResourceWarning: Async generator '...' was garbage collected before it had been exhausted.
Surround its use in 'async with aclosing(...):' to ensure that it gets cleaned up as soon as you're done using it.

Note: closing the iterator (rather than calling request.stream.aclose()) is what fixes this, e.g. httpx's ByteStream.aclose() is a no-op, while the abandoned object is the generator created by stream.__aiter__(). The aclose call is getattr-guarded so streams whose iterators lack aclose keep working.

Test evidence

New regression test test_http11_write_error_closes_request_body in tests/_async/test_http11.py: a network stream that raises WriteError mid-body, asserting the body iterator receives GeneratorExit while the WriteError stays suppressed and the response remains readable.

Before (on master, trio backend):

ResourceWarning: Async generator '...TrackingBody.__aiter__.<locals>.gen' was garbage collected before it had been exhausted. ...

After: no warning; tests/_async/test_http11.py, 28 passed (asyncio + trio backends). ruff check and ruff format clean.

If a `WriteError` is raised part-way through `_send_request_body`,
the request body's async iterator was abandoned mid-iteration and
later garbage collected without ever being exhausted, triggering
`ResourceWarning: async generator was garbage collected before it
had been exhausted` (encode/httpx#3597).

Close the iterator in a `finally` block so it is cleaned up on both
success and failure paths.
The _sync/http11.py and _sync/test_http11.py files are generated from
their _async counterparts by scripts/unasync.py (enforced by
'scripts/check' in CI). Also annotate the new test's generator so mypy
is clean.
@Dextheking1
Dextheking1 marked this pull request as ready for review September 23, 2026 23:54
@Dextheking1
Dextheking1 force-pushed the fix/3597-writeerror-stream-aclose branch from 6eed176 to d6d35b7 Compare September 28, 2026 11:29
@jacksonsr451

Copy link
Copy Markdown

Hi! I investigated the async request-body iterator cleanup related to encode/httpx#3597 and reviewed this PR.

I found one additional cancellation-safety edge case: when an async generator's finally block contains an await checkpoint, Trio cancellation can interrupt the unshielded aclose() before the generator finishes cleanup.

I reproduced this deterministically against the current PR implementation. The regression test fails under Trio without shielding and passes under both asyncio and Trio when aclose() is wrapped with HTTPCore's existing AsyncShieldCancellation.

I also ran the full HTTPCore and HTTPX test suites, comparing the complementary change against the PR head. The observed failures were reproducible on the unmodified PR implementation.

I have a minimal complementary patch and regression test prepared locally. Would you be interested in incorporating this cancellation-safety improvement into the existing PR?

Thanks!

@jacksonsr451

Copy link
Copy Markdown

Hi! I've completed the investigation and prepared two complementary changes for this PR.

1. Cancellation-safe async iterator cleanup

I found that cancellation can interrupt aclose() before an async generator finishes its cleanup, particularly when its finally block contains an await checkpoint.

The proposed change wraps only await aclose() with HTTPCore's existing AsyncShieldCancellation, preserving the current request-body handling and exception behavior.

The regression test reproduces the issue against the original PR implementation under both asyncio and Trio (RED), and passes with the fix applied (GREEN).

2. Build compatibility improvement

I also reproduced a build compatibility issue involving Hatchling-generated metadata and Twine 6.1.

Hatchling 1.32 generates metadata version 2.5, which Twine 6.1 rejects. Constraining the build dependency to hatchling<1.32 resolved the locally reproduced issue.

Build and Twine checks passed under Linux with Python 3.8 and 3.12.

Validation results:

  • Cancellation regression: 2 passed.
  • HTTP/1.1 and cancellation tests: 52 passed, 6 XPASS.
  • Ruff, mypy, and generated-source consistency checks: passed.
  • Build, Twine, and MkDocs: passed.
  • Full Windows suite: 216 passed, 3 failed, 6 XPASS. The three failures also reproduce against the unmodified PR baseline.

I've prepared these as two separate local commits to keep the cancellation fix independent from the build compatibility change.

Would you prefer that I share the commits as patches, open a separate PR targeting your branch (if possible), or follow another approach to incorporate them into #1119?

I'd be happy to adapt to your preferred workflow. Thanks!

@jacksonsr451

Copy link
Copy Markdown

Thanks for the iterator-finalization work in #1119!

I've prepared two complementary commits on my fork:

  • Cancellation-safe iterator cleanup — Shields the awaited aclose() call using the existing AsyncShieldCancellation mechanism, with deterministic regression coverage. The regression test fails against the unshielded PR head and passes with the fix under both asyncio and Trio.

  • Hatchling/Twine build compatibility — Constrains Hatchling to <1.32 to avoid the metadata 2.5 incompatibility with Twine 6.1. Build and Twine validation passed, including Python 3.8 compatibility checks.

Branch: contrib/pr1119-cancellation-shield

Validation

  • Cancellation regression tests: 2 passed (asyncio and Trio).
  • Generated-source consistency check: passed.
  • Full Windows suite: 216 passed, 3 baseline-confirmed failures, 6 XPASS.
  • Build and Twine compatibility checks: passed.

Both commits are separate and based directly on the original PR head, so they can be incorporated independently.

Would you prefer to cherry-pick the cancellation-safety fix into #1119 and handle the build compatibility change separately, or incorporate both commits into this PR?

I'm happy to make any adjustments based on your feedback.

Thanks!

This branch has not been deployed

No deployments
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.

ResourceWarning in test_write_timeout due to Unclosed Async Generator

2 participants