This is the mail archive of the cygwin-developers 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] winsup/cygwin: Protect fork() against dll- and exe-updates.


Hi Michael,

On Jul 24 17:43, Michael Haubenwallner wrote:
> Hi!
> 
> When starting to port Gentoo Prefix to Cygwin, the first real problem
> discovered is that fork() does use the original executable's location
> to Create the child's Process, probably finding linked dlls that just
> have emerged in the current directory (sth. like /my/prefix/usr/bin),
> causing "Loaded different DLL with same basename in forked child" errors.

Unfortunately there's some red tape to get over with, first.  We need a
copyright assignment from you before we can go much further.  See
https://cygwin.com/contrib.html, "Before you get started".  Please fill
out the standard assignment form and send the signed PDF to the address
given therein.

> Diving into the details, I'm coming up with (a patch-draft based on) the
> idea to create hardlinks for the binaries-in-use into some cygwin-specific
> directory, like \proc\<ntpid>\object\ ('ve seen this name on AIX),
> and use these hardlinks instead to create the new child's process.
> 
> Thoughts so far?

Well, yes.  Off the top of my head a couple of potential problems come
to mind:

- /proc is already available as virtual filesystem as on Linux.  Reusing
  it for some purposes is ok, but in this case we're talking about a
  real directory of the same name, which then would be hidden beneath
  the virtual one.  Is that deliberate?  The directory wouldn't be
  accessible from Cygwin applications while native Windows apps would
  see the dir.  I think hidden is bad.  Something like this should take
  place in a visible cache dir.  /var/cache or /var/spool come to mind.

  Also, using the Windows PID as dir name seems a bit weird, given that
  the virtual /proc obviously uses the Cygwin PID.  This sounds like a
  source for confusion.

- What about running Cygwin on filesystems not supporting hardlinks,
  like FAT?

- There's a meme along the lines of "Why is Cygwin soooo Slow!!!1!!11".

  The most important factor for this slowness is the way fork() has to
  be emulated.  The method you're proposing would add to the overhead by
  having to create hardlinks on fork and deleting them again at exit or
  execve time.

  Did you run tests to find out the cost of this additional overhead?

> For now, when <cygroot>\proc\ directory does exist, the patch (roughly) does:
> 
> For /bin/bash.exe, cygwin1.dll creates these hardlinks at process startup:
>   \proc\<ntpid>\object\bash.exe         -> /bin/bash.exe
>   \proc\<ntpid>\object\bash.exe.local      (empty file for dll redirection)
>   \proc\<ntpid>\object\cygwin1.dll      -> /bin/cygwin1.dll
>   \proc\<ntpid>\object\cygreadline7.dll -> /bin/cygreadline7.dll
> 
> And frok::parent then does:
> 
>   CreateProcess("\proc\<ntpid>\object\bash.exe", "/bin/bash.exe", ...)
> 
> Resulting in another \proc\<ntpid>\object\ directory with same hardlinks.
> 
> While attached patch does work so far already, there's a few issues:
> 
> *) dll-redirection for LoadLibrary using "app.exe.local" file does operate on
>    the dll's basename only, breaking perl's Hash::Util and List::Util at least.
>    So creating hardlinks for dynamically loaded dlls is disabled for now.
>    Eventually, manifests and/or app.exe.config could help here, but I'm still
>    failing to really grok them...

Hmm.  The DLLs are loaded dynamically anyway, so they will be loaded
dynamically in the child as well in dll_list::load_after_fork_impl.  Why
not simply hardlinking them using a unique filename (e.g. using the
inode number), storing the unique number or name in the dll struct and
then calling LoadLibrary on this name?

> *) Who can clean up \proc\<ntpid>\ directory after power-loss or similar?
>    For now, if stale \proc\<ntpid>\ is found, it is removed beforehand.
>    But when this was from a different user, cleanup will fail. However,
>    using \proc\S-<current-user-id>\<ntpid>\ instead could help here...

Yes, that seems necessary.  The requirement to remove a complete
directory on process startup is a lot of effort, though.  I'm feeling a
sweat attack coming...

> *) Is it really necessary to create these hardlinks in the real filesystem?
>    I could imagine to create them directly in $Recycle.bin instead, or some
>    (other) memory-only thing...

Uh, well, they are hardlinks after all.  They must be created on the
same filesystem.

> Thoughts welcome!

In general I like the basic idea behind this.  But given the overhead it
adds to the already slow fork, I'm rather reluctant.  I'm sure this
needs at least a lot more discussion (for which the cygwin-developers
mailing list, redirected to, would be better suited).  For instance:

- What if a EXE/DLL is replace more than once during the lifetime of
  a process?

- What about reducing the overhead by implementing some kind of generic
  exe/dll cache used by all processes?  It would reduce the requirement
  to cleanup, reduce the footprint of the cache, speed up subsequent
  forks.

- Given that the /bin directory alone can be easily 0.5 Gigs and more,
  the cache(s) can take as much memory.  This really asks for some
  cleanup mechanism.

- The heretical question of course:  Is the underlying problem really
  worth the additional overhead?  The patch is pretty intrusive.
  Is there a simpler way to achieve the same or, at least, a similar
  result?

> Thank you!
> /haubi/

Thank you,
Corinna

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

Attachment: pgpqBQL9QsP2H.pgp
Description: PGP signature


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