Re: [PATCH 2/3] Cygwin: console: Fix NOFLSH mode a little
Takashi Yano <[email protected]> Tue, 30 Jun 2026 01:14:56 +0900
| Newsgroups | gmane.os.cygwin.patches |
|---|---|
| Message-ID | <[email protected]> |
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.
--
Takashi Yano <[email protected]>
0001-Cygwin-console-discard_key_events-patch-1-2.patch
(text/plain, 1.8 KB)
From 2d04a679e90a6e009d239322e0fb4b8064f4d22c Mon Sep 17 00:00:00 2001 From: Takashi Yano <[email protected]> Date: Tue, 30 Jun 2026 00:31:52 +0900 Subject: [PATCH] Cygwin: console discard_key_events patch (1/2) --- winsup/cygwin/fhandler/console.cc | 8 +++++++- winsup/cygwin/local_includes/fhandler.h | 1 + 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/winsup/cygwin/fhandler/console.cc b/winsup/cygwin/fhandler/console.cc index 730bb0b45..ae7940039 100644 --- a/winsup/cygwin/fhandler/console.cc +++ b/winsup/cygwin/fhandler/console.cc @@ -430,6 +430,7 @@ fhandler_console::cons_master_thread (handle_set_t *p, tty *ttyp) return sizeof (INPUT_RECORD) * n; } }; + con.master_thread_tid = GetCurrentThreadId (); termios &ti = ttyp->ti; while (con.owner == GetCurrentProcessId ()) { @@ -1765,12 +1766,17 @@ out: void fhandler_console::discard_key_events (size_t n) { + /* Do not touch key events if called from cons_master_thread. */ + if (con.master_thread_tid == GetCurrentThreadId ()) + return; + DWORD discarded = 0; INPUT_RECORD input_rec[INREC_SIZE]; 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/local_includes/fhandler.h b/winsup/cygwin/local_includes/fhandler.h index 8e9cbef4b..3b3589c25 100644 --- a/winsup/cygwin/local_includes/fhandler.h +++ b/winsup/cygwin/local_includes/fhandler.h @@ -2150,6 +2150,7 @@ class dev_console char *cons_rapoi; bool cursor_key_app_mode; volatile bool disable_master_thread; + DWORD master_thread_tid; tty::cons_mode curr_input_mode; tty::cons_mode curr_output_mode; DWORD prev_input_mode; -- 2.51.0