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]>