fix(robot): make lockstep lifecycle and control RPCs race-safe - #520
fix(robot): make lockstep lifecycle and control RPCs race-safe#520MagellaX wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dcd40c15d2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if token is not None and owned_token is not None and token != owned_token: | ||
| raise ValueError( | ||
| f"result token {token!r} does not match this session's claim {owned_token!r}" | ||
| ) |
There was a problem hiding this comment.
Reject tokens when the session has no live claim
When a session has never claimed a slot or has already completed its own result, owned_token is None, so this condition permits any explicit token. Because concurrent sessions share the endpoint's control connection and the bridge validates ownership only at the connection level, such a call can grade and release another session's slot, corrupting that rollout. Require an explicit token to match a live session claim whenever session_id is present.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit dcd40c1. Configure here.
# Conflicts: # hud/environment/tests/test_robot_bridge.py # hud/environment/tests/test_robot_claims.py

Summary
Follow-up hardening for #481. This keeps the unified batched robot design while closing correctness gaps found under concurrent
Sharedrollouts and real LeRobot inference.torch.from_numpy;Root cause and impact
The tick loop previously advanced the simulator outside the reset/result lifecycle transaction. A concurrent
resultcould return a pre-step grade after the action had already been consumed. Cancellation after sending an RPC could also leave its reply unread on the shared stream and strand a reset claim.The former boolean idle state also conflated never-dialed, disconnected, terminal, and consumed slots, making late admission and reconnect behavior ambiguous. Separately, msgpack-decoded arrays are read-only but were passed directly to PyTorch.
These changes preserve action/observation/grade identity for vectorized
Sharedrollouts, prevent canceled or dropped control calls from wedging a generation, and avoid PyTorch's unsafe non-writeable NumPy path without copying writable arrays.Validation
git diff --check: passed.lerobot/smolvla_libero@31d453f7edd78c839a8bbc39744a292686daf0de:libero_spatial, task 0, seed 0;[1, 50, 7];The smoke run scored
0/success=false, as expected from the 12-step cap; it validates execution correctness, not benchmark success.Note
High Risk
Changes core robot bridge lifecycle, lockstep stepping, and grading under concurrency and cancellation; incorrect behavior would corrupt trajectories or wedge batches, though behavior is heavily covered by new lifecycle tests.
Overview
Hardens vectorized robot sim lockstep and shared control RPCs so concurrent
Sharedrollouts keep trajectory, grading, and slot identity under races and cancellation.Bridge (
RobotBridge) replaces booleanidle/usedwith explicit_SlotPhase(dialing, active, idle, expired, terminated, spent, sealed)._lifecycle_locktiesreset,result, and the tick loop together so grading cannot race an in-flight step or consume actions afterresultstarts. The tick loop only steps when every active claimed slot has an action; never-dialed slots expire at the initial dial deadline instead of blocking peers; connected slow policies are not stepped with hold timeouts. After the first step, available slots are sealed so late rollouts cannot join an advanced batch. Control connections own reset tokens and discard them on disconnect without user grading;LeRobotAdaptercopies read-only msgpack arrays beforetorch.from_numpy.Endpoint shields RPC exchanges, drains replies on cancel (with bounded timeout and connection poison on hang), validates session–token ownership on
result, and releases stranded claims on cancelledreset.Docs clarify lockstep holds, claim-before-first-step admission, and that
Sharedwidthis a cap—not a start barrier—for late rollouts.Reviewed by Cursor Bugbot for commit fa4877b. Bugbot is set up for automated code reviews on this repo. Configure here.