Re: [PATCH] Cygwin: pty: Fix cursor position on new nat app in existing pcon
Johannes Schindelin <[email protected]> Thu, 23 Jul 2026 13:47:13 +0200 (CEST)
| Newsgroups | gmane.os.cygwin.patches |
|---|---|
| Message-ID | <[email protected]> |
Hi Takashi,
Thanks! This is correct and well-targeted. It should go in.
On Tue, 21 Jul 2026, Takashi Yano wrote:
> Previously, the steps: cmd.exe -> bash -> cmd.exe exhibit broken
> cursor position even with the commit b34394d456b6 ("Cygwin: pty:
> Fixup pty state after a cygwin app exits").
>=20
> This patch sets req_fixup_pcon_cur_pos also when reusing existing
> pseudo console as well as req_xfer_input.
The coupling in `setup_pseudoconsole()` is sound: Before this patch, only
`req_xfer_input` would be set, and a reply to CSI6n would simply be
swallowed. With this patch, the reply is consumed instead (and
intentionally not forwarded), the same handshake `req_fixup_pcon_state()`
already performs.
It complements b34394d456 (Cygwin: pty: Fixup pty state after a cygwin app
exits, 2026-06-13) by covering the sibling boundary: a new native app
reusing the existing pseudo console (your cmd.exe -> bash -> cmd.exe
case), rather than a cygwin app exiting.
One pre-existing race, not a blocker: the reuse branch sets
`pcon_start_pid` without the spin-until-clear reservation loop
`req_fixup_pcon_state()` uses, and the master clears `pcon_start_pid`
outside `input_mutex` (the store sits just past `ReleaseMutex`). So an
overlapping handshake from another process can clobber it to zero early,
and the slave stops spinning before its own DSR reply is handled. Before
this patch that stray late reply was harmless (swallowed); now it could
drive a stale `fixup_pcon_cursor_position()`. Probability is very low, and
the same exposure already exists via `req_fixup_pcon_state()`, so the
verdict stands. Optional separate fix: move the clear inside
`input_mutex`, and/or give the reuse branch the reservation loop.
Reviewed-by: Johannes Schindelin <[email protected]>
Thank you,
Johannes
> Signed-off-by: Takashi Yano <[email protected]>
> Reviewed-by:
> ---
> winsup/cygwin/fhandler/pty.cc | 1 +
> 1 file changed, 1 insertion(+)
>=20
> diff --git a/winsup/cygwin/fhandler/pty.cc b/winsup/cygwin/fhandler/pty.=
cc
> index f3df55f34..a100c868e 100644
> --- a/winsup/cygwin/fhandler/pty.cc
> +++ b/winsup/cygwin/fhandler/pty.cc
> @@ -3850,6 +3850,7 @@ fhandler_pty_slave::setup_pseudoconsole ()
> WaitForSingleObject (input_mutex, mutex_timeout);
> get_ttyp ()->req_xfer_input =3D true; /* indicates that this "ESC[6n=
"
> is just for transfer input */
> + get_ttyp ()->req_fixup_pcon_cur_pos =3D true;
> get_ttyp ()->pcon_start =3D true;
> get_ttyp ()->pcon_start_pid =3D myself->pid;
> WriteFile (get_output_handle (), "\033[6n", 4, &n, NULL);
> --=20
> 2.51.0
>=20
>=20