Don't re-arm read timeout on a connection returned to the pool by daragok · Pull Request #12954 · aio-libs/aiohttp

daragok · 2026-06-17T16:10:32Z

What do these changes do?

ResponseHandler.resume_reading() rescheduled the sock_read read timeout on every call. But resume_reading() is also invoked while draining an already-buffered, completed response (StreamReader._read_nowait_chunk / feed_eof), which happens after the connection has been released to the keep-alive pool. That armed a sock_read timer on an idle pooled connection; the timer later fired, stamped a SocketTimeoutError on the connection via _on_read_timeout, and the next request that reused the connection failed immediately (DataQueue.read() → raise self._exception), long before any real sock_read window.

The fix only reschedules the read timeout when genuinely resuming a transport that was paused for backpressure (a read is still in flight). The in-flight read timeout is otherwise armed by start_timeout() and refreshed in data_received(), so this resume_reading() path was the only one that needed guarding. The parser-resume (data_received(b"")) used by the decompression-continuation path (#11966) is preserved.

This restores the 3.13.5 behavior (where resume_reading() only ran when the transport was actually paused).

Are there changes in behavior for the user?

Yes — a regression introduced in 3.14 is fixed. Clients that set ClientTimeout(sock_read=...) together with the keep-alive pool no longer get pooled connections poisoned by a stray read timeout, so reused connections stop failing with an immediate SocketTimeoutError. The exposure is worst when keepalive_timeout > sock_read. No public API change.

Is it a substantial burden for the maintainers to support this?

No — the change is three lines, with a regression test. It narrows when the read timeout is rescheduled to match pre-3.14 semantics.

Related issue number

Fixes #12953

Checklist

I think the code is well written
Unit tests for the changes exist
Documentation reflects the changes — N/A (no public API/behavior-contract change)
If you provide code modification, please add yourself to CONTRIBUTORS.txt
Add a new news fragment into the CHANGES/ folder

Test logs

# New regression test passes with the fix and fails without it (verified both
# with pure-Python and the compiled C extensions).

$ python -m pytest tests/test_client_functional.py::test_sock_read_timeout_not_rearmed_on_pooled_connection
============================== 1 passed in 0.39s ===============================

# (with the fix reverted)
FAILED tests/test_client_functional.py::test_sock_read_timeout_not_rearmed_on_pooled_connection
  assert proto._read_timeout_handle is None   # AssertionError -- stray timer armed on pooled conn

# Related suites, with C extensions built:
$ python -m pytest tests/test_client_proto.py tests/test_streams.py tests/test_http_parser.py
1 failed, 909 passed, 15 skipped, 6 deselected, 3 xfailed
  # the single failure is test_feed_eof_no_err_brotli, pre-existing and unrelated
  # (brotli not installed in this env; fails identically on a clean checkout)

$ python -m pytest tests/test_client_functional.py -k "timeout or keepalive or reuse or read or chunk or stream or eof"
60 passed, 8 skipped, 243 deselected

$ python -m pytest tests/test_client_ws_functional.py -k "timeout or heartbeat or receive"
19 passed, 42 deselected

Drafted with Claude Code (Opus 4.8); reviewed by @daragok.

ResponseHandler.resume_reading() rescheduled the sock_read timeout unconditionally. Reading the buffered body of a completed response calls resume_reading() after the connection has already been released to the keep-alive pool, which armed a read-timeout timer on the idle pooled connection. The timer later fired, stamped a SocketTimeoutError on the connection, and the next request that reused it failed immediately. Only reschedule the read timeout when genuinely resuming a transport that was paused for backpressure; the in-flight read timeout is armed via start_timeout() and refreshed in data_received(), so this is the only path that needed guarding. Restores the 3.13.5 behavior while keeping the parser-resume needed by the decompression-continuation path. Closes aio-libs#12953

codecov · 2026-06-17T16:19:51Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.95%. Comparing base (f8ae266) to head (d1d1dbe).
⚠️ Report is 1 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #12954   +/-   ##
=======================================
  Coverage   98.95%   98.95%           
=======================================
  Files         131      131           
  Lines       47998    48019   +21     
  Branches     2494     2495    +1     
=======================================
+ Hits        47498    47519   +21     
  Misses        376      376           
  Partials      124      124

Flag	Coverage Δ
Autobahn	`22.27% <13.63%> (-0.01%)`	⬇️
CI-GHA	`98.90% <100.00%> (+<0.01%)`	⬆️
OS-Linux	`98.66% <100.00%> (-0.01%)`	⬇️
OS-Windows	`97.03% <100.00%> (+<0.01%)`	⬆️
OS-macOS	`97.94% <100.00%> (-0.01%)`	⬇️
Py-3.10	`98.13% <100.00%> (+<0.01%)`	⬆️
Py-3.11	`98.41% <100.00%> (+<0.01%)`	⬆️
Py-3.12	`98.50% <100.00%> (+<0.01%)`	⬆️
Py-3.13	`98.46% <100.00%> (-0.01%)`	⬇️
Py-3.14	`98.48% <100.00%> (-0.01%)`	⬇️
Py-3.14t	`97.57% <100.00%> (-0.01%)`	⬇️
Py-pypy-3.11	`97.44% <100.00%> (+<0.01%)`	⬆️
VM-macos	`97.94% <100.00%> (-0.01%)`	⬇️
VM-ubuntu	`98.66% <100.00%> (-0.01%)`	⬇️
VM-windows	`97.03% <100.00%> (+<0.01%)`	⬆️
cython-coverage	`38.06% <100.00%> (+0.02%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

The doc-spelling job rejects 'backpressure' (not in the docs wordlist). Also remove the verbose comment in resume_reading() to match the project's comment style.

codspeed-hq · 2026-06-17T16:40:11Z

Merging this PR will not alter performance

✅ 83 untouched benchmarks
⏩ 83 skipped benchmarks¹

_{Comparing daragok:fix/sock-read-rearm-pooled-conn (6aa9848) with master (db5c238)²}

83 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
No successful run was found on master (f8ae266) during the generation of this report, so db5c238 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

Dreamsorcerer · 2026-06-18T19:44:31Z

@aiolibsbot review

aiolibsbot · 2026-06-18T22:57:11Z

PR Review — Don't re-arm read timeout on a connection returned to the pool

Correct, minimal fix that restores pre-3.14 read-timeout semantics. Merge-ready.

Root cause verified: _read_nowait_chunk resume after payload EOF re-armed a stray sock_read timer on pooled idle connections; the fix gates rescheduling on was_paused.
In-flight read-timeout refresh (data_received/start_timeout) and the Allow decompression to continue after exceeding max_length #11966 decompression-continuation resume are both preserved.
Solid regression test with a deterministic handle assertion; only nit is an extra real-time sleep(0.3) that re-checks the same condition.
CHANGES symlink (12954→12953), CONTRIBUTORS alphabetical insert, and changelog :user: attribution all conform to AGENTS.md conventions.

🟢 Suggestions

1. Fix is correct and minimal (`aiohttp/client_proto.py`, L198-201)

The guard is right. super().resume_reading() sets _reading_paused = False first, so capturing was_paused before the super call is necessary and correctly distinguishes a genuine backpressure resume from a buffer-drain resume.

Confirmed the bug path: after a payload reaches EOF, _drop_timeout runs via payload.on_eof (client_proto.py:366). Later, StreamReader._read_nowait_chunk drains the buffered body below low-water and calls resume_reading() (streams.py:539) — at that point the unconditional _reschedule_timeout() re-armed a stray timer on an already-pooled idle connection, with no further drop to cancel it. Gating on was_paused eliminates that path while preserving the in-flight refresh (data_received at client_proto.py:303, start_timeout at :283) and the decompression-continuation resume (_reading_paused is True there).

No change needed.

2. Test leans on real wall-clock sleep (`tests/test_client_functional.py`, L1290-1293)

The assertion at line 1284 (proto._read_timeout_handle is None) is the deterministic check that actually fails without the fix — good.

The trailing await asyncio.sleep(0.3) against sock_read=0.1 adds ~0.3s real time and only re-confirms the same condition behaviorally. Under --numprocesses=auto contention the 3× margin is usually safe, but it's a real-time-sensitive tail. Optional: drop the sleep block and rely on the handle assertion, or shrink the margin, to keep the suite fast and deterministic. Not blocking.

Checklist

Fix targets the actual bug path (buffer-drain resume after EOF)
No legitimate timeout reschedule is lost
Regression test fails without fix, passes with it
Test isolation / no order dependence
Changelog, CONTRIBUTORS, symlink conventions followed
Test avoids fragile timing — suggestion ＃2

Automated review by Kōan (Claude) HEAD=e9a4a6a 2 min 31s

Dreamsorcerer · 2026-06-18T22:58:45Z

Well, I beat the bot to it...

patchback · 2026-06-18T23:32:21Z

Backport to 3.14: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply 5c293f4 on top of patchback/backports/3.14/5c293f4f71f6188b446afd331afa47262a874f4f/pr-12954

Backporting merged PR #12954 into master

Ensure you have a local repo clone of your fork. Unless you cloned it
from the upstream, this would be your origin remote.
Make sure you have an upstream repo added as a remote too. In these
instructions you'll refer to it by the name upstream. If you don't
have it, here's how you can add it:
```
$ git remote add upstream https://github.com/aio-libs/aiohttp.git
```

Ensure you have the latest copy of upstream and prepare a branch
that will hold the backported code:

$ git fetch upstream
$ git checkout -b patchback/backports/3.14/5c293f4f71f6188b446afd331afa47262a874f4f/pr-12954 upstream/3.14

Now, cherry-pick PR Don't re-arm read timeout on a connection returned to the pool #12954 contents into that branch:
```
$ git cherry-pick -x 5c293f4f71f6188b446afd331afa47262a874f4f
```
If it'll yell at you with something like fatal: Commit 5c293f4f71f6188b446afd331afa47262a874f4f is a merge but no -m option was given., add -m 1 as follows instead:
```
$ git cherry-pick -m1 -x 5c293f4f71f6188b446afd331afa47262a874f4f
```
At this point, you'll probably encounter some merge conflicts. You must
resolve them in to preserve the patch from PR Don't re-arm read timeout on a connection returned to the pool #12954 as close to the
original as possible.

Push this branch to your fork on GitHub:

$ git push origin patchback/backports/3.14/5c293f4f71f6188b446afd331afa47262a874f4f/pr-12954

Create a PR, ensure that the CI is green. If it's not — update it so that
the tests and any other checks pass. This is it!
Now relax and wait for the maintainers to process your pull request
when they have some cycles to do reviews. Don't worry — they'll tell you if
any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback · 2026-06-18T23:32:28Z

Backport to 3.15: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply 5c293f4 on top of patchback/backports/3.15/5c293f4f71f6188b446afd331afa47262a874f4f/pr-12954

Backporting merged PR #12954 into master

Ensure you have a local repo clone of your fork. Unless you cloned it
from the upstream, this would be your origin remote.
Make sure you have an upstream repo added as a remote too. In these
instructions you'll refer to it by the name upstream. If you don't
have it, here's how you can add it:
```
$ git remote add upstream https://github.com/aio-libs/aiohttp.git
```

Ensure you have the latest copy of upstream and prepare a branch
that will hold the backported code:

$ git fetch upstream
$ git checkout -b patchback/backports/3.15/5c293f4f71f6188b446afd331afa47262a874f4f/pr-12954 upstream/3.15

Now, cherry-pick PR Don't re-arm read timeout on a connection returned to the pool #12954 contents into that branch:
```
$ git cherry-pick -x 5c293f4f71f6188b446afd331afa47262a874f4f
```
If it'll yell at you with something like fatal: Commit 5c293f4f71f6188b446afd331afa47262a874f4f is a merge but no -m option was given., add -m 1 as follows instead:
```
$ git cherry-pick -m1 -x 5c293f4f71f6188b446afd331afa47262a874f4f
```
At this point, you'll probably encounter some merge conflicts. You must
resolve them in to preserve the patch from PR Don't re-arm read timeout on a connection returned to the pool #12954 as close to the
original as possible.

Push this branch to your fork on GitHub:

$ git push origin patchback/backports/3.15/5c293f4f71f6188b446afd331afa47262a874f4f/pr-12954

Create a PR, ensure that the CI is green. If it's not — update it so that
the tests and any other checks pass. This is it!
Now relax and wait for the maintainers to process your pull request
when they have some cycles to do reviews. Don't worry — they'll tell you if
any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

Dreamsorcerer · 2026-06-18T23:33:34Z

Could you handle the above backports?

daragok · 2026-06-19T08:13:11Z

@Dreamsorcerer, hey, thanks for the merge 🤗

I could handle the backports, if that's a question for me. Do you want me to create 2 PRs for those? to the 3.14 and 3.15 branches

…nnection returned to the pool (#12967) **This is a backport of PR #12954 as merged into master (5c293f4).**

…nnection returned to the pool (#12966) **This is a backport of PR #12954 as merged into master (5c293f4).**

…imeouts aiohttp 3.14.0 and 3.14.1 re-arm the sock_read timer on a keep-alive connection after it has already been returned to the idle pool. The stray timer stamps a SocketTimeoutError on the pooled connection without closing it, so the pool keeps handing it out and the next request to pick it up fails instantly on an error left behind by an earlier, unrelated request. Because a single pool is shared across providers, the failures appear simultaneously across Vertex AI, Bedrock, Anthropic and OpenAI-compatible deployments as sub-millisecond "Connection timed out" errors. uv.lock resolved aiohttp 3.14.1 and the published images install via `uv sync --frozen`, so every image built from that lock shipped the regression. The wheel's own metadata declared `aiohttp>=3.10,<4.0`, which also left pip consumers free to resolve into the same broken window, so both the runtime floor and the uv constraint move to >=3.14.2. Upstream fixed this in aio-libs/aiohttp#12954, released in aiohttp 3.14.2; the lock now resolves 3.14.3. Raising the floor rather than capping below 3.14 keeps the advisories that the existing 3.14.1 floor cleared, so no osv-scanner ignores are needed. litellm requires Python >=3.10 and aiohttp 3.14.2 requires >=3.10, so no supported interpreter loses support. Both new tests fail on the previous pins and pass on these.

…imeouts aiohttp 3.14.0 and 3.14.1 re-arm the sock_read timer on a keep-alive connection after it has already been returned to the idle pool. The stray timer stamps a SocketTimeoutError on the pooled connection without closing it, so the pool keeps handing it out and the next request to pick it up fails instantly on an error left behind by an earlier, unrelated request. Because a single pool is shared across providers, the failures appear simultaneously across Vertex AI, Bedrock, Anthropic and OpenAI-compatible deployments as sub-millisecond "Connection timed out" errors. uv.lock resolved aiohttp 3.14.1 and the published images install via `uv sync --frozen`, so every image built from that lock shipped the regression. The wheel's own metadata declared `aiohttp>=3.10,<4.0`, which also left pip consumers free to resolve into the same broken window, so both the runtime floor and the uv constraint move to >=3.14.2. Upstream fixed this in aio-libs/aiohttp#12954, released in aiohttp 3.14.2; the lock now resolves 3.14.3. Raising the floor rather than capping below 3.14 keeps the advisories that the existing 3.14.1 floor cleared, so no osv-scanner ignores are needed. litellm requires Python >=3.10 and aiohttp 3.14.2 requires >=3.10, so no supported interpreter loses support. Both new tests fail on the previous pins and pass on these. (cherry picked from commit ffd6ac5)

psf-chronographer Bot added the bot:chronographer:provided There is a change note present in this PR label Jun 17, 2026

Symlink PR-numbered changelog fragment to the issue fragment

e93b5e9

Drop the explanatory comment and avoid 'backpressure' in the changelog

6aa9848

The doc-spelling job rejects 'backpressure' (not in the docs wordlist). Also remove the verbose comment in resume_reading() to match the project's comment style.

daragok mentioned this pull request Jun 17, 2026

Connection Pool Poisoning #12795

Closed

1 task

daragok marked this pull request as ready for review June 17, 2026 17:09

daragok requested review from asvetlov and webknjaz as code owners June 17, 2026 17:09

Dreamsorcerer added backport-3.14 Trigger automatic backporting to the 3.14 release branch by Patchback robot backport-3.15 Trigger automatic backporting to the 3.15 release branch by Patchback robot labels Jun 18, 2026

Dreamsorcerer added 2 commits June 18, 2026 23:54

Shorten test slightly to reduce runtime

e9a4a6a

Update test_client_functional.py

d1d1dbe

Dreamsorcerer merged commit 5c293f4 into aio-libs:master Jun 18, 2026
48 of 49 checks passed

This was referenced Jun 19, 2026

[PR #12954/5c293f4f backport][3.14] Don't re-arm read timeout on a connection returned to the pool #12966

Merged

[PR #12954/5c293f4f backport][3.15] Don't re-arm read timeout on a connection returned to the pool #12967

Merged

Dreamsorcerer pushed a commit that referenced this pull request Jun 19, 2026

[PR #12954/5c293f4f backport][3.15] Don't re-arm read timeout on a co…

eae0621

…nnection returned to the pool (#12967) **This is a backport of PR #12954 as merged into master (5c293f4).**

Dreamsorcerer pushed a commit that referenced this pull request Jun 19, 2026

[PR #12954/5c293f4f backport][3.14] Don't re-arm read timeout on a co…

4cce8a5

…nnection returned to the pool (#12966) **This is a backport of PR #12954 as merged into master (5c293f4).**

deepanshululla mentioned this pull request Jul 18, 2026

[Bug]: aiohttp 3.14.x connection-pool poisoning causes cross-provider "Connection timed out" failures (litellm pins aiohttp>=3.14.1, v1.91.0+) BerriAI/litellm#33820

Closed

devin-ai-integration Bot mentioned this pull request Jul 18, 2026

fix(deps): pin aiohttp below 3.14 to avoid pooled-connection timeout regression BerriAI/litellm#33822

Open

5 tasks

yuneng-berri mentioned this pull request Jul 31, 2026

fix(deps): raise aiohttp floor to 3.14.2 to clear pooled-connection timeouts BerriAI/litellm#35337

Merged

5 tasks

Navigation Menu

Uh oh!

Uh oh!

Don't re-arm read timeout on a connection returned to the pool - #12954

Don't re-arm read timeout on a connection returned to the pool#12954
Dreamsorcerer merged 5 commits into
aio-libs:masterfrom
daragok:fix/sock-read-rearm-pooled-conn

daragok commented Jun 17, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Jun 17, 2026 •

edited

Loading

Uh oh!

codspeed-hq Bot commented Jun 17, 2026

Uh oh!

Dreamsorcerer commented Jun 18, 2026

Uh oh!

aiolibsbot commented Jun 18, 2026

Uh oh!

Dreamsorcerer commented Jun 18, 2026

Uh oh!

Uh oh!

patchback Bot commented Jun 18, 2026 •

edited

Loading

Uh oh!

patchback Bot commented Jun 18, 2026 •

edited

Loading

Uh oh!

Dreamsorcerer commented Jun 18, 2026

Uh oh!

daragok commented Jun 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

daragok commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What do these changes do?

Are there changes in behavior for the user?

Is it a substantial burden for the maintainers to support this?

Related issue number

Checklist

Uh oh!

codecov Bot commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

codspeed-hq Bot commented Jun 17, 2026

Merging this PR will not alter performance

Footnotes

Uh oh!

Dreamsorcerer commented Jun 18, 2026

Uh oh!

aiolibsbot commented Jun 18, 2026

PR Review — Don't re-arm read timeout on a connection returned to the pool

🟢 Suggestions

Checklist

Uh oh!

Dreamsorcerer commented Jun 18, 2026

Uh oh!

Uh oh!

patchback Bot commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Backport to 3.14: 💔 cherry-picking failed — conflicts found

Backporting merged PR #12954 into master

Uh oh!

patchback Bot commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Backport to 3.15: 💔 cherry-picking failed — conflicts found

Backporting merged PR #12954 into master

Uh oh!

Dreamsorcerer commented Jun 18, 2026

Uh oh!

daragok commented Jun 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

daragok commented Jun 17, 2026 •

edited

Loading

codecov Bot commented Jun 17, 2026 •

edited

Loading

patchback Bot commented Jun 18, 2026 •

edited

Loading

patchback Bot commented Jun 18, 2026 •

edited

Loading