public inbox for cygwin-cvs@sourceware.org
help / color / mirror / Atom feed
From: Takashi Yano <tyan0@sourceware.org>
To: cygwin-cvs@sourceware.org
Subject: [newlib-cygwin/cygwin-3_3-branch] Cygwin: console: Improve the code to avoid typeahead key swapping.
Date: Mon, 28 Feb 2022 11:19:11 +0000 (GMT)	[thread overview]
Message-ID: <20220228111911.3008E3858C60@sourceware.org> (raw)

https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=c61fe51e99c08bdce3afa75881880a35dbdb389a

commit c61fe51e99c08bdce3afa75881880a35dbdb389a
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Mon Feb 28 20:02:01 2022 +0900

    Cygwin: console: Improve the code to avoid typeahead key swapping.
    
    - The commit "Cygwin: console: Prevent the order of typeahead input
      from swapped." did not fully resolve the issue. If keys are typed
      during input buffer fix, the order of key event may be swapped.
      This patch fixes the issue again.

Diff:
---
 winsup/cygwin/fhandler_console.cc | 75 ++++++++++++++++++++++-----------------
 1 file changed, 42 insertions(+), 33 deletions(-)

diff --git a/winsup/cygwin/fhandler_console.cc b/winsup/cygwin/fhandler_console.cc
index 5552db685..656a46d89 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
@@ -188,12 +188,23 @@ cons_master_thread (VOID *arg)
 void
 fhandler_console::cons_master_thread (handle_set_t *p, tty *ttyp)
 {
+  const int additional_space = 128; /* Possible max number of incoming events
+				       during the process. Additional space
+				       should be left for writeback fix. */
+  const int inrec_size = INREC_SIZE + additional_space;
+  struct
+  {
+    inline static size_t bytes (size_t n)
+      {
+	return sizeof (INPUT_RECORD) * n;
+      }
+  } m;
   termios &ti = ttyp->ti;
   int processed_up_to = -1;
   while (con.owner == myself->pid)
     {
       DWORD total_read, n, i;
-      INPUT_RECORD input_rec[INREC_SIZE];
+      INPUT_RECORD input_rec[inrec_size];
 
       if (con.disable_master_thread)
 	{
@@ -203,6 +214,7 @@ fhandler_console::cons_master_thread (handle_set_t *p, tty *ttyp)
 
       WaitForSingleObject (p->input_mutex, mutex_timeout);
       total_read = 0;
+      bool nowait = false;
       switch (cygwait (p->input_handle, (DWORD) 0))
 	{
 	case WAIT_OBJECT_0:
@@ -211,16 +223,13 @@ fhandler_console::cons_master_thread (handle_set_t *p, tty *ttyp)
 	  if (total_read == INREC_SIZE /* Working space full */
 	      && cygwait (p->input_handle, (DWORD) 0) == WAIT_OBJECT_0)
 	    {
-	      const int incr = 1;
-	      size_t bytes = sizeof (INPUT_RECORD) * (total_read - incr);
-	      /* Discard oldest incr events. */
-	      memmove (input_rec, input_rec + incr, bytes);
-	      total_read -= incr;
-	      processed_up_to =
-		(processed_up_to + 1 >= incr) ? processed_up_to - incr : -1;
+	      const int incr = min (processed_up_to + 1, additional_space);
 	      ReadConsoleInputW (p->input_handle,
 				 input_rec + total_read, incr, &n);
-	      total_read += n;
+	      /* Discard oldest n events. */
+	      memmove (input_rec, input_rec + n, m.bytes (total_read));
+	      processed_up_to -= n;
+	      nowait = true;
 	    }
 	  break;
 	case WAIT_TIMEOUT:
@@ -297,7 +306,7 @@ remove_record:
 	    { /* Remove corresponding record. */
 	      if (total_read > i + 1)
 		memmove (input_rec + i, input_rec + i + 1,
-			 sizeof (INPUT_RECORD) * (total_read - i - 1));
+			 m.bytes (total_read - i - 1));
 	      total_read--;
 	      i--;
 	    }
@@ -305,45 +314,45 @@ remove_record:
       processed_up_to = total_read - 1;
       if (total_read)
 	{
-	  /* Writeback input records other than interrupt. */
-	  WriteConsoleInputW (p->input_handle, input_rec, total_read, &n);
-	  size_t bytes = sizeof (INPUT_RECORD) * total_read;
 	  do
 	    {
-	      const int additional_size = 128; /* Possible max number of
-						  incoming events during
-						  above process. */
-	      const int new_size = INREC_SIZE + additional_size;
-	      INPUT_RECORD tmp[new_size];
+	      INPUT_RECORD tmp[inrec_size];
+	      /* Writeback input records other than interrupt. */
+	      WriteConsoleInputW (p->input_handle, input_rec, total_read, &n);
 	      /* Check if writeback was successfull. */
-	      PeekConsoleInputW (p->input_handle, tmp, new_size, &n);
-	      if (memcmp (input_rec, tmp, bytes) == 0)
+	      PeekConsoleInputW (p->input_handle, tmp, inrec_size, &n);
+	      if (n < total_read)
+		break; /* Someone has read input without acquiring
+			  input_mutex. ConEmu cygwin-connector? */
+	      if (memcmp (input_rec, tmp, m.bytes (total_read)) == 0)
 		break; /* OK */
 	      /* Try to fix */
 	      DWORD incr = n - total_read;
 	      DWORD ofst;
 	      for (ofst = 1; ofst <= incr; ofst++)
-		if (memcmp (input_rec, tmp + ofst, bytes) == 0)
+		if (memcmp (input_rec, tmp + ofst, m.bytes (total_read)) == 0)
 		  {
-		    ReadConsoleInputW (p->input_handle, tmp, new_size, &n);
-		    DWORD m;
-		    WriteConsoleInputW (p->input_handle, tmp + ofst,
-					total_read, &m);
-		    WriteConsoleInputW (p->input_handle, tmp, ofst, &m);
-		    if ( n > ofst + total_read)
-		      WriteConsoleInputW (p->input_handle,
-					  tmp + ofst + total_read,
-					  n - (ofst + total_read), &m);
+		    ReadConsoleInputW (p->input_handle, tmp, inrec_size, &n);
+		    memcpy (input_rec, tmp + ofst, m.bytes (total_read));
+		    memcpy (input_rec + total_read, tmp, m.bytes (ofst));
+		    if (n > ofst + total_read)
+		      memcpy (input_rec + total_read + ofst,
+			      tmp + ofst + total_read,
+			      m.bytes (n - (ofst + total_read)));
+		    total_read = n;
 		    break;
 		  }
-	      if (ofst > incr) /* Hard to fix */
-		break; /* Giving up */
+	      if (ofst > incr)
+		break; /* Writeback was not atomic. Or someone has read
+			  input without acquiring input_mutex.
+			  Giving up because hard to fix. */
 	    }
 	  while (true);
 	}
 skip_writeback:
       ReleaseMutex (p->input_mutex);
-      cygwait (40);
+      if (!nowait)
+	cygwait (40);
     }
 }


                 reply	other threads:[~2022-02-28 11:19 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220228111911.3008E3858C60@sourceware.org \
    --to=tyan0@sourceware.org \
    --cc=cygwin-cvs@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).