Re: [PATCH] Cygwin: console: Clear readahead buffer on tcflush()
Johannes Schindelin <[email protected]>
| Newsgroups | gmane.os.cygwin.patches |
|---|---|
| Message-ID | <[email protected]> |
Hi Takashi,
On Fri, 7 Aug 2026, Takashi Yano wrote:
> Previously, tcflush(TCIFLUSH) only discarded the input events and
> did not clear the readahead buffer. Because of this bug, when user
> program called select() -> tcflush() -> read(), the last read()
> returned the contents of the readahead buffer instead of blocking
> as it should.
>
> Correctly, tcflush() must discard all pending input, so read()
> should block until new input arrives. With this patch, the read-
> ahead buffer is also cleared as well as input events.
>
> Fixes: 8382778cdb57 ("Cygwin: console: fix select() behaviour")
> Suggested-by: Johannes Schindelin <[email protected]>
> Signed-off-by: Takashi Yano <[email protected]>
> Reviewed-by:
> ---
> winsup/cygwin/fhandler/console.cc | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/winsup/cygwin/fhandler/console.cc b/winsup/cygwin/fhandler/console.cc
> index ba35ca44c..d3fb2fcbd 100644
> --- a/winsup/cygwin/fhandler/console.cc
> +++ b/winsup/cygwin/fhandler/console.cc
> @@ -2362,6 +2362,8 @@ fhandler_console::tcflush (int queue)
> res = -1;
> }
> con.num_processed = 0;
> + eat_readahead (-1);
> + input_ready = false;
The core fix is right: with `con_ra` drained and `input_ready` cleared,
`read()` keeps waiting (its loop gates on `input_ready` and
`get_cons_readahead_valid()`), so the select() -> tcflush() -> read() path
no longer hands back the stale readahead. I confirmed it fires only for
`TCIFLUSH`/`TCIOFLUSH`, not `TCOFLUSH`, and that clearing `input_ready`
cannot drop a window-resize or signal event, since those travel via
`input_winch` and `input_signalled`.
Two things should change before this lands, though, I think.
First, this input flush is not serialized against `input_mutex`.
`tcflush()` here runs holding only `output_mutex` (taken by `ioctl()`) and
`attach_mutex`, not `input_mutex`, yet `con_ra` and `input_ready` are
otherwise mutated only under `input_mutex`: `process_input_message()`
fills them, `read()` clears `input_ready`, and `peek_console()` reads
them. So a concurrent `read()`/`select()` on the same console can, right
after this flush empties the buffer, line-edit already-drained events back
into `con_ra`, and the pre-flush input reappears once `tcflush()` returns.
Please take `input_mutex` around the flush body.
One caveat there: the signal path (`sigflush()` -> `tcflush(TCIFLUSH)`)
may already hold `input_mutex`, so it is worth confirming there is no
self-deadlock. Windows mutexes are recursive per-thread, and
`process_sigs` releases `input_mutex` around `kill_pgrp`, so it is
probably fine, but that should be checked rather than assumed.
Second, `get_cons_readahead_valid()` gates on `con.cons_rapoi`, which is a
separate buffer holding terminal query responses (the cursor-position and
device-attribute reports that `char_command()` queues), not `con_ra`. The
patch leaves `con.cons_rapoi` untouched, so if such a response is pending,
`read()` after `tcflush(TCIFLUSH)` still returns it instead of blocking.
That is arguably the POSIX "data received but not read" that the flush is
meant to discard. Either clear `con.cons_rapoi` as part of the same
(locked) flush, or, if leaving the query-response buffer intact is
intentional, say so in the commit message so the exemption is on record.
Ciao,
Johannes
> }
> return res;
> }
> --
> 2.51.0
>
>