Re: [PATCH] Cygwin: pty: Fix cursor position on new nat app in existing pcon

Takashi Yano <[email protected]> Mon, 3 Aug 2026 11:40:55 +0900
Newsgroups gmane.os.cygwin.patches
Message-ID <[email protected]>
Hi Johannes,

Thanks for reviewing and sorry for late action.

On Thu, 23 Jul 2026 13:47:13 +0200 (CEST)
Johannes Schindelin <[email protected]> wrote:
> 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").
> > 
> > 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]>

Thanks for the advice. I'll push this patch to master branch,
with the fix in which 'pcon_start_pid' is cleared inside the
`input_mutex`.

-- 
Takashi Yano <[email protected]>