[PATCH] Cygwin: pty: Improve CSI6n handling in pcon_start state
Thomas Wolff
towo@towo.net
Sat Feb 28 13:19:30 GMT 2026
Am 27.02.2026 um 18:58 schrieb Johannes Schindelin:
> Hi Takashi,
>
> On Mon, 23 Feb 2026, Takashi Yano wrote:
>
>> Previsouly, CSI6n was not handled correctly if the some sequences
>> are appended after the responce for CSI6n. Especially, if the
>> appended sequence is a ESC sequence, which is longer than the
>> expected maximum length of the CSI6n responce, the sequence will
>> not be written atomically. With this patch, pcon_start state
>> is cleared at the end of CSI6n responce, and appended sequence
>> will be written outside of the CSI&n handling block.
I encounter stray CSI6n when I invoke wsl.exe via execl(p) within a
forkpty child and I was puzzled why that occurs.
Could it be related to this issue (and hopefully fixed by the patch)?
Thomas
> The idea of breaking out of the CSI 6n loop at `R` and falling through to
> the normal write paths is sound, but I think the `towrite` accounting has
> a bug, and the commit message could use some work.
>
>> Fixes: f20641789427 ("Cygwin: pty: Reduce unecessary input transfer.")
>> Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
>> Reviewed-by:
>> ---
>> winsup/cygwin/fhandler/pty.cc | 20 +++++++++++++-------
>> 1 file changed, 13 insertions(+), 7 deletions(-)
>>
>> diff --git a/winsup/cygwin/fhandler/pty.cc b/winsup/cygwin/fhandler/pty.cc
>> index 838be4a2b..c1e03db41 100644
>> --- a/winsup/cygwin/fhandler/pty.cc
>> +++ b/winsup/cygwin/fhandler/pty.cc
>> @@ -2137,6 +2137,8 @@ fhandler_pty_master::close (int flag)
>> ssize_t
>> fhandler_pty_master::write (const void *ptr, size_t len)
>> {
>> + size_t towrite = len;
>> +
>> ssize_t ret;
>> char *p = (char *) ptr;
>> termios &ti = tc ()->ti;
>> @@ -2171,6 +2173,8 @@ fhandler_pty_master::write (const void *ptr, size_t len)
>> }
>> if (state == 1)
>> {
>> + towrite--;
>> + ptr = p + i + 1;
> The per-byte `towrite--` only fires inside `state == 1`, so bytes before
> the ESC (which go through the `else` / `line_edit` branch) are never
> subtracted. If there happen to be N bytes before the ESC in the same
> write, `towrite` ends up N too large, and the `nat` pipe fast path reads
> past the end of the buffer.
>
> This can be fixed in a simpler way by not tracking `towrite` in the loop
> at all, and instead computing it once at the break, see below...
>
>> if (ixput < wpbuf_len)
>> wpbuf[ixput++] = p[i];
>> else
>> @@ -2184,7 +2188,10 @@ fhandler_pty_master::write (const void *ptr, size_t len)
>> else
>> line_edit (p + i, 1, ti, &ret);
>> if (state == 1 && p[i] == 'R')
>> - state = 2;
>> + {
>> + state = 2;
>> + break;
>> + }
>
> If you initialize `towrite = 0` and make this hunk look like this instead:
>
> if (state == 1 && p[i] == 'R')
> - state = 2;
> + {
> + state = 2;
> + towrite = len - i - 1;
> + ptr = p + i + 1;
> + break;
> + }
>
> then no per-byte bookkeeping is needed, making the entire logic a lot more
> robust, not to mention: easier on the reader's brain.
>
> Regarding the commit message: beyond the typos ("Previsouly" =>
> "Previously", "responce" => "response" x3, "CSI&n" => "CSI 6n"), the body
> describes the bug as "the sequence will not be written atomically", but
> isn't the actual problem that bytes after the `R` terminator go through
> per-byte `line_edit` inside the CSI 6n loop and then hit `return len`
> without ever reaching the `nat` pipe fast path? In that case, it would be
> a routing problem, not an atomicity problem, and something like:
>
> Cygwin: pty: Fix data after CSI 6n response bypassing normal write paths
>
> When the terminal's CSI 6n response and subsequent data (e.g.
> keystrokes) arrive in the same write buffer, `master::write()`
> processes all of it inside the pcon_start loop and returns early.
> Bytes after the 'R' terminator go through per-byte `line_edit()` in
> that loop instead of falling through to the `nat` pipe fast path or
> the normal bulk `line_edit()` call.
>
> Fix this by breaking out of the loop when 'R' is found and letting the
> remaining data fall through to the normal write paths, which are now
> reachable because `pcon_start` has been cleared.
>
> would be potentially more accurate in describing what is going on.
>
> I am still quite fuzzy on the exact goings-on in the pty code, essentially
> cobbling it all together "on the side" because I unfortunately cannot
> afford to spend much focus on the Cygwin/MSYS2 runtime. Hopefully what I
> said above makes some sense?
>
> Ciao,
> Johannes
>
>> }
>> if (state == 2)
>> {
>> @@ -2220,8 +2227,8 @@ fhandler_pty_master::write (const void *ptr, size_t len)
>> }
>> get_ttyp ()->pcon_start_pid = 0;
>> }
>> -
>> - return len;
>> + if (towrite == 0)
>> + return len;
>> }
>>
>> /* Write terminal input to to_slave_nat pipe instead of output_handle
>> @@ -2233,15 +2240,14 @@ fhandler_pty_master::write (const void *ptr, size_t len)
>> is activated. */
>> tmp_pathbuf tp;
>> char *buf = (char *) ptr;
>> - size_t nlen = len;
>> + size_t nlen = towrite;
>> if (get_ttyp ()->term_code_page != CP_UTF8)
>> {
>> static mbstate_t mbp;
>> buf = tp.c_get ();
>> nlen = NT_MAX_PATH;
>> - convert_mb_str (CP_UTF8, buf, &nlen,
>> - get_ttyp ()->term_code_page, (const char *) ptr, len,
>> - &mbp);
>> + convert_mb_str (CP_UTF8, buf, &nlen, get_ttyp ()->term_code_page,
>> + (const char *) ptr, towrite, &mbp);
>> }
>>
>> for (size_t i = 0; i < nlen; i++)
>> --
>> 2.51.0
>>
>>
More information about the Cygwin-patches
mailing list