Skip to content

fix(net): never let the reader block on delivery — pong-starvation was the hours-idle disconnect (v1.81.6)#45

Merged
SyntaxNyah merged 2 commits into
mainfrom
claude/client-idle-disconnect-9ueqmt
Jul 24, 2026
Merged

fix(net): never let the reader block on delivery — pong-starvation was the hours-idle disconnect (v1.81.6)#45
SyntaxNyah merged 2 commits into
mainfrom
claude/client-idle-disconnect-9ueqmt

Conversation

@SyntaxNyah

Copy link
Copy Markdown
Owner

The bug

Users left minimized in the background for hours got disconnected on perfectly stable networks and servers. v1.81.3–v1.81.5 didn't resolve it.

Root cause

coder/websocket answers the server's WebSocket PING control frames only from inside an in-flight ws.Read() (read.go handleControl). readLoop stopped re-entering ws.Read() the moment its 256-cap incoming channel filled, because it then blocked on the channel send — and that channel is drained only by the render loop, which Windows stalls while the app is minimized behind a fullscreen window (the v1.81.5 finding). At idle-room packet rates the 256 slots take hours to fill — the reported timescale. Once full: no goroutine in ws.Read() → server pings unanswered → the server's ping-timeout (python websockets: 20 s/20 s defaults) drops a healthy client.

v1.81.5 moved only the write-side CH keepalive off the render loop — which kept the server's app-level idle timer satisfied and left WS pong-starvation as the sole kill path.

The fix

Split delivery off the read goroutine so it always returns to ws.Read():

  • readLoop now does only a non-blocking send into a new bounded backlog channel (readBacklogCap = 32768 packets, readBacklogMaxBytes = 32 MiB via atomic accounting) and immediately re-enters ws.Read() — pongs flow for the entire stall, whatever the window state.
  • A new deliverLoop goroutine does the blocking backlog → Incoming hand-off (blocking there is fine — it isn't the goroutine that must keep reading). Single delivery path preserves FIFO order; on restore the backlog drains immediately, and the courtroom's bounded IC queue (drop-oldest + packed-room catch-up) collapses the replay.
  • readBacklogCap is sized for a busy room's full occluded stretch (~0.5 pkt/s → ~18 h; idle rooms → days), since an occluded-but-healthy client is indistinguishable from a wedged one at this layer. ~1.6 MiB preallocated per connection.
  • Overflow of either bound = the drain has been dead for many busy hours with traffic still arriving → deliberate teardown with a distinct errReadBacklogOverflow.
  • Transport drop while stalled: deliberately no Close() from the read-error path — that would abandon parked packets for zero benefit; on resume pumpConnection drains the parked packets and surfaces the close in the same call.
  • Parked background tabs shared Conn.readLoop, so their identical wedge is fixed for free.
  • DialOptions.ReadBacklogCap: test hook, mirroring KeepaliveInterval.

Tests

  • TestReaderKeepsPongingWhileConsumerStalled — server WS pings must keep succeeding while the consumer stalls past the old 256 limit, then all 400 frames arrive in FIFO order. Fails on the pre-fix reader.
  • TestReadBacklogOverflowDisconnects — wedged-client contract + sentinel error.
  • TestIdleOnlyPingsNeverWedge — pure-idle baseline.

Gate: go test -race -count=1 ./internal/protocol/ green (5× repeat, flake-checked); courtroom/assets/network/config/cache/theme suites green. Root cause and fix were additionally adversarially reviewed from concurrency / behavior / efficacy angles with all findings incorporated.

Releasing

Merging this PR does not publish a release by itself — the release pipeline triggers on a v* tag push. After merging, tag the merge commit v1.81.6 and push the tag; the pipeline builds all platforms and publishes with the CHANGELOG's ## v1.81.6 section as the notes.


Generated by Claude Code

claude added 2 commits July 24, 2026 15:53
…s the hours-idle disconnect (v1.81.6)

Root cause of the remaining 'disconnected after hours minimized' reports
(v1.81.3-v1.81.5 didn't resolve them): coder/websocket answers the server's
WS PING control frames only from inside an in-flight ws.Read (read.go
handleControl), and readLoop blocked on the full 256-cap incoming channel
once the OS-stalled render loop stopped draining it. At idle-room packet
rates the queue took hours to fill; the moment it did, the reader stopped
re-entering ws.Read, the server's pings went unanswered, and its
ping-timeout (python websockets: 20s/20s defaults) dropped a perfectly
healthy client. v1.81.5 moved only the WRITE-side CH keepalive off the
render loop — which kept the app-level idle timer satisfied and left
WS pong-starvation as the sole kill path.

Fix: split delivery off the read goroutine so it always returns to ws.Read.

- readLoop now does only a NON-blocking send into a new bounded backlog
  channel (readBacklogCap=32768 packets, readBacklogMaxBytes=32MiB tracked
  via atomic backlogBytes) and immediately re-enters ws.Read — pongs flow
  for the entire stall, however long, whatever the window state.
- New deliverLoop goroutine does the blocking backlog -> Incoming hand-off
  (blocking THERE is fine — it is not the goroutine that must keep
  reading). Single delivery path preserves FIFO order; on restore the
  backlog drains to the UI immediately, and the courtroom's bounded IC
  queue (drop-oldest + packed-room catch-up) collapses the replay, so
  depth never becomes a replay storm.
- readBacklogCap is sized for a BUSY room's full occluded stretch (~0.5
  packets/s -> ~18h; idle rooms -> days), because an occluded-but-healthy
  client is indistinguishable from a wedged one at this layer and must not
  be torn down inside any plausible stall. ~1.6MiB preallocated per conn.
- Overflow of either bound = the drain has been dead for many busy hours
  with traffic still arriving — torn down deliberately with
  errReadBacklogOverflow (distinct from transport drops).
- Transport drop while stalled: deliberately NO Close() from the read-error
  path — closing c.closed would make deliverLoop abandon parked packets,
  and surfacing 'immediately' buys nothing while nobody is watching; on
  resume pumpConnection drains the parked packets and hits the channel
  close in the same call.
- DialOptions.ReadBacklogCap: test hook, mirroring KeepaliveInterval.
- Parked background tabs shared Conn.readLoop, so their identical wedge is
  fixed for free.

Tests: TestReaderKeepsPongingWhileConsumerStalled (server pings must keep
succeeding while the consumer stalls past the old 256 limit, then all 400
frames arrive in order — fails on the pre-fix reader),
TestReadBacklogOverflowDisconnects (wedged-client contract + sentinel),
TestIdleOnlyPingsNeverWedge (pure-idle baseline). Adversarially reviewed
(concurrency / behavior / efficacy lenses). Gate: go test -race -count=1
./internal/protocol/ green (5x repeat); courtroom/assets/network/config/
cache/theme suites green.
.gitignore already lists scheduled_tasks.lock, but the file was committed
before the rule existed, and ignore rules don't apply to tracked files —
so every session that schedules a task rewrote it and dirtied the tree.
Untrack it; the existing ignore rule takes over from here.
@SyntaxNyah
SyntaxNyah merged commit 410b073 into main Jul 24, 2026
1 check failed
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