Re: Rare character glitch in pty?

[Thomas Wolff <towo@towo.net>]

I had reported this to the the main cygwin list:

Am 20.07.2023 um 08:33 schrieb Thomas Wolff via Cygwin:
> There was a report that on switching terminal windows with the mouse,
> focus event reports (as enabled with ESC[?1004h) would sometimes get 
> mangled.
> This is further described in issue reports to mintty/wsltty 
> (https://github.com/mintty/wsltty/issues/335) and also to tmux 
> (https://github.com/tmux/tmux/issues/3601).
>
> I could reproduce the issue without either of tmux or WSL. However, it 
> happens only once in a few hundred instances:
> The escape sequence is changed so that the initial ESC is replaced by 
> one of the two subsequent characters, so instead of ESC[O, either of 
> [[O or sometimes O[O would be sent to the foreground application.
>
> I traced the issue down to the pty that connects mintty and its client 
> application, verified that the correct sequence is passed into write() 
> at the pty and the mangled sequence appears at read() in the client 
> process. In addition to a minimised test program that behaves like 
> `cat -v`, I also also put the test output directly into the client 
> process within the mintty code.
> This leaves the only conclusion that the issue must be happening 
> somehow within pty handling, maybe in the context of some timing 
> condition or Windows messages being passed to mintty.
> Anyhow, I was about to prepare testing with older mintty versions, in 
> case I overlooked something, when the OP reported that the issue does 
> not happen with cygwin 3.2.0.
> Is there any recent change that could possibly cause such weird 
> behaviour?
> Thomas
>

I have now automated the test case to chase the issue and associate it 
with a cygwin version. As unlikely as it may appear, results suggest 
that there is a case within the pty handling that may change sequences 
sent through it once in a while, more likely on a system under heavy load.
I took some of the latest cygwin snapshots (by the way, there are no 
newer ones on the snapshots page):
I can reproduce the issue with
     3.3.5-341.x86_64.snap 2022-02-17 11:53 UTC
while it did not occur with
     3.4.0-341.x86_64.snap 2022-01-27 14:34 UTC
Weird enough, the failing newer one is a cygwin 3.3.5 although the older 
one is a 3.4.0 version.

Checking the git log around that time, there are a number of changes 
related to pty as listed below. I added some ! or ? marks to indicate 
more or less likely commits to be a culprit of this issue, judged on 
commit messages and browsing of code changes.

I would also try to identify a commit more precisely but unfortunately 
the cygwin build process seems broken for me as it does not create the 
dll anymore...

Thomas


commit 2f2b91550547001b005393097c5a4328451343d6
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Thu Feb 24 00:57:37 2022 +0900

     Cygwin: pty, console: Add a workaround for GDB SIGINT handling.

     - The inferior of the GDB cannot be continued after SIGINT even
       though nopass option is set. This seems because cygwin GDB does
       not support hooking cygwin signal. Therefore, a workaround for
       GDB is added. With this patch, only CTRL_C_EVENT is sent to the
       GDB inferior by Ctrl-C and sending SIGINT is omitted. Note that
       "handle SIGINT (no)pass" command does not take effect even with
       or without this patch.

commit 2cab4d0bb4af5ad7ec97914068d5fa1b8954909c
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Tue Feb 15 23:23:50 2022 +0900
?
     Cygwin: pty, console: Refactor the code processing special keys.

     - This patch commonize the code which processes special keys in pty
       and console to improve maintanancibility. As a result, some small
       bugs have been fixed.

commit bed1add783a13b3304b1625b962707f89e90e323
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Sun Feb 13 05:12:18 2022 +0900

     Cygwin: pty: Fix a bug in tty_min::segpgid().

     - In tty_min::setpgid(), a pointer to fhandler instance is casted to
       fhandler_pty_slave and accessed even if terminal is not a pty slave.
       This patch fixes the issue.

commit c4704c7c204e377859f657a3b06c112133adff09
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Sun Feb 13 02:16:32 2022 +0900

     Cygwin: pty: Discard input in from_master_nat pipe on signal as well.

     - Currently, pty discards input only in from_master pipe on signal.
       Due to this, if pty is started without pseudo console support and
       start a non-cygwin process from cmd.exe, type adhead input is not
       discarded on signals such as Ctrl-C. This patch fixes the issue.

commit b958e1f03acd1732bb0a433a589efb645458aac4
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Thu Feb 10 17:47:51 2022 +0900
?
     Cygwin: pty: Revise the code to wait for completion of forwarding.

     - With this patch, the code to wait for completion of forwarding of
       output from non-cygwin app is revised so that it can more reliably
       detect the completion.

commit bddd9c1c417e97791c44241cf94753e90464501c
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Thu Feb 10 17:22:11 2022 +0900
!?
     Cygwin: pty: Prevent deadlock on echo output.

     - If the slave process writes a lot of text output, doecho() can
       cause deadlock. This is because output_mutex is held in slave::
       write() and if WriteFile() is blocked due to pipe full, doecho()
       tries to acquire output_mutex and gets into deadlock. With this
!      patch, the deadlock is prevented on the sacrifice of atomicity
       of doecho().

commit b04aea00f1bb503dcc364c83ee3ef3da3bc1305e
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Sat Feb 12 17:51:55 2022 +0900
??
     Cygwin: pty: Pass Ctrl-Z (EOF) to non-cygwin apps with disable_pcon.

     - Previously, non-cygwin app running in pty started without pseudo
       console support was suspended by Ctrl-Z rather than sending EOF.
       Even worse, suspended app could not be resumed by fg command. With
       this patch, Ctrl-Z (EOF for non-cygwin apps) is passed to non-cygwin
       app instead of suspending that app. This patch also handles Ctrl-\
       (QUIT) and Ctrl-D (EOF) as well.

commit c05c36a7c8af5e97fa379abb7571ed35a0088e98
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Thu Feb 10 16:53:47 2022 +0900
??
     Cygwin: pty, console: Fix Ctrl-C handling for non-cygwin apps.

     - Currently, if cat is started from cmd.exe which is started in cygwin
       console, Ctrl-C terminates not only cat but also cmd.exe. This also
       happens in pty in which pseudo console is disabled. This patch fixes
       the issue.

commit 909ed837ccb932ea70f71cc41891fa2c8143133f
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Fri Jan 14 19:35:12 2022 +0900
??
     Cygwin: pty: Fix race issue between closing and opening master.

     - If the from_master is closed before cleaning up other pipes, such
       as from_slave_nat, the same pty may be allocated and pty master may
       try to open the pipe which is not closed yet, and it will fail.
       This patch fixes the issue.

commit 3af461092e7ee9b76f3a1c18a6d95ed6e226df81
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Thu Jan 13 19:44:43 2022 +0900
!?
     Cygwin: pty: Stop closing and recreating attach_mutex.

     - Closing attach_mutex and recreating it causes the race issue
       between pty and console codes. With this patch, attach_mutex
       is created only once in a process which opens pty, and never
       closed in order to avoid this issue.

     Addresses:
https://cygwin.com/pipermail/cygwin-developers/2021-December/012548.html

commit 4f490c4cef8fd527642dbb5ea5373b373193a49b
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Thu Jan 13 18:18:00 2022 +0900

     Cygwin: pty: Fix memory leak in master_fwd_thread.

     - If master_fwd_thread is terminated by cygthread::terminate_thread(),
       the opportunity to release tmp_pathbuf is missed, resulting in a
       memory leak. This patch fixes the issue.

commit aa49985245e4e5b278778b0a47f6ce83cceb4ad4
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Thu Jan 13 18:16:31 2022 +0900
?
     Cygwin: pty, console: Fix deadlock in GDB regarding mutex.

     - GDB inferior may be suspended while the inferior grabs mutex.
       This causes deadlock in terminal I/O. With this patch, timeout
       for waiting mutex is set to 0 for the debugger process when the
       process calls CreateProcess() with DEBUG_PROCESS flag to avoid
       deadlock. This may cause the race issue in GDB, however, there
       is no other way than that.

     Addresses:
https://cygwin.com/pipermail/cygwin-developers/2021-December/012542.html