Client disconnect does not cancel generation; GPU keeps running for abandoned requests #9

New Issue

hugo · 2026-07-25T09:39:28Z

hugo commented

2026-07-25 09:39:28 +00:00

Symptom

When an HTTP client disconnects mid-request, generation keeps running to completion on the GPU. Because the runtime is a single worker thread (src/runtime.rs), every queued request behind the abandoned one is blocked for the full duration of a generation that nobody will read.

Evidence

ActiveGeneration carries the cancel flag:

src/runtime.rs:17-20 — ActiveGeneration { events, cancel: Arc<AtomicBool> }

Only the chat streaming path sets it on write failure:

src/server/response.rs:755, :762, :772 — active.cancel.store(true, Ordering::Relaxed) after a failed send_chat_projection_events / send_sse
src/server/response.rs:912 — keepalive write failure during prefill

Every other exit path drops ActiveGeneration with the flag still false, leaving the worker generating:

src/server/response.rs:155 (completion_stream_response) — send_sse(...).map_err(|error| (500, error))? returns early, no cancel. Same pattern in the Anthropic and Responses stream functions.
src/server/response.rs:743 and :751 — send_sse_headers(...)? and send_stream_start(...)? inside the chat path also use ? and skip the cancel that the surrounding code is careful to do.
src/server/response.rs:925 (wait_for_output) — the non-streaming path never writes to the socket until the generation is finished, so a disconnect is not detected at all.

When the receiver is dropped, runtime.rs's let _ = command.events.send(...) swallows the SendError, so nothing upstream notices either.

How ds4 handles this

../ds4/ds4_server.c treats a failed client write as a decode-loop terminator, for every protocol, not just one:

ds4_server.c:11371 — the decode loop, and inside it four sibling branches (OpenAI SSE, Anthropic, OpenAI live, Responses) that each do snprintf(err, sizeof(err), "client stream write failed"); break; when their respective *_sse_stream_update() returns false.
ds4_server.c:9969 — a sticky bool stream_failed on the prefill progress struct, set at :10435 / :10442 when the SSE headers or the : prefill keepalive fail to send.
ds4_server.c:11281 — before decoding starts, if (progress.stream_failed) { ...log "stream closed during prefill"...; return; }. The job is abandoned before a single token is generated.
ds4_server.c:4473 send_all() makes detection fast: a 2 s stall deadline (DS4_SERVER_SEND_STALL_TIMEOUT_MS) with poll(POLLOUT), returning false on POLLERR|POLLHUP|POLLNVAL.

So the reference implementation's invariant is "a write failure anywhere ends the job". DS4Server implements that invariant in one of four protocol paths.

Suggested fix

Rather than adding cancel.store to a dozen more early-return sites, make it structural — one impl in src/runtime.rs covers every current and future exit path:

impl Drop for ActiveGeneration {
    fn drop(&mut self) {
        self.cancel.store(true, Ordering::Relaxed);
    }
}

This is harmless on the success path (generation has already finished when the handle drops). The six explicit cancel.store calls in src/server/response.rs and the one in src/app/generation.rs:220 then become redundant and can be removed — except keep the one at response.rs:759, which cancels on seeing a tool-call terminator, not on failure, and is unrelated.

Consider also matching ds4's stream_failed behaviour for the non-streaming path: wait_for_output cannot notice a disconnect, so a TcpStream poll for POLLHUP between events, or simply accepting the current behaviour with a comment, is a judgement call worth making explicitly.

Acceptance criteria

Killing a client mid-generation (curl ... & sleep 2; kill %1) stops GPU work within a token or two, for all four protocols: /v1/chat/completions, /v1/completions, /v1/messages, /v1/responses, both stream: true and stream: false where detectable.
A second request submitted right after the kill starts promptly instead of waiting out the abandoned generation.
Test: a unit test that drops an ActiveGeneration and asserts the cancel flag flipped.

