Re: [PATCH] Cygwin: console: Fix regression in console input

Hi Takashi,

On Mon, 3 Aug 2026, Takashi Yano wrote:

> The commit fac73911f5a0 ("Cygwin: console: Fix typeahead input for
> bash") introduced a bug where select() consumes some input chars in
> canonical mode, preventing read() from reading them. This is due to
> discarding input events when process_input_message() does not return
> `input_ok` even if it is called from select().
> The basic idea of that commit was making process_input_message()
> not to store processed chars into readahead buffer. This was not
> correct because the key input events ware processed twice, once by
> select() and again by read(). Thus even if that commit worked as
> intended, the side effect such as input echo would be applied twice.
> 
> With this patch, process_input_message() handles only the minimum
> necessary of input events in both cases, those processed by select()
> and those processed by read(). To achive this behaviour, the function
> returns without processing when `input_ready` is already satisfied,
> or after it has processed the specified number of chars.
> 
> Addresses: https://cygwin.com/pipermail/cygwin/2026-August/259915.html
> Reported-by: Steven Doerfler <[email protected]>
> Fixes: fac73911f5a0 ("Cygwin: console: Fix typeahead input for bash")
> Signed-off-by: Takashi Yano <[email protected]>
> Revewed-by:

This explanation, as well as the diff, look sound to me.

You may want to fix the typos "Revewed" -> "Reviewed", "ware" -> "were"
and "achive" -> "achieve", but those are tiny nits.

You could also fix the pre-existing typo in the comment "recored" ->
"recorded" while at it, but again, that's just a nit.

> ---
>  winsup/cygwin/fhandler/console.cc | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/winsup/cygwin/fhandler/console.cc b/winsup/cygwin/fhandler/console.cc
> index 0219a37ef..3904fbd57 100644
> --- a/winsup/cygwin/fhandler/console.cc
> +++ b/winsup/cygwin/fhandler/console.cc
> @@ -1406,7 +1406,6 @@ fhandler_console::input_states
>  fhandler_console::process_input_message (size_t len)
>  {
>    char tmp[60];
> -  size_t num_chars = 0;
>  
>    if (!shared_console_info[unit])
>      return input_error;
> @@ -1429,6 +1428,10 @@ fhandler_console::process_input_message (size_t len)
>        return input_error;
>      }
>  
> +  /* len == 0 if called from select.cc:peek_console() */
> +  if (input_ready && (len == 0 || (get_ttyp ()->ti.c_lflag & ICANON)))
> +    return input_ok;

It is worth pointing out that while this `input_ready &&` part looks
superfluous (both callers guard the call behind `!input_ready`), it is a
concurrency guard: a second thread reading the same console fd can set
`inpuy_ready` in the window between `read()`'s lock-free `while
(!input_ready)` check and `acquire_input_mutex`.

Maybe worth an extra code comment?

> +
>    for (i = 0; i < total_read; i ++)
>      {
>        DWORD nread = 1;
> @@ -1794,7 +1797,6 @@ fhandler_console::process_input_message (size_t len)
>  	}
>  
>        num_input_events_processed = i + 1;
> -      num_chars += nread;
>        if (toadd)
>  	{
>  	  ssize_t ret;
> @@ -1813,21 +1815,17 @@ fhandler_console::process_input_message (size_t len)
>  	    }
>  	}
>        /* len == 0 if called from select.cc:peek_console() */
> -      if (len && num_chars >= len)
> +      if (input_ready && (len == 0 || con_ra.ralen >= len))

Something GPT 5.6 Sol stumbled over, before noticing that this is a
pre-existing issue: the raw multi-pass caller passes full `buflen` (not
`buflen - copied_chars`). That was already there before this patch (and
both GPT and Opus think that this is incorrect, I haven't had time to wrap
my head around it), and the new `ralen`-based check is actually
neutral-to-better; and `tcflush(TCIFLUSH)` doesn't flush `con_ra`.

I'm fairly certain that this is out of scope for this here bug fix, if it
is an issue at all (what do you think? Is this `buflen` issue real? Should
`tcflush()` also flush `con_ra`?).

In any case, I'm happy with this patch! Feel free to add my "Reviewed-by".

Thanks!
Johannes

>  	goto out;
>      }
>  out:
> -  if (len == 0)
> -    /* If len == 0, cancel reading from console input buffer.
> -       Clear readahead buffer. */
> -    eat_readahead (-1);
>    /* Discard processed recored. */
>    DWORD discard_len = min (total_read, i + 1);
>    /* If input is signalled, do not discard input here because
>       discard_key_events() is already called from line_edit(). */
>    if (stat == input_signalled)
>      discard_len = 0;
> -  if (discard_len && (len || stat != input_ok))
> +  if (discard_len)
>      discard_key_events (discard_len);
>    return stat;
>  }
> -- 
> 2.51.0
> 
>