[PATCH] Cygwin: pty: Fix input transferring from nat pipe to cyg pipe

[Takashi Yano]

Hi Johannes,

Thanks for the review!

On Thu, 5 Mar 2026 11:12:50 +0100 (CET)
Johannes Schindelin wrote:
> Hi Takashi,
> 
> On Tue, 3 Mar 2026, Takashi Yano wrote:
> 
> > In disable_pcon mode, input transferring from nat pipe to cyg pipe
> > does not work after the commit 04f386e9af99. This is because nat
>                                  ^^^^^^^^^^^^
> 
> When I imagine myself reading this in a couple of months, I foresee
> wishing for the commit OID to be amended by the usual information
> (`--format=reference` in Git parlance). In this instance: 04f386e9af
> (Cygwin: console: Inherit pcon hand over from parent pty, 2024-10-31).

OK. That makes sense.

> > pipe hand over is wrongly handled in get_winpid_to_hand_over().
> > This function should return the PID that should acquire ownership
> > of the nat pipe. However, when pseudo console is disabled, it
> > returns PID of cygwin process (such as cygwin shell) when no
> > other native (non-cygwin) app is not found.
> > 
> > The case that even cygwin app should take over the ownership
> > of nat pipe, is happen only in pcon_activated case. This patch
> > adds pcon_activated check to the condition where even a cygwin
> > app is allowable as a target to hand over.
> 
> While this looks technically correct, I suggest that the commit message
> should lead with the context and impact, illustrating the bug's symptoms.
> The reason? While I now understand what this patch is about, I would have
> liked the commit message to help me get there quicker.
> 
> Also, I was scratching my head a bit why there is only one changed
> condition in the diff when the function contains _three_ calls to
> `get_console_process_id()`. The explanation is that the other two calls
> filter out Cygwin processes (because those calls use `nat = true`). I
> guess it cannot hurt to keep trying to get potential other candidates
> connected to the console (even if I thought that `disable_pcon` meant that
> there simply is no console).
> 
> With that in mind, I iterated a bit on a commit message with the help of
> Claude Opus, and this is a draft I'd like to take as inspiration:
> 
>    Cygwin: pty: Fix nat pipe hand-over when pcon is disabled
> 
>    The nat pipe ownership hand-over mechanism relies on the console
>    process list ― the set of processes attached to a console, enumerable
>    via `GetConsoleProcessList()`. This list only exists when there is a
>    pseudo console. When pseudo console support is disabled, there is no
>    console associated with the PTY, so this list is meaningless.

Actually, in disable_pcon case, the process on the PTY is associated with
an invisible console.

>    04f386e9af (Cygwin: console: Inherit pcon hand over from parent pty,
>    2024-10-31) added a last-resort fallback in `get_winpid_to_hand_over()`
>    that hands nat pipe ownership to any process in the console process
>    list, including Cygwin processes. This fallback is needed when a
>    Cygwin process must take over management of an active pseudo console
>    after the original owner exits.
> 
>    When the pseudo console is disabled, this fallback incorrectly finds a
>    Cygwin process (such as the shell) and assigns it nat pipe ownership.
>    Since there is no pseudo console for that process to manage, ownership
>    never gets released, input stays stuck on the nat pipe, and keyboard
>    input to the shell breaks.
> 
>    Only the third (last-resort) call in the cascade needs guarding: the
>    first two calls filter for native (non-Cygwin) processes via the `nat`
>    parameter, and handing ownership to another native process is fine
>    regardless of pcon state. It is only the fallback to Cygwin processes
>    that is dangerous without an active pseudo console.
> 
>    Guard the fallback with a `pcon_activated` check, since handing nat
>    pipe ownership to a Cygwin process only makes sense when there is an
>    active pseudo console for it to manage.
> 
> It could probably use some clarification as to why the other two calls
> aren't disabled (I guess the explanation is along the lines that even in
> `disable_pcon` mode, there _could_ be an attached Console instance and we
> would want to find processes attached to that instance).
> 
> Thoughts?

I'll update the commit message and submit v2 patch.

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