Skip to content

[Frontend] Abort engine request on client disconnect (free leaked KV)#1562

Open
yhl-amd wants to merge 2 commits into
ROCm:mainfrom
yhl-amd:feat/api-abort-on-disconnect
Open

[Frontend] Abort engine request on client disconnect (free leaked KV)#1562
yhl-amd wants to merge 2 commits into
ROCm:mainfrom
yhl-amd:feat/api-abort-on-disconnect

Conversation

@yhl-amd

@yhl-amd yhl-amd commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Non-streaming API handlers were not cancelled when the client hung up (Starlette only cancels StreamingResponse, not plain handlers), so the engine kept generating and the sequence's KV blocks leaked until it hit max_tokens. This adds a client-disconnect abort path.

Changes

  • engine: EngineCoreMgr.abort_request broadcasts an abort_request utility command; _handle_abort_request marks the seq aborted; the scheduler finishes it at the next step via the normal stop path (frees KV, emits a finished RequestOutput). Adds Sequence.aborted.
  • api_server: _run_nonstream_with_disconnect runs generate_async in a task and polls request.is_disconnected(); on disconnect it cancels the task, whose teardown aborts + pops the request. Wired into /v1/chat/completions and /v1/completions (return HTTP 499). generate_async / generate_async_multimodal / generate_async_fanout / cleanup_streaming_request abort + pop on early exit.

Test plan

  • Disconnect a non-streaming client mid-generation → seq finishes at next step, KV freed (no leak until max_tokens).
  • Normal (non-disconnect) requests unaffected.

🤖 Generated with Claude Code

…st leak

The server never reacted to a client hanging up mid-request, and the
non-streaming path never released finished requests. Under retrying
clients (opencode / Claude Code) this piled up "pending requests"
without bound and wasted GPU on output nobody reads.

Two independent bugs, both verified with controlled curl tests:

1. No disconnect cancellation. Neither the streaming generators nor the
   non-stream handlers aborted the engine sequence when the client went
   away, so an abandoned request ran to max_tokens. Add an engine abort
   path (Sequence.aborted -> Scheduler reuses the normal finish/deallocate
   stop path; EngineUtilityHandler.abort_request; CoreManager.abort_request)
   and wire it into the API layer. Streaming generators
   (stream_chat_response / _fanout) now run their body under try/finally so
   cleanup_streaming_request (which aborts the seq) fires on GeneratorExit.
   For non-stream requests Starlette does NOT cancel the handler on
   disconnect, so add `_run_nonstream_with_disconnect()` which races the
   generation against `raw_request.is_disconnected()` and cancels on
   disconnect. `chat_completions` and `completions` gain a `raw_request`
   parameter for this.

2. Non-stream request leak. generate_async / _multimodal / _fanout only
   removed the request from io_processor.requests on a setup exception,
   never on normal completion, so every completed non-stream request
   leaked a Sequence (pending requests grew forever). Wrap their token
   loops in try/finally that always pops the request (and aborts on early
   exit). Streaming already pops via cleanup_streaming_request.

Also add `"choices": []` to the streaming usage chunk so AI-SDK clients
(opencode) stop rejecting the final usage frame.

Verified (DeepSeek-V4, TP8): 6 sequential non-stream completions keep
pending flat (was 1->8); 3 mid-generation disconnects -> abort found,
GPU drops immediately, pending flat (was +3 leak, GPU pinned to
max_tokens); normal streaming + non-stream answers unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: yihonglie <hyi@amd.com>
@github-actions

Copy link
Copy Markdown
Contributor

🏷️ CI Guide

Runs automatically on every eligible PR before approval:

  • ✅ Pre Checkin: Black, Ruff, catalog schema validation, non-GPU unit tests

Heavy model tests:

  • ✅ Run after the PR is approved and Pre Checkin passes
  • ✅ Run immediately when an approval review is submitted
  • ✅ Can be requested before approval with labels
Label Tests
ci:full Run all heavy PR model tests: native ATOM, vLLM, and SGLang
ci:atom Run native ATOM model accuracy tests
ci:vllm Run ATOM vLLM OOT model accuracy tests
ci:sglang Run ATOM SGLang model accuracy tests

Heavy jobs are skipped when the PR is not approved and no matching ci:* label is present.
Add labels via the sidebar or gh pr edit 1562 --add-label <label>

@yhl-amd yhl-amd marked this pull request as ready for review July 12, 2026 14:40
@yhl-amd yhl-amd force-pushed the feat/api-abort-on-disconnect branch from 77348b7 to 40c6c03 Compare July 12, 2026 15:50
…ct-abort

# Conflicts:
#	atom/entrypoints/openai/api_server.py
@yhl-amd yhl-amd force-pushed the feat/api-abort-on-disconnect branch from 40c6c03 to 7a0492f Compare July 12, 2026 16:20
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.

1 participant