[PATCH v6 3/3] Cygwin: console: Fix master thread for OpenConsole.exe

[Johannes Schindelin]

Hi Takashi,

Thank you for the new patch. A few observations:

On Mon, 6 Apr 2026, Takashi Yano wrote:

> If the console is originating from a pseudo console, current master
> thread code does not work as expected. This is because the pseudo
> console does not keep all the event as is. All bKeyDown == 0 events
> will be omitted from the input record written by WriteConsoleInput().
>
> [...]

The commit message describes this as general pseudo console behavior, but
the code comment in `strip_inrec()` is more specific:

> diff --git a/winsup/cygwin/fhandler/console.cc b/winsup/cygwin/fhandler/console.cc
> index 1dd5dfa1d..1693a5be7 100644
> --- a/winsup/cygwin/fhandler/console.cc
> +++ b/winsup/cygwin/fhandler/console.cc
> @@ -305,6 +305,23 @@ cons_master_thread (VOID *arg)
>    return 0;
>  }
>  
> +static inline DWORD
> +strip_inrec (INPUT_RECORD *r, DWORD n)
> +{
> +  /* Pseudo console with OpenConsole.exe removes the events
> +     whose bKeyDown is 0 as well as ones whose charcode is 0. */

And the patch title itself says "Fix master thread for OpenConsole.exe".

Can you help me understand: does legacy conhost.exe _also_ strip these
events when used as a pseudo console host? If it does, the commit message
is fine but the code comment should drop the "with OpenConsole.exe" part.
If it does _not_, then guarding with `inside_pcon` is too broad: when the
user sets `use_legacy_pcon` (introduced in patch 1/3 of this series),
`strip_inrec()` would discard events on the Cygwin side that conhost.exe
actually preserves in its input buffer. Those stripped records would then
not be written back at lines 579-584, and the `inrec_eq()` comparison
against the peeked buffer would also see a mismatch.

In that case, could the guard be tightened to `inside_pcon &&
!use_legacy_pcon` (or a dedicated flag) so that stripping only happens
when OpenConsole.exe is the actual host?

> +  DWORD j = 0;
> +  for (DWORD i = 0; i < n; i++)
> +    {
> +      if (r[i].EventType != KEY_EVENT)
> +	r[j++] = r[i];
> +      else if (r[i].Event.KeyEvent.bKeyDown
> +	       && r[i].Event.KeyEvent.uChar.UnicodeChar)
> +	r[j++] = r[i];

Note: this strips not only key-up events (`bKeyDown == 0`) but also
KEY_EVENTs where `UnicodeChar == 0`, which includes arrow keys and
function keys. For signal processing that is harmless because the existing
loop already skips exactly those events.

However, it means those events are permanently removed from
`input_rec`/`input_tmp` before writeback, so any downstream code that
reads raw INPUT_RECORDs from the console input buffer (e.g. via
`ReadConsoleInput()`) will never see them.

Is that the intended behavior with OpenConsole.exe? I.e., does
OpenConsole.exe _also_ strip those events when it processes
`WriteConsoleInput()`, so the records would be lost regardless? If so, the
commit message should say so explicitly (it currently only mentions
`bKeyDown == 0` events, not the `UnicodeChar == 0` case). If not, we might
be discarding more than necessary.

Ciao,
Johannes

> +    }
> +  return j;
> +}
> +
>  /* Compare two INPUT_RECORD sequences */
>  static inline bool
>  inrec_eq (const INPUT_RECORD *a, const INPUT_RECORD *b, DWORD n)
> @@ -482,6 +499,8 @@ fhandler_console::cons_master_thread (handle_set_t *p, tty *ttyp)
>  	      total_read += len;
>  	    }
>  	  release_attach_mutex ();
> +	  if (inside_pcon)
> +	    total_read = strip_inrec (input_rec, total_read);
>  	  break;
>  	case WAIT_TIMEOUT:
>  	  con.num_processed = 0;
> @@ -606,6 +625,8 @@ remove_record:
>  	      acquire_attach_mutex (mutex_timeout);
>  	      PeekConsoleInputW (p->input_handle, input_tmp, inrec_size, &n);
>  	      release_attach_mutex ();
> +	      if (inside_pcon)
> +		n = strip_inrec (input_tmp, n);
>  	      if (n < min (total_read, inrec_size))
>  		break; /* Someone has read input without acquiring
>  			  input_mutex. ConEmu cygwin-connector? */
> @@ -624,6 +645,8 @@ remove_record:
>  		  n += len;
>  		}
>  	      release_attach_mutex ();
> +	      if (inside_pcon)
> +		n = strip_inrec (input_tmp, n);
>  	      bool fixed = false;
>  	      for (DWORD ofs = n - total_read; ofs > 0; ofs--)
>  		{
> -- 
> 2.51.0
> 
> 
>