use plain write(2) for terminal stdout instead of stdlib's pwritev writer#1124
Draft
adamnroman wants to merge 2 commits into
Draft
use plain write(2) for terminal stdout instead of stdlib's pwritev writer#1124adamnroman wants to merge 2 commits into
adamnroman wants to merge 2 commits into
Conversation
99c1d0e to
4e24770
Compare
Collaborator
|
I assume this breaks on windows... |
StdoutOutput.write in renderer-output.zig drove stdout through
std.fs.File.stdout().writer(), which attempts pwritev(fd, iov, n,
/*offset=*/0) as a fast path before falling back to writev on ESPIPE.
Stdout connected to a TTY is not seekable, so the pwritev fast path
collapses to writev inside the kernel on Linux and gains nothing. It is
also where things go wrong under gVisor (runsc), which returns EINVAL
instead of ESPIPE for pwritev on a non-seekable fd. The ESPIPE fallback
therefore never fires and every render frame is silently dropped inside
a gVisor sandbox.
Replace the buffered writer interface with std.fs.File.stdout().writeAll,
which is cross-platform (posix.write on Unix, WriteFile on Windows) and
does not take the pwritev fast path. The 4096-byte stdoutBuffer field is
no longer needed and is removed; the struct becomes zero-sized but is
still allocated via the existing ownership scheme in
BufferedBackend.createStdout (the `.{}` initializer still works on a
zero-field struct).
Other TUI projects with hand-rolled renderers do the same: ncurses uses
write(2) in _nc_flush (ncurses/tinfo/lib_tputs.c:139) and _nc_outch
(ncurses/tinfo/lib_tputs.c:194, :199); earendil-works/pi routes its
differential renderer through ProcessTerminal.write, which calls
process.stdout.write (packages/tui/src/terminal.ts:496-497); OpenAI's
codex CLI delegates to ratatui + crossterm (codex-rs/Cargo.toml:274,
:338), which write through io::Write on io::stdout(). None reach for
pwrite/pwritev.
No behavior change on Linux/macOS/Windows. Fixes silently-dropped
rendering on gVisor.
4e24770 to
9ef2313
Compare
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
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
StdoutOutput.writeinrenderer-output.zigdrives stdout throughstd.fs.File.stdout().writer(), whose bufferedWriterinterface attemptspwritev(fd, iov, n, /*offset=*/0)as a fast path before falling back towritevonESPIPE. Stdout connected to a TTY is not seekable, so thepwritevfast path collapses towritevinside the kernel on Linux and gains nothing. It is also where things go wrong under gVisor (runsc), which returnsEINVALinstead ofESPIPEforpwritevon a non-seekable fd. TheESPIPEfallback never fires and every render frame is silently dropped inside a gVisor sandbox.This PR replaces the buffered
Writerinterface withstd.fs.File.stdout().writeAll(data), which loops overposix.writeon Unix andWriteFileon Windows (nopwritevpath on either), and removes the now-unusedstdoutBufferfield.Repro
Run any OpenTUI program (for example opencode under
@opentui/core@0.3.0) inside a runsc-backed Kubernetes pod (AKS gVisorRuntimeClass) inside a tmux pane. Expected: full TUI. Observed: just the shell prompt.straceon the binary shows the first render emit as:and nothing after it. The same setup under
runcreturnsESPIPEand thewritevfallback succeeds.Why this change is correct
Stdout connected to a terminal is not seekable.
pwritev(fd, iov, n, /*offset=*/0)on a non-seekable fd is a no-op at best and a portability hazard at worst. On Linux it collapses towritevinside the kernel, so the fast path buys nothing. On gVisor it returnsEINVALinstead of the POSIX-expectedESPIPE, which defeatspwritev-vs-writevfallbacks (Bun, Zig stdlib) and silently drops every render frame inside a gVisor sandbox.Other TUI projects, including ones with hand-rolled renderers, all emit their frames with plain
write(2)/WriteFileor the language-native stream wrapper on top of it. Three direct references:ncurses does the terminal byte-push with
write(2)in_nc_flushand_nc_outch:ncurses/tinfo/lib_tputs.c#L139(_nc_flush:ssize_t res = write(SP_PARM->_ofd, buf, amount);)ncurses/tinfo/lib_tputs.c#L194and#L199(_nc_outch)earendil-works/pi has its own from-scratch differential renderer (
pi-tui). ItsProcessTerminal.writeis the single sink the renderer drives, and it usesprocess.stdout.write:packages/tui/src/terminal.ts#L496-L497(write(data: string): void { process.stdout.write(data); ... })OpenAI codex CLI delegates terminal rendering to
ratatui+crossterm. The dependency declarations:codex-rs/Cargo.toml#L338(ratatui = "0.29.0")codex-rs/Cargo.toml#L274(crossterm = "0.28.1")codex-rs/tui/Cargo.toml#L71and#L82(thecodex-tuicrate)Through that stack, codex's render path is
ratatui::CrosstermBackend::flush-> innerW: Write(io::stdout()) ->write(2). Nopwrite*is reached on the rendered-bytes path.This change routes OpenTUI's
StdoutOutputthe same way.Cross-platform note
std.fs.File.stdout()resolves to the correct handle on every supported platform (verified against Zig 0.15.2 stdlib inlib/std/fs/File.zig):File.writethen routes towindows.WriteFileorposix.writedepending on platform.File.writeAllloops overFile.write. Neither touchespwrite*on any platform.Notes
StdoutOutputbecomes a zero-sized struct after thestdoutBufferfield is removed. The existing.{}initializer inBufferedBackend.createStdoutstill works on a zero-field struct.File.stdout()sites in the repo (bench-utils.zig,bench.zig) are benchmark harness output, not the renderer's user-facing path; intentionally untouched.Testing
zig build -Dtarget=x86_64-linux-gnu.2.17 -Doptimize=ReleaseFastagainst the patched tree produces a workinglibopentui.so, cross-compiled successfully..sowas swapped into a Bun-compiled opencode binary built against@opentui/core@0.3.0and deployed to an AKS pod running the gVisorRuntimeClassinside tmux. TUI renders correctly. Same binary continues to render correctly on runc (Minikube) and on macOS/Linux laptops.std.fs.File.write/WriteFileroute used by the rest of OpenTUI.zig build test --summary all) onnative-native-muslfails to compile in our build environment, but this failure also reproduces on a clean checkout ofmainwith no changes; it is pre-existing and unrelated to this patch.No behavior change on Linux/macOS/Windows. Fixes silently-dropped rendering on gVisor.