Skip to content

[Bugfix] Replace assertion with guard in finished_sending KV transfer - #5

Open
Jackie2049 wants to merge 1 commit into
mainfrom
fix/finished-sending-race-condition
Open

[Bugfix] Replace assertion with guard in finished_sending KV transfer#5
Jackie2049 wants to merge 1 commit into
mainfrom
fix/finished-sending-race-condition

Conversation

@Jackie2049

@Jackie2049 Jackie2049 commented May 21, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes #1

In disaggregated prefill/decode (PD separation) deployments, a race condition exists in the scheduler's handling of finished_sending from the KV connector output.

Problem

At vllm/v1/core/sched/scheduler.py:2153:

for req_id in kv_connector_output.finished_sending or ():
    assert req_id in self.requests  # ← hard crash on race condition
    self._free_blocks(self.requests[req_id])

When a request is aborted (user cancellation, timeout, preemption, or KV load failure) while a KV transfer is still in flight, the request is removed from self.requests. When the KV connector later reports finished_sending for that request, the assertion fires and crashes the entire serving process.

Timeline of the race

1. Request A starts KV transfer to remote instance
2. Request A is aborted (e.g., client disconnect, timeout)
3. Scheduler removes A from self.requests, frees A's blocks
4. KV connector completes send, reports finished_sending = ["A"]
5. assert req_id in self.requests → AssertionError → crash

Fix

Replace the assertion with a defensive guard:

if req_id not in self.requests:
    logger.warning(
        "Request %s not found when finishing KV send; "
        "may have been aborted during transfer.", req_id)
    continue

This is safe because:

  • Aborted requests already have their blocks freed during the abort path (_free_request)
  • Skipping the duplicate free is the correct behavior
  • The continue ensures remaining requests in the loop are still processed

Testing

  • Verified the fix compiles without errors
  • Unit test for the race condition can be added if maintainers request

In disaggregated prefill/decode deployments, a race condition exists
where a request may be aborted (user cancellation, timeout, preemption)
before the KV connector reports finished_sending. The previous code used
`assert req_id in self.requests` which causes a hard crash instead of
gracefully handling the race.

Replace the assertion with a defensive guard check that logs a warning
and skips the request. This is safe because aborted requests already
have their blocks freed during the abort path.

Fixes #1

Co-Authored-By: Claude
@Jackie2049
Jackie2049 force-pushed the fix/finished-sending-race-condition branch from 48f7882 to 449b6f5 Compare June 6, 2026 17:18
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.

[Bug] Race condition in scheduler finished_sending KV transfer causes production crash

1 participant