Re: [PATCH 1/3] Cygwin: console: Ensure the master thread runs only when it is supposed to

Hi Johannes,

On Mon, 29 Jun 2026 20:23:55 +0900
Takashi Yano wrote:
> Hi Johannes,
> 
> On Sat, 27 Jun 2026 10:34:20 +0200 (CEST)
> Johannes Schindelin wrote:
> > Hi Takashi & Mark,
> > 
> > An AutoHotKey-based test I added to
> > https://github.com/git-for-windows/msys2-runtime/pull/131 bisected a
> > Ctrl-C regression to this commit after it landed on master: Ctrl-C stopped
> > interrupting `cat` in the pipeline `cat | ping`. The diagnosis and the fix
> > I would propose are below.
> > 
> > On Thu, 11 Jun 2026, Takashi Yano wrote:
> > 
> > > @@ -1190,8 +1191,8 @@ fhandler_console::bg_check (int sig, bool dontsignal)
> > >       in the same process group. */
> > >    if (sig == SIGTTIN && con.curr_input_mode != tty::cygwin)
> > >      {
> > > -      set_disable_master_thread (false, this);
> > >        set_input_mode (tty::cygwin, &tc ()->ti, get_handle_set ());
> > > +      set_disable_master_thread (false, this);
> > >      }
> > >    if (sig == SIGTTOU && con.curr_output_mode != tty::cygwin)
> > >      set_output_mode (tty::cygwin, &tc ()->ti, get_handle_set ());
> > > @@ -2087,8 +2088,8 @@ fhandler_console::post_open_setup (int fd)
> > >    /* Setting-up console mode for cygwin app started from non-cygwin app. */
> > >    if (fd == 0)
> > >      {
> > > -      set_disable_master_thread (false, this);
> > >        set_input_mode (tty::cygwin, &get_ttyp ()->ti, &handle_set);
> > > +      set_disable_master_thread (false, this);
> > >      }
> > >    else if (fd == 1 || fd == 2)
> > >      set_output_mode (tty::cygwin, &get_ttyp ()->ti, &handle_set);
> > 
> > The console only delivers Ctrl-C as a raw `0x03` byte (which the console
> > master thread then reads and turns into a `SIGINT` for the foreground
> > process group) while that thread is live. When the master thread is
> > suspended or disabled, `set_input_mode (tty::cygwin)` instead requests
> > `ENABLE_PROCESSED_INPUT`, so the console raises a `CTRL_C_EVENT` and the
> > `0x03` byte never reaches the master thread.
> > 
> > The reorder above has `set_input_mode (tty::cygwin)` run while
> > `disable_master_thread` is still set, so `ENABLE_PROCESSED_INPUT` stays on
> > and a cygwin program sharing a foreground pgrp with a non-cygwin program
> > (e.g. the pipeline `cat | ping`) never receives its `SIGINT`. Clearing
> > `disable_master_thread` first, so the mode is configured with the master
> > thread already live, restores the previous behavior in both `bg_check ()`
> > and `post_open_setup ()`. The disable paths and the synchronous suspension
> > this commit added are load-bearing for non-cygwin programs and are left
> > untouched, so the master thread is still reliably suspended for them.
> > 
> > The fix I would propose is in
> > https://github.com/git-for-windows/msys2-runtime/pull/131/commits/73aae37a62d8246e1abaac6e52d6c6bb89bc4c5d:
> 
> Thanks for catching this!
> 
> > -- snip --
> > From 73aae37a62d8246e1abaac6e52d6c6bb89bc4c5d Mon Sep 17 00:00:00 2001
> > From: Johannes Schindelin <[email protected]>
> > Date: Fri, 26 Jun 2026 09:16:49 +0200
> > Subject: [PATCH] Cygwin: console: re-enable the master thread before selecting
> >  cygwin input mode
> > 
> > When a cygwin program and a non-cygwin program run in the same foreground
> > process group (for example the pipeline `cat | ping`), Ctrl-C stopped
> > interrupting the cygwin program after "Cygwin: console: Ensure the master
> > thread runs only when it is supposed to".
> > 
> > The console only delivers Ctrl-C as a raw 0x03 byte (which the console
> > master thread reads and turns into a SIGINT for the foreground process
> > group) while that thread is live. When it is suspended or disabled,
> > set_input_mode (tty::cygwin) instead requests ENABLE_PROCESSED_INPUT, so
> > the console raises a CTRL_C_EVENT and the 0x03 byte never reaches the
> > master thread. The referenced commit reordered the two enable paths,
> > bg_check () and post_open_setup (), so that set_input_mode (tty::cygwin)
> > runs while disable_master_thread is still set; that leaves
> > ENABLE_PROCESSED_INPUT on and the cygwin program never receives its SIGINT.
> > 
> > Clear disable_master_thread before selecting cygwin input mode in those two
> > paths, so the mode is configured with the master thread already live and
> > ENABLE_PROCESSED_INPUT stays off. The disable paths and the synchronous
> > suspension that the referenced commit added are left unchanged, so
> > non-cygwin programs still get the master thread reliably suspended.
> > 
> > Fixes: 733d5a953fa9 ("Cygwin: console: Ensure the master thread runs only when it is supposed to")
> > Assisted-by: Opus 4.8
> > Signed-off-by: Johannes Schindelin <[email protected]>
> > ---
> >  winsup/cygwin/fhandler/console.cc | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/winsup/cygwin/fhandler/console.cc b/winsup/cygwin/fhandler/console.cc
> > index 0136652878..685e99d62c 100644
> > --- a/winsup/cygwin/fhandler/console.cc
> > +++ b/winsup/cygwin/fhandler/console.cc
> > @@ -1147,8 +1147,8 @@ fhandler_console::bg_check (int sig, bool dontsignal)
> >       in the same process group. */
> >    if (sig == SIGTTIN && con.curr_input_mode != tty::cygwin)
> >      {
> > -      set_input_mode (tty::cygwin, &tc ()->ti, get_handle_set ());
> >        set_disable_master_thread (false, this);
> > +      set_input_mode (tty::cygwin, &tc ()->ti, get_handle_set ());
> >      }
> >    if (sig == SIGTTOU && con.curr_output_mode != tty::cygwin)
> >      set_output_mode (tty::cygwin, &tc ()->ti, get_handle_set ());
> > @@ -2035,8 +2035,8 @@ fhandler_console::post_open_setup (int fd)
> >    /* Setting-up console mode for cygwin app started from non-cygwin app. */
> >    if (fd == 0)
> >      {
> > -      set_input_mode (tty::cygwin, &get_ttyp ()->ti, &handle_set);
> >        set_disable_master_thread (false, this);
> > +      set_input_mode (tty::cygwin, &get_ttyp ()->ti, &handle_set);
> >      }
> >    else if (fd == 1 || fd == 2)
> >      set_output_mode (tty::cygwin, &get_ttyp ()->ti, &handle_set);
> > -- snap --
> 
> I think we also need the following.
> 
> diff --git a/winsup/cygwin/fhandler/console.cc b/winsup/cygwin/fhandler/console.cc
> index d2ffaa0c4..340bc3f2e 100644
> --- a/winsup/cygwin/fhandler/console.cc
> +++ b/winsup/cygwin/fhandler/console.cc
> @@ -991,6 +994,7 @@ fhandler_console::cleanup_for_non_cygwin_app (handle_set_t *p)
>    termios *ti = shared_console_info[unit] ?
>      &(shared_console_info[unit]->tty_min_state.ti) : &dummy;
>    /* Cleaning-up console mode for non-cygwin app. */
> +  set_disable_master_thread (con.owner == GetCurrentProcessId ());
>    /* conmode can be tty::restore when non-cygwin app is
>       exec'ed from login shell. */
>    tty::cons_mode conmode = cons_mode_on_close (p);
> @@ -998,7 +1002,6 @@ fhandler_console::cleanup_for_non_cygwin_app (handle_set_t *p)
>      set_output_mode (conmode, ti, p);
>    if (con.curr_input_mode != conmode)
>      set_input_mode (conmode, ti, p);
> -  set_disable_master_thread (con.owner == GetCurrentProcessId ());
>  }
> 
>  /* Return the tty structure associated with a given tty number.  If the

If you agree with:
https://cygwin.com/pipermail/cygwin-patches/2026q2/015136.html
, I'll push this to master and cygwin-3_6-branch.

-- 
Takashi Yano <[email protected]>