Re: [PATCH] malloc: Run fork handler as late as possible [BZ #19431]

[Torvald Riegel <triegel at redhat dot com>]

On Wed, 2016-02-10 at 22:44 +0100, Florian Weimer wrote:
> Previously, a thread M invoking fork would acquire locks in this order:
> 
>   (M1) malloc arena locks (in the registered fork handler)
>   (M2) libio list lock
> 
> A thread F invoking flush (NULL) would acquire locks in this order:
> 
>   (F1) libio list lock
>   (F2) individual _IO_FILE locks
> 
> A thread G running getdelim would use this order:
> 
>   (G1) _IO_FILE lock
>   (G2) malloc arena lock
> 
> After executing (M1), (F1), (G1), none of the threads can make progress.
> 
> This commit changes the fork lock order to:
> 
>   (M'1) libio list lock
>   (M'2) malloc arena locks
> 
> It explicitly encodes the lock order in the implementations of fork,
> and does not rely on the registration order, thus avoiding the deadlock.
> 
> I couldn't test the Hurd bits, but the changes look straightforward enough.

Are those changes, and thus the new scheme, documented anywhere?

I think it's really worthwhile to always update and improve the
documentation, especially if we're kind of (re)discovering knowledge
about the current implementation such as in this case.  I know this is
extra work, but if this isn't documented, it's less likely anybody but
you will be aware of how this all works.

> diff --git a/malloc/bug19431.c b/malloc/bug19431.c
> new file mode 100644
> index 0000000..3bd876b
> --- /dev/null
> +++ b/malloc/bug19431.c

I haven't seen this naming scheme before. Could you put a tst- prefix in
there, or something like that?

> diff --git a/malloc/malloc-private.h b/malloc/malloc-private.h
> new file mode 100644
> index 0000000..2fef840
> --- /dev/null
> +++ b/malloc/malloc-private.h

Should this perhaps be malloc-internal.h to match libc-internal.h?