Skip to content

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

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

[PR #12954/5c293f4f backport][3.15] Don't re-arm read timeout on a connection returned to the pool#12967
Dreamsorcerer merged 1 commit into
aio-libs:3.15from
daragok:patchback/backports/3.15/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 (e67a2e8) to head (0a3362f).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             3.15   #12967   +/-   ##
=======================================
  Coverage   98.23%   98.23%           
=======================================
  Files         135      135           
  Lines       48720    48741   +21     
  Branches     2621     2622    +1     
=======================================
+ Hits        47862    47883   +21     
  Misses        678      678           
  Partials      180      180           
Flag Coverage Δ
CI-GHA 98.26% <100.00%> (+<0.01%) ⬆️
OS-Linux 98.03% <100.00%> (+<0.01%) ⬆️
OS-Windows 95.71% <100.00%> (+<0.01%) ⬆️
OS-macOS 97.24% <100.00%> (+<0.01%) ⬆️
Py-3.10 97.40% <100.00%> (+<0.01%) ⬆️
Py-3.11 97.70% <100.00%> (-0.01%) ⬇️
Py-3.12 97.78% <100.00%> (+<0.01%) ⬆️
Py-3.13 97.76% <100.00%> (+<0.01%) ⬆️
Py-3.14 97.87% <100.00%> (+<0.01%) ⬆️
Py-3.14t 96.86% <100.00%> (-0.01%) ⬇️
Py-pypy-3.11 96.71% <100.00%> (+<0.01%) ⬆️
VM-macos 97.24% <100.00%> (+<0.01%) ⬆️
VM-ubuntu 98.03% <100.00%> (+<0.01%) ⬆️
VM-windows 95.71% <100.00%> (+<0.01%) ⬆️
cython-coverage 38.39% <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

✅ 83 untouched benchmarks
⏩ 86 skipped benchmarks1


Comparing daragok:patchback/backports/3.15/5c293f4f71f6188b446afd331afa47262a874f4f/pr-12954 (0a3362f) with 3.15 (e67a2e8)

Open in CodSpeed

Footnotes

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

Copy link
Copy Markdown
Member

Thanks

@Dreamsorcerer
Dreamsorcerer merged commit eae0621 into aio-libs:3.15 Jun 19, 2026
47 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