Rare character glitch in pty?

[Thomas Wolff]

Am 04.08.2023 um 13:33 schrieb Takashi Yano:
> On Fri, 4 Aug 2023 17:52:11 +0900
> Takashi Yano wrote:
>> Hi Thomas,
>>
>> On Wed, 2 Aug 2023 08:32:39 +0200
>> Thomas Wolff wrote:
>>> Am 02.08.2023 um 00:35 schrieb Takashi Yano:
>>>> On Tue, 1 Aug 2023 23:05:18 +0200
>>>> Thomas Wolff wrote:
>>>>> 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.
>>>> Can you provide the automated test case? I would like to
>>>> investigate what is happning.
>>> Manual test case:
>>> 1. Run `mintty echokey1` with the attached test program that runs like
>>> `cat -v` and detects the glitch
>>> 2. Run a second test window, same setup to increase the hit rate
>>> 3. Switch these 2 windows by clicking them in change a few hundred
>>> times, until the test program outputs "FAIL"
>>>
>>> Automated test case:
>>> 1. Use the tool TinyTask (https://www.tinytask.net/) to save the
>>> clicking. Sorry, I didn't manage to replace this with a self-made click
>>> simulator.
>>> 2. Place the 2 test windows in a specific position (e.g. covering the
>>> two screen halves in a top position)
>>> 3. Use the Record function of the tool and click to switch windows a
>>> dozen times, stop recording
>>> 4. Save the sequence as a TinyTask script, or use mine (2w.rec
>>> attached), better: load the script (Open), then create a standalone test
>>> executable (button "exe") to create tinytask2w.exe (I can send that to
>>> you if you like)
>>> 5. Optionally, create some background load on your system
>>> 6. Minimize other windows in case the test script does not hit the test
>>> windows
>>> 7. Run the test script minptytest (attached) to start the 2 windows and
>>> run the TinyTask standalone test in a loop; the script detects a marker
>>> file created by the test program; it also detect premature termination
>>> of a window (e.g. by hitting ^D in it)
>> Thanks for the test case. I could reproduce the issue using
>> the automated test case.
>>
>> I looked into this problem, and found the cause.
>>
>> Previously, though readahead buffer handling in pty master was not
>> fully thread-safe, accept_input() was called from peek_pipe() thread
>> in select.cc.
>>
>> The mechanism of the problem is:
>> 1) accept_input() which is called from peek_pipe() thread calls
>>     eat_readahead(-1) before reading readahead buffer. This allows
>>     writing to the readahead buffer from another (main) thread.
>> 2) The main thread calls fhandler_pty_master::write() just after
>>     eat_readahead(-1) was called and before reading the readahead
>>     buffer by accept_input() called from peek_pipe() thread. This
>>     overwrites the readahead buffer.
>> 3) The read result from readahead buffer which was overwritten is
>>     sent to the slave.
>>
>> This issue has existed this decade.
>>
>> I'll submit a patch for this issue shortly.
> Now, cygwin-3.5.0-0.370.g3b4f6217c371 (Test) is ready.
> Please test.
Great! Thank you for fixing such a subtle issue in a core component.
Any chance to get this back-ported to 3.4.7 (as the last version to 
support ... whatever)?
Thomas