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

Hi Johannes,

Thanks for finding that.

On Sat, 27 Jun 2026 09:27:12 +0200 (CEST)
Johannes Schindelin wrote:
> 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 root cause of above situation is that the master-thread is not
disable even when line_edit() is called via read(). This may cause
other issues we have not seen yet. Usually, input_mutex is held by
the thread calling line_edit(), so the master-thread does not touch
the input events. However, the current code calls discard_key_events()
after release_input_mutex_if_necessary().

So, what about the patch (1/2) attached instead?

> 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.

Yeah, you are right. Then, what about the patch (2/2) attached?

Sorry for no commit message for both patches yet.

-- 
Takashi Yano <[email protected]>

From 47377a1e286de55ea5b24529e35afdad6c747ea4 Mon Sep 17 00:00:00 2001
From: Takashi Yano <[email protected]>
Date: Mon, 29 Jun 2026 23:22:05 +0900
Subject: [PATCH 1/2] Cygwin: console: discard_key_events patch (1/2)

---
 winsup/cygwin/fhandler/console.cc |  3 ++-
 winsup/cygwin/fhandler/termios.cc | 12 +++++-------
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/winsup/cygwin/fhandler/console.cc b/winsup/cygwin/fhandler/console.cc
index 730bb0b45..ccd087f92 100644
--- a/winsup/cygwin/fhandler/console.cc
+++ b/winsup/cygwin/fhandler/console.cc
@@ -1770,7 +1770,8 @@ fhandler_console::discard_key_events (size_t n)
   DWORD n1 = min (INREC_SIZE, n);
   while (n)
     {
-      ReadConsoleInputW (get_handle (), input_rec, n1, &n1);
+      if (!ReadConsoleInputW (get_handle (), input_rec, n1, &n1) || !n1)
+	break;
       n -= n1;
       discarded += n1;
       n1 = min (INREC_SIZE, n);
diff --git a/winsup/cygwin/fhandler/termios.cc b/winsup/cygwin/fhandler/termios.cc
index 605258731..a2b14d14b 100644
--- a/winsup/cygwin/fhandler/termios.cc
+++ b/winsup/cygwin/fhandler/termios.cc
@@ -449,6 +449,8 @@ fhandler_termios::process_sigs (char c, tty* ttyp, fhandler_termios *fh)
 	  fh->eat_readahead (-1);
 	  fh->discard_input ();
 	}
+      if (is_flush_sig (sig) && cygheap->ctty)
+	cygheap->ctty->discard_key_events (1);
       if (fh)
 	fh->release_input_mutex_if_necessary ();
       ttyp->kill_pgrp (sig, pgid);
@@ -666,13 +668,9 @@ 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))
-    {
-      if (!(tc ()->ti.c_lflag & NOFLSH))
-	tcflush (TCIFLUSH);
-      else
-	discard_key_events (1);
-    }
+      && (tc ()->getpgid () == myself->pgid)
+      && !(tc ()->ti.c_lflag & NOFLSH))
+    tcflush (TCIFLUSH);
 }
 
 pid_t
-- 
2.51.0

From 6b8f413c29345a902d2cd23bd9798597d5cbb548 Mon Sep 17 00:00:00 2001
From: Takashi Yano <[email protected]>
Date: Mon, 29 Jun 2026 23:24:42 +0900
Subject: [PATCH 2/2] Cgywin: console: discard_key_events patch (2/2)

---
 winsup/cygwin/fhandler/console.cc | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/winsup/cygwin/fhandler/console.cc b/winsup/cygwin/fhandler/console.cc
index ccd087f92..c1497d97c 100644
--- a/winsup/cygwin/fhandler/console.cc
+++ b/winsup/cygwin/fhandler/console.cc
@@ -1752,13 +1752,7 @@ out:
   if (stat == input_signalled && !(ti->c_lflag & NOFLSH))
     discard_len = 0;
   if (discard_len && (len || stat != input_ok))
-    {
-      acquire_attach_mutex (mutex_timeout);
-      DWORD resume_pid = attach_console (con.owner);
-      discard_key_events (discard_len);
-      detach_console (resume_pid, con.owner);
-      release_attach_mutex ();
-    }
+    discard_key_events (discard_len);
   return stat;
 }
 
@@ -1768,6 +1762,8 @@ fhandler_console::discard_key_events (size_t n)
   DWORD discarded = 0;
   INPUT_RECORD input_rec[INREC_SIZE];
   DWORD n1 = min (INREC_SIZE, n);
+  acquire_attach_mutex (mutex_timeout);
+  DWORD resume_pid = attach_console (con.owner);
   while (n)
     {
       if (!ReadConsoleInputW (get_handle (), input_rec, n1, &n1) || !n1)
@@ -1776,6 +1772,8 @@ fhandler_console::discard_key_events (size_t n)
       discarded += n1;
       n1 = min (INREC_SIZE, n);
     }
+  detach_console (resume_pid, con.owner);
+  release_attach_mutex ();
   con.num_processed -= min (con.num_processed, discarded);
 }
 
-- 
2.51.0