Repository navigation
[Bugfix][NIXL] Harden handshake listener against bad requests - #1
Closed
jdonkervliet wants to merge 2 commits into
Closed
jdonkervliet wants to merge 2 commits into
jdonkervliet wants to merge 2 commits into
Conversation
jdonkervliet
force-pushed
the
jesse/nixl-handshake-hardening
branch
2 times, most recently
from
September 1, 2026 09:06
e2a20e8 to
988fc3e
Compare
Owner
Author
|
Closing and reopening to force GitHub to recompute a stuck diff cache. |
jdonkervliet
force-pushed
the
jesse/nixl-handshake-hardening
branch
2 times, most recently
from
September 1, 2026 10:51
8df4d33 to
c0ef774
Compare
The prefill-side handshake listener is a single background thread that lives for the whole process, answering metadata queries from every decode worker that needs to pull KV cache from this engine. Its only exception handling was for the idle-poll timeout; anything raised while actually processing a request (a malformed message, a request for a pp/tp rank this engine doesn't serve, or a peer that simply doesn't speak the REQ/ROUTER envelope this listener expects) propagated out of the thread's target function and killed it silently, with nothing to restart or report it. Once dead, every subsequent handshake against this engine times out and fails until the process is restarted, no matter which decode worker is asking. Move the frame-count unpacking and message handling into a guarded block, narrowed to the concrete failure modes a bad request can trigger: msgspec.DecodeError (malformed msgpack), ValueError/TypeError (wrong message shape, e.g. a non-REQ client that doesn't send the expected 3-part envelope), KeyError (unknown rank), and zmq.ZMQError (reply-side socket errors). A single bad request is now logged (with the raw bytes it sent, safely escaped and capped) and dropped, instead of taking the side channel down for every decode worker talking to this engine. Anything outside that list still crashes the thread loudly, since that indicates a genuine bug rather than a bad peer. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Jesse Donkervliet <jesse@ubicloud.com>
A decode worker's handshake request times out identically whether the remote's listener never started, is too busy to respond, or has died: ZMQ hides the underlying connection state, so a bare "Resource temporarily unavailable" is all that ever reaches the caller. Wrap the timeout with the remote address/rank and a pointer to the existing side-channel probe script, so the failure can be triaged directly instead of guessing from logs that show nothing useful. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Jesse Donkervliet <jesse@ubicloud.com>
jdonkervliet
force-pushed
the
jesse/nixl-handshake-hardening
branch
from
September 1, 2026 11:05
c0ef774 to
eccd4ca
Compare
Owner
Author
|
Opened upstream as vllm-project#54757. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
The prefill-side NIXL handshake listener (
_nixl_handshake_listenerinbase_scheduler.py) is a single background thread, per engine, that lives for the whole process and answers metadata queries from every decode worker that needs to pull KV cache from it. Its only exception handling covered the idle-poll timeout; any exception raised while actually processing a received message killed the thread silently, with nothing to restart or report it.This fails whenever anything reaches the side-channel port that isn't a well-formed request from a real
REQclient: a malformed message, a request for a pp/tp rank this engine doesn't serve, or a peer that doesn't speak theREQ/ROUTERenvelope (aREQsocket always auto-adds an empty delimiter frame; anything that doesn't, such as a bareDEALERsocket, produces a different frame count on theROUTERside). Once the thread dies, every subsequent handshake against that engine times out and fails for every decode worker until the process is restarted: a single bad request causing a lasting, silent, engine-wide DoS.This isn't tied to an existing filed issue. It was found via production investigation, reproduced locally, and checked against
vllm-project/vllm(issues, PRs, currentmain) for existing work on this listener's exception handling; nothing addresses it. The closest related work, vllm-project#48633 and vllm-project#49238 / vllm-project#50047, covers different NIXL reliability gaps (push-mode handshake latency, stale peer state after a restart), not this one.Changes:
base_scheduler.py: move the frame unpacking and message handling into a guarded block, narrowed to the concrete failure modes a bad request can trigger (msgspec.DecodeError,ValueError/TypeError,KeyError,zmq.ZMQError). A bad request is now logged (with the raw bytes that triggered it, safely escaped and capped) and dropped, instead of taking the whole side channel down. Anything outside that list still crashes the thread loudly, since that's a genuine bug rather than a bad peer.base_worker.py: wrap the decode-side handshake timeout (zmq.error.Again) with the remote address/rank and a pointer to the existing side-channel probe script (tests/v1/kv_connector/nixl_integration/nixl_side_channel_probe.py), since ZMQ hides the underlying connection state and the bare timeout gives no way to triage which of several possible causes it was.tests/v1/kv_connector/unit/test_nixl_connector.py: two new regression tests (test_handshake_listener_survives_bad_envelope,test_handshake_listener_survives_malformed_content) that run the real listener against a live socket and a stray non-REQclient. Verified both fail against the unpatched listener (thread dies) and pass against the fix.AI assistance was used to investigate, reproduce, and implement this fix.
Test Plan
ruff check/ruff format --check/pyrighton changed files.python -m pytest tests/v1/kv_connector/unit/test_nixl_connector.py -v(full file).python -m pytest tests/v1/kv_connector/unit/test_bidirectional_kv_transfer.py -v(full file; calls_nixl_handshakeand_nixl_handshake_listenerdirectly)._nixl_handshake_listeneragainst a realROUTERsocket, connected with aDEALERclient (noREQenvelope) to reproduce the crash, then reran the same scenario against the patched listener to confirm it survives and keeps serving; now captured permanently as the two new regression tests above.Test Result
ruff/pyright: clean (0 errors).ValueError/msgspec.DecodeError, thread dies) and pass against the fix.Essential Elements of an Effective PR Description Checklist