Skip to content

[BugFix] Make async recv timeout bound the whole call - #4184

Merged
vmoens merged 1 commit into
mainfrom
async-recv-deadline
Aug 28, 2026
Merged

vmoens merged 1 commit into
mainfrom
async-recv-deadline

Conversation

@vmoens

@vmoens vmoens commented Aug 28, 2026 •

Copy link
Copy Markdown
Contributor

No description provided.

The timeout of async_step_recv / async_step_and_maybe_reset_recv /
async_reset_recv only bounded the max_get top-up phase: the wait for
the first result and the min_get phase used blocking, timeout-less
gets, so recv(min_get=1, timeout=...) could block forever.

The deadline is now anchored at call entry and covers the entire call
on all three paths (queue exchange, shm exchange, threading futures):

- If fewer than min_get results arrive before the deadline, a
  TimeoutError is raised. No result is lost: the multiprocessing paths
  requeue the partial harvest, the threading path leaves completed
  futures in the pool's pending lists, and pending-result accounting
  is only updated on success.
- Once min_get is satisfied, the call returns at the deadline with
  whatever is ready (up to max_get), including a final non-blocking
  drain at expiry.
- timeout=0 is a non-blocking poll; timeout=None is unchanged.

Adds docstrings for the recv contract, restructures the two deadline
tests that encoded the old semantics, and adds a regression test
asserting both the TimeoutError and the no-result-lost guarantee.

The API is unreleased (#4109, post v0.13.3), so this is a plain
behavior fix with no deprecation cycle.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@pytorch-bot

pytorch-bot Bot commented Aug 28, 2026

Copy link
Copy Markdown

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/rl/4184

Note: Links to docs will display an error until the docs builds have been completed.

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 28, 2026
@vmoens
vmoens merged commit d8b05ca into main Aug 28, 2026
157 of 161 checks passed
@vmoens
vmoens deleted the async-recv-deadline branch August 28, 2026 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BugFix CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant