deno
133cef32 - fix(ext/http): close active WebSockets on Deno.serve shutdown (#35105)

Commit
53 days ago
fix(ext/http): close active WebSockets on Deno.serve shutdown (#35105) ## Summary Fixes [denoland/deno#22387](https://github.com/denoland/deno/issues/22387). `server.shutdown()` (and an abort-signal forced shutdown) used to leave any upgraded `serverWebSocket` resource alive: after the listener stopped, the upgraded streams were no longer reachable from the hyper connection, so nothing tore them down. The minimal repro from the issue: ```js const port = 3006; Deno.test('httpServerWebSocketShutdown', async () => { const wsOpen = Promise.withResolvers(); const server = Deno.serve({ port }, req => { const { socket, response } = Deno.upgradeWebSocket(req); socket.onopen = wsOpen.resolve; return response; }); const ws = new WebSocket(\`ws://localhost:\${port}\`); await wsOpen.promise; await server.shutdown(); }); ``` Reported: ``` error: Leaking resources: serverWebSocket ``` ## Approach * `HttpServerState` gains a per-server `ActiveWebSockets` registry of `Weak<ServerWebSocket>`. The websocket-upgrade op registers every new server-side `ServerWebSocket` there. * Each registered `ServerWebSocket` carries a `WebSocketGuard` that holds a `SignallingRc<HttpServerState>` clone (so the graceful `poll_complete` waits for the websocket to drain) and removes itself from the registry on `Drop`. * `ServerWebSocket` gains an embedder-facing `server_shutdown()` helper. It is idempotent: it sends a `Close(1001 Going Away)` frame in the background and then cancels the pending `op_ws_next_event` read via a per-resource `CancelHandle`. * `op_ws_next_event` now wraps `read_frame` with `or_cancel(read_cancel)`; on cancel it returns `ClosedDefault` so the JS event loop dispatches `close` and calls `core.tryClose(rid)`, releasing the guard. * `op_http_close` iterates the registry on both graceful and forceful shutdown and calls `server_shutdown()` on each entry. Graceful shutdown then waits for `server_state.poll_complete`. Client and Node-side websockets are not affected — only `ServerWebSocket` resources created via `op_http_upgrade_websocket_next` opt in to the registry. ## Test plan - [x] `cargo check -p deno_websocket`, `cargo check -p deno_http`, `cargo clippy -p deno_http -p deno_websocket --no-deps` - [x] `dprint check` clean - [ ] CI green - [ ] New regression test `httpServerWebSocketShutdownNoLeak` in `tests/unit/serve_test.ts` passes locally (blocked here by a stale prebuilt `librusty_v8.a` missing `v8__Isolate__SetIdle`; relying on CI) Refs #22387 Also fixes #25792 (same root cause: upgraded websockets are not reachable from the hyper connection cancel handle, so an abort-signal shutdown leaves them open and `server.finished` never resolves). Supersedes #35142, which fixed only the abort-signal path. Closes #25792 Closes denoland/divybot#546 --------- Co-authored-by: divybot <divybot@users.noreply.github.com> Co-authored-by: Divy Srivastava <me@littledivy.com>
Author
Parents
Loading