[Bugfix][NIXL] Fix multiple handles xfer race by NickLucche · Pull Request #56104 · vllm-project/vllm

NickLucche · 2026-09-09T15:35:47Z

A request can own multiple in-flight NIXL transfer handles (one per remote TP rank under heterogeneous TP).
On main, the first handle to reach ERR immediately invalidates the request's blocks and reports the failure to the scheduler, while the sibling handles may still be posting RDMA writes into those same blocks.
Posted transfers cannot be aborted, so once the scheduler reassigns the blocks, the in-flight writes corrupt whichever request owns them next.

This PR fixes this race by deferring NIXL receive-failure reporting and block invalidation until all transfers for the request have stopped, preventing in-flight RDMA writes from corrupting reused KV blocks.

Some cascading changes to keep purpose of data structures clear:

background threads use the existing _failed_recv_reqs Q to report failures
main engine thread failures are reported on _recv_failures set to minimize overhead
_failed_recv_reqs Q failures are addressed in get_finished; _pop_done_transfers also returns current poll failures, so we can track them and check whether all handles have been processed (or we delay, bulk of this PR)

claude

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

NickLucche · 2026-09-09T15:36:49Z

/ci run

github-actions · 2026-09-09T15:37:10Z

✅ Triggered Buildkite CI #87944 for commit 231c1deba790.

njhill · 2026-09-09T21:21:58Z

Thanks @NickLucche! I think this change may introduce some problems on the push side though, will add more detail soon.

mergify · 2026-09-10T11:32:08Z

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @NickLucche.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

njhill

Thanks @NickLucche.

Also here are tests you could include to cover the push mode changes fixed by the inline suggestion: njhill@4143de0

njhill · 2026-09-09T20:37:01Z

+        self,
+        req_id: str,
+        handle: int | None,
+        failed_req_ids: set[str] | None = None,


nit:

Suggested change

self,

req_id: str,

handle: int | None,

failed_req_ids: set[str] | None = None,

self, req_id: str, handle: int | None, failed_req_ids: set[str] | None = None

njhill · 2026-09-10T22:27:06Z

+            if not self._handle_failed_transfer(
+                request_id, handle, self._recv_failures
+            ):
+                assert handle is not None


nit: redundant assert

njhill · 2026-09-10T22:39:38Z

        # writer thread also appends to it, so guard the pop.
        with self._sending_transfers_lock:
-            done_pushing = self._pop_done_transfers(self._sending_transfers)
+            done_pushing, _ = self._pop_done_transfers(self._sending_transfers)


Suggested change

done_pushing, _ = self._pop_done_transfers(self._sending_transfers)

done_pushing, failed_pushing = self._pop_done_transfers(

self._sending_transfers

)

# A failed send must never be reported as done: its blocks

# are freed via the lease / watchdog instead.

done_pushing = {

req_id

for req_id in done_pushing - failed_pushing

if req_id in self._recving_metadata

}

Otherwise failed sends could be reported as succesful (done_sending) and successful sends could complete early at write handle time bypassing the consumer notif accounting.

NickLucche · 2026-09-11T12:46:47Z

/ci run

github-actions · 2026-09-11T12:47:04Z

✅ Triggered Buildkite CI #88324 for commit 551a9dbbb59f.

NickLucche · 2026-09-11T13:52:46Z

/ci run

github-actions · 2026-09-11T13:53:05Z

✅ Triggered Buildkite CI #88336 for commit 47f5a96fb6cd.

NickLucche · 2026-09-11T14:02:37Z

/ci run

NickLucche · 2026-09-11T14:03:00Z

Thanks for reviewing @njhill , added your push fix

njhill

Thanks @NickLucche

njhill · 2026-09-11T16:56:16Z

/ci run

github-actions · 2026-09-11T16:56:34Z

✅ Triggered Buildkite CI #88377 for commit c551acbea2d5.

mergify · 2026-09-12T06:42:16Z

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @NickLucche.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

njhill · 2026-09-12T19:47:41Z

+            if handle is not None and not self._handle_failed_transfer(
+                request_id, handle, self._recv_failures
+            ):


Suggested change

if handle is not None and not self._handle_failed_transfer(

request_id, handle, self._recv_failures

):

if not self._handle_failed_transfer(

request_id, handle, self._recv_failures

):

assert handle is not None

@NickLucche my bad I think this was my incorrect prior suggestion. This should fix the test ^^

