Skip to content

fix: buffer writes during disconnect to prevent programs in terminal from freezing#731

Merged
MisterTea merged 2 commits intoMisterTea:masterfrom
ejc3:fix-disconnect-buffer
Apr 1, 2026
Merged

fix: buffer writes during disconnect to prevent programs in terminal from freezing#731
MisterTea merged 2 commits intoMisterTea:masterfrom
ejc3:fix-disconnect-buffer

Conversation

@ejc3
Copy link
Copy Markdown
Contributor

@ejc3 ejc3 commented Feb 4, 2026

Summary

When a client disconnects, writePacket() would loop forever waiting for the socket, causing the entire pipe chain to back up. This freezes any program writing to the terminal (e.g. Claude Code would no longer make progress).

This fix buffers data during disconnect (up to 4MB) before entering the blocking loop. On reconnect, buffered data is replayed via ET's existing recovery mechanism.

Problem

The root cause was in BackedWriter::write():

  1. When socketFd < 0, it returned SKIPPED immediately without buffering
  2. Connection::write() returned false on SKIPPED
  3. writePacket() looped forever with 100ms sleeps waiting for reconnect
  4. This blocked the terminal output pipe, filling the PTY buffer
  5. Programs writing to stdout would block on write() and freeze

Solution

  • Buffer packets before checking socket state
  • Add BUFFERED_ONLY return state for successful buffering without socket
  • Add DISCONNECT_BUFFER_BYTES constant (4MB) to control when to start blocking
  • Keep MAX_BACKUP_BYTES at 64MB for normal connected operation

Behavior

State Action
Connected Buffer + send (trim old at 64MB)
Disconnected, buffer < 4MB Buffer data, return success (no blocking)
Disconnected, buffer >= 4MB Return SKIPPED → writePacket() blocks until reconnect

The blocking loop still exists but now only triggers after 4MB is buffered. This preserves data for replay on reconnect while still providing backpressure for extended outages.

Changes

  • BackedWriter.hpp: Add BUFFERED_ONLY enum, add DISCONNECT_BUFFER_BYTES constant
  • BackedWriter.cpp: Buffer before socket check, respect disconnect limit
  • Connection.cpp: Handle BUFFERED_ONLY as success
  • BackedIOTest.cpp: Update existing test, add new disconnect buffer test

Test plan

  • All 111 existing tests pass (942 assertions)
  • New unit test fills 4MB buffer and verifies SKIPPED is returned
  • Manual testing: disconnect/reconnect with active terminal output

Evidence from production logs

After deploying this fix, the server logs show successful buffering during disconnects:

2026-02-04T06:30:23,822 Write buffered (no socket)
2026-02-04T06:30:23,932 Write buffered (no socket)
2026-02-04T06:00:23,821 Write buffered (no socket)
2026-02-04T06:00:23,934 Write buffered (no socket)
...

These writes completed successfully (returned BUFFERED_ONLY) instead of blocking. The terminal remained responsive during the disconnect, and output was replayed on reconnect.

@ejc3 ejc3 force-pushed the fix-disconnect-buffer branch from 94cb593 to 4f2cf10 Compare February 4, 2026 07:22
@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 4, 2026

Codecov Report

❌ Patch coverage is 93.65079% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.96%. Comparing base (ee8ddc2) to head (7814e0c).
⚠️ Report is 7 commits behind head on master.

Files with missing lines Patch % Lines
test/unit_tests/BackedIOTest.cpp 93.61% 3 Missing ⚠️
src/base/BackedWriter.cpp 92.30% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #731      +/-   ##
==========================================
- Coverage   87.05%   86.96%   -0.09%     
==========================================
  Files          70       70              
  Lines        5161     5210      +49     
  Branches      483      487       +4     
==========================================
+ Hits         4493     4531      +38     
- Misses        667      679      +12     
+ Partials        1        0       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

When a client disconnects, writePacket() would previously loop forever
waiting for the socket to return, causing the entire pipe chain to back
up and freeze programs writing to the terminal (like Claude Code).

This change buffers data during disconnect (up to 4MB) and only blocks
when the buffer is full. On reconnect, buffered data is replayed via
ET existing recovery mechanism.

Changes:
- Add BUFFERED_ONLY state to BackedWriterWriteState enum
- Buffer packets before checking socket state in BackedWriter::write()
- Add DISCONNECT_BUFFER_BYTES constant (4MB) to control when to block
- Keep MAX_BACKUP_BYTES at 64MB for normal connected operation
- Connection::write() treats BUFFERED_ONLY as success
- Add unit test for disconnect buffering behavior

Behavior:
- Disconnected, buffer < 4MB: buffer data, return success (no drop)
- Disconnected, buffer >= 4MB: return SKIPPED, writePacket() blocks (no drop)
- Connected, buffer > 64MB: trim oldest packets (original ET behavior)

This ensures programs do not freeze on short disconnects while still
preserving backpressure for extended outages. Data is never dropped
when disconnected - it is either buffered or we block.
@ejc3 ejc3 force-pushed the fix-disconnect-buffer branch from 4f2cf10 to 9f75c17 Compare February 4, 2026 15:33
Adds test coverage for the MAX_BACKUP_BYTES (64MB) trim behavior when
socket is connected. Verifies that:
- Old data is trimmed when buffer exceeds 64MB
- Recovery from old sequence numbers fails (data trimmed)
- Recovery from recent sequence numbers succeeds
@ejc3 ejc3 force-pushed the fix-disconnect-buffer branch from a53cc21 to 7814e0c Compare February 5, 2026 04:54
@MisterTea
Copy link
Copy Markdown
Owner

Thanks!

@MisterTea MisterTea merged commit 3b8813d into MisterTea:master Apr 1, 2026
26 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