Skip to content

fix(async): apply socket_timeout per read in async parsers - #4106

Open
goingforstudying-ctrl wants to merge 3 commits into
redis:masterfrom
goingforstudying-ctrl:fix-async-timeout-per-read
Open

fix(async): apply socket_timeout per read in async parsers#4106
goingforstudying-ctrl wants to merge 3 commits into
redis:masterfrom
goingforstudying-ctrl:fix-async-timeout-per-read

Conversation

@goingforstudying-ctrl

@goingforstudying-ctrl goingforstudying-ctrl commented Jun 6, 2026

Copy link
Copy Markdown

Fixes #3454

The async client was wrapping the entire parser.read_response() call in async_timeout, so socket_timeout applied to the whole response. The sync client applies it per socket recv() instead.

This change passes the timeout down to the async parsers (RESP2, RESP3, and Hiredis) and applies it around each individual stream read, matching the sync semantics.


Note

Medium Risk
Changes core async I/O and timeout behavior for all connections (RESP2/RESP3/Hiredis), which can alter when slow large replies time out versus succeeding; buffer handling for pipelining was also adjusted.

Overview
Fixes #3454 by aligning async socket_timeout with the sync client: the timeout applies to each stream read while parsing a reply, not to the whole read_response() call.

Connection.read_response no longer wraps the parser in a single outer async_timeout; it passes read_timeout into parser.read_response(timeout=...).

Async parsers (Python RESP2/RESP3 base, Hiredis) accept a timeout argument and apply async_timeout around individual stream.read() calls via new _read_from_stream. RESP2/RESP3 recursive parsing threads timeout through _readline / _read. Hiredis applies it in read_from_socket and on recursive push handling.

The Python async RESP buffer _clear now keeps unconsumed bytes after _pos for pipelining, with a full buffer reset on on_connect. _read / _readline were reworked to fill the buffer incrementally instead of relying on readexactly for whole segments.

New tests in test_async_timeout_per_read.py cover multi-chunk success, per-chunk failure, nested arrays, SENTINEL default, Hiredis, and connection-to-parser timeout forwarding.

Reviewed by Cursor Bugbot for commit 54d8a65. Bugbot is set up for automated code reviews on this repo. Configure here.

@goingforstudying-ctrl
goingforstudying-ctrl force-pushed the fix-async-timeout-per-read branch from 15d783a to 77d17d1 Compare June 8, 2026 10:25
@petyaslavova

Copy link
Copy Markdown
Collaborator

Hi @goingforstudying-ctrl, thank you for your contribution! I'll review it shortly.

@goingforstudying-ctrl
goingforstudying-ctrl force-pushed the fix-async-timeout-per-read branch 3 times, most recently from eb9ab5b to e8d64de Compare June 9, 2026 17:13
Comment thread redis/_parsers/base.py

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Reviewed by Cursor Bugbot for commit d159301. Configure here.

Comment thread redis/_parsers/base.py
@goingforstudying-ctrl
goingforstudying-ctrl force-pushed the fix-async-timeout-per-read branch 3 times, most recently from 28e0c92 to d64a8ee Compare June 12, 2026 07:51
@goingforstudying-ctrl

Copy link
Copy Markdown
Author

Hi @petyaslavova, this PR (async socket_timeout per-read fix) has been waiting for CI approval. Would you be able to approve the workflow run when you get a chance? Thanks!

@goingforstudying-ctrl
goingforstudying-ctrl force-pushed the fix-async-timeout-per-read branch 2 times, most recently from f3fecb4 to b4c0415 Compare June 17, 2026 17:00
@goingforstudying-ctrl

goingforstudying-ctrl commented Jun 21, 2026

Copy link
Copy Markdown
Author

Addressed the review comments:

  1. Pipelined data discarded after parse — fixed by preserving bytes beyond _pos in _clear()
  2. Reconnect retains stale parser buffer — fixed by doing a full buffer reset in on_connect() instead of calling _clear()

Both issues have been resolved in the latest commits.

@goingforstudying-ctrl

goingforstudying-ctrl commented Jun 23, 2026

Copy link
Copy Markdown
Author

Hi @petyaslavova, I addressed the review issues in the latest commits and CI looks green now. Would you have a chance to take another look? Thanks!

@goingforstudying-ctrl
goingforstudying-ctrl force-pushed the fix-async-timeout-per-read branch from b4c0415 to 9714948 Compare June 26, 2026 07:35
The async client was wrapping the entire parser.read_response() call in
async_timeout, which meant socket_timeout applied to the whole response.
The sync client applies it per socket recv() instead.

Change the async Python parsers (RESP2/RESP3) and the Hiredis async parser
to accept a timeout parameter and apply it around each individual stream
read. Connection.read_response now passes the timeout down to the parser
rather than wrapping the parser call.

Fixes redis#3454
_clear() now retains unconsumed bytes beyond _pos so pipelined
responses buffered during a readline() are not discarded after a
successful parse. on_connect() performs a full buffer reset on
(re)connect to avoid carrying stale data from a previous session.
@goingforstudying-ctrl
goingforstudying-ctrl force-pushed the fix-async-timeout-per-read branch from 38501cd to ceb0bd4 Compare July 2, 2026 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

socket_timeout parameter on async connections actually apply to multiple socket read operations

2 participants