akiomik / akiomik/tears

Graceful WebSocket shutdown: send Close frame on cancellation (opt-in)

Aperta
#103 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub
documentation enhancement
Lingua principale
Rust
Stelle
9
Fork
1
Merge medio
11h 46m
PR unite (30g)
62

Descrizione

## Summary

On abort, a WebSocket subscription does **not** send a WebSocket-level Close frame — only the TCP connection is closed (by the OS on `TcpStream` drop). This is a **limitation, not a bug**: the server still detects the disconnect via read error / EOF, and there is no resource leak. RFC 6455 treats a bare TCP close (no Close-frame handshake) as an *abnormal* closure, so a graceful shutdown that emits `0x88` would be more correct, but the current behavior is acceptable.

This issue tracks offering an **opt-in graceful shutdown** path. See the `FIXME` in `src/subscription/websocket.rs` (`WsStreamState::Running`).

## Why it happens

`SubscriptionManager` stops subscriptions with `JoinHandle::abort()` (in both `update()` and `shutdown()`). Abort drops the `stream::unfold` future at an `.await` point, and Rust has no async `Drop`, so the WebSocket stream cannot run `write.close().await` during teardown.

## Cheapest mitigation (available today, no framework change)

The application already receives an `UnboundedSender` via `WebSocketMessage::Connected`, and `WebSocketCommand::Close` already performs `write.send(Message::Close(..)).await` + `write.close().await`. So an app can send `Close` before quitting to get a clean close.

Gap: `run()` aborts subscriptions immediately after the loop breaks, so a `Close` sent in the same turn may still be queued when the task is aborted. Closing this cleanly would need either the app to await `Disconnected` before quitting, or a short grace period before the final abort.

**Recommended first step:** document the "close on quit" pattern and consider a short pre-abort grace period. This is non-breaking and may satisfy most needs.

## Framework-level design (only if a guarantee without app cooperation is needed)

Cooperative cancellation + grace period + hard-abort fallback:

1. Add a `SubscriptionContext` carrying a `CancellationToken` (with a public accessor, since `SubscriptionSource` is a public trait) and a default `stream_with_context` method (source-compatible for normal implementors).
2. Store the token per running subscription in `RunningSubscription`.
3. Have `WebSocket` observe cancellation in its `select!` and send a Close frame.
4. Add `SubscriptionManager::shutdown_gracefully(grace).await` — cancel tokens, await each handle with `timeout(grace, &mut handle)` (keep ownership so timed-out handles can still be aborted), then `abort()` on timeout.
5. Keep the existing hard `shutdown()` as immediate-abort fallback.
6. Await graceful shutdown from the already-async `run()` exit path (non-breaking; there is no public `Runtime::shutdown()`). The only genuinely breaking piece would be async-ifying the public `SubscriptionManager::shutdown()` in place — so add a new method instead.

Handling removed subscriptions in `update()` gracefully is a separate, not-yet-adopted step; any cleanup tasks must be **tracked inside `SubscriptionManager`**, not detached, to avoid orphaned background tasks (guarded by `tests/websocket_leak.rs`).

## Scope / decision

- Not a bug; low priority. Reasonable to defer.
- Prefer the app-driven `Close` pattern (+ optional pre-abort grace) first.
- Adopt the full cancellation-token design only if a framework-enforced clean close (without app cooperation, or on subscription replacement) is genuinely required — ideally alongside a future major that can add the graceful API.

Guida per i contributori

Apri la guida per i contributori

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.