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

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


-- 
Takashi Yano <[email protected]>