[PATCH] Cygwin: pty: Fix the issue regarding open and close multiple PTYs.

[Corinna Vinschen]

Hi Takashi,

On Jan  1 15:47, Takashi Yano wrote:
> - If two PTYs are opened in the same process and the first one
>   is closed, the helper process for the first PTY remains running.
>   This patch fixes the issue.
> ---
> [...]
> diff --git a/winsup/utils/cygwin-console-helper.cc b/winsup/utils/cygwin-console-helper.cc
> index 66004bd15..6255fb93d 100644
> --- a/winsup/utils/cygwin-console-helper.cc
> +++ b/winsup/utils/cygwin-console-helper.cc
> @@ -4,14 +4,24 @@ int
>  main (int argc, char **argv)
>  {
>    char *end;
> +  HANDLE parent = NULL;
>    if (argc < 3)
>      exit (1);
> +  if (argc == 5)
> +    parent = OpenProcess (PROCESS_DUP_HANDLE, FALSE,
> +			  strtoull (argv[4], &end, 0));
>    HANDLE h = (HANDLE) strtoull (argv[1], &end, 0);
> +  if (parent)
> +    DuplicateHandle (parent, h, GetCurrentProcess (), &h,
> +		     0, FALSE, DUPLICATE_SAME_ACCESS);
>    SetEvent (h);
> -  if (argc == 4) /* Pseudo console helper mode for PTY */
> +  if (argc == 4 || argc == 5) /* Pseudo console helper mode for PTY */
>      {
>        SetConsoleCtrlHandler (NULL, TRUE);
>        HANDLE hPipe = (HANDLE) strtoull (argv[3], &end, 0);
> +      if (parent)
> +	DuplicateHandle (parent, hPipe, GetCurrentProcess (), &hPipe,
> +			 0, FALSE, DUPLICATE_SAME_ACCESS);
>        char buf[64];
>        sprintf (buf, "StdHandles=%p,%p\n",
>  	       GetStdHandle (STD_INPUT_HANDLE),
> @@ -21,6 +31,9 @@ main (int argc, char **argv)
>        CloseHandle (hPipe);
>      }
>    h = (HANDLE) strtoull (argv[2], &end, 0);
> +  if (parent)
> +    DuplicateHandle (parent, h, GetCurrentProcess (), &h,
> +		     0, FALSE, DUPLICATE_SAME_ACCESS);

I think it would be better if cygwin-console-helper closes the parent
handle at this point before waiting.  I have a bad feeling keeping the
PROCESS_DUP_HANDLE handle open for longer than necessary, after the
handle leakage we had in execve lately.

But then again, given that Cygwin uses EXTENDED_STARTUPINFO_PRESENT
anyway, wouldn't it makes sense to pass the required handles with
PROC_THREAD_ATTRIBUTE_HANDLE_LIST to avoid having to open the
parent with PROCESS_DUP_HANDLE?


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://cygwin.com/pipermail/cygwin-patches/attachments/20200113/762dbddd/attachment.sig>