Typed characters are mis-ordered when CPU usage is high

Typed characters are mis-ordered when CPU usage is high

[Orgad Shaneh]

Hi,

I'm using cygwin runtime 3.3.4-2.

When a foreground job is running, and the general CPU usage of the
machine is high, characters are mis-ordered.

To reproduce, I use this script to produce load:
#!/bin/sh

for i in $(seq $(nproc)); do
  while true; do :; done &
done
wait

Run it in the background, then run a long foreground job, and while it
is running, type something.

Example:
$ spin.sh &
$ sleep 3 # While it is running, I quickly typed git status
$ sigt tatus

This reproduces on Windows Terminal and on cmd (Cygwin.bat)

Please CC me on replies, as I'm not registered to the mailing list.

Thanks,
- Orgad

Re: Typed characters are mis-ordered when CPU usage is high

[Takashi Yano]

On Sun, 27 Feb 2022 16:39:24 +0200
Orgad Shaneh wrote:
> Hi,
> 
> I'm using cygwin runtime 3.3.4-2.
> 
> When a foreground job is running, and the general CPU usage of the
> machine is high, characters are mis-ordered.
> 
> To reproduce, I use this script to produce load:
> #!/bin/sh
> 
> for i in $(seq $(nproc)); do
>   while true; do :; done &
> done
> wait
> 
> Run it in the background, then run a long foreground job, and while it
> is running, type something.
> 
> Example:
> $ spin.sh &
> $ sleep 3 # While it is running, I quickly typed git status
> $ sigt tatus
> 
> This reproduces on Windows Terminal and on cmd (Cygwin.bat)

Thanks for the report.

I think this is due to a bug which I recently fixed.
https://cygwin.com/pipermail/cygwin-patches/2022q1/011791.html


-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

Re: Typed characters are mis-ordered when CPU usage is high

[Takashi Yano]

Hi Orgad,

On Sun, 27 Feb 2022 23:53:03 +0900
Takashi Yano wrote:
> On Sun, 27 Feb 2022 16:39:24 +0200
> Orgad Shaneh wrote:
> > Hi,
> > 
> > I'm using cygwin runtime 3.3.4-2.
> > 
> > When a foreground job is running, and the general CPU usage of the
> > machine is high, characters are mis-ordered.
> > 
> > To reproduce, I use this script to produce load:
> > #!/bin/sh
> > 
> > for i in $(seq $(nproc)); do
> >   while true; do :; done &
> > done
> > wait
> > 
> > Run it in the background, then run a long foreground job, and while it
> > is running, type something.
> > 
> > Example:
> > $ spin.sh &
> > $ sleep 3 # While it is running, I quickly typed git status
> > $ sigt tatus
> > 
> > This reproduces on Windows Terminal and on cmd (Cygwin.bat)
> 
> Thanks for the report.
> 
> I think this is due to a bug which I recently fixed.
> https://cygwin.com/pipermail/cygwin-patches/2022q1/011791.html

Now, new developer snapshot is ready: https://cygwin.com/snapshots/
Please try.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

Re: Typed characters are mis-ordered when CPU usage is high

[Orgad Shaneh]

On Wed, Mar 2, 2022 at 1:13 AM Takashi Yano <takashi.yano@nifty.ne.jp> wrote:
>
> Hi Orgad,
>
> On Sun, 27 Feb 2022 23:53:03 +0900
> Takashi Yano wrote:
> > On Sun, 27 Feb 2022 16:39:24 +0200
> > Orgad Shaneh wrote:
> > > Hi,
> > >
> > > I'm using cygwin runtime 3.3.4-2.
> > >
> > > When a foreground job is running, and the general CPU usage of the
> > > machine is high, characters are mis-ordered.
> > >
> > > To reproduce, I use this script to produce load:
> > > #!/bin/sh
> > >
> > > for i in $(seq $(nproc)); do
> > >   while true; do :; done &
> > > done
> > > wait
> > >
> > > Run it in the background, then run a long foreground job, and while it
> > > is running, type something.
> > >
> > > Example:
> > > $ spin.sh &
> > > $ sleep 3 # While it is running, I quickly typed git status
> > > $ sigt tatus
> > >
> > > This reproduces on Windows Terminal and on cmd (Cygwin.bat)
> >
> > Thanks for the report.
> >
> > I think this is due to a bug which I recently fixed.
> > https://cygwin.com/pipermail/cygwin-patches/2022q1/011791.html
>
> Now, new developer snapshot is ready: https://cygwin.com/snapshots/
> Please try.

Tested. Looks much better. Thanks!

Is a release planned soon?

- Orgad

Re: Typed characters are mis-ordered when CPU usage is high

[Takashi Yano]

On Wed, 2 Mar 2022 22:13:30 +0200
Orgad Shaneh wrote:
> On Wed, Mar 2, 2022 at 1:13 AM Takashi Yano <takashi.yano@nifty.ne.jp> wrote:
> >
> > Hi Orgad,
> >
> > On Sun, 27 Feb 2022 23:53:03 +0900
> > Takashi Yano wrote:
> > > On Sun, 27 Feb 2022 16:39:24 +0200
> > > Orgad Shaneh wrote:
> > > > Hi,
> > > >
> > > > I'm using cygwin runtime 3.3.4-2.
> > > >
> > > > When a foreground job is running, and the general CPU usage of the
> > > > machine is high, characters are mis-ordered.
> > > >
> > > > To reproduce, I use this script to produce load:
> > > > #!/bin/sh
> > > >
> > > > for i in $(seq $(nproc)); do
> > > >   while true; do :; done &
> > > > done
> > > > wait
> > > >
> > > > Run it in the background, then run a long foreground job, and while it
> > > > is running, type something.
> > > >
> > > > Example:
> > > > $ spin.sh &
> > > > $ sleep 3 # While it is running, I quickly typed git status
> > > > $ sigt tatus
> > > >
> > > > This reproduces on Windows Terminal and on cmd (Cygwin.bat)
> > >
> > > Thanks for the report.
> > >
> > > I think this is due to a bug which I recently fixed.
> > > https://cygwin.com/pipermail/cygwin-patches/2022q1/011791.html
> >
> > Now, new developer snapshot is ready: https://cygwin.com/snapshots/
> > Please try.
> 
> Tested. Looks much better. Thanks!

Thanks for testing. But what do you mean by 'better'?
Do you mean that the problem still happens in lower probability?

> Is a release planned soon?

This fix was also applied in cygwin-3_3-branch, so the next
release (probabliy 3.3.5) will come with the fix.


-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

Re: Typed characters are mis-ordered when CPU usage is high

[Orgad Shaneh]

On Thu, Mar 3, 2022, 2:02 AM Takashi Yano <takashi.yano@nifty.ne.jp> wrote:

> On Wed, 2 Mar 2022 22:13:30 +0200
> Orgad Shaneh wrote:
> > On Wed, Mar 2, 2022 at 1:13 AM Takashi Yano <takashi.yano@nifty.ne.jp>
> wrote:
> > >
>
> > > Now, new developer snapshot is ready: https://cygwin.com/snapshots/
> > > Please try.
> >
> > Tested. Looks much better. Thanks!
>
> Thanks for testing. But what do you mean by 'better'?
> Do you mean that the problem still happens in lower probability?
>
> No, it's fixed.
>
> > Is a release planned soon?
>
> This fix was also applied in cygwin-3_3-branch, so the next
> release (probably 3.3.5) will come with the fix.
>
> I understand. I meant to ask if there is a planned schedule for the next
> release.
>
> - Orgad

