Re: [musl] [PATCH v2] MT fork

[Szabolcs Nagy <nsz@port70.net>]

* Rich Felker <dalias@libc.org> [2020-11-09 12:07:36 -0500]:
> On Sun, Nov 08, 2020 at 05:12:15PM +0100, Szabolcs Nagy wrote:
> > * Rich Felker <dalias@libc.org> [2020-11-05 22:36:17 -0500]:
> > > On Fri, Oct 30, 2020 at 11:31:17PM -0400, Rich Felker wrote:
> > > > One thing I know is potentially problematic is interaction with malloc
> > > > replacement -- locking of any of the subsystems locked at fork time
> > > > necessarily takes place after application atfork handlers, so if the
> > > > malloc replacement registers atfork handlers (as many do), it could
> > > > deadlock. I'm exploring whether malloc use in these systems can be
> > > > eliminated. A few are almost-surely better just using direct mmap
> > > > anyway, but for some it's borderline. I'll have a better idea sometime
> > > > in the next few days.
> > > 
> > > OK, here's a summary of the affected locks (where there's a lock order
> > > conflict between them and application-replaced malloc):
> > 
> > if malloc replacements take internal locks in atfork
> > handlers then it's not just libc internal locks that
> > can cause problems but locks taken by other atfork
> > handlers that were registered before the malloc one.
> 
> No other locks taken there could impede forward process in libc. The

the problem is not in the libc: if malloc locks are
taken and user code can run (which can in an atfork
handler) then that's a problem between the malloc
lock and other user lock.

i.e. it's simply not valid for a malloc implementation
to register an atfork handler to take locks since it
cannot guarantee lock ordering. (the only reason it
works in practice because atfork handlers are rare)

> only reason malloc is special here is because we allowed the
> application to redefine a family of functions used by libc. For
> MT-fork with malloc inside libc, we do the malloc_atfork code last so
> that the lock isn't held while other libc components' locks are being
> taken. But we can't start taking libc-internal locks until the
> application atfork handlers run, or the application could see a
> deadlocked libc state (e.g. if something in the atfork handlers used
> time functions, maybe to log time of fork, or gettext functions, maybe
> to print a localized message, etc.).

yes i understood this.

> > if malloc interposers want to do something around fork
> > then libc may need to expose some better api than atfork.
> 
> Most of them use existing pthread_atfork if they're intended to be
> usable with MT-fork. I don't think inventing a new musl-specific API
> is a solution here. Their pthread_atfork approach already fully works
> with glibc because glibc *doesn't* make anything but malloc work in
> the MT-forked child. All the other subsystems mentioned here will
> deadlock or blow up in the child with glibc, but only with 0.01%
> probability since it's rare for them to be under hammering at fork
> time.

yes the libc internal locks are a problem, but
other atfork handlers are a problem too that
cannot be fixed.

we can make the atfork ordering an application
responsibility but it feels a bit sketchy (locks
can be library internals an application does not
know about), so i think to solve the more general
problem of locking around fork needs some new api,
pthread_atfork is not good for that.

> One solution you might actually like: getting rid of
> application-provided-malloc use inside libc. This could be achieved by
> making malloc a thin wrapper for __libc_malloc or whatever, which
> could be called by everything in libc that doesn't actually have a
> contract to return "as-if-by-malloc" memory. Only a few functions like
> getdelim would be left still calling malloc.

if none of the as-if-by-malloc allocations are
behind libc internal locks then this sounds good.

(not ideal, since then interposers can't see all
allocations, which some tools would like to see,
but at least correct and robust. and it is annoying
that we have to do all this extra work just because
of mt-fork)

> 
> The other pros of such an approach are stuff like making it so
> application code doesn't get called as a callback from messy contexts
> inside libc, e.g. with dynamic linker in inconsistent state. The major
> con I see is that it precludes omitting the libc malloc entirely when
> static linking, assuming you link any part of libc that uses malloc
> internally. However, almost all such places only call malloc, not
> free, so you'd just get the trivial bump allocator gratuitously
> linked, rather than full mallocng or oldmalloc, except for dlerror
> which shouldn't come up in static linked programs anyway.

i see.
that sounds fine to me.