Conversation
waitOnSocketData treated EBADF/EINVAL from select() like EINTR and returned false. Callers (SocketHandler::readAll, RawSocketUtils::readAll) treat false as "retry", but a closed fd never becomes readable again, so an fd closed by another thread spun at 100% CPU. Throw instead, so retry loops treat it as socket death like any other read failure. The etterminal and jumphost init loops read packets outside any try/catch; give them one so a router that dies during init produces a logged fatal instead of an uncaught exception.
…isterTea#631) When a process produces output faster than the network can deliver it, the excess queues in kernel buffers - mostly the server's TCP send buffer, which autotunes to many MB. On a saturated link the display falls steadily behind real time and Ctrl-C appears dead: the interrupt is delivered, but the prompt has to wait behind the entire queue. On a 100KB/s test link, Ctrl-C took 56-116s to take effect. This adds a per-session flow control mode, chosen by the client: - none (default): the existing behavior, unchanged. The mode field is not even serialized when unset, so new binaries interoperate with old ones exactly as before, and users who don't opt in see zero change. - backpressure: terminal output is staged in a small (64KB) buffer that is drained only when select() reports the socket writable; when it fills, the server stops reading the PTY and the remote process pauses, like plain ssh. Lossless - safe for consumers that need every byte, such as tmux -CC. - discard: same buffer, but when it fills the oldest chunk is dropped. The remote process never stalls - even while the client is disconnected - and the display stays near real time. Output that exceeded the link rate is missing from scrollback. Application-level buffering only helps if the backlog actually waits in the application buffer, so opted-in sessions also shrink the kernel queues via SocketHandler::minimizeKernelBuffering (a no-op by default): - TCP_NOTSENT_LOWAT=32KB on the client connection: select() reports writable only when <32KB is unsent, keeping the backlog in the WriteBuffer where it can be gated or dropped. In-flight (sent-but-unacked) data is not limited, so throughput on high-BDP links is unaffected. It is reapplied every loop iteration because the connection gets a new socket on reconnect and fd numbers are reused. - SO_SNDBUF=64KB on the etterminal->etserver unix socket (pure queue, no in-flight component, so clamping costs nothing locally). etterminal learns the mode via a new TermInit field. In the jumphost path, discard only drops TERMINAL_BUFFER packets; control packets (port forwarding, responses, keepalives) are always delivered, and reads pause when the queue is dominated by them so it stays bounded. The client suppresses its missed-keepalive disconnect while it is intentionally not reading (console buffer full), since the keepalive echo may be sitting unread in the socket. Measured on a 100KB/s throttled link with a 2.7MB/s producer: display lag @30s producer Ctrl-C -> prompt none (unchanged) ~31s, growing throttled 56-116s backpressure ~2.5s, bounded throttled 2-3s discard ~1.2s, bounded full speed 2.1s In discard mode the producer also runs at full speed through a 60s network outage, and on reconnect the display recovers to ~1.2s of lag instead of replaying the backlog.
A userspace throttle proxy (100KB/s, with a clamped receive buffer so it models a real slow link instead of absorbing megabytes into its own autotuned rcvbuf) sits between etserver and the et client, with a timestamp-printing workload behind a PTY driven from tmux. - do_scenario.sh: per-mode scenario (trunk/none/backpressure/discard) measuring display lag, producer lag (is the process stalled?), throughput, and Ctrl-C-to-prompt latency, with an optional 60s disconnect simulation. Emits JSONL metrics. Refuses to run on a dirty tree because its cleanup restores source dirs from HEAD. - throughput_test.sh: bulk throughput on a fast link (no throttle). - REPORT.md: methodology and measured results. To run the components standalone, et gains --idpasskey to skip the SSH handshake and connect directly to a manually started etterminal, and etterminal's --idpasskey handling is fixed to actually use the passed value (it was assigned to a shadowing local and dropped).
Contributor
Author
|
Validation context and linked production failure report are also posted on the original PR: #730 (comment). This follow-up should be treated as a tested rebase aid for #730. |
xz-dev
marked this pull request as draft
August 27, 2026 07:25
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #797 +/- ##
==========================================
- Coverage 87.14% 87.13% -0.01%
==========================================
Files 75 77 +2
Lines 6462 6740 +278
Branches 610 650 +40
==========================================
+ Hits 5631 5873 +242
- Misses 831 867 +36 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
Author
|
Superseded by the focused follow-up PR against Ben Maurer's branch: bmaurer#1 Closing this PR because the original contribution should remain primary. The focused follow-up preserves Ben's existing commits/authorship, merges current upstream, resolves the #788 conflict, and fixes the existing E2E cleanup bug without introducing another upstream-facing copy of PR #730. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This follow-up carries Ben Maurer's three-commit PR #730 stack rebased onto current
MisterTea/EternalTerminal@b74a12ef(masteras of 2026-08-06). Original authorship and commit order are preserved.Upstream PR: #730
Related issues: #631, #787, #782
Why this matters beyond interactive lag: we reproduced a recovery deadloop under fast terminal output. With the default
nonemode, stale output grew into a multi-megabyte queue; reconnect repeatedly hit the recovery timeout and restarted from the beginning. In our exact v7 baseline reproduction, recovery never finished. A long-running tmux/Pi session on a lossy, high-RTT path can therefore appear permanently frozen even though the remote program andetserverremain alive. This matches the failed-recovery class reported in #787; #782 explains why users only see a frozen terminal while reconnect/recovery progress exists only in logs.With this patch and explicit
--flow-control backpressure, matched-binary tests bounded the queue, stalled the producer while congested, resumed it after recovery, and returned display lag to about 1–2 seconds.discardalso recovered while allowing producer progress, with the documented scrollback trade-off. Default behavior remains unchanged unless a client opts in.Validation on the current rebased branch:
cmake -S . -B build -DDISABLE_VCPKG=ON -DBUILD_TESTING=ON -DCMAKE_BUILD_TYPE=RelWithDebInfocmake --build build -j1ctest --test-dir build --output-on-failure -j1: 159/159 passed, 2 privilege-dependent skipsgit diff --check origin/master..HEAD: passednonetimed out;backpressureanddiscardrecoveredbackpressurerecovered without restarting the serverSocket Timeout, display caught up below 2 sThe only rebase conflict was in
TerminalClient::run; resolution preserves both PR #730 output-buffer draining and current master’s #788 non-TTY guard (console && consoleFd >= 0).This is intended to help the original PR move forward, not replace its author. If Ben can update #730 directly, this branch can be used as a tested rebase reference or cherry-picked as the same three-commit stack.