Re: [PATCH v2] Cygwin: pty: Fix nat_pipe_owner_pid when gdb runs non-cygwin app

Hi Takashi,

On Wed, 8 Jul 2026, Takashi Yano wrote:

> Previously, nat_pipe_owner_pid was incorrectly set to 0 when the
> inferior of gdb was a non-cygwin app. Due to this bug, repeatedly
> running a non-cygwin app under gdb could lead to an unexpected crash.
>=20
> This occurred because the previous code in setup_for_non_cygwin_app()
> set nat_pipe_owner_pid to exec_dwProcessId, which is correct when the
> caller is the stub process of the non-cygwin app. However, when the
> caller is gdb, the owner should be gdb itself, so nat_pipe_owner_pid
> must be set to myself->dwProcessId.
>=20
> With this fix, attach_console_temporarily() can be called with target
> pid equal to the process's own pid, in which case the attach operation
> is skipped.
>=20
> Fixes: 1e6c51d74136 ("Cygwin: pty: Reorganize the code path of setting u=
p and closing pcon.")
> Signed-off-by: Takashi Yano <[email protected]>
> Reviewed-by:
> ---
> v2: Skip attaching operation when attaching to myself is requested.

=46rom what I can see, the fix is correct. Three non-blocking notes:

The new source comment (and the commit message) has exec_dwProcessId and
dwProcessId the wrong way round, I think. exec_dwProcessId holds the
stub's own pid, saved before the overlay; dwProcessId is what gets
repointed to the native child. The code still does the right thing because
`exec_dwProcessId ?: dwProcessId` reduces to "our own pid" on every path
that reaches it, which is exactly what the owner-self check already
assumes. But the commentary as written will mislead the next reader.

The sibling assignment in `setup_pseudoconsole()` still writes
exec_dwProcessId directly, without the fallback. Harmless today because
its sole caller sets the owner first, but leaving the two spots
inconsistent invites a future regression. Worth applying the same `?:`
there.

Also: I believe that the bug is not really gdb-specific: _any_ Cygwin
process that spawns a non-cygwin app through the CreateProcess hook
without _P_OVERLAY hits it. If that is so, the subject and log undersell
the scope.

In any case, I happily provide my:

  Reviewed-by: Johannes Schindelin <[email protected]>

Ciao,
Johannes

>=20
>  winsup/cygwin/fhandler/pty.cc | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>=20
> diff --git a/winsup/cygwin/fhandler/pty.cc b/winsup/cygwin/fhandler/pty.=
cc
> index 1b453a499..6ef4fa506 100644
> --- a/winsup/cygwin/fhandler/pty.cc
> +++ b/winsup/cygwin/fhandler/pty.cc
> @@ -4734,7 +4734,11 @@ fhandler_pty_slave::setup_for_non_cygwin_app (boo=
l nopcon,
>        fhandler_pty_slave *ptys =3D (fhandler_pty_slave *) fh;
>        ptys->get_ttyp ()->switch_to_nat_pipe =3D true;
>        if (!process_alive (ptys->get_ttyp ()->nat_pipe_owner_pid))
> -	ptys->get_ttyp ()->nat_pipe_owner_pid =3D myself->exec_dwProcessId;
> +	/* In normal case where the current process is the stub process for
> +	   non-cygwin app, set owner to exec_dwProcessId (non-cygwin app).
> +	   However, in gdb case, gdb itself should be the owner. */
> +	ptys->get_ttyp ()->nat_pipe_owner_pid =3D
> +	  myself->exec_dwProcessId ? : myself->dwProcessId;
>      }
>    bool pcon_enabled =3D false;
>    if (!nopcon)
> @@ -4862,6 +4866,8 @@ fhandler_pty_common::attach_console_temporarily (D=
WORD target_pid)
>  {
>    DWORD resume_pid =3D 0;
>    acquire_attach_mutex (mutex_timeout);
> +  if (target_pid =3D=3D GetCurrentProcessId ())
> +    return target_pid;
>    pinfo pinfo_resume (myself->ppid);
>    if (pinfo_resume)
>      resume_pid =3D pinfo_resume->dwProcessId;
> @@ -4880,6 +4886,11 @@ fhandler_pty_common::attach_console_temporarily (=
DWORD target_pid)
>  void
>  fhandler_pty_common::resume_from_temporarily_attach (DWORD resume_pid)
>  {
> +  if (resume_pid =3D=3D GetCurrentProcessId ())
> +    {
> +      release_attach_mutex ();
> +      return;
> +    }
>    bool console_exists =3D (resume_pid !=3D (DWORD) -1);
>    if (!console_exists || resume_pid)
>      {
> --=20
> 2.51.0
>=20
>=20