I would have pushed this but don't have permission for your branch.

@njhill sorry about that, I'll look into branch permissions

Signed-off-by: NickLucche <nicolo.lucchesi@mistral.ai>

In get_finished, filter popped write-transfer req_ids against failed_pushing and _recving_metadata: a failed send must never be reported as done (its blocks are freed via the lease / watchdog), and a successful send still waits for the consumer notification instead of completing at write-handle time. Also apply review nits (collapse _handle_failed_transfer signature, drop the redundant assert in pull_worker by folding the handle check into the condition) and adapt two tests from njhill's review branch covering the failed-send and consumer-notif cases. Generated by Mistral Vibe. Co-Authored-By: Mistral Vibe <vibe@mistral.ai> Signed-off-by: NickLucche <nicolo.lucchesi@mistral.ai> Co-authored-by: Nick Hill <nickhill123@gmail.com>

Always record setup failures, including when make_prepped_xfer raises before returning a handle. Retain a handle only when releasing it fails. Co-authored-by: OpenAI Codex <noreply@openai.com> Signed-off-by: NickLucche <nicolo.lucchesi@mistral.ai>

Assert delayed receive failures through get_transfer_results, initialize the push stub for direct receive synchronization, and keep the upstream push failure test consistent with the reviewed lease/watchdog behavior. Validation: all 48 push tests and five focused receive lifecycle tests pass; pre-commit passes on all five PR files. The setup-failure test is blocked on macOS because CPU_ATTN rejects its block size of 16. Co-authored-by: OpenAI Codex <noreply@openai.com> Signed-off-by: NickLucche <nicolo.lucchesi@mistral.ai>

NickLucche · 2026-09-14T11:09:00Z

/ci run

github-actions · 2026-09-14T11:09:19Z

✅ Triggered Buildkite CI #88801 for commit 7ce48f91b922.

njhill · 2026-09-14T15:06:19Z

/ci run

github-actions · 2026-09-14T15:06:54Z

✅ CI is already running for this commit: https://buildkite.com/vllm/ci/builds/88801

njhill · 2026-09-14T18:01:36Z

