Concurrent HTTP/2 uploads deadlock in `_wait_for_outgoing_flow`
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 1.5k
- Forks
- 78
- Avg merge
- 8h 59m
- Merged PRs (30d)
- 24
Description
When several large request bodies are uploaded concurrently over one HTTP/2
connection, one of the streams stops sending and hangs until its read timeout,
even though the peer has granted it plenty of flow-control window. The other
streams on the connection complete normally.
AsyncHTTP2Connection._wait_for_outgoing_flow waits for window by calling
_receive_events(request) with no stream id. That call takes the connection's
shared _read_lock and then unconditionally performs a blocking network read.
It never rechecks whether the WINDOW_UPDATE the caller is waiting for was
already delivered by a read that another stream performed while this one was
queued on the lock. Once the other streams have finished, the peer has nothing
further to send, and that read never returns.
Impact
The stalled request fails with ReadTimeout
after the full read timeout, and until then it holds the connection's read lock,
so it is a per-connection stall rather than a per-request one. It surfaced in
production as an upload that took 28.5 seconds instead of 3.5, which was long
enough for the upstream to reset the stream and fail the request outright.
The redundant reads cost time even when they do eventually return: removing them
made the concurrent case roughly an order of magnitude faster (see the table).
Reproduction
Concurrent 1.4 MB POST bodies through one AsyncClient(http1=False, http2=True), against a minimal h2 server that acknowledges flow control
promptly. Twenty rounds per row; a "stall" is a round in which at least one
upload hit the 10 s read timeout.
| build | concurrency | stalls | mean round |
|---|---|---|---|
| httpx2 2.10.0 | 2 | 0/20 | 0.04 s |
| httpx2 2.10.0 | 4 | 2/20 | 1.08 s |
| httpx2 2.10.0 | 8 | 3/20 | 1.67 s |
| httpx2 2.10.0 | 16 | 2/20 | 1.34 s |
Concurrency of one or two never stalls. Every failure is a ReadTimeout, and
the upload count confirms exactly one stream per stalled round fails to finish.
Server-side state at the moment of a hang shows the client, not the server, is
at fault: the connection's inbound window is full and the stalled stream has
tens of kilobytes of remote window available, while the client has stopped
sending with tens of kilobytes still to write.
Suggested fix
Recheck the caller's window after taking the read lock, and skip the read if it
has already been replenished. With this applied, every configuration above goes
to 0 stalls, and the mean round time at concurrency 4 falls from 1.08 s to
0.08 s. Verified on httpcore2 2.10.0.
async def _receive_events(
self,
request: Request,
stream_id: int | None = None,
awaiting_flow_on: int | None = None,
) -> None:
async with self._read_lock:
# A task waiting on outgoing flow control queues behind other tasks on
# this lock. By the time it gets in, one of their reads may already have
# delivered the WINDOW_UPDATE it needs; without this check it commits to
# a blocking read that nothing is left to answer.
if (
awaiting_flow_on is not None
and self._h2_state.local_flow_control_window(awaiting_flow_on) > 0
):
return
...
and at the single call site in _wait_for_outgoing_flow:
while flow <= 0:
await self._receive_events(request, awaiting_flow_on=stream_id)
The early return skips the trailing await self._write_outgoing_data(request).
That is safe as written, because the early-returning path generates no frames to
flush, but a variant that keeps the flush also clears every configuration if you
would rather preserve the method's postcondition.
No spin is introduced: the read is still performed whenever the window really is
zero, and _wait_for_outgoing_flow rechecks the window immediately after the
call with no intervening await.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with h2_upload_stall_repro.py and inspect AsyncHTTP2Connection._wait_for_outgoing_flow and _receive_events, focusing on the shared _read_lock and flow-control checks. Run the reproduction against the listed concurrency levels; done means all configurations reach 0 stalls without ReadTimeout and the concurrent case avoids redundant blocking reads.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 76/100