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

Takashi Yano <[email protected]> Tue, 30 Jun 2026 13:11:35 +0900
Newsgroups gmane.os.cygwin.patches
Message-ID <[email protected]>
Hi Johannes,

On Tue, 30 Jun 2026 01:14:56 +0900
Takashi Yano wrote:
> On Mon, 29 Jun 2026 23:30:17 +0900
> Takashi Yano wrote:
> > 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().
> 
> No, this is not correct. The root problem is that the key event should
> not be touched if process_sigs() is called from cons_master_thread().
> 
> > So, what about the patch (1/2) attached instead?
> 
> New patch (1/2) attached.

This was still broken... Sorry.
Please check out:
https://cygwin.com/pipermail/cygwin-patches/2026q2/015129.html

The commit message is borrowed from your patch. Thanks!

-- 
Takashi Yano <[email protected]>