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
>
>
