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]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.