## Symptom When an HTTP client disconnects mid-request, generation keeps running to completion on the GPU. Because the runtime is a single worker thread (`src/runtime.rs`), every queued request behind the abandoned one is blocked for the full duration of a generation that nobody will read. ## Evidence `ActiveGeneration` carries the cancel flag: - `src/runtime.rs:17-20` — `ActiveGeneration { events, cancel: Arc<AtomicBool> }` Only the **chat** streaming path sets it on write failure: - `src/server/response.rs:755`, `:762`, `:772` — `active.cancel.store(true, Ordering::Relaxed)` after a failed `send_chat_projection_events` / `send_sse` - `src/server/response.rs:912` — keepalive write failure during prefill Every other exit path drops `ActiveGeneration` with the flag still `false`, leaving the worker generating: - `src/server/response.rs:155` (`completion_stream_response`) — `send_sse(...).map_err(|error| (500, error))?` returns early, no cancel. Same pattern in the Anthropic and Responses stream functions. - `src/server/response.rs:743` and `:751` — `send_sse_headers(...)?` and `send_stream_start(...)?` inside the *chat* path also use `?` and skip the cancel that the surrounding code is careful to do. - `src/server/response.rs:925` (`wait_for_output`) — the non-streaming path never writes to the socket until the generation is finished, so a disconnect is not detected at all. When the receiver is dropped, `runtime.rs`'s `let _ = command.events.send(...)` swallows the `SendError`, so nothing upstream notices either. ## How ds4 handles this `../ds4/ds4_server.c` treats a failed client write as a decode-loop terminator, for **every** protocol, not just one: - `ds4_server.c:11371` — the decode loop, and inside it four sibling branches (OpenAI SSE, Anthropic, OpenAI live, Responses) that each do `snprintf(err, sizeof(err), "client stream write failed"); break;` when their respective `*_sse_stream_update()` returns false. - `ds4_server.c:9969` — a sticky `bool stream_failed` on the prefill progress struct, set at `:10435` / `:10442` when the SSE headers or the `: prefill` keepalive fail to send. - `ds4_server.c:11281` — before decoding starts, `if (progress.stream_failed) { ...log "stream closed during prefill"...; return; }`. The job is abandoned before a single token is generated. - `ds4_server.c:4473` `send_all()` makes detection fast: a 2 s stall deadline (`DS4_SERVER_SEND_STALL_TIMEOUT_MS`) with `poll(POLLOUT)`, returning false on `POLLERR|POLLHUP|POLLNVAL`. So the reference implementation's invariant is "a write failure anywhere ends the job". DS4Server implements that invariant in one of four protocol paths. ## Suggested fix Rather than adding `cancel.store` to a dozen more early-return sites, make it structural — one impl in `src/runtime.rs` covers every current and future exit path: ```rust impl Drop for ActiveGeneration { fn drop(&mut self) { self.cancel.store(true, Ordering::Relaxed); } } ``` This is harmless on the success path (generation has already finished when the handle drops). The six explicit `cancel.store` calls in `src/server/response.rs` and the one in `src/app/generation.rs:220` then become redundant and can be removed — except keep the one at `response.rs:759`, which cancels on *seeing a tool-call terminator*, not on failure, and is unrelated. Consider also matching ds4's `stream_failed` behaviour for the non-streaming path: `wait_for_output` cannot notice a disconnect, so a `TcpStream` poll for `POLLHUP` between events, or simply accepting the current behaviour with a comment, is a judgement call worth making explicitly. ## Acceptance criteria - Killing a client mid-generation (`curl ... & sleep 2; kill %1`) stops GPU work within a token or two, for all four protocols: `/v1/chat/completions`, `/v1/completions`, `/v1/messages`, `/v1/responses`, both `stream: true` and `stream: false` where detectable. - A second request submitted right after the kill starts promptly instead of waiting out the abandoned generation. - Test: a unit test that drops an `ActiveGeneration` and asserts the cancel flag flipped.

hugo added the bug label 2026-07-25 09:39:28 +00:00

hugo commented

2026-07-25 10:49:15 +00:00

Fixed in 218c947. ActiveGeneration now cancels its worker when the handle is dropped, covering every early-exit path. Added a regression test for the drop behavior. Verified with cargo fmt, Clippy (-D warnings), make bundle, and cargo test --all-features.

Fixed in 218c947. ActiveGeneration now cancels its worker when the handle is dropped, covering every early-exit path. Added a regression test for the drop behavior. Verified with cargo fmt, Clippy (-D warnings), make bundle, and cargo test --all-features.

hugo closed this issue

2026-07-25 10:49:19 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: hugo/DS4Server#9