Re: [PATCH 2/3] Cygwin: console: Fix NOFLSH mode a little

Hi Takashi, Mark,

I have a fix for the issue below sitting in
https://github.com/git-for-windows/msys2-runtime/pull/131/commits/0b0976a5e85de52312c17d21f1d3fc41dc572179
that I should have sent earlier, but I was struggling to find the time to
validate the fix via automated AutoHotKey-based tests. Sorry for the
delay.

On Thu, 11 Jun 2026, Takashi Yano wrote:

> If you run "stty noflsh; cat" in "bash", and stop "cat" by Ctrl-C,
> a stray ^C is passed to "bash". The current code calls tcflush() if
> NOFLSH is not set, however, tcflush() is not called when NOFLSH is
> set. So, Ctrl-C remains in console input buffer. This should be
> discarded even in NOFLSH mode. This patch introduces a helper
> function discard_key_events() and call it to erase Ctrl-C in the
> console input buffer.
> 
> Note that even with this patch, NOFLSH is not fully functional in
> console because the readahead buffer is unique to process, so it
> cannot be inherited to other processes. However, it should work
> intra process.
> 
> Fixes: 118e51be1d04 ("(tty_min::kill_pgrp): Handle tty flush when signal detected.")
> Signed-off-by: Takashi Yano <[email protected]>
> Reviewed-by:
> ---
>  winsup/cygwin/fhandler/console.cc       | 20 +++++++++++++++++---
>  winsup/cygwin/fhandler/termios.cc       | 10 +++++++---
>  winsup/cygwin/local_includes/fhandler.h |  2 ++
>  3 files changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/winsup/cygwin/fhandler/console.cc b/winsup/cygwin/fhandler/console.cc
> index a5e6cd89d..9ac492980 100644
> --- a/winsup/cygwin/fhandler/console.cc
> +++ b/winsup/cygwin/fhandler/console.cc
> @@ -1744,17 +1744,31 @@ out:
>      discard_len = 0;
>    if (discard_len)
>      {
> -      DWORD discarded;
>        acquire_attach_mutex (mutex_timeout);
>        DWORD resume_pid = attach_console (con.owner);
> -      ReadConsoleInputW (get_handle (), input_rec, discard_len, &discarded);
> +      discard_key_events (discard_len);
>        detach_console (resume_pid, con.owner);
>        release_attach_mutex ();
> -      con.num_processed -= min (con.num_processed, discarded);
>      }
>    return stat;
>  }
>  
> +void
> +fhandler_console::discard_key_events (size_t n)
> +{
> +  DWORD discarded = 0;
> +  INPUT_RECORD input_rec[INREC_SIZE];
> +  DWORD n1 = min (INREC_SIZE, n);
> +  while (n)
> +    {
> +      ReadConsoleInputW (get_handle (), input_rec, n1, &n1);
> +      n -= n1;
> +      discarded += n1;
> +      n1 = min (INREC_SIZE, n);
> +    }
> +  con.num_processed -= min (con.num_processed, discarded);
> +}

`discard_key_events()` loops on `ReadConsoleInputW()` until it has
consumed the requested count, and `ReadConsoleInputW()` blocks while the
input buffer is empty. The `sigflush()` caller passes a hard-coded `1`
with no guarantee that a record is actually queued: in the master-thread
path the signalling record has already been read out of the buffer before
`sigflush()` runs, so the call blocks until the user's next keystroke
arrives, and then swallows it. And because `ReadConsoleInputW()`'s return
is unchecked, a failed read leaves `n1` indeterminate, so `n -= n1` can
underflow and spin.

The fix is to consume only what `GetNumberOfConsoleInputEvents()` reports
as currently queued (so the helper never waits for input that is not
there) and to bail on a failed or zero-length read (so it cannot
underflow).

One separate point worth flagging: the `process_input_message()` caller
wraps `discard_key_events()` in `acquire_attach_mutex()` + `attach_console
(con.owner)`, but the `sigflush()` call site does not, so the
`ReadConsoleInputW()` there runs against whatever console the calling
process happens to be attached to. With the guard above the worst case is
a no-op when the calling process happens not to be attached, but it would
be more correct to wrap the `sigflush()` discard the same way, or to move
the attach into the helper itself.

The fix I propose is:

-- snip --
From 0b0976a5e85de52312c17d21f1d3fc41dc572179 Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <[email protected]>
Date: Thu, 25 Jun 2026 13:41:44 +0200
Subject: [PATCH] Cygwin: console: do not block or spin in discard_key_events()

