[PATCH] Cygwin: console: Improve the performance of peek_console

[Johannes Schindelin]

Hi Takashi,

On Fri, 4 Sep 2026, Takashi Yano wrote:

> Previously, peek_console() used PeekConsoleInput() to confirm whether
> the console input buffer has some input records. However, this need
> attaching to the console if the process does not attach to the console.
> To reduce that overhead, this patch uses WaitForSingleObject() with
> console input handle instead. WaitForSingleObject() works without
> attaching to the console, so this simplifies the peek_console()
> code.

Great! I like when Cygwin's code is taught to avoid expensive operations.

One issue, though:

> 
> Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
> Reviewed-by:
> ---
>  winsup/cygwin/select.cc | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc
> index 592f6d14d..d565f4d5b 100644
> --- a/winsup/cygwin/select.cc
> +++ b/winsup/cygwin/select.cc
> @@ -1149,8 +1149,6 @@ peek_console (select_record *me, bool)
>        return 1;
>      }
>  
> -  INPUT_RECORD irec;
> -  DWORD events_read;
>    HANDLE h;
>    set_handle_or_return_if_not_open (h, me);
>  
> @@ -1161,12 +1159,7 @@ peek_console (select_record *me, bool)
>        else
>  	{
>  	  fh->acquire_input_mutex (mutex_timeout);
> -	  acquire_attach_mutex (mutex_timeout);
> -	  DWORD resume_pid = fh->attach_console (fh->get_owner ());
> -	  BOOL r = PeekConsoleInputW (h, &irec, 1, &events_read);
> -	  fh->detach_console (resume_pid, fh->get_owner ());
> -	  release_attach_mutex ();
> -	  if (!r || !events_read)
> +	  if (WaitForSingleObject (h, 0) != WAIT_OBJECT_0)

With this change, a host can report readiness without leaving any records
to process. Here is a concrete source counterexample in released
OpenConsole `v1.24.11911.0`:

1. `SetConsoleActiveScreenBufferImpl()`
   (https://github.com/microsoft/terminal/blob/v1.24.11911.0/src/host/getset.cpp#L474-L498)
   calls `SetActiveScreenBuffer()`
   (https://github.com/microsoft/terminal/blob/v1.24.11911.0/src/host/output.cpp#L446-L476),
   which invokes `FlushAllButKeys()`.
2. If the queue contained only non-key records, it becomes empty while the
   input-available event remains signaled.
3. An empty subsequent peek returns before the event-reset code
   (https://github.com/microsoft/terminal/blob/v1.24.11911.0/src/host/inputBuffer.cpp#L460-L470).
   With no other input activity repairing that state, Cygwin's new loop
   can repeatedly see "signaled handle" followed by "zero records."

In other words: changing active screen buffers can discard the remaining
non-key records without resetting the input-available event. The queue is
then empty but the handle remains signaled; an empty peek does not repair
the event. The new preliminary wait consequently succeeds repeatedly,
while `process_input_message(0)` finds zero records and returns
`input_processing` without changing readiness. `peek_console()` loops
without progress, so even a zero-timeout `select()` can hang. The old
preliminary peek explicitly broke out when its record count was zero.

This is not merely the normal distinction between a keypress and a
complete line. It is a stale host notification paired with an actually
empty queue. The source counterexample keeps Cygwin's worker paused
through Win32 input mode, with no new input or other reader repairing the
state.

I think we can keep the cheap `WaitForSingleObject()` check, but could we
let `process_input_message()` report an empty queue separately from
`input_processing`? If its `PeekConsoleInputW()` call finds no records,
`peek_console()` needs to stop retrying rather than spin on a handle that
stays signaled. Simply breaking on `input_processing` would also stop when
we consumed input but hadn't yet completed a canonical line, so we need to
keep those cases separate. Please handle input errors explicitly, too.

Ciao,
Johannes

>  	    {
>  	      fh->release_input_mutex ();
>  	      break;
> -- 
> 2.51.0
> 
>