Re: Typed characters are mis-ordered when CPU usage is high

[Orgad Shaneh]

On Thu, Mar 3, 2022 at 2:02 AM Takashi Yano <takashi.yano@nifty.ne.jp> wrote:
>
> On Wed, 2 Mar 2022 22:13:30 +0200
> Orgad Shaneh wrote:
> > On Wed, Mar 2, 2022 at 1:13 AM Takashi Yano <takashi.yano@nifty.ne.jp> wrote:
> > >
> > > Hi Orgad,
> > >
> > > On Sun, 27 Feb 2022 23:53:03 +0900
> > > Takashi Yano wrote:
> > > > On Sun, 27 Feb 2022 16:39:24 +0200
> > > > Orgad Shaneh wrote:
> > > > > Hi,
> > > > >
> > > > > I'm using cygwin runtime 3.3.4-2.
> > > > >
> > > > > When a foreground job is running, and the general CPU usage of the
> > > > > machine is high, characters are mis-ordered.
> > > > >
> > > > > To reproduce, I use this script to produce load:
> > > > > #!/bin/sh
> > > > >
> > > > > for i in $(seq $(nproc)); do
> > > > >   while true; do :; done &
> > > > > done
> > > > > wait
> > > > >
> > > > > Run it in the background, then run a long foreground job, and while it
> > > > > is running, type something.
> > > > >
> > > > > Example:
> > > > > $ spin.sh &
> > > > > $ sleep 3 # While it is running, I quickly typed git status
> > > > > $ sigt tatus
> > > > >
> > > > > This reproduces on Windows Terminal and on cmd (Cygwin.bat)
> > > >
> > > > Thanks for the report.
> > > >
> > > > I think this is due to a bug which I recently fixed.
> > > > https://cygwin.com/pipermail/cygwin-patches/2022q1/011791.html
> > >
> > > Now, new developer snapshot is ready: https://cygwin.com/snapshots/
> > > Please try.
> >
> > Tested. Looks much better. Thanks!
>
> Thanks for testing. But what do you mean by 'better'?
> Do you mean that the problem still happens in lower probability?


After using it on more scenarios (3.3 branch), I see have strange problems.

For example, if I run git log -1 and immediately type, my input is
lost until the prompt appears again.

It doesn't happen with other commands like git status, I'm not sure
why there's a difference.

- Orgad

Re: Typed characters are mis-ordered when CPU usage is high

[Takashi Yano]

On Thu, 10 Mar 2022 15:17:03 +0200
Orgad Shaneh wrote:
> After using it on more scenarios (3.3 branch), I see have strange problems.
> 
> For example, if I run git log -1 and immediately type, my input is
> lost until the prompt appears again.
> 
> It doesn't happen with other commands like git status, I'm not sure
> why there's a difference.

Thanks for the report. This is not a new problem but has been
occurring for many years.

I looked into this problem and found the cause.
This seems to be due to a bug of fsync(). Cygwin's fsync()
flushes the console input buffer unlike linux.

I will propose a patch for this issue.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

Re: Typed characters are mis-ordered when CPU usage is high

[Orgad Shaneh]

On Fri, Mar 11, 2022 at 11:36 PM Takashi Yano <takashi.yano@nifty.ne.jp> wrote:
> I looked into this problem and found the cause.
> This seems to be due to a bug of fsync(). Cygwin's fsync()
> flushes the console input buffer unlike linux.
>
> I will propose a patch for this issue.
>
> --
> Takashi Yano <takashi.yano@nifty.ne.jp>

Thank you very much. Looks better now.

I'm sorry for nudging, but on msys2 I still get frequent mistypes when
typing fast.

I (still) don't have a consistent reproduction, but if I get it
correctly, it looks like one or more characters I type right when the
prompt appears show up before the buffered characters.

For instance, I run git fetch, and while it is running I type git
status, *sometimes* 1-2 characters "pop" to the left, so I get
something like tgit satus.

I wasn't able to reproduce it with cygwin, but on msys2 (with cygwin
3.3 branch merged in) it happens to me all the time :/

- Orgad

Re: Typed characters are mis-ordered when CPU usage is high

[Takashi Yano]

[-- Attachment #1: Type: text/plain, Size: 3676 bytes --]

On Thu, 17 Mar 2022 20:40:15 +0200
Orgad Shaneh wrote:
> On Fri, Mar 11, 2022 at 11:36 PM Takashi Yano <takashi.yano@nifty.ne.jp> wrote:
> > I looked into this problem and found the cause.
> > This seems to be due to a bug of fsync(). Cygwin's fsync()
> > flushes the console input buffer unlike linux.
> >
> > I will propose a patch for this issue.
> >
> > --
> > Takashi Yano <takashi.yano@nifty.ne.jp>
> 
> Thank you very much. Looks better now.
> 
> I'm sorry for nudging, but on msys2 I still get frequent mistypes when
> typing fast.
> 
> I (still) don't have a consistent reproduction, but if I get it
> correctly, it looks like one or more characters I type right when the
> prompt appears show up before the buffered characters.
> 
> For instance, I run git fetch, and while it is running I type git
> status, *sometimes* 1-2 characters "pop" to the left, so I get
> something like tgit satus.
> 
> I wasn't able to reproduce it with cygwin, but on msys2 (with cygwin
> 3.3 branch merged in) it happens to me all the time :/

Thansk for the report.

I cloned the msys2-runtime repository from
https://github.com/msys2/msys2-runtime
and applied patches in cygwin-3_3-branch against msys2-3_3_4-release
branch. The patches applied are listed in cygwin-3_3-branch-merged.log
attached. A few patches, which are not actually in cygwin-3_3-branch,
are also applied just for avoiding conflict easily.

However, I cannot reproduce your problem. Have you surely applied the
following patches especially important for this issue?

Could you please also check if the code of cons_master_thread() in
fhandler_console.cc exactly matches with cons_master_thread.cc attached?

commit 4d0721a66f94427fa14601d0c58a0762709b5bf2
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Fri Feb 25 17:10:03 2022 +0900

    Cygwin: console: Prevent the order of typeahead input from swapped.

    - If a lot of keys are typed very quickly in the app which does
      not read console, the order of input keys in console input buffer
      occasionally swapped. Although this extremely rarely happens,
      is obviously a bug of cons_master_thread. This patch fixes the
      issue.

commit f48870148c7ebcaf8163b08a040d15ef048f4e94
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Mon Feb 28 20:02:01 2022 +0900

    Cygwin: console: Improve the code to avoid typeahead key swapping.

    - The commit "Cygwin: console: Prevent the order of typeahead input
      from swapped." did not fully resolve the issue. If keys are typed
      during input buffer fix, the order of key event may be swapped.
      This patch fixes the issue again.

commit 6e872adb9f153ba0504c898a91ac8dcf0ed7e68a
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Wed Mar 2 09:55:52 2022 +0900

    Cygwin: console: Prevent special keys processing from drop.

    - There was a potential risk to drop special key processing when
      process_input_messsage() is called intermittently. This patch
      fixes the issue.

commit d88768187a72662d1692ee279e7dea3e948d256e
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Wed Mar 2 11:08:27 2022 +0900

    Cygwin: console: Fix a bug from comparison between int and DWORD.

