Re: [PATCH] Cygwin: pty: keep interactive console input for native programs via Cygwin

Hi Johannes,

On Mon, 20 Jul 2026 09:43:18 +0000
"Johannes Schindelin wrote:
> From: Johannes Schindelin <[email protected]>
> 
> Currently, when a native Windows program starts a Cygwin program while a
> pseudo console is active, and the Cygwin program then starts another
> native Windows program, the final program can lose access to console
> input. It then behaves as though its standard input were redirected
> instead of remaining interactive.
> 
> For example, a native `git.exe` may invoke shell aliases (i.e. execute a
> shell command) that would in turn call interactive Git commands who
> would no longer work because their standard input appeared to be
> redirected. This can be demonstrated as follows:
> 
>   git -c 'alias.console-probe=!powershell.exe -NoLogo -NoProfile -Command "
>     Write-Output ([Console]::IsInputRedirected)
>     try {
>       [void][Console]::KeyAvailable
>       exit 0
>     } catch {
>       exit 1
>     }
>   "' console-probe
> 
> Running this command with a Win32 version of `git.exe` currently prints
> `True` and exits with exit code 1. In the latest official release, where
> this bug is not present, it prints `False` and results in exit code 0.
> 
> The reason is to be fonud in the archetype code. Reminder: For each
> pseudo terminal (pty), the archetype is the shared pty fhandler that
> owns the underlying native handles and supplies them to every
> per-file-descriptor fhandler for that pty.
> 
> `open_with_arch()` calls `open()`, copies the first pty fhandler's state
> into the archetype, and then calls `open_setup()`. At that stage, pcon
> handle adoption already took place in `open_setup()`. This was not
> anticipated by 60a88896dc (Cygwin: pty: do not leak nat handles when
> adopting the pcon's in open_setup(), 2026-06-25), which tried to fix a
> leak by closing the superseded native handles as they were replaced in
> `open_setup()`.  Because `open_with_arch()` had already copied those
> handle values into the archetype, closing them invalidated the
> archetype's copies.
> 
> The archetype therefore retained stale values for those closed handles,
> which later pty fd fhandlers would inherit. If Windows reuses one of
> those values for a newly duplicated pcon handle, closing the stale value
> closes the new handle instead. The nested native program then receives
> unusable console input.
> 
> Preserve usable console input by moving the unchanged transactional pcon
> handle adoption to `open()`, before the archetype snapshot. The archetype
> then receives valid pcon handles, all pty fd fhandlers inherit live
> handles, and the superseded raw pipe handles are closed exactly once.
> 
> This commit is best viewed with `--color-moved`.
> 
> Fixes: 60a88896dce0 ("Cygwin: pty: do not leak nat handles when
>  adopting the pcon's in open_setup()")
> Assisted-by: GPT-5.6 Sol
> Signed-off-by: Johannes Schindelin <[email protected]>
> ---
>     Fix a regression on cygwin/master
>     
>     I noticed this regression in Git for Windows' fork, to which I had
>     backported the faulty commit from Cygwin's master branch.
> 
> Published-As: https://github.com/cygwingitgadget/cygwin/releases/tag/pr-8%2Fdscho%2Ffix-nat-handle-leakfix-cygwin-v1
> Fetch-It-Via: git fetch https://github.com/cygwingitgadget/cygwin pr-8/dscho/fix-nat-handle-leakfix-cygwin-v1
> Pull-Request: https://github.com/cygwingitgadget/cygwin/pull/8
> 
>  winsup/cygwin/fhandler/pty.cc | 44 +++++++++++++++++------------------
>  1 file changed, 22 insertions(+), 22 deletions(-)
> 
> diff --git a/winsup/cygwin/fhandler/pty.cc b/winsup/cygwin/fhandler/pty.cc
> index f3df55f34..37a480933 100644
> --- a/winsup/cygwin/fhandler/pty.cc
> +++ b/winsup/cygwin/fhandler/pty.cc
> @@ -1220,26 +1220,6 @@ fhandler_pty_slave::open (int flags, mode_t)
>        release_attach_mutex ();
>      }
>  
> -  set_open_status ();
> -  return 1;
> -
> -err:
> -  if (GetLastError () == ERROR_FILE_NOT_FOUND)
> -    set_errno (ENXIO);
> -  else
> -    __seterrno ();
> -err_no_errno:
> -  termios_printf (errmsg);
> -err_no_msg:
> -  for (HANDLE **h = handles; *h; h++)
> -    if (**h && **h != INVALID_HANDLE_VALUE)
> -      CloseHandle (**h);
> -  return 0;
> -}
> -
> -bool
> -fhandler_pty_slave::open_setup (int flags)
> -{
>    if (get_ttyp ()->pcon_activated)
>      {
>        HANDLE pcon_owner = OpenProcess (PROCESS_DUP_HANDLE, FALSE,
> @@ -1255,8 +1235,8 @@ fhandler_pty_slave::open_setup (int flags)
>  				        0, TRUE, DUPLICATE_SAME_ACCESS);
>  	  if (ok_in && ok_out)
>  	    {
> -	      /* Close the cyg master-side handles open() installed before
> -		 replacing them, so they do not leak. */
> +	      /* Replace these before open_with_arch() copies them into the
> +		 archetype shared by all pty slave fhandlers. */
>  	      CloseHandle (get_handle_nat ());
>  	      CloseHandle (get_output_handle_nat ());
>  	      set_handle_nat (new_in);
> @@ -1273,6 +1253,26 @@ fhandler_pty_slave::open_setup (int flags)
>  	}
>      }
>  
> +  set_open_status ();
> +  return 1;
> +
> +err:
> +  if (GetLastError () == ERROR_FILE_NOT_FOUND)
> +    set_errno (ENXIO);
> +  else
> +    __seterrno ();
> +err_no_errno:
> +  termios_printf (errmsg);
> +err_no_msg:
> +  for (HANDLE **h = handles; *h; h++)
> +    if (**h && **h != INVALID_HANDLE_VALUE)
> +      CloseHandle (**h);
> +  return 0;
> +}
> +
> +bool
> +fhandler_pty_slave::open_setup (int flags)
> +{
>    set_flags ((flags & ~O_TEXT) | O_BINARY);
>    myself->set_ctty (this, flags);
>    report_tty_counts (this, "opened", "");
> 
> base-commit: 524d75ff73986b263161665af771cc90e55b5e01

At first glance, I thought this patch was incorrect, because open()
is only called in the process that initially opened the pty.
But that was a mistake on my part.

Since the commit 6eed1ef74869 ("Cygwin: pty: detect pcon-backed pty
for non-Cygwin-spawned children"), open() is also called when a cygwin
app is started from non-cygwin app. So open() is indeed the right
place to fix nat-handle, rather that open_setup().

In the first place, my patch b34394d456b6 ("Cygwin: pty: Fixup pty
state after a cygwin app exits") should place that logic in open().

Thanks for fixing that. Pushed to master branch.

-- 
Takashi Yano <[email protected]>