This is the mail archive of the cygwin-patches mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Possibly correct fix to strace phantom process entry


On Apr 24 18:14, Daniel Santos wrote:
> On 04/24/2017 07:19 AM, Corinna Vinschen wrote:
> > Hi Daniel,
> > 
> > On Apr 24 04:37, Daniel Santos wrote:
> > > The root cause of problem with strace causing long delays when any
> > > process enumerates the process database appears to be calling
> > > myself.thisproc () from child_info_spawn::handle_spawn() when we've
> > > dynamically loaded cygwin1.dll.  It definately fixes the problem, but I
> > > don't konw what other processes dynamically load cygwin1.dll and, thus,
> > > what other side-effects that this may have.  Please verify correctness.
> > > 
> > > Please see discussion here: https://cygwin.com/ml/cygwin/2017-04/msg00240.html
> > > 
> > > Daniel
> > > 
> > > Signed-off-by: Daniel Santos <daniel.santos@pobox.com>
> > > ---
> > I was just looking into this myself, but I was looking into the weird
> > Sleep loop itself and was wondering if that makes any sense at all.
> > 
> > Assuming pinfo::init is called from process enumeration (winpids::add)
> > then there's no good reason to handle this procinfo block at all.  It
> > doesn't resolve into an existing "real" Cygwin process.  And that's
> > exactly the case that hangs.
> 
> Yeah, and it doesn't represent a unique windows process either.

Right.  The problem is, I didn't look too deeply into the pinfo
implementation yet (except for /proc and signal stuff), so I'm quite
fuzzy on why this is done this way myself.  This was originally cgf's
domain (my ex-co-maintainer).

Here's another interesting observation: If you start ps under strace
from another Cygwin process like this:

  $ strace -o xyz ps

then strace is in the process list with filled out data like any other
process.  However, if you do the same from cmd:

  C:\> strace -o xyz ps

then strace is not in the list of Cygwin processes.  The reason is
apparently that a parent creates a child procinfo on fork (in
frok::parent), while the standalone start of strace only creates the
minimal procinfo via the thisproc call in child_info_spawn::handle_spawn.

I wonder if the handling of processes like strace isn't a bit lacking.
I appreciate any effort to improve this code.

> > So my curent fix would have been this:
> > [...]
> 
> Yeah, I hacked this loop up many times, mostly to diagnose the problem.  I
> presume that it was originally added for a reason, but as I said before, I
> know that on x86 procinfo->ppid is almost certain to compile into a mov that
> will be atomic, but when I expect another thread to change something larger
> than one byte, I prefer to use a macro or function that is always atomic and
> conveys the intention.

I don't think this is the problem here.  The test in done only to
check if ppid is written at all under the assumption that the ppid
value is always written last.  The actual value doesn't matter as long
as it's != 0.

> > Btw., would you mind to send a BSD waiver per
> > https://cygwin.com/contrib.html and
> > https://cygwin.com/git/?p=newlib-cygwin.git;f=winsup/CONTRIBUTORS;hb=HEAD
> > Your patches are covered by the "trivial patch" rule yet, but if you
> > look into providing more patches you don't have to care anymore.
> 
> Thanks, I had overlooked that.  Is this sufficient?
> 
> I am providing my patches to the Cygwin sources under the 2-clause BSD
> license.

Yep, that's ok.


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

Attachment: signature.asc
Description: PGP signature


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]