discard_key_events(), added in "Cygwin: console: Fix NOFLSH behaviour a
bit", loops on ReadConsoleInputW() until it has consumed the requested
number of records, but ReadConsoleInputW() blocks while the console
input buffer is empty. sigflush() calls it with a hard-coded count of
one and no guarantee that a record is actually queued: in the
master-thread path the signalling record has already been read out of
the buffer before sigflush() runs, so the call blocks until, and then
swallows, the user's next keystroke. And because the ReadConsoleInputW()
return value is unchecked, a failed read leaves the count indeterminate,
so "n -= n1" can underflow and spin.

Consume only what GetNumberOfConsoleInputEvents() reports as queued, so
the helper never waits for input that is not there, and bail out on a
failed or zero-length read so it cannot underflow.

Fixes: 56dfa4db988c ("Cygwin: console: Fix NOFLSH behaviour a bit")
Assisted-by: Opus 4.8
Signed-off-by: Johannes Schindelin <[email protected]>
---
 winsup/cygwin/fhandler/console.cc | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/winsup/cygwin/fhandler/console.cc b/winsup/cygwin/fhandler/console.cc
index 685e99d62c..4949d0494c 100644
--- a/winsup/cygwin/fhandler/console.cc
+++ b/winsup/cygwin/fhandler/console.cc
@@ -1702,13 +1702,18 @@ fhandler_console::discard_key_events (size_t n)
 {
   DWORD discarded = 0;
   INPUT_RECORD input_rec[INREC_SIZE];
-  DWORD n1 = min (INREC_SIZE, n);
   while (n)
     {
-      ReadConsoleInputW (get_handle (), input_rec, n1, &n1);
+      /* Only ever consume events that are actually queued, so this never
+	 blocks waiting for the user's next keystroke. */
+      DWORD avail = 0;
+      if (!GetNumberOfConsoleInputEvents (get_handle (), &avail) || !avail)
+	break;
+      DWORD n1 = min (min ((DWORD) INREC_SIZE, (DWORD) n), avail);
+      if (!ReadConsoleInputW (get_handle (), input_rec, n1, &n1) || !n1)
+	break;
       n -= n1;
       discarded += n1;
-      n1 = min (INREC_SIZE, n);
     }
   con.num_processed -= min (con.num_processed, discarded);
 }
-- snap --

Ciao,
Johannes

> +
>  bool
>  dev_console::fillin (HANDLE h)
>  {
> diff --git a/winsup/cygwin/fhandler/termios.cc b/winsup/cygwin/fhandler/termios.cc
> index ca5fa4b7e..605258731 100644
> --- a/winsup/cygwin/fhandler/termios.cc
> +++ b/winsup/cygwin/fhandler/termios.cc
> @@ -666,9 +666,13 @@ fhandler_termios::sigflush ()
>       be NULL while this is alive.  However, we can conceivably close a
>       ctty while exiting and that will zero this. */
>    if ((!have_execed || have_execed_cygwin) && tc ()
> -      && (tc ()->getpgid () == myself->pgid)
> -      && !(tc ()->ti.c_lflag & NOFLSH))
> -    tcflush (TCIFLUSH);
> +      && (tc ()->getpgid () == myself->pgid))
> +    {
> +      if (!(tc ()->ti.c_lflag & NOFLSH))
> +	tcflush (TCIFLUSH);
> +      else
> +	discard_key_events (1);
> +    }
>  }
>  
>  pid_t
> diff --git a/winsup/cygwin/local_includes/fhandler.h b/winsup/cygwin/local_includes/fhandler.h
> index 4f5605524..49e0e7983 100644
> --- a/winsup/cygwin/local_includes/fhandler.h
> +++ b/winsup/cygwin/local_includes/fhandler.h
> @@ -1983,6 +1983,7 @@ class fhandler_termios: public fhandler_base
>    pid_t tcgetsid ();
>    virtual int fstat (struct stat *buf);
>    int tcflow (int);
> +  virtual void discard_key_events (size_t n) {}
>  
>    fhandler_termios (void *) {}
>  
> @@ -2363,6 +2364,7 @@ private:
>    void wpbuf_put (char c);
>    void wpbuf_send ();
>    int fstat (struct stat *buf);
> +  void discard_key_events (size_t n);
>  
>    class console_unit
>    {
> -- 
> 2.51.0
> 
>