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 Corinna,

On 07/27/2015 09:50 AM, Corinna Vinschen wrote:
> On Jul 24 17:43, Michael Haubenwallner wrote:
>>
>> When starting to port Gentoo Prefix to Cygwin, the first real problem
>> discovered is that fork() does use the original executable's location

> Unfortunately there's some red tape to get over with, first.  We need a
> copyright assignment from you before we can go much further.

Copyright assignment is in progress.

>> 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.

There's no particular reason for /proc/ actually - just came to my mind
first. I've also seen /run/ on recent Linux boxes...

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

This is the functional reason to keep these hardlinks optional:
I don't want Cygwin itself to require NTFS, but Gentoo Prefix only - which
IMHO is a corner use-case for Cygwin, but requires an updates-protected fork.

> - 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.

Agreed, and this is the performance reason to keep hardlinks optional.

However, I've been using Interix before - and Cygwin feels faster even
with hardlinks enabled. And I do prefer "slow" over "broken" setups.

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

On a 4-core Windows Server 2012r2 VM, three times building cygwin takes:
$ ( time for x in {1..3}; do
           rm -rf cygwin-2.2.0-0.1.x86_64 ;
           cygport -8 cygwin.cygport prep compile install ;
         done ) > outfile 2>&1

vanilla cygwin-2.2.0-0.1:
  real    31m35.530s
  user    30m11.245s
  sys     22m20.365s

patched cygwin-2.2.0-0.1:
  real    52m26.049s
  user    30m39.820s
  sys     26m41.637s

>> 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?

This might be necessary in the initial dlopen() already: I've tried hardlinks
for loaded dlls mangling the full path into the hardlink's filename, but
encountered different load addresses in the child - most likely due to the
now different dll's filename.
Haven't tried creating a mangled subdir only to keep the filename, though.

>> *) 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...

Removing that subdirectory at process startup should be rarely needed, as the
original process removes it at pinfo::exit - unless something goes really wrong.

>> 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?

This wouldn't make any difference: The hardlinks are created upon the first
use of some exe/dll in parent (even if that process won't ever use fork),
and the forked child gets the parent's first-use versions. Still there is
a short timeframe between process startup and hardlink creation, but that
is not a real problem (yet). The main issue is with programs (shell scripts,
bots) that do run the Gentoo Prefix bootstrap&update itself - these are
started before the actual dll/exe updates and do some forks afterwards.

> - 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.

I'm all for it, but I've no idea of currently available cross-process
mechanisms in cygwin/windows that could help here ...

But: From the LoadLibrary docs I've got the impression that even an exe
can be dynamically loaded. Iff it would be possible to branch to an exe's
main() - what if there is some pool of cygwin-starter processes, that do
nothing but wait for some cygwin-process-start event, to dynamically load
the exe in question and branch to it's main()? And if that starter does
create the new exe's (and its linked dll's) optional hardlinks before
dynamically loading them, even the timeframe mentioned above would be gone.

> - 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.

Of course unused hardlinks have to be removed at some time. However,
without performing updates (of dlls in use), only hardlinks would be
cached, which shouldn't consume a lot of diskspace.

> - The heretical question of course:  Is the underlying problem really
>   worth the additional overhead?  The patch is pretty intrusive.

The underlying problem is:
Gentoo Prefix breaks on Cygwin with current fork implementation. OTOH,
both - enabling the hardlink creation plus the performance overhead - is
acceptable to me (and for now) to allow for Gentoo Prefix on Cygwin.
The alternative - to not have some POSIX-like buildsystem on Windows (since
Interix is gone) for our otherwise portable application - is... an issue.
While our final application is built using the MSVC toolchain - wrapped
to some gcc-like commandline tools using parity[1] (which provides some
additional features like preloading, embedded rpath, transparent
static/dynamic/runtime linking), the application's build system relies
on a POSIX-like system. We have tried using Linux with remote execution[2]
of the MSVC toolchain, but that failed due to filesystem sync issues.
[1] http://sourceforge.net/projects/parity/
[2] https://github.com/mduft/rex/

>   Is there a simpler way to achieve the same or, at least, a similar
>   result?

Hmm - most likely there is a faster way than the current patch,
but I doubt there is a simpler way...

> Thank you,
> Corinna

Thank you!
/haubi/


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