Skip to content

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

Merged
Dreamsorcerer merged 1 commit into
aio-libs:3.14from
daragok:patchback/backports/3.14/5c293f4f71f6188b446afd331afa47262a874f4f/pr-12954
Jun 19, 2026
Merged

[PR #12954/5c293f4f backport][3.14] Don't re-arm read timeout on a connection returned to the pool#12966
Dreamsorcerer merged 1 commit into
aio-libs:3.14from
daragok:patchback/backports/3.14/5c293f4f71f6188b446afd331afa47262a874f4f/pr-12954

Conversation

@daragok

@daragok daragok commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

This is a backport of PR #12954 as merged into master (5c293f4).

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.

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

codecov Bot commented Jun 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.23%. Comparing base (248cc2c) to head (2499993).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             3.14   #12966   +/-   ##
=======================================
  Coverage   98.23%   98.23%           
=======================================
  Files         135      135           
  Lines       48625    48646   +21     
  Branches     2608     2609    +1     
=======================================
+ Hits        47767    47788   +21     
  Misses        678      678           
  Partials      180      180           
Flag Coverage Δ
CI-GHA 98.28% <100.00%> (+<0.01%) ⬆️
OS-Linux 98.05% <100.00%> (+<0.01%) ⬆️
OS-Windows 95.72% <100.00%> (+<0.01%) ⬆️
OS-macOS 97.25% <100.00%> (+<0.01%) ⬆️
Py-3.10 97.44% <100.00%> (-0.01%) ⬇️
Py-3.11 97.72% <100.00%> (+<0.01%) ⬆️
Py-3.12 97.79% <100.00%> (+<0.01%) ⬆️
Py-3.13 97.77% <100.00%> (-0.01%) ⬇️
Py-3.14 97.88% <100.00%> (+<0.01%) ⬆️
Py-3.14t 96.88% <100.00%> (+<0.01%) ⬆️
Py-pypy-3.11 96.72% <100.00%> (+<0.01%) ⬆️
VM-macos 97.25% <100.00%> (+<0.01%) ⬆️
VM-ubuntu 98.05% <100.00%> (+<0.01%) ⬆️
VM-windows 95.72% <100.00%> (+<0.01%) ⬆️
cython-coverage 38.41% <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.

@codspeed-hq

codspeed-hq Bot commented Jun 19, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 72 untouched benchmarks
⏩ 7 skipped benchmarks1


Comparing daragok:patchback/backports/3.14/5c293f4f71f6188b446afd331afa47262a874f4f/pr-12954 (2499993) with 3.14 (248cc2c)

Open in CodSpeed

Footnotes

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

@Dreamsorcerer
Dreamsorcerer merged commit 4cce8a5 into aio-libs:3.14 Jun 19, 2026
46 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:chronographer:provided There is a change note present in this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants