Release Bun.serve handler Strongs once the server is idle (native↔JS cycle leak) - #32086
Closed
alii wants to merge 14 commits into
Closed
Release Bun.serve handler Strongs once the server is idle (native↔JS cycle leak)#32086alii wants to merge 14 commits into
alii wants to merge 14 commits into
Claude / Claude Code Review
completed
Jun 12, 2026 in 30m 23s
Code review found 1 important issue
Found 5 candidates, confirmed 2. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 1 |
| 🟡 Nit | 1 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | src/runtime/server/ServerWebSocket.rs:656-662 |
Server-side ws.close()/ws.terminate() never decrement active_websocket_count |
| 🟡 Nit | src/runtime/server/mod.rs:3530-3537 |
TERMINATED gate in on_websocket_closed defeats idle release when stop(true) is called from inside a close handler |
Annotations
Check failure on line 662 in src/runtime/server/ServerWebSocket.rs
claude / Claude Code Review
Server-side ws.close()/ws.terminate() never decrement active_websocket_count
Server-side `ws.close()` / `ws.terminate()` set `flags.closed = true` *before* calling `websocket().end()`/`.close()`, which synchronously re-enters `on_close`; the re-entrant `on_close` reads `was_closed = true` and this scopeguard skips `server.on_websocket_closed()`, so `active_websocket_count` is never decremented. The PR's new comment in `on_open`'s error path (ServerWebSocket.rs:438-439) documents this exact mechanism and adds a compensating `on_websocket_closed()` there, but `close()`/`te
Check warning on line 3537 in src/runtime/server/mod.rs
claude / Claude Code Review
TERMINATED gate in on_websocket_closed defeats idle release when stop(true) is called from inside a close handler
Edge case: when `server.stop(true)` is called *from inside* a websocket `close` handler, the idle release never fires. The outer `on_close`'s scopeguard hasn't decremented yet, so `stop()`'s own `deinit_if_we_can()` sees `active_websocket_count==1` and skips the idle block; when the scopeguard then drops the count to 0, `TERMINATED` is set and `on_websocket_closed` skips `deinit_if_we_can()` here too — leaving the handler protections (and the cycle this PR breaks) in place for that path. Not a r
Loading