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

[Johannes Schindelin via GitGitGadget]

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. */
+      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