commit cef38f41f2ce0510855bb4b872c7f6514f6deca5
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Wed Mar 2 16:00:31 2022 +0900

    Cygwin: console, pty: Revamp the acquire/release_attach_mutex timing.

    - This patch revises the acquiring/releasing timing for attach_mutex
      to make the period in which it is being acquired shorter. Further,
      acquiring/releasing are added to where they are missing but needed.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

[-- Attachment #2: cygwin-3_3-branch-merged.log --]
[-- Type: application/octet-stream, Size: 20665 bytes --]

commit 190ee356a11424145e56783b1221ade49d216d3e
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Fri Mar 11 17:26:30 2022 +0900

    Cygwin: path: Add fallback for DFS mounted drive.
    
    - If UNC path for DFS is mounted to a drive with drive letter, the
      error "Too many levels of symbolic links" occurs when accessing
      to that drive. This is because GetDosDeviceW() returns unexpected
      string such as "\Device\Mup\DfsClient\;Z:000000000003fb89\dfsserver
      \dfs\linkname" for the mounted UNC path "\??\UNC\fileserver\share".
      This patch adds a workaround for this issue.
    
      Addresses: https://cygwin.com/pipermail/cygwin/2022-March/250979.html

commit d6e75fe45acab6b3f8f461f243a4d00a35e9aca2
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Sat Mar 12 06:19:53 2022 +0900

    Cygwin: fsync: Return EINVAL for special files.
    
    - Unlike linux, fsync() calls FlushFileBuffers() even for special
      files. This causes the problem reported in:
        https://cygwin.com/pipermail/cygwin/2022-March/251022.html
      This patch fixes the issue.

commit 79b252767c4aa481c26a1d013c763c55a19a6362
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Thu Mar 10 20:20:00 2022 +0900

    Cygwin: console, pty: Fix segfault in child_info_spawn::worker().
    
    - After the commit "Cygwin: pty, console: Fix handle leak which
      occurs on exec() error.", startxwin cannot start X due to the
      error "Failed to activate virtual core keyboard: 2". The problem
      is access violation in the code retrieving the pgid of the ctty.
      This patch fixes the issue.
    
      Addresses: https://cygwin.com/pipermail/cygwin/2022-March/251013.html

commit 5614852bee92120d90e2b5c30df5712f7d0710c1
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Sat Mar 5 09:39:26 2022 +0900

    Cygwin: pty: Adopt the variable name to the name generally used.
    
    - Generally, '\n' is called "line feed" (not "new line"), so the
      variable name p_nl has been changed to p_lf.

commit 96d5c2a5455e908a6979970456c818131c6d0153
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Sat Mar 5 08:39:05 2022 +0900

    Cygwin: pty: Add several further comments to the pty code.

commit 7f300a5a717cd13d7562d6d007420bc239a00849
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Fri Mar 4 23:57:12 2022 +0900

    Cygwin: pty: Take account of CR+NL line feed in input.
    
    - Currently, individual CR or NL is treated as line feed in
      accept_input() and transfer_input(). This patch takes account
      of CR+NL as well.

commit c9dea2f5192c7416f3d8f022b71d02bd5cd0374c
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Fri Mar 4 22:02:35 2022 +0900

    Cygwin: pty: Fix a possible race issue in initialization of pcon.
    
    - Currently, tty::pcon_start flag is cleared before transfer_input()
      in master::write(), however, the code in setup_pseudoconsole()
      waits for transfer_input() using tty::pcon_start. This possibly
      causes the race issue. The patch fixes this potential issue.

commit 79153a114e7d0e2b52df2d2be5ffc7972765f4fc
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Fri Mar 4 22:00:24 2022 +0900

    Cygwin: pty: Update some comments in pty code.

commit bf402e9d47658583758e2789cf969e7d8f59c951
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Fri Mar 4 18:54:18 2022 +0900

    Cygwin: pty: Omit transfer_input() call where it is no longer needed.
    
    - This patch removes the old code which calls transfer_input() but
      is no longer needed. These code was necessary indeed in the past,
      however, as a result of recent frequent code changes, it is no
      longer needed.

commit 9490b9c766841b4d7f7007476bf6da90de2a7c9d
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Fri Mar 4 14:42:46 2022 +0900

    Cygwin: pty: Rearrange reset_switch_to_nat_pipe() calls.
    
    - Previously, reset_switch_to_nat_pipe() is called from many places
      in pty code. This patch reorganizes that. With this patch, it is
      called only from bg_check() and setpgid_aux(). The calls which
      does not have enough reason have been omitted.

commit dc8143826b5e5b641824e3b62f30427f29e223e1
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Fri Mar 4 16:05:35 2022 +0900

    Cygwin: pty: Treat both CR and NL as line feed in transfer_inpup().
    
    - To make read() work properly in canonical mode, writing to the pty
      pipe should be done line by line. However, only CR was treated as
      line separator previously in transfer_input(). This patch fixes
      the issue.

commit 9385adbd3d8d1e518c6b261221448b4cae1840f7
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Thu Mar 3 11:43:07 2022 +0900

    Cygwin: pty: Stop to use PID_NEW_PG flag as a marker for GDB.
    
    - Previously, the PID_NEW_PG flag was also used as a marker for GDB
      with non-cygwin inferior, unlike its original meaning. With this
      patch, the condition exec_dwProcessId == dwProcessId is used as a
      marker for that instead.

commit b52943358a36d1dd7dbf209fc4bf056016e7806d
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Thu Mar 3 10:55:12 2022 +0900

    Cygwin: pty: Simplify the setup code for GDB a bit.
    
    - This patch omits the unnecessary code path for setup for GDB.

commit c69f2ae8b137ae13fda1a8bf44ada07df4193e89
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Thu Mar 3 10:38:05 2022 +0900

    Cygwin: pty: Rename nat_pipe_owner_alive() to process_alive().
    
    - The function nat_pipe_owner_alive() is used even for the process
      which is not a nat pipe owner, so, it is renamed to process_alive().

commit 29714cecd6e494144bed647d639b2d3ff7719de8
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Thu Mar 3 04:06:38 2022 +0900

    Cygwin: pty: Add still missing acquire/release_attach_mutex.
    
    - transfer_input() function uses console api, so it should be guarded
      by attach_mutex. However, in most cases, it is missing. This patch
      fixes the issue.

commit 18a96ac4a225cbe1521ef3ed0090251a00885d97
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Wed Mar 2 21:35:40 2022 +0900

    Cygwin: pty: Communalize the code for temporary attach to console.
    
    - This patch communalizes the code for attaching another console
      temporarily and resuming to the original attach state, because
      there were a plurality of similar codes throughout.

commit cef38f41f2ce0510855bb4b872c7f6514f6deca5
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Wed Mar 2 16:00:31 2022 +0900

    Cygwin: console, pty: Revamp the acquire/release_attach_mutex timing.
    
    - This patch revises the acquiring/releasing timing for attach_mutex
      to make the period in which it is being acquired shorter. Further,
      acquiring/releasing are added to where they are missing but needed.

commit d88768187a72662d1692ee279e7dea3e948d256e
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Wed Mar 2 11:08:27 2022 +0900

    Cygwin: console: Fix a bug from comparison between int and DWORD.

commit 6e872adb9f153ba0504c898a91ac8dcf0ed7e68a
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Wed Mar 2 09:55:52 2022 +0900

    Cygwin: console: Prevent special keys processing from drop.
    
    - There was a potential risk to drop special key processing when
      process_input_messsage() is called intermittently. This patch
      fixes the issue.

commit d353c7a1ae057f0813ecdaf0a347a3fbfd121fc3
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Wed Mar 2 08:35:09 2022 +0900

    Cygwin: console: Stop to create struct instance which is not needed.
    
    - In fhandler_console::cons_master_thread(), a struct which has
      only a static function is used. In this case, struct instance
      is not necessary. So with this patch, the static function is
      invoked without creating instance.

commit 39592fd005dbe9638823d253b6e00c74702ff34a
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Tue Mar 1 22:09:01 2022 +0900

    Cygwin: pty: Rename some functions/variables with the name *pcon*.
    
    - With this patch, some pty functions/variables have been renamed
      so that the name *pcon* is not used for those that are called
      even when the pseudo console is not active.

commit a6b2c7055286faede0d86356b15ee24222fa2d82
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Tue Mar 1 18:04:35 2022 +0900

    Cygwin: pty: Reorganize the code path of setting up and closing pcon.
    
    - This patch reorganizes the code path of setting-up and cleaning-up
      of the pseudo console to improve readability and maintainability
      of pty code.

commit a4697a09283fcf53e4e90144fb933b890285ff1d
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Tue Mar 1 11:34:16 2022 +0900

    Cygwin: pty: Avoid cutting the branch the pty master is sitting on.
    
    - When Ctrl-C terminates a non-cygwin process on a pseudo console,
      pty master attaches to the pseudo console first, and send
      CTRL_C_EVENT. If the non-cygwin process closes the pseudo console
      before the pty master calls FreeConsole(), the pty master process
      will crash. With this patch, pty master process takes over the
      ownership of the pseudo console, and closes it by myself.

commit 162e40938dd35cab6c30e3862bc2cae9c4deb0de
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Mon Feb 28 20:25:09 2022 +0900

    Cygwin: pty: Isolate CTRL_C_EVENTs between ptys.
    
    - With this patch, unique invisible consoles are created for each pty
      to isolate CTRL_C_EVENTs between ptys. This is necessary by Ctrl-C
      handling in fhandler_termios::process_sigs() for non-cygwin apps
      started in pty if the pseudo console is disabled.

commit f48870148c7ebcaf8163b08a040d15ef048f4e94
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Mon Feb 28 20:02:01 2022 +0900

    Cygwin: console: Improve the code to avoid typeahead key swapping.
    
    - The commit "Cygwin: console: Prevent the order of typeahead input
      from swapped." did not fully resolve the issue. If keys are typed
      during input buffer fix, the order of key event may be swapped.
      This patch fixes the issue again.

commit 362bf69bd42742bae7bd8bf1c0a37c9ffae78500
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Sat Feb 26 15:13:13 2022 +0900

    Cygwin: pinfo: Fix exit code for non-cygwin apps which reads console.
    
    - The recent commit "Cygwin: pinfo: Fix exit code when non-cygwin app
      exits by Ctrl-C." did not fix enough the issue. If a non-cygwin app
      is reading the console, it will not return STATUS_CONTROL_C_EXIT
      even if it is terminated by Ctrl-C. As a result, the previous patch
      does not take effect.
      This patch solves this issue by setting sigExeced to SIGINT in
      ctrl_c_handler(). In addition, sigExeced will be cleared if the app
      does not terminated within predetermined time period. The reason is
      that the app does not seem to be terminated by the signal sigExeced.

commit 6dc4dc83582af87c4d72ba06533a75ff849edb5b
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Mon Feb 28 07:24:45 2022 +0900

    Cygwin: termios: Ensure detection of GDB inferior in process_sigs().
    
    - In some situations, some cygwin processes might wrongly identified
      as GDB inferior. This patch ensures the detection of GDB inferior.

commit 46828257dc8a199bbb9b81ce0fde06d8bef8234e
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Sun Feb 27 20:40:24 2022 +0900

    Cygwin: termios: Add comments to the code refactored recently.
    
    - This patch adds some comments to the source code whose intent may
      not be clear enough from the code which is refactored recently.

commit af50bcb86f24471142c51c55a9884968abc85f7f
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Sun Feb 27 12:33:08 2022 +0900

    Cygwin: console: Correct the past fix for apps which open pty.
    
    - The commit "Cygwin: console: Fix issues of apps which open pty."
      did not fix the second problem correctly. That commit looked to
      fix the issue, but the actual problem was that ctrl_c_handler()
      should be reregistered *AFTER* FreeConsole()/AttachConsole().
      This patch correct that.

commit d80ecdd5c2b920bb05856b221397ecdc211a4bb2
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Sat Feb 26 23:31:59 2022 +0900

    Cygwin: console: Revert experimental code mixed accidentally.
    
    - The commit "Cygwin: console: Restore CTRL_BREAK_EVENT handling."
      was accidentally mixed with experimental code in exceptions.cc.
      Due to this, non-cygwin app receives CTRL_C_EVENT twice in the
      following scenario.
       1) Run 'sleep 10 | <non-cygwin app>'
       2) Hit Ctrl-C.
       3) The non-cygwin app receives CTRL_C_EVENT twice.
      This patch reverts the code with the problem.

