Cygwin: pty: detect pcon-backed pty for non-Cygwin-spawned children#131
Draft
dscho wants to merge 1 commit into
Draft
Cygwin: pty: detect pcon-backed pty for non-Cygwin-spawned children#131dscho wants to merge 1 commit into
dscho wants to merge 1 commit into
Conversation
dscho
force-pushed
the
fix-pcon-grandchild-cons0
branch
2 times, most recently
from
51f9d40 to
8555725
Compare
When a Cygwin process (e.g. `bash` under MinTTY) spawns a native Win32 child (e.g. `git.exe`) with pseudo console support enabled, the child gets a pseudo console that bridges the pty. If that native child then spawns a Cygwin grandchild (e.g. `vim`, `less`), the grandchild inherits the pseudo console's console handles. In `init_std_file_from_handle()`, the grandchild's msys2-runtime sees `GetConsoleScreenBufferInfo()` succeed on those handles and, with no valid `ctty` set, falls back to `FH_CONSOLE` and gives the process `cons0` instead of connecting to the pty. This causes scrollback clobbering in MinTTY because alternate screen sequences (`ESC[?1049h` / `ESC[?1049l`) are handled by `fhandler_console`'s `save_restore()` against the pseudo console's buffer, which has no correspondence to MinTTY's scrollback. Fix this in the existing console branch of `init_std_file_from_handle()`: when there is no valid `ctty` and we are about to fall back to `FH_CONSOLE`, first scan the shared tty table for an entry whose `pcon_activated` is set and whose `nat_pipe_owner_pid` is in our console's process list (via `GetConsoleProcessList`). If found, parse the device as that pty slave instead of as a real console. The handle is closed in either fallback path, matching the existing `FH_CONSOLE` behavior. `myself->ctty` is left untouched; the regular `fhandler_pty_slave::open_setup()` path will set it via `myself->set_ctty()` when the pty slave is opened. The structure of `find_pcon_pty()` matters and is easy to get wrong in case a keen developer would like to refactor this code in the future. This code runs on every Cygwin process startup whose parent is non-Cygwin, so the common path (no pty with an active pseudo console) must remain free of expensive operations. Two pitfalls to avoid: filtering tty entries with `tty::exists()` looks correct but creates and destroys a named pipe per entry (128 entries on every call), and hoisting the `GetConsoleProcessList()` call out of the loop pays the cross-process cost even when no candidate exists. The current shape, a cheap shared-memory boolean check first and a lazily fetched process list only on the first candidate, keeps the common case at a handful of pointer reads. This was reported in git-for-windows/git#5303 and bisected to Git for Windows v2.41.0, where the msys2-runtime was upgraded from 3.3.6 (no pcon) to 3.4.6 (pcon architecture introduced). Assisted-by: Claude Opus 4.7 (1M context) Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho
force-pushed
the
fix-pcon-grandchild-cons0
branch
from
8555725 to
7017080
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
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.
When
bash(Cygwin, under MinTTY) spawnsgit.exe(native Win32) with pseudo console support enabled, andgit.exethen spawns a Cygwin grandchild (vim,less,sh), the grandchild inherits the pseudo console's console handles. The msys2-runtime classifies those handles asFH_CONSOLE, giving the processcons0instead of connecting to the pty (pty0).On
cons0, alternate screen sequences (ESC[?1049h/ESC[?1049l) are handled byfhandler_console'ssave_restore()against the pseudo console's buffer rather than by MinTTY against its scrollback, causing the visible scrollback to be clobbered when the editor/pager exits.This was reported in git-for-windows/git#5303 and bisected to Git for Windows v2.41.0 (msys2-runtime 3.3.6 to 3.4.6, when the pseudo console architecture was introduced).
The fix scans the shared tty table during
stdio_init()for a tty whose pseudo console owner PID is in our console's process list (viaGetConsoleProcessList). If found,myself->cttyis set to that pty device before the standardGetStdHandle/init_std_file_from_handlepath runs, so the existingCTTY_IS_VALIDbranch creates anfhandler_pty_slavethat connects to the pty.Verified locally: with this fix,
GIT_EDITOR=vim git commit --amendno longer clobbers the scrollback in MinTTY.