Repository navigation
Close editor sockets before draining server shutdown - #14
Conversation
DebelToni
left a comment
There was a problem hiding this comment.
Reviewed the entire two-file diff and surrounding Fastify/gateway/collaboration/compiler shutdown lifecycle. Moving gateway.close() into preClose resolves the HTTP-drain dependency while keeping compiler drain, collaboration persistence, checkpoint and database close ordered in onClose. The existing gateway tracks accepted sockets and sends 1001; its WebSocketServer rejects subsequent upgrades after closing starts, and collaboration remains alive until accepted sockets have drained. Four listening-server regressions cover zero/one/three event sockets, broadcasts/1001 closure and an unpersisted Yjs edit checked by reopening SQLite. All four passed in GitHub CI, along with the full suite and the separate real-Docker compiler regression. Required test/analyze and CodeQL succeeded for this exact head. No dependency, permission, authentication, host automation or policy changes. No blocking findings; no contributor code/tests were executed on DGX. Exact head/base/main, unchanged protections/settings and review threads verified before squash merge.
Open editor and event WebSockets can block graceful shutdown because the gateway closes them in
onClose, which Fastify reaches only after the HTTP server has drained those same sockets. Deployments can consequently wait for systemd's 90-second stop timeout.Close the WebSocket gateway in
preClose. Keep compiler drain, pending collaborative edit persistence, checkpointing, and database closure in their existing finalization order. Clients receive the normal1001server-stopping close; the existing WebSocket handshake timeout remains in place.Validation: four live-server regressions cover zero, one, and three event sockets, broadcasts around closure, and a pending collaborative edit verified by reopening SQLite after shutdown. Against the old code, all three active-socket cases timed out; the no-socket control passed. After the fix, 67 server tests pass (the Docker test is left to required CI), along with server lint, typecheck, build, and diff checks. A separate built-server check closed three active sockets in 10 ms locally. No authentication, permissions, dependencies, or deployment configuration changes.