commit 4d0721a66f94427fa14601d0c58a0762709b5bf2
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Fri Feb 25 17:10:03 2022 +0900

    Cygwin: console: Prevent the order of typeahead input from swapped.
    
    - If a lot of keys are typed very quickly in the app which does
      not read console, the order of input keys in console input buffer
      occasionally swapped. Although this extremely rarely happens,
      is obviously a bug of cons_master_thread. This patch fixes the
      issue.

commit 72ac2a7ba3183bd68d36f273c0d3f5c815dbe2e1
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Sat Feb 26 04:19:03 2022 +0900

    Cygwin: pty: Stop to send CTRL_C_EVENT if pcon activated.
    
    - The commit "Cygwin: console: Redesign handling of special keys."
      removes special treatment for pty in with pseudo console activated,
      however, it is necessary on second thought. This is because sending
      CTRL_C_EVENT to non-cygwin apps will be done in pseudo console,
      therefore, sending it in fhandler_pty_master::write() duplicates
      that event for non-cygwin apps.

commit e4007c316b4ecf63a8a282010149e98d920f4fdf
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Fri Feb 25 16:23:58 2022 +0900

    Cygwin: console: Fix issues of apps which open pty.
    
    - After some recent changes for special keys handling break the
      apps which open pty (such as script command). If the app which
      opens pty is executed in console, the following issues occur.
        1) If the script command was started from non-cygwin shell
           (such as cmd.exe), another cygwin app started in pty slave
           cannot receive Ctrl-C.
        2) If non-cygwin app is executed in pty slave, the app which
           opened the pty (e.g. script command) crashes by Ctrl-C.
      This patch fixes these issues.

commit b2f2f38c53a8ad5af540a498fe4f76eaaefd4f66
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Thu Feb 24 21:04:49 2022 +0900

    Cygwin: pinfo: Fix exit code when non-cygwin app exits by Ctrl-C.
    
    - Previously, if non-cygwin app exits by Ctrl-C, exit code was
      0x00007f00. With this patch, the exit code will be 0x00000002,
      which means process exited by SIGINT.