Not sure what is going on with the CI here :(

njhill · 2026-09-15T03:16:04Z

/ci cancel

njhill · 2026-09-15T03:37:55Z

/ci run --allow-stale

github-actions · 2026-09-15T03:38:13Z

✅ Triggered Buildkite CI #89004 for commit 7ce48f91b922.

⚠️ This PR is 52 commits behind upstream main. Running CI at your own risk because --allow-stale was requested; outdated CI configuration may cause failures. Before merging, merge or rebase onto the latest main, then rerun /ci run on the latest PR commit.

Signed-off-by: NickLucche <nicolo.lucchesi@mistral.ai> Co-authored-by: Nick Hill <nickhill123@gmail.com>

NickLucche requested review from ApostaC, ivanium, orozery and xuechendi as code owners September 9, 2026 15:35

claude Bot reviewed Sep 9, 2026

View reviewed changes

NickLucche mentioned this pull request Sep 9, 2026

[Bugfix][NIXL] Defer recv-failure reporting until sibling transfers finish #55900

Closed

mergify Bot added bug Something isn't working kv-connector labels Sep 9, 2026

NickLucche mentioned this pull request Sep 9, 2026

[KVConnector][NIXL] Support attention-HMA layouts in pipeline-parallel push prefill #50494

Merged

zixi-qi mentioned this pull request Sep 10, 2026

[KVConnector][NIXL] Support packed MLA KV layouts in pipeline-parallel push prefill #50499

Merged

mergify Bot added the needs-rebase label Sep 10, 2026

NickLucche mentioned this pull request Sep 10, 2026

[Bugfix][NIXL] Release a dead peer's NIXL state without waiting for TTL #50047

Merged

njhill reviewed Sep 10, 2026

View reviewed changes

NickLucche force-pushed the fix-multiple-handle-xfers branch from 231c1de to 551a9db Compare September 11, 2026 12:46

NickLucche removed the needs-rebase label Sep 11, 2026

mergify Bot added needs-rebase and removed needs-rebase labels Sep 11, 2026

NickLucche force-pushed the fix-multiple-handle-xfers branch from 551a9db to 47f5a96 Compare September 11, 2026 13:52

NickLucche force-pushed the fix-multiple-handle-xfers branch from 47f5a96 to c551acb Compare September 11, 2026 13:56

njhill approved these changes Sep 11, 2026

View reviewed changes

mergify Bot added the needs-rebase label Sep 12, 2026

njhill reviewed Sep 12, 2026

View reviewed changes

NickLucche and others added 5 commits September 14, 2026 12:39

init

6016707

Signed-off-by: NickLucche <nicolo.lucchesi@mistral.ai>

add tests

5b1a5a4

Signed-off-by: NickLucche <nicolo.lucchesi@mistral.ai>

NickLucche force-pushed the fix-multiple-handle-xfers branch from c551acb to 7ce48f9 Compare September 14, 2026 11:08

NickLucche enabled auto-merge (squash) September 14, 2026 12:30

github-actions Bot added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 14, 2026

mergify Bot removed the needs-rebase label Sep 14, 2026

NickLucche merged commit 45779ef into vllm-project:main Sep 15, 2026
53 of 54 checks passed

ItsRoy69 pushed a commit to ItsRoy69/vllm that referenced this pull request Sep 15, 2026

[Bugfix][NIXL] Fix multiple handles xfer race (vllm-project#56104)

cfbcd68

Signed-off-by: NickLucche <nicolo.lucchesi@mistral.ai> Co-authored-by: Nick Hill <nickhill123@gmail.com>

khluu mentioned this pull request Sep 15, 2026

[CI] Fix NIXL push worker test stub after failure deferral #56955

Merged

keneoneth pushed a commit to keneoneth/vllm that referenced this pull request Sep 16, 2026

[Bugfix][NIXL] Fix multiple handles xfer race (vllm-project#56104)

9dbaa77

Signed-off-by: NickLucche <nicolo.lucchesi@mistral.ai> Co-authored-by: Nick Hill <nickhill123@gmail.com>

Dao007forever mentioned this pull request Sep 22, 2026

[Bugfix][NIXL] Restore successful push completion reporting #58188

Merged

This was referenced Oct 3, 2026

[Bug]: NIXL pull: an aborted request whose READ never completes keeps its decode KV blocks until restart #59872

Open

[Bugfix][NIXL] Time out stalled KV loads and release aborted pulls #59874

Open

-            done_pushing, _ = self._pop_done_transfers(self._sending_transfers)
+            done_pushing, failed_pushing = self._pop_done_transfers(
+                self._sending_transfers
+            )
+        # A failed send must never be reported as done: its blocks
+        # are freed via the lease / watchdog instead.
+        done_pushing = {
+            req_id
+            for req_id in done_pushing - failed_pushing
+            if req_id in self._recving_metadata
+        }

Conversation

NickLucche commented Sep 9, 2026

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Claude Code Review

Uh oh!

NickLucche commented Sep 9, 2026

Uh oh!

github-actions Bot commented Sep 9, 2026

Uh oh!

njhill commented Sep 9, 2026

Uh oh!

mergify Bot commented Sep 10, 2026

Uh oh!

njhill left a comment

Choose a reason for hiding this comment

Uh oh!

njhill Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

njhill Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

njhill Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

NickLucche commented Sep 11, 2026

Uh oh!

github-actions Bot commented Sep 11, 2026

Uh oh!

NickLucche commented Sep 11, 2026

Uh oh!

github-actions Bot commented Sep 11, 2026

Uh oh!

NickLucche commented Sep 11, 2026

Uh oh!

NickLucche commented Sep 11, 2026

Uh oh!

njhill left a comment

Choose a reason for hiding this comment

Uh oh!

njhill commented Sep 11, 2026

Uh oh!

github-actions Bot commented Sep 11, 2026

Uh oh!

mergify Bot commented Sep 12, 2026

Uh oh!

njhill Sep 12, 2026

Choose a reason for hiding this comment

Uh oh!

njhill Sep 13, 2026

Choose a reason for hiding this comment

Uh oh!

NickLucche Sep 14, 2026

Choose a reason for hiding this comment

Uh oh!

NickLucche commented Sep 14, 2026

Uh oh!

github-actions Bot commented Sep 14, 2026

Uh oh!

njhill commented Sep 14, 2026

Uh oh!

github-actions Bot commented Sep 14, 2026

Uh oh!

njhill commented Sep 14, 2026

Uh oh!

njhill commented Sep 15, 2026

Uh oh!

njhill commented Sep 15, 2026

Uh oh!

github-actions Bot commented Sep 15, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development