fix: run interactive pty in raw mode so stdout is not truncated - #845
fix: run interactive pty in raw mode so stdout is not truncated#845SomSamantray wants to merge 6 commits into
Conversation
The pty slave allocated for interactive containers was left in canonical mode, whose line discipline buffers output and causes large writes to be truncated at 1024 bytes (reported in apple/container#1148). A process writing more than 1024 bytes to stdout and exiting loses the buffered tail because the canonical-mode tty holds it waiting for a line terminator. Configure the slave with the existing Terminal.setraw() recipe (cfmakeraw plus OPOST preserved) before dup3'ing it onto stdio, so output passes through without truncation while newline-to-CRLF translation is kept. Add a regression test asserting the termios flags: a fresh pty slave is canonical, and setraw() clears ICANON while preserving OPOST.
Unify the ICANON and OPOST assertions through one flag-membership helper operating on a tcflag_t field, instead of re-inlining the same bit-mask idiom with different shapes.
cfmakeraw clears OPOST but leaves ONLCR set; re-adding OPOST in Terminal.setraw() restores newline-to-CRLF output translation. Lock both flags so the raw-mode recipe's output behavior is pinned by the test.
|
One terminal behaviour needs preserving before this lands: I’ve put a focused follow-up on It adds an opt-in Please feel free to cherry-pick the commit or take the branch. |
Guards the ICANON/OPOST/ONLCR assertions already in this test with an ISIG check, so a future change that flips the preserveSignalGeneration default would fail this test instead of passing silently.
Cherry-picked as 625e774 (on top of your commit at f4ccfb7) — thanks for the fast, well-scoped fix. Also added one more regression assertion: the default (non- One open question from that same review pass, noted in the PR description under "Unapplied review findings": restoring |
2 participants
Interactive containers (
container run --interactiveon apple/container) truncated stdout at 1024 bytes when output went to a terminal or file: the guest pty slave was left in canonical mode, whose line discipline buffers large writes, so a process writing more than 1024 bytes and exiting lost the buffered tail.The pty slave is now put into raw mode before it is wired onto the container's stdio, using the existing
Terminal.setraw()recipe (cfmakeraw with OPOST/ONLCR preserved so CRLF output translation is kept). Putting the slave fully into raw mode also clearsISIG, and @stephenlclarke pointed out in review that this would stop Ctrl-C from generatingSIGINTfor the container's own process.Terminal.setraw()now takes an opt-inpreserveSignalGenerationparameter (defaultfalse, so every other caller — the host-side terminals incctland the examples — keeps its current behavior); only the guest pty slave invmexecenables it, so Ctrl-C still interrupts the running process.Regression tests cover both paths: default
setraw()clearsICANONandISIGwhile preservingOPOST/ONLCR; withpreserveSignalGeneration: true,ICANONstill clears butISIGstays set.Related: apple/container#1148
Unapplied review findings
Sources/ContainerizationOS/Terminal.swift:150— RestoringISIGon the guest pty slave re-armsSIGQUIT(Ctrl-) andSIGTSTP(Ctrl-Z) generation, not justSIGINT(Ctrl-C).cfmakerawclears the wholeISIGbit, so re-setting it (rather than selectively re-enabling only theINTRspecial character) restores all three signal-generating characters at once. This matches this repo's pre-truncation-fix behavior exactly (a freshly allocated pty slave hasISIGon by default), so it isn't a new regression relative tomain— but it's worth an explicit call on whether that's the intended scope (matching runc's own console contract, which does the same) or whether onlySIGINTshould be restored (via_POSIX_VDISABLEonVQUIT/VSUSP). No code change proposed pending that call.