Skip to content

Close editor sockets before draining server shutdown - #14

Merged
DebelToni merged 1 commit into
mainfrom
fix/websocket-graceful-shutdown
Oct 19, 2026
Merged

DebelToni merged 1 commit into
mainfrom
fix/websocket-graceful-shutdown

Conversation

@Slavov88

Copy link
Copy Markdown
Collaborator

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 normal 1001 server-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.

@DebelToni DebelToni left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@DebelToni
DebelToni merged commit 8c9f8a5 into main Oct 19, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants