[PATCH] malloc: Run fork handler as late as possible [BZ #19431]
Torvald Riegel
triegel@redhat.com
Tue Feb 16 10:24:00 GMT 2016
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?
More information about the Libc-alpha
mailing list