fix(client): render off the select loop so a slow terminal can't wedge it (phux-fysb)#51
Open
phall1 wants to merge 1 commit into
Open
fix(client): render off the select loop so a slow terminal can't wedge it (phux-fysb)#51phall1 wants to merge 1 commit into
phall1 wants to merge 1 commit into
Conversation
…e it (phux-fysb) Re-attaching to a multi-pane/window session froze the client: screen stuck, Ctrl-C and detach dead. The attach select! loop rendered SYNCHRONOUSLY -- every paint_full_frame/render_at ends in out.flush(), and when `out` is the real tty that flush BLOCKS until the terminal drains. Because the paint runs inside the biased conn.recv() arm, a slow flush starves the stdin/signal arms. Multi-pane re-attach is the trigger: its paint_full_frame burst is ~4x a single pane's bytes (measured 5.6KB -> 22KB at 1->6 panes), enough to cross the wedge threshold a single pane never reaches. (The "pane shells die" the user saw is collateral: force-killing the wedged terminal EOFs the session.) New StdoutSink threads through main_loop as `out`: writes buffer in memory, flush() ships the complete-frame bytes to a dedicated OS thread (phux-stdout) that owns real stdout and does the blocking write off the runtime thread. The select! loop never blocks on the terminal, so input and signals are always serviced. Backpressure: if the queued backlog exceeds 256KiB the sink drops it and sets needs_resync; main_loop polls that and repaints the latest state via paint_full_frame (self-contained, supersedes the dropped diffs) -- memory bounded, screen renders at the sink's pace. Wiring keeps the synchronous test seam: run/run_with_predict -> run_buffered (spawns the writer) -> attach_session; run_with_stdout(_predict) still passes None,None so phux-server integration tests are unchanged. On Detached/Err the writer is drained+joined before the terminal-reset writes (so detach is prompt and the reset isn't garbled); SwitchTo keeps the writer across the session switch. Tests: stdout_writer unit tests incl. flush_does_not_block_on_a_slow_sink (flush stays <25ms behind a 50ms/chunk sink -- the core property) and overflow-drops-backlog-sets-resync. No render-path code writes stdout directly, so every flush goes through the sink. 343 client tests + attach lifecycle/snapshot/reattach/graceful-restore green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary
Re-attaching to a multi-pane/window session froze the client — screen stuck, Ctrl-C and detach dead — and then the pane shells appeared to die. This decouples rendering from the input loop so a slow terminal can never wedge it.
Root cause
The attach
tokio::select!loop renders synchronously: everypaint_full_frame/render_atends inout.flush(), and whenoutis the real tty that flush blocks until the terminal drains. The paint runs inside thebiasedconn.recv()arm, so a slow flush starves thestdin/signal arms → the client can't process Ctrl-C or detach = wedge.Why multi-pane specifically: a re-attach
paint_full_frameburst scales ~4× with panes (measured 5.6 KB → 22 KB at 1 → 6 panes). That crosses the wedge threshold a single pane (which drains instantly on any terminal) never reaches. The "pane shells die" is collateral — force-killing the wedged terminal EOFs the session, which reaps the panes (exit code 1).The fix
A new
StdoutSink(attach/stdout_writer.rs) threads throughmain_loopasout:write*only buffer in memory;flush()ships complete frames to a dedicated OS thread (phux-stdout) that owns real stdout and does the blocking write off the runtime thread.select!loop therefore never blocks on the terminal — input and signals are always serviced.needs_resync;main_looppolls it and repaints the latest state viapaint_full_frame(self-contained — supersedes the dropped diffs). Memory stays bounded; the screen renders at the terminal's pace.Lifecycle
run/run_with_predict→run_buffered(spawns the writer) →attach_session. The synchronous test seamrun_with_stdout(_predict)passesNone, None, so phux-server integration tests are unchanged. OnDetached/Errthe writer is drained + joined before the terminal-reset writes (detach stays prompt, reset isn't garbled);SwitchTokeeps the writer across the session switch.Validation
flush_does_not_block_on_a_slow_sink—flush()stays <25 ms even when the inner sink sleeps 50 ms/chunk (the core "loop can't be starved" property). Plus overflow-drops-backlog-sets-resync and ship-in-order.just cigreen.Caveat — please confirm on a real terminal
I could not reproduce the exact wedge end-to-end headlessly: a throttled pty reader trips macOS pty flow-control that gates input on any binary (a harness artifact, not phux), and the detach keybinding won't fire through raw pty writes. The fix is mechanism-proven, but the definitive check is on a real terminal: open a multi-pane session, detach, re-attach — it should stay responsive instead of freezing.
🤖 Generated with Claude Code