[PATCH] Cygwin: console: Correct previous NOFLSH fix

The previous fix for NOFLSH mode does not work as intended.

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.

To avoid this, this patch does not discard input when process_sigs()
is called from cons_master_thread, where the value of `fh` is NULL,
because discarding will be done in cons_master_thread.

And because the ReadConsoleInputW() return value is unchecked, a failed
read leaves the count indeterminate, so "n -= n1" can underflow and spin.
Check return value of ReadConsoleInputW() and abort if it fails.

Moreover, discard_key_event(1) does not work as intended if the first
key event is not a bKeyDown event correspoding to the signalling key.
Use tcflush() instead(). Since the ey-strokes prior to the signalling
key are already in the readahead buffer, so tcflush() discards only
the signalling key. The important point here is to discard input before
releasing input_mutex by release_input_mutex_if_necessary(), because,
if not, cons_master_thread starts to process key events before discarding
signalling key event because the thread can acquire input_mutex. This
causes the signalling key is processed twice.

One separate point: 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, so
it would be more correct to move the attach into the helper itself.

Fixes: 66324edf64a9 ("Cygwin: console: Fix NOFLSH behaviour a bit")
Co-authored-by: Johannes Schindelin <[email protected]>
Signed-off-by: Takashi Yano <[email protected]>
Reviewed-by:
---
 winsup/cygwin/fhandler/console.cc | 17 ++++++++---------
 winsup/cygwin/fhandler/termios.cc | 21 +++++++++++----------
 2 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/winsup/cygwin/fhandler/console.cc b/winsup/cygwin/fhandler/console.cc
index 730bb0b45..925db828c 100644
--- a/winsup/cygwin/fhandler/console.cc
+++ b/winsup/cygwin/fhandler/console.cc
@@ -1749,16 +1749,10 @@ out:
   DWORD discard_len = min (total_read, i + 1);
   /* If input is signalled, do not discard input here because
      tcflush() is already called from line_edit(). */
-  if (stat == input_signalled && !(ti->c_lflag & NOFLSH))
+  if (stat == input_signalled)
     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,13 +1762,18 @@ 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)
     {
-      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);
     }
+  detach_console (resume_pid, con.owner);
+  release_attach_mutex ();
   con.num_processed -= min (con.num_processed, discarded);
 }
 
diff --git a/winsup/cygwin/fhandler/termios.cc b/winsup/cygwin/fhandler/termios.cc
index 605258731..c59027093 100644
--- a/winsup/cygwin/fhandler/termios.cc
+++ b/winsup/cygwin/fhandler/termios.cc
@@ -444,10 +444,15 @@ fhandler_termios::process_sigs (char c, tty* ttyp, fhandler_termios *fh)
 	goto not_a_sig;
 
       termios_printf ("got interrupt %d, sending signal %d", c, sig);
-      if (!(ti.c_lflag & NOFLSH) && fh)
+      if (fh)
 	{
-	  fh->eat_readahead (-1);
-	  fh->discard_input ();
+	  if (!(ti.c_lflag & NOFLSH))
+	    {
+	      fh->eat_readahead (-1);
+	      fh->discard_input ();
+	    }
+	  if (fh->is_console ())
+	    fh->tcflush (TCIFLUSH);
 	}
       if (fh)
 	fh->release_input_mutex_if_necessary ();
@@ -666,13 +671,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