[PATCH 3/4] Cygwin: pty: Prevent premature pseudo console teardown that amplifies oscillation

[Takashi Yano]

Hi Johannes,

On Mon, 02 Mar 2026 14:24:39 +0000
Johannes Schindelin wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> The two preceding commits removed transfer code paths that stole
> readline's data during pseudo console oscillation.  This commit
> addresses the oscillation itself: a guard function that tears down
> active pseudo console sessions prematurely, causing more frequent
> oscillation cycles and thus more opportunities for the remaining
> (less harmful) timing issues to manifest.
> 
> The function reset_switch_to_nat_pipe() runs from bg_check() in the
> slave process.  Its purpose is to clean up the nat pipe state when
> no native process is using the pseudo console anymore.  Its guard
> logic was:
> 
>   if (!nat_pipe_owner_self(pid) && process_alive(pid))
>     return;   /* someone else owns it, don't reset */
>   /* fall through to destructive cleanup: clear pty_input_state,
>      nat_pipe_owner_pid, switch_to_nat_pipe, pcon_activated */
> 
> The nat_pipe_owner_pid is set to bash's own PID during
> setup_for_non_cygwin_app() (because bash is the process that calls
> exec() to launch the native program).  When bg_check() runs and
> calls reset_switch_to_nat_pipe(), nat_pipe_owner_self() returns
> true -- the first condition becomes false, the && short-circuits,
> and the function falls through to the destructive cleanup.  It
> clears pcon_activated, switch_to_nat_pipe, pty_input_state, and
> nat_pipe_owner_pid -- even though the native process is still
> alive and actively using the pseudo console.
> 
> This forced every subsequent code path to re-initialize the pseudo
> console from scratch, creating exactly the rapid oscillation
> described in the earlier commits.
> 
> Restructure the guard into two separate checks:
> 
>   if (process_alive(pid))
>     {
>       if (!nat_pipe_owner_self(pid))
>         return;   /* someone else owns it */
>       if (pcon_activated || switch_to_nat_pipe)
>         return;   /* we own it, but session is still active */
>     }
>   /* fall through: owner died or session ended */
> 
> When a different process owns the nat pipe, the behavior is
> unchanged.  When bash itself is the owner, the function now also
> returns early if pcon_activated or switch_to_nat_pipe is still set.
> Both flags are checked because during pseudo console handovers
> between parent and child native processes, pcon_activated is
> briefly false while switch_to_nat_pipe remains true.
> 
> The cleanup still runs when it should: when the owner process has
> exited or when both flags indicate the session has truly ended.
> 
> Regression note: this change is strictly more conservative -- it
> adds conditions that prevent cleanup, never removes them.  Every
> scenario where the original code returned early still returns early.
> 
> Addresses: https://github.com/git-for-windows/git/issues/5632
> Fixes: 919dea66d3ca ("Cygwin: pty: Fix a race issue in startup of pseudo console.")
> Assisted-by: Claude Opus 4.6
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  winsup/cygwin/fhandler/pty.cc | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/winsup/cygwin/fhandler/pty.cc b/winsup/cygwin/fhandler/pty.cc
> index 2450057c1..dd7ea9038 100644
> --- a/winsup/cygwin/fhandler/pty.cc
> +++ b/winsup/cygwin/fhandler/pty.cc
> @@ -1171,12 +1171,23 @@ fhandler_pty_slave::reset_switch_to_nat_pipe (void)
>    DWORD wait_ret = WaitForSingleObject (pipe_sw_mutex, mutex_timeout);
>    if (wait_ret == WAIT_TIMEOUT)
>      return;
> -  if (!nat_pipe_owner_self (get_ttyp ()->nat_pipe_owner_pid)
> -      && process_alive (get_ttyp ()->nat_pipe_owner_pid))
> +  if (process_alive (get_ttyp ()->nat_pipe_owner_pid))
>      {
> -      /* There is a process which owns nat pipe. */
> -      ReleaseMutex (pipe_sw_mutex);
> -      return;
> +      if (!nat_pipe_owner_self (get_ttyp ()->nat_pipe_owner_pid))
> +	{
> +	  /* There is a process which owns nat pipe. */
> +	  ReleaseMutex (pipe_sw_mutex);
> +	  return;
> +	}
> +      /* We are the nat pipe owner.  Don't reset while a native process
> +	 is still using the nat pipe -- check both pcon_activated and
> +	 switch_to_nat_pipe since the latter stays true during pcon
> +	 handovers when pcon_activated is briefly false. */

In what situation does the code path reach here?
Normal cygwin process usually cannot be a nat_pipe_owner.
Two exceptions are:
 1. GDB with non-cygwin inferior
 2. stub process for non-cygwin app

The stub process never calls reset_switch_to_nat_pipe(), therefore
we should consider only of GDB. However, the code for GDB exists
at the begining of reset_switch_to_nat_pipe().
(See 'if (h_gdb_inferior)' block.)

So, simplly

  if (process_alive (get_ttyp ()->nat_pipe_owner_pid))
    {
      /* There is a process which still owns nat pipe. */
      ReleaseMutex (pipe_sw_mutex);
      return;
    }

might be enough. Am I overlooking something?

> +      if (get_ttyp ()->pcon_activated || get_ttyp ()->switch_to_nat_pipe)
> +	{
> +	  ReleaseMutex (pipe_sw_mutex);
> +	  return;
> +	}
>      }
>    /* Clean up nat pipe state */
>    get_ttyp ()->pty_input_state = tty::to_cyg;
> -- 
> cygwingitgadget
> 


-- 
Takashi Yano <takashi.yano@nifty.ne.jp>