public inbox for cygwin@cygwin.com
 help / color / mirror / Atom feed
From: Alexey Izbyshev <izbyshev@ispras.ru>
To: Takashi Yano <takashi.yano@nifty.ne.jp>
Cc: cygwin@cygwin.com
Subject: Re: Deadlock of the process tree when running make
Date: Sun, 10 Apr 2022 23:49:29 +0300	[thread overview]
Message-ID: <b937a782f8b8993e3d4a058a354596a7@ispras.ru> (raw)
In-Reply-To: <0e1a53626639cb21369225ff9092ecfc@ispras.ru>

On 2022-04-10 15:13, Alexey Izbyshev wrote:
> On 2022-04-10 10:34, Takashi Yano wrote:
>> On Sat, 09 Apr 2022 23:26:51 +0300
>> Thanks for investigating. In the normal case, conhost.exe is 
>> terminated
>> when hWritePipe is closed.
> 
> Thanks for confirming.
> 
>> 
>> Possibly, the hWritePipe has incorrect handle value.
> 
> I've verified that the handle was correct by attaching via gdb to the
> hanging bash and checking that hWritePipe field is now zeroed (which
> happens only in the branch where _HandleIsValid returns true and
> hWritePipe is closed).
> 
> I've found something interesting though. I've modeled a similar
> situation on another machine:
> 
> 1. I've run a native process via bash.
> 2. I've attached to bash via gdb and set a breakpoint on 
> ClosePseudoConsole().
> 3. I've killed the native process.
> 4. The breakpoint was hit, and I looked at hWritePipe value.
> 
> ProcessHacker shows it as "Unnamed file: \FileSystem\Npfs". Both bash
> and conhost had a single handle with such name, and after I've
> forcibly closed it in the bash process (while it was still suspended
> by gdb), conhost.exe indeed died.
> 
> Then I looked at the original hanging tree and found that the hanging
> bash.exe still has a single handle displayed as "Unnamed file:
> \FileSystem\Npfs". I don't know how to check what kernel object it
> refers to, but at least its access rights are the same as for
> hWritePipe that I've seen on another machine, and its handle count is
> 1. So could it be another copy of hWritePipe, e.g. due to some handle
> leak?
> 
> I don't know how to verify whether this suspicious handle in bash.exe
> is paired with "Unnamed file: \FileSystem\Npfs" in conhost.exe, other
> than by forcibly closing it. If I close it and conhost.exe dies, it
> will confirm "the extra handle" theory, but will also prevent further
> investigation with the hanging tree. Do you have any advice?
> 
I've found something that looked strange to me by checking handles in 
the hanging process tree: the hanging conhost.exe and the hanging 
bash.exe belong to different tests. Each test is a separate shell script 
in a separate make recipe, so it looks like conhost.exe was created by 
one test (which is still hanging at a later point in its script, trying 
to run grep), but then bash.exe belonging to another test somehow got a 
pseudoconsole referring to this conhost.exe and now hangs trying to 
close it. So it looks that Cygwin migrated the pseudoconsole between 
processes, and indeed fhandler_pty_slave::close_pseudoconsole() contains 
something looking like migration logic. And this logic contains the 
following call:

DuplicateHandle (GetCurrentProcess (),
                  ttyp->h_pcon_write_pipe,
                  new_owner, &new_write_pipe,
                  0, TRUE, DUPLICATE_SAME_ACCESS);

Is it safe to create an *inheritable* handle in another process here? 
Could it be that the target process spawns a child at the wrong moment 
(e.g. before it even knows about the newly created handle), and that 
handle unintentionally leaks into the child, triggering the hang 
afterwards?

A similarly suspicious code is also in 
fhandler_pty_common::resize_pseudo_console():

   DuplicateHandle (pcon_owner, get_ttyp ()->h_pcon_write_pipe,
                    GetCurrentProcess (), &hpcon_local.hWritePipe,
                    0, TRUE, DUPLICATE_SAME_ACCESS);
   ResizePseudoConsole ((HPCON) &hpcon_local, size);
   CloseHandle (pcon_owner);
   CloseHandle (hpcon_local.hWritePipe);

If another thread spawns a child using 
CreateProcess(bInheritHandles=TRUE) between DuplicateHandle() and 
CloseHandle(hpcon_local.hWritePipe), the handle will leak into the 
child.

Sorry if this is a false lead, I haven't tried to really understand the 
pseudoconsole-related code yet.

Thanks,
Alexey

  reply	other threads:[~2022-04-10 20:49 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-07 21:53 Alexey Izbyshev
2022-04-07 23:54 ` Brian Inglis
2022-04-08  8:42   ` Alexey Izbyshev
2022-04-08 17:04     ` Brian Inglis
2022-04-11 13:27       ` Alexey Izbyshev
2022-04-09 10:17 ` Takashi Yano
2022-04-09 11:00   ` Alexey Izbyshev
2022-04-09 11:02     ` Alexey Izbyshev
2022-04-09 11:46       ` Takashi Yano
2022-04-09 16:07         ` Alexey Izbyshev
2022-04-09 16:57           ` Takashi Yano
2022-04-09 17:23             ` Alexey Izbyshev
2022-04-09 17:54               ` Takashi Yano
2022-04-09 19:35                 ` Alexey Izbyshev
2022-04-09 20:26                   ` Alexey Izbyshev
2022-04-10  7:34                     ` Takashi Yano
2022-04-10 12:13                       ` Alexey Izbyshev
2022-04-10 20:49                         ` Alexey Izbyshev [this message]
2022-04-11  8:35                           ` Takashi Yano
2022-04-11 10:10                             ` Alexey Izbyshev
2022-04-13 16:48                               ` Alexey Izbyshev
2022-04-13 17:22                                 ` Takashi Yano
2022-04-13 17:27                                   ` Alexey Izbyshev
2022-04-13 23:17                                 ` Alexey Izbyshev
2022-04-16  9:39                                   ` Takashi Yano
2022-04-16 13:21                                     ` Alexey Izbyshev
2022-04-27 11:22                                       ` Takashi Yano
2022-04-27 12:19                                         ` Alexey Izbyshev
2022-04-11  5:23               ` Jeremy Drake
2022-04-11  8:36                 ` Takashi Yano
2022-04-11 15:28                 ` Alexey Izbyshev
2022-04-11 17:02                   ` Jeremy Drake

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b937a782f8b8993e3d4a058a354596a7@ispras.ru \
    --to=izbyshev@ispras.ru \
    --cc=cygwin@cygwin.com \
    --cc=takashi.yano@nifty.ne.jp \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).