Re: [PATCH] Cygwin: console: Clear readahead buffer on tcflush()
Takashi Yano <[email protected]>
| Newsgroups | gmane.os.cygwin.patches |
|---|---|
| Message-ID | <[email protected]> |
Hi Johannes,
Thanks for reviewing.
On Tue, 11 Aug 2026 12:14:13 +0200 (CEST)
Johannes Schindelin wrote:
> 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.
Done.
> 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.
kill_pgrp() is called the places other than process_sigs(), but yes,
a mutex of Windows can be acquired recursively, so adding input_mutex
guard in tcflush() is safe, I think.
https://learn.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-releasemutex
> 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.
Done.
--
Takashi Yano <[email protected]>