commit 28fa493baeafce2fc272062646d69b0470b4466b
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Thu Feb 24 17:28:32 2022 +0900

    Cygwin: console: Restore CTRL_BREAK_EVENT handling.
    
    - The recent change by the commit "Cygwin: console: Redesign handling
      of special keys." breaks the handling of CTRL_BREAK_EVENT. The login
      shell in console exits on Ctrl-Break key. This patch fixes the issue.

commit 926c019dc314374fb96a3411bd17582854b72ae4
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 6ff7aab4a4e70d20fea85f63bda664a7e8513ebe
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Wed Feb 23 13:24:39 2022 +0900

    Cygwin: console: Redesign handling of special keys.
    
    - This patch rearranges the cooperation between cons_master_thread,
      line_edit, and ctrl_c_handler so that only one of them operates
      at the same time. Since these handle Ctrl-C individually, so the
      signal may be sent multiple times to the process. This patch fixes
      the issue.

commit 8061898334f3842e737206f8f00b0bca4db6fe65
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Mon Feb 21 22:40:48 2022 +0900

    Cygwin: pty: Encapsulate pty code in tty::setpgid().
    
    - This patch hides complex pty codes in tty::setpgid() to transfer
      input into the class fhandler_pty_slave by encapsulating it.

commit ca4dfdb1214a85135a6cdb8a17e5460a459810a4
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Mon Feb 21 21:20:48 2022 +0900

    Cygwin: pty, console: Fix handle leak which occurs on exec() error.
    
    - This patch fixes the handle leak which occurs when exec() fails
      with an error. The duplicated handles will be closed when the
      exec'ed process is terminated. However, if exec() fails, the code
      path does not reach to the code closing the duplicated handles.
      To implement this fix more appropriately, the setup, cleanup and
      closing pty codes which was previously located in spawn.cc are
      encapsulated into the fhandler_pty_slave class functions.

commit 75ef5e6ecb1459d76ac3920c9785e303691f48e9
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Wed Feb 16 15:43:42 2022 +0900

    Cygwin: console: Rearrange set_(in|out)put_mode() calls.
    
    - With this patch, all set_(in|out)put_mode() calls are rearranged
      as follows.
    
      1) Setup for cygwin apps, started from non-cygwin app, is done
         in fhandler_console::post_open_setup(), which overrides
         fhandler_base::post_open_setup() called from dtable.cc.
      2) Cleanup for cygwin app is done in fhandler_console::close().
      3) Setup for cygwin apps is also in fhandler_console::bg_check(),
         which overrides fhandler_termios::bg_check(). This is called
         on read(), write() and select() for console. It is necessary
         if cygwin and non-cygwin apps are started simultaneously in
         the same process group.
      4) Setup for non-cygwin apps is done in spawn.cc via
         fhandler_console::setup_console_for_non_cygwin_app().
      5) Cleanup for non-cygwin app is done in spawn.cc vid
         fhandler_console::cleanup_console_for_non_cygwin_app().
      6) Setup for non-cygwin app started by GDB is done in
         fhandler_console::set_console_mode_to_native().
      7) No explicit cleanup for non-cygwin app started by GDB, because
         console mode is automatically reset to tty::cygwin on read()/
         write() in GDB thanks to 3).

commit 65ca6a6caf39ce8fb522bcd9a752c01a2e4d1410
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 2de3dd14a5b675deb6cda6d6e40ce8896dc6dc03
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Sat Feb 5 16:52:33 2022 +0900

    Cygwin: console: Remove unnecessary (redundant) code.

commit ffa08bd9b015e971e788c44eaf788fff2b894d9d
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Thu Feb 3 21:09:44 2022 +0900

    Cygwin: console: Fix IL/DL escape sequence on the last line.
    
    - Currently, escape sequence IL/DL (CSI Ps L, CSI Ps M) does not
      work correctly at the last (bottom end) line. This patch fixes
      the issue.
    
    Addresses:
      https://cygwin.com/pipermail/cygwin/2022-February/250736.html

commit 3acfcc7b633aab10aa2dca2bc3be25d81c148299
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Thu Feb 3 12:00:14 2022 +0900

    Cygwin: path: Fix UNC path handling for SMB3 mounted to a drive.
    
    - If an UNC path is mounted to a drive using SMB3.11, accessing to
      the drive fails with error "Too many levels of symbolic links."
      This patch fixes the issue.

