Skip to content

fix(ws): drop dead connections, bound message floods and pending sketch input - #167

Merged
ttbombadil merged 1 commit into
mainfrom
fix/ws-connection-lifecycle
Oct 3, 2026
Merged

ttbombadil merged 1 commit into
mainfrom
fix/ws-connection-lifecycle

Conversation

@ttbombadil

Copy link
Copy Markdown
Collaborator

Purpose

R4b of the refactoring series (docs/UNOSIM_REFACTORING_OPL.md), audit finding A7 (WebSocket part).

Findings re-verified

  • No heartbeat (confirmed): a half-open connection (closed laptop, dropped Wi-Fi) kept its runner and admission reservation until the simulation timeout (default 60 s, up to 300 s). The student's reconnect was rejected with SIMULATION_ALREADY_ACTIVE meanwhile.
  • No inbound limit (confirmed): only start_simulation was rate-limited; each serial_input / set_pin_value became a stdin write. If the sketch never reads Serial, Node buffered every write without bound.
  • "No per-connection serialization" (falsified as a defect): concurrency is required so that stop_simulation / code_changed can abort a start still waiting for a runner (abortQueuedAcquire). Double starts are blocked by the reservation, and the synchronous handlers keep their order. Serializing would break stop-while-queued, so it is not changed. Recorded as FALSIFIED.

Change

  • Protocol-level ping every WS_HEARTBEAT_INTERVAL_MS (default 30 s). A connection that has not answered the previous ping is terminated. The existing close handler releases runner and reservation. Browsers answer pings automatically; the interval is unref'd and cleared on wss.close.
  • Token bucket per connection (500 messages/s, burst 1000). Excess messages are dropped, with one warning per connection. The analog slider sends far below this.
  • ProcessController.writeStdin refuses input once 1 MiB is pending in the child's stdin.

Tests

  • RED → GREEN: tests/server/routes/simulation-connection-lifecycle.test.ts:
    • a client with autoPong: false is terminated and its simulation freed (runner stopped and released, admission 0);
    • a responsive client stays open;
    • a flood of 200 messages is cut to the bucket while the connection stays open.
  • RED → GREEN: tests/server/services/process-controller-stdin-bound.test.ts (child that never reads stdin: writes are refused; the pending backlog stays bounded).
  • npm run check, ESLint, unit 2721 passed, Docker integration 27/27 locally, pre-push incl. Sonar quality gate PASSED.

🤖 Generated with Claude Code

…ch input

A half-open WebSocket kept its runner and admission until the simulation
timeout, and inbound messages were unbounded. The server now pings every 30 s
and terminates connections that miss a pong, drops messages beyond a
per-connection token bucket, and stops buffering stdin past 1 MiB.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ttbombadil
ttbombadil merged commit df62bf8 into main Oct 3, 2026
5 checks passed
@ttbombadil
ttbombadil deleted the fix/ws-connection-lifecycle branch October 3, 2026 22:21
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.

1 participant