Re: [PATCH] Cygwin: console: Fix undesired mode change at exit of non-cygwin apps

Hi Takashi,

On Tue, 14 Jul 2026, Takashi Yano wrote:

> Previously, if two non-cygwin apps are started and one of them
> exits first, the other one loosed appropriate console mode, since
> the first one restored it to tty::cygwin. This patch introduce a
> counter `non_cygwin_cnt` that counts the number of non-cygwin apps
> currently running, and restores console mode only when the last
> non-cygwin app exits.

Thank you for chasing this down. The underlying diagnosis is correct:
cleanup performed by one spawning Cygwin process must not restore console
modes while another spawned native process still requires native modes.
However, v1 is not yet safe to apply. What is missing here is explicit
lifetime/ownership tracking of native-mode acquisition; I found three
concrete correctness gaps and one historical nit.

First, the counter's lifetime is unbalanced, and I can reproduce it in
isolation. `fhandler_termios::spawn_worker::setup()` calls
`setup_for_non_cygwin_app()`, which increments `con.non_cygwin_cnt` before
`CreateProcessW()` runs. But `spawn_worker::cleanup()`, which decrements
it, is never reached when `CreateProcessW()` fails, nor for `_P_NOWAIT`,
`_P_NOWAITO`, `_P_DETACH`, or `_P_VFORK` spawn modes; those paths only
close the duplicated handles. A failed or non-waiting `spawnl()`
invocation therefore leaves the count permanently positive, and every
later cleanup returns early.

I applied v1 to Git for Windows' fork of the MSYS2 runtime, built an
isolated DLL, and reproduced this in an isolated console created with
`CREATE_NEW_CONSOLE`: invoke a valid PE whose machine type is unsupported,
so non-Cygwin classification succeeds but `CreateProcessW()` fails, then
invoke a native zero-exit executable. After that second, successful
process-spawn attempt, with v1 applied the console modes stayed at input
`0x000000e7`, output `0x00000003`; the unpatched runtime correctly
restored input `0x000002e8`, output `0x00000007`.

Second, the counter transition and the console-mode transition are not one
cross-process transaction. `InterlockedIncrement`/`InterlockedDecrement`
only serialize the integer. The current locking permits this execution
order: the Cygwin process performing cleanup decrements 1 to 0 and is
about to restore Cygwin modes; concurrently, the Cygwin process performing
setup for a different spawned native process increments 0 to 1 and
completes native-mode setup; the Cygwin process performing cleanup then
restores Cygwin modes anyway. The counter records one outstanding
native-mode acquisition, yet the console is back in Cygwin mode.

The separate input/output mutexes do not close this gap, since they are
acquired after the counter operation, independently of it. This is the
same category of issue we ran into with the PTY start/exit race: the
decision to change state and the actual state change need one shared
synchronization boundary.

Third, background spawned native processes are counted even though they
never acquire native console modes: the increment in
`setup_for_non_cygwin_app()` happens before the `if (get_ttyp()->getpgid()
=3D=3D myself->pgid)` check. A background spawned native process which nev=
er
acquired native-mode ownership must not suppress restoration. Ownership
acquisition must be recorded explicitly for the matching setup/cleanup
lifetime; a process that did not change the console modes must not defer
restoration.

>=20
> Fixes: 29d8a8300812 ("Cygwin: console: Rearrange set_(in|out)put_mode() =
calls.")

Smaller point: `Fixes: 29d8a8300812` is not the introducing commit;
`29d8a8300812^` already has the setup/restoration for each process-spawn
operation inline in `spawn.cc`, and that commit mostly factors it into
helpers. The exact `tty::native`/`tty::cygwin` pairing traces back to
`48285aa36c2c` ("Cygwin: console: Fix handling of Ctrl-S in Win7."). Worth
pointing `Fixes:` there, or dropping the trailer if you find a more
appropriate boundary.

For v2, could you pair every native-mode acquisition with its release for
every process-spawn outcome, and fold the ownership/count decision into
the same synchronization boundary as the master-thread/input/output mode
transition? Whichever concrete scheme you settle on, it needs to recover
cleanly if the spawning Cygwin process exits unexpectedly or uses a
non-waiting spawn mode before releasing ownership.

Ciao,
Johannes

> Signed-off-by: Takashi Yano <[email protected]>
> Reviewed-by:
> ---
>  winsup/cygwin/fhandler/console.cc       | 4 ++++
>  winsup/cygwin/local_includes/fhandler.h | 1 +
>  2 files changed, 5 insertions(+)
>=20
> diff --git a/winsup/cygwin/fhandler/console.cc b/winsup/cygwin/fhandler/=
console.cc
> index d4c87f29f..474e169ea 100644
> --- a/winsup/cygwin/fhandler/console.cc
> +++ b/winsup/cygwin/fhandler/console.cc
> @@ -841,6 +841,7 @@ fhandler_console::setup ()
>        con.num_processed =3D 0;
>        con.curr_input_mode =3D tty::restore;
>        con.curr_output_mode =3D tty::restore;
> +      con.non_cygwin_cnt =3D 0;
>      }
>  }
> =20
> @@ -975,6 +976,7 @@ fhandler_console::setup_for_non_cygwin_app ()
>       in background, tty settings of the shell is reflected
>       to the console mode of the app. So, do not change the
>       console mode. */
> +  InterlockedIncrement (&con.non_cygwin_cnt);
>    if (get_ttyp ()->getpgid () =3D=3D myself->pgid)
>      {
>        set_disable_master_thread (true, this);
> @@ -987,6 +989,8 @@ void
>  fhandler_console::cleanup_for_non_cygwin_app (handle_set_t *p)
>  {
>    const _minor_t unit =3D p->unit;
> +  if (InterlockedDecrement (&con.non_cygwin_cnt) !=3D 0)
> +    return;
>    termios dummy =3D {0, };
>    termios *ti =3D shared_console_info[unit] ?
>      &(shared_console_info[unit]->tty_min_state.ti) : &dummy;
> diff --git a/winsup/cygwin/local_includes/fhandler.h b/winsup/cygwin/loc=
al_includes/fhandler.h
> index d11b3ec4f..eafb7c581 100644
> --- a/winsup/cygwin/local_includes/fhandler.h
> +++ b/winsup/cygwin/local_includes/fhandler.h
> @@ -2082,6 +2082,7 @@ class dev_console
>    DWORD owner;
>    bool is_legacy;
>    bool orig_virtual_terminal_processing_mode;
> +  LONG non_cygwin_cnt;
> =20
>    WORD default_color, underline_color, dim_color;
> =20
> --=20
> 2.51.0
>=20
>=20