Add optional server-side player keepalive to reap dead connections#912
Merged
mcottontensor merged 1 commit intoJun 19, 2026
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Relevant components:
Problem statement:
The signalling server never actively checks that a connected player is still alive.
KeepaliveMonitoris only wired up on the client (browser to server); the server just replies to pings, it doesn't send its own. A player is only removed from the registry when its WebSocket firesclose.That's fine when a player disconnects cleanly, but if the socket dies without a close frame — laptop going to sleep, Wi-Fi dropping, a tunnel resetting, the tab being killed — the server never hears about it. The dead player stays in the registry and stays subscribed to its streamer until the OS TCP keepalive eventually reaps the socket, which on Windows defaults to around two hours (and
wsdoesn't enable it).With the default
--max_players 0(unlimited) this is mostly harmless, which is probably why it hasn't come up. But as soon as you cap subscribers —--max_players 1for a single-viewer experience — that ghost holds the only slot, and every subsequent viewer is turned away with "Max players reached" until the server is restarted.Solution
Add the missing server-to-browser half of the keepalive. When a player connects, the server can now start a
KeepaliveMonitoron that connection (the same class already used client-side); the browser answers these pings automatically viahandlePingMessage, so no frontend change is needed. If a player misses the keepalive, the server callsws.terminate()— a forceful close rather than a graceful one, because a dead peer never completes the close handshake — which firescloseimmediately and frees the slot.It's controlled by a new
IServerConfig.playerKeepaliveTimeout(milliseconds,0disables it). The signalling web server exposes it as--player_keepalive_timeout. I've defaulted it to 30000 (30s): reaping a genuinely dead connection is a correctness improvement that I think is worth having on by default, but it's fully tunable and can be turned off. A live browser always responds to the ping, so a real viewer is never evicted; only a connection that has gone completely silent is. The monitor stops itself on transportclose, so there's no extra teardown.Documentation
SignallingWebServer/README.mdis updated with the new option, and the config field and the logic both have explanatory comments.Test Plan and Compatibility
npm run buildandnpm run lintpass forCommon,SignallingandSignallingWebServer. I tested by connecting a viewer and then dropping it ungracefully (killing the browser process / sleeping the machine / pulling Wi-Fi) — the server logs the keepalive failure and frees the slot within roughly one to two timeout intervals, and a new viewer can connect. The clean paths (normal disconnect, and a long idle-but-alive session) behave exactly as before and never evict a live viewer. Setting--player_keepalive_timeout 0restores the previous behaviour exactly.