[-- Attachment #3: cons_master_thread.cc --]
[-- Type: text/x-c++src, Size: 5451 bytes --]

/* This thread processes signals derived from input messages.
   Without this thread, those signals can be handled only when
   the process calls read() or select(). This thread reads input
   records, processes signals and removes corresponding record.
   The other input records are kept back for read() or select(). */
void
fhandler_console::cons_master_thread (handle_set_t *p, tty *ttyp)
{
  const int additional_space = 128; /* Possible max number of incoming events
				       during the process. Additional space
				       should be left for writeback fix. */
  const int inrec_size = INREC_SIZE + additional_space;
  struct m
  {
    inline static size_t bytes (size_t n)
      {
	return sizeof (INPUT_RECORD) * n;
      }
  };
  termios &ti = ttyp->ti;
  while (con.owner == myself->pid)
    {
      DWORD total_read, n, i;
      INPUT_RECORD input_rec[inrec_size];

      if (con.disable_master_thread)
	{
	  cygwait (40);
	  continue;
	}

      WaitForSingleObject (p->input_mutex, mutex_timeout);
      total_read = 0;
      bool nowait = false;
      switch (cygwait (p->input_handle, (DWORD) 0))
	{
	case WAIT_OBJECT_0:
	  acquire_attach_mutex (mutex_timeout);
	  ReadConsoleInputW (p->input_handle,
			     input_rec, INREC_SIZE, &total_read);
	  if (total_read == INREC_SIZE /* Working space full */
	      && cygwait (p->input_handle, (DWORD) 0) == WAIT_OBJECT_0)
	    {
	      const int incr = min (con.num_processed, additional_space);
	      ReadConsoleInputW (p->input_handle,
				 input_rec + total_read, incr, &n);
	      /* Discard oldest n events. */
	      memmove (input_rec, input_rec + n, m::bytes (total_read));
	      con.num_processed -= n;
	      nowait = true;
	    }
	  release_attach_mutex ();
	  break;
	case WAIT_TIMEOUT:
	  con.num_processed = 0;
	case WAIT_SIGNALED:
	case WAIT_CANCELED:
	  break;
	default: /* Error */
	  ReleaseMutex (p->input_mutex);
	  return;
	}
      /* If ENABLE_VIRTUAL_TERMINAL_INPUT is not set, changing
	 window height does not generate WINDOW_BUFFER_SIZE_EVENT.
	 Therefore, check windows size every time here. */
      if (!wincap.has_con_24bit_colors () || con_is_legacy)
	{
	  SHORT y = con.dwWinSize.Y;
	  SHORT x = con.dwWinSize.X;
	  con.fillin (p->output_handle);
	  if (y != con.dwWinSize.Y || x != con.dwWinSize.X)
	    {
	      con.scroll_region.Top = 0;
	      con.scroll_region.Bottom = -1;
	      ttyp->kill_pgrp (SIGWINCH);
	    }
	}
      for (i = con.num_processed; i < total_read; i++)
	{
	  wchar_t wc;
	  char c;
	  bool processed = false;
	  switch (input_rec[i].EventType)
	    {
	    case KEY_EVENT:
	      if (!input_rec[i].Event.KeyEvent.bKeyDown)
		continue;
	      wc = input_rec[i].Event.KeyEvent.uChar.UnicodeChar;
	      if (!wc || (wint_t) wc >= 0x80)
		continue;
	      c = (char) wc;
	      switch (process_sigs (c, ttyp, NULL))
		{
		case signalled:
		case not_signalled_but_done:
		case done_with_debugger:
		  processed = true;
		  ttyp->output_stopped = false;
		  if (ti.c_lflag & NOFLSH)
		    goto remove_record;
		  con.num_processed = 0;
		  goto skip_writeback;
		default: /* not signalled */
		  break;
		}
	      processed = process_stop_start (c, ttyp);
	      break;
	    case WINDOW_BUFFER_SIZE_EVENT:
	      SHORT y = con.dwWinSize.Y;
	      SHORT x = con.dwWinSize.X;
	      con.fillin (p->output_handle);
	      if (y != con.dwWinSize.Y || x != con.dwWinSize.X)
		{
		  con.scroll_region.Top = 0;
		  con.scroll_region.Bottom = -1;
		  if (wincap.has_con_24bit_colors () && !con_is_legacy)
		    fix_tab_position (p->output_handle);
		  ttyp->kill_pgrp (SIGWINCH);
		}
	      processed = true;
	      break;
	    }
remove_record:
	  if (processed)
	    { /* Remove corresponding record. */
	      if (total_read > i + 1)
		memmove (input_rec + i, input_rec + i + 1,
			 m::bytes (total_read - i - 1));
	      total_read--;
	      i--;
	    }
	}
      con.num_processed = total_read;
      if (total_read)
	{
	  do
	    {
	      INPUT_RECORD tmp[inrec_size];
	      /* Writeback input records other than interrupt. */
	      acquire_attach_mutex (mutex_timeout);
	      WriteConsoleInputW (p->input_handle, input_rec, total_read, &n);
	      /* Check if writeback was successfull. */
	      PeekConsoleInputW (p->input_handle, tmp, inrec_size, &n);
	      release_attach_mutex ();
	      if (n < total_read)
		break; /* Someone has read input without acquiring
			  input_mutex. ConEmu cygwin-connector? */
	      if (memcmp (input_rec, tmp, m::bytes (total_read)) == 0)
		break; /* OK */
	      /* Try to fix */
	      DWORD incr = n - total_read;
	      DWORD ofst;
	      for (ofst = 1; ofst <= incr; ofst++)
		if (memcmp (input_rec, tmp + ofst, m::bytes (total_read)) == 0)
		  {
		    acquire_attach_mutex (mutex_timeout);
		    ReadConsoleInputW (p->input_handle, tmp, inrec_size, &n);
		    release_attach_mutex ();
		    memcpy (input_rec, tmp + ofst, m::bytes (total_read));
		    memcpy (input_rec + total_read, tmp, m::bytes (ofst));
		    if (n > ofst + total_read)
		      memcpy (input_rec + total_read + ofst,
			      tmp + ofst + total_read,
			      m::bytes (n - (ofst + total_read)));
		    total_read = n;
		    break;
		  }
	      if (ofst > incr)
		break; /* Writeback was not atomic. Or someone has read
			  input without acquiring input_mutex.
			  Giving up because hard to fix. */
	    }
	  while (true);
	}
skip_writeback:
      ReleaseMutex (p->input_mutex);
      if (!nowait)
	cygwait (40);
    }
}

Re: Typed characters are mis-ordered when CPU usage is high

[Takashi Yano]

On Fri, 18 Mar 2022 13:23:35 +0900
Takashi Yano <takashi.yano@nifty.ne.jp> wrote:

> On Thu, 17 Mar 2022 20:40:15 +0200
> Orgad Shaneh wrote:
> > On Fri, Mar 11, 2022 at 11:36 PM Takashi Yano <takashi.yano@nifty.ne.jp> wrote:
> > > I looked into this problem and found the cause.
> > > This seems to be due to a bug of fsync(). Cygwin's fsync()
> > > flushes the console input buffer unlike linux.
> > >
> > > I will propose a patch for this issue.
> > >
> > > --
> > > Takashi Yano <takashi.yano@nifty.ne.jp>
> > 
> > Thank you very much. Looks better now.
> > 
> > I'm sorry for nudging, but on msys2 I still get frequent mistypes when
> > typing fast.
> > 
> > I (still) don't have a consistent reproduction, but if I get it
> > correctly, it looks like one or more characters I type right when the
> > prompt appears show up before the buffered characters.
> > 
> > For instance, I run git fetch, and while it is running I type git
> > status, *sometimes* 1-2 characters "pop" to the left, so I get
> > something like tgit satus.
> > 
> > I wasn't able to reproduce it with cygwin, but on msys2 (with cygwin
> > 3.3 branch merged in) it happens to me all the time :/
> 
> Thansk for the report.
> 
> I cloned the msys2-runtime repository from
> https://github.com/msys2/msys2-runtime
> and applied patches in cygwin-3_3-branch against msys2-3_3_4-release
> branch. The patches applied are listed in cygwin-3_3-branch-merged.log
> attached. A few patches, which are not actually in cygwin-3_3-branch,
> are also applied just for avoiding conflict easily.
> 
> However, I cannot reproduce your problem. Have you surely applied the
> following patches especially important for this issue?
> 
> Could you please also check if the code of cons_master_thread() in
> fhandler_console.cc exactly matches with cons_master_thread.cc attached?

I also tried merge-3.3 branch from
https://github.com/orgads/msys2-runtime-1 (perhaps it's your repository),
and confirmed it works without the issue. What is the difference?

I tried the following steps.

1) Start cmd.exe (command prompt).
2) Run \msys64\usr\bin\bash -l
3) cd msys2-runtime
4) git fetch
5) Quickly type "git status" before the shell prompt is shown.
6) Repeat 4) and 5) many times.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

Re: Typed characters are mis-ordered when CPU usage is high

[Orgad Shaneh]

On Fri, Mar 18, 2022 at 7:22 AM Takashi Yano <takashi.yano@nifty.ne.jp> wrote:
>
> On Fri, 18 Mar 2022 13:23:35 +0900
> Takashi Yano <takashi.yano@nifty.ne.jp> wrote:
>
> > On Thu, 17 Mar 2022 20:40:15 +0200
> > Orgad Shaneh wrote:
> > > On Fri, Mar 11, 2022 at 11:36 PM Takashi Yano <takashi.yano@nifty.ne.jp> wrote:
> > > > I looked into this problem and found the cause.
> > > > This seems to be due to a bug of fsync(). Cygwin's fsync()
> > > > flushes the console input buffer unlike linux.
> > > >
> > > > I will propose a patch for this issue.
> > > >
> > > > --
> > > > Takashi Yano <takashi.yano@nifty.ne.jp>
> > >
> > > Thank you very much. Looks better now.
> > >
> > > I'm sorry for nudging, but on msys2 I still get frequent mistypes when
> > > typing fast.
> > >
> > > I (still) don't have a consistent reproduction, but if I get it
> > > correctly, it looks like one or more characters I type right when the
> > > prompt appears show up before the buffered characters.
> > >
> > > For instance, I run git fetch, and while it is running I type git
> > > status, *sometimes* 1-2 characters "pop" to the left, so I get
> > > something like tgit satus.
> > >
> > > I wasn't able to reproduce it with cygwin, but on msys2 (with cygwin
> > > 3.3 branch merged in) it happens to me all the time :/
> >
> > Thansk for the report.
> >
> > I cloned the msys2-runtime repository from
> > https://github.com/msys2/msys2-runtime
> > and applied patches in cygwin-3_3-branch against msys2-3_3_4-release
> > branch. The patches applied are listed in cygwin-3_3-branch-merged.log
> > attached. A few patches, which are not actually in cygwin-3_3-branch,
> > are also applied just for avoiding conflict easily.
> >
> > However, I cannot reproduce your problem. Have you surely applied the
> > following patches especially important for this issue?
> >
> > Could you please also check if the code of cons_master_thread() in
> > fhandler_console.cc exactly matches with cons_master_thread.cc attached?
>
> I also tried merge-3.3 branch from
> https://github.com/orgads/msys2-runtime-1 (perhaps it's your repository),
> and confirmed it works without the issue. What is the difference?
>
> I tried the following steps.
>
> 1) Start cmd.exe (command prompt).
> 2) Run \msys64\usr\bin\bash -l
> 3) cd msys2-runtime
> 4) git fetch
> 5) Quickly type "git status" before the shell prompt is shown.
> 6) Repeat 4) and 5) many times.

I'm using the dll that was built here:
https://github.com/git-for-windows/msys2-runtime/actions/runs/2000018081
for my PR: https://github.com/git-for-windows/msys2-runtime/pull/43

I can reproduce in msys2-runtime repo, but it's quite hard. Try this:
1. git fetch
2. While it is running, write git and then very quickly type
ququququququququququququququququ until the prompt appears.

Sometimes you'll get q or u before git.

- Orgad

Re: Typed characters are mis-ordered when CPU usage is high

[Takashi Yano]

On Fri, 18 Mar 2022 07:57:27 +0200
Orgad Shaneh wrote:
> I'm using the dll that was built here:
> https://github.com/git-for-windows/msys2-runtime/actions/runs/2000018081
> for my PR: https://github.com/git-for-windows/msys2-runtime/pull/43
> 
> I can reproduce in msys2-runtime repo, but it's quite hard. Try this:
> 1. git fetch
> 2. While it is running, write git and then very quickly type
> ququququququququququququququququ until the prompt appears.
> 
> Sometimes you'll get q or u before git.

Do you run git in Git for Windows? or msys2 git?

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

Re: Typed characters are mis-ordered when CPU usage is high

[Orgad Shaneh]

On Fri, Mar 18, 2022, 8:31 AM Takashi Yano <takashi.yano@nifty.ne.jp> wrote:

> On Fri, 18 Mar 2022 07:57:27 +0200
> Orgad Shaneh wrote:
> > I'm using the dll that was built here:
> > https://github.com/git-for-windows/msys2-runtime/actions/runs/2000018081
> > for my PR: https://github.com/git-for-windows/msys2-runtime/pull/43
> >
> > I can reproduce in msys2-runtime repo, but it's quite hard. Try this:
> > 1. git fetch
> > 2. While it is running, write git and then very quickly type
> > ququququququququququququququququ until the prompt appears.
> >
> > Sometimes you'll get q or u before git.
>
> Do you run git in Git for Windows? or msys2 git?
>

Git for Windows

>

Re: Typed characters are mis-ordered when CPU usage is high

[Orgad Shaneh]

On Fri, Mar 18, 2022 at 8:32 AM Orgad Shaneh <orgads@gmail.com> wrote:
>
> On Fri, Mar 18, 2022, 8:31 AM Takashi Yano <takashi.yano@nifty.ne.jp> wrote:
>>
>> On Fri, 18 Mar 2022 07:57:27 +0200
>> Orgad Shaneh wrote:
>> > I'm using the dll that was built here:
>> > https://github.com/git-for-windows/msys2-runtime/actions/runs/2000018081
>> > for my PR: https://github.com/git-for-windows/msys2-runtime/pull/43
>> >
>> > I can reproduce in msys2-runtime repo, but it's quite hard. Try this:
>> > 1. git fetch
>> > 2. While it is running, write git and then very quickly type
>> > ququququququququququququququququ until the prompt appears.
>> >
>> > Sometimes you'll get q or u before git.
>>
>> Do you run git in Git for Windows? or msys2 git?
>
>
> Git for Windows

Were you able to reproduce?

I found an easier way to reproduce, which works almost every time.

It still happens only on Git Bash, and not on MSYS2 MINGW64, although
I use the same dll in both. I have no idea why there's a difference.
:/

I run Windows Terminal, but it reproduces also in cmd, as you tried.

1. In Control Panel -> Keyboard, set Repeat delay to shortest and
Repeat rate to fastest.
2. In msys2-runtime run git fetch
3. Type git and press and hold q

- Orgad

Re: Typed characters are mis-ordered when CPU usage is high

[Takashi Yano]

[-- Attachment #1: Type: text/plain, Size: 1138 bytes --]

On Fri, 18 Mar 2022 13:21:00 +0200
Orgad Shaneh wrote:
> > Git for Windows
> 
> Were you able to reproduce?
> 
> I found an easier way to reproduce, which works almost every time.
> 
> It still happens only on Git Bash, and not on MSYS2 MINGW64, although
> I use the same dll in both. I have no idea why there's a difference.
> :/
> 
> I run Windows Terminal, but it reproduces also in cmd, as you tried.
> 
> 1. In Control Panel -> Keyboard, set Repeat delay to shortest and
> Repeat rate to fastest.
> 2. In msys2-runtime run git fetch
> 3. Type git and press and hold q

Thanks. I can reproduce the issue. I think I found the cause.
The two unexpected things happen.

(1) wVirtualKeyCode and wVirtualScanCode of readback key event may
    be null'ed even if they are not zero on WriteConsoleInputW().
    Therefore, memcmp() report the event is not equal.
(2) WriteConsoleInputW() may not be atomic. The event sequence which
    is written by WriteConsoleInputW() may be inserted by key input
    in the middle of the sequence.

A patch for these issues is attached. Could you please test?

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

[-- Attachment #2: prevent-swap.patch --]
[-- Type: application/octet-stream, Size: 2884 bytes --]

diff --git a/winsup/cygwin/fhandler_console.cc b/winsup/cygwin/fhandler_console.cc
index adac69e89..44328cb6f 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
@@ -180,6 +180,27 @@ cons_master_thread (VOID *arg)
   return 0;
 }
 
+static inline bool
+inrec_eq (const INPUT_RECORD *a, const INPUT_RECORD *b, DWORD n)
+{
+  for (DWORD i = 0; i < n; i++)
+    {
+      if (a[i].EventType == KEY_EVENT && b[i].EventType == KEY_EVENT)
+	{ /* wVirtualKeyCode and wVirtualScanCode of the readback
+	     key event may be different from that of written event. */
+	  const KEY_EVENT_RECORD *ak = &a[i].Event.KeyEvent;
+	  const KEY_EVENT_RECORD *bk = &b[i].Event.KeyEvent;
+	  if (ak->bKeyDown != bk->bKeyDown
+	      || ak->uChar.UnicodeChar != bk->uChar.UnicodeChar
+	      || ak->dwControlKeyState != bk->dwControlKeyState)
+	    return false;
+	}
+      else if (memcmp (a + i, b + i, sizeof (INPUT_RECORD)) != 0)
+	return false;
+    }
+  return true;
+}
+
 /* This thread processes signals derived from input messages.
    Without this thread, those signals can be handled only when
    the process calls read() or select(). This thread reads input
@@ -327,30 +348,26 @@ remove_record:
 	      if (n < total_read)
 		break; /* Someone has read input without acquiring
 			  input_mutex. ConEmu cygwin-connector? */
-	      if (memcmp (input_rec, tmp, m::bytes (total_read)) == 0)
+	      if (inrec_eq (input_rec, tmp, total_read))
 		break; /* OK */
 	      /* Try to fix */
-	      DWORD incr = n - total_read;
-	      DWORD ofst;
-	      for (ofst = 1; ofst <= incr; ofst++)
-		if (memcmp (input_rec, tmp + ofst, m::bytes (total_read)) == 0)
+	      acquire_attach_mutex (mutex_timeout);
+	      ReadConsoleInputW (p->input_handle, tmp, inrec_size, &n);
+	      release_attach_mutex ();
+	      for (DWORD i = 0, j = 0; j < n; j++)
+		if (i == total_read
+		    || !inrec_eq (input_rec + i, tmp + j, 1))
 		  {
-		    acquire_attach_mutex (mutex_timeout);
-		    ReadConsoleInputW (p->input_handle, tmp, inrec_size, &n);
-		    release_attach_mutex ();
-		    memcpy (input_rec, tmp + ofst, m::bytes (total_read));
-		    memcpy (input_rec + total_read, tmp, m::bytes (ofst));
-		    if (n > ofst + total_read)
-		      memcpy (input_rec + total_read + ofst,
-			      tmp + ofst + total_read,
-			      m::bytes (n - (ofst + total_read)));
-		    total_read = n;
-		    break;
+		    if (total_read + j - i >= n)
+		      { /* Something is wrong... Giving up. */
+			WriteConsoleInputW (p->input_handle, tmp, n, &n);
+			goto skip_writeback;
+		      }
+		    input_rec[total_read + j - i] = tmp[j];
 		  }
-	      if (ofst > incr)
-		break; /* Writeback was not atomic. Or someone has read
-			  input without acquiring input_mutex.
-			  Giving up because hard to fix. */
+		else
+		  i++;
+	      total_read = n;
 	    }
 	  while (true);
 	}

Re: Typed characters are mis-ordered when CPU usage is high

[Orgad Shaneh]

On Fri, Mar 18, 2022 at 2:15 PM Takashi Yano <takashi.yano@nifty.ne.jp> wrote:
>
> On Fri, 18 Mar 2022 13:21:00 +0200
> Orgad Shaneh wrote:
> > > Git for Windows
> >
> > Were you able to reproduce?
> >
> > I found an easier way to reproduce, which works almost every time.
> >
> > It still happens only on Git Bash, and not on MSYS2 MINGW64, although
> > I use the same dll in both. I have no idea why there's a difference.
> > :/
> >
> > I run Windows Terminal, but it reproduces also in cmd, as you tried.
> >
> > 1. In Control Panel -> Keyboard, set Repeat delay to shortest and
> > Repeat rate to fastest.
> > 2. In msys2-runtime run git fetch
> > 3. Type git and press and hold q
>
> Thanks. I can reproduce the issue. I think I found the cause.
> The two unexpected things happen.
>
> (1) wVirtualKeyCode and wVirtualScanCode of readback key event may
>     be null'ed even if they are not zero on WriteConsoleInputW().
>     Therefore, memcmp() report the event is not equal.
> (2) WriteConsoleInputW() may not be atomic. The event sequence which
>     is written by WriteConsoleInputW() may be inserted by key input
>     in the middle of the sequence.
>
> A patch for these issues is attached. Could you please test?
>
> --
> Takashi Yano <takashi.yano@nifty.ne.jp>

Awesome, looks good now. Thank you very much!

Hope this is the last bit :)

- Orgad

Re: Typed characters are mis-ordered when CPU usage is high

[Takashi Yano]

On Fri, 18 Mar 2022 15:04:31 +0200
Orgad Shaneh wrote:
> On Fri, Mar 18, 2022 at 2:15 PM Takashi Yano <takashi.yano@nifty.ne.jp> wrote:
> > Thanks. I can reproduce the issue. I think I found the cause.
> > The two unexpected things happen.
> >
> > (1) wVirtualKeyCode and wVirtualScanCode of readback key event may
> >     be null'ed even if they are not zero on WriteConsoleInputW().
> >     Therefore, memcmp() report the event is not equal.
> > (2) WriteConsoleInputW() may not be atomic. The event sequence which
> >     is written by WriteConsoleInputW() may be inserted by key input
> >     in the middle of the sequence.
> >
> > A patch for these issues is attached. Could you please test?
> 
> Awesome, looks good now. Thank you very much!
> 
> Hope this is the last bit :)

Thanks for testing!

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

Re: Typed characters are mis-ordered when CPU usage is high

[n952162]

On 3/17/22 19:40, Orgad Shaneh wrote:
> On Fri, Mar 11, 2022 at 11:36 PM Takashi Yano <takashi.yano@nifty.ne.jp> wrote:
>> I looked into this problem and found the cause.
>> This seems to be due to a bug of fsync(). Cygwin's fsync()
>> flushes the console input buffer unlike linux.
>>
>> I will propose a patch for this issue.
>>
>> --
>> Takashi Yano <takashi.yano@nifty.ne.jp>
> Thank you very much. Looks better now.
>
> I'm sorry for nudging, but on msys2 I still get frequent mistypes when
> typing fast.
>

I guess we're just going to have to change the keyboard layout again....

Re: Typed characters are mis-ordered when CPU usage is high

[Kaz Kylheku (Cygwin)]

On 2022-02-27 06:53, Takashi Yano wrote:
> I think this is due to a bug which I recently fixed.
> https://cygwin.com/pipermail/cygwin-patches/2022q1/011791.html

Was that upstreamed? Sorry to be a pain, but something in the
log message about the cause and the fix (not just the manifested
issue being addressed) would be good.

Reversed characters might mess not only with the user's input,
but with input coming from terminal emulation, like escape
sequences, which may be a worse problem.

Re: Typed characters are mis-ordered when CPU usage is high

[Takashi Yano]

On Wed, 02 Mar 2022 12:28:47 -0800
"Kaz Kylheku wrote:
> On 2022-02-27 06:53, Takashi Yano wrote:
> > I think this is due to a bug which I recently fixed.
> > https://cygwin.com/pipermail/cygwin-patches/2022q1/011791.html
> 
> Was that upstreamed? Sorry to be a pain, but something in the
> log message about the cause and the fix (not just the manifested
> issue being addressed) would be good.
> 
> Reversed characters might mess not only with the user's input,
> but with input coming from terminal emulation, like escape
> sequences, which may be a worse problem.

What do you mean by upstream? The patch was applied the current
cygwin git head (master) and cygwin-3_3-branch. This bug only
affected to the key typed input (user key typing) in console
(e.g. command prompt or Windows Termainal).

If you mean output to the terminal by "input coming from terminal
emulation", that is not affected. If you mean returned escape
sequence such as 'ESC [ 24;1R' against 'ESC [6n', this should not
be affected as well.

The problem was that the console input buffer:
https://docs.microsoft.com/en-us/windows/console/console-input-buffer
could be messed up by the conflict between WriteConsoleInputW()
called in console code (cons_master_thread()) and the keyboard
events comes from user key typing. This has been fixed by checking
the order of the input events in console input buffer after
WriteConsoleInputW() and reordering that if the older events are
placed after newer events.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>