This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] malloc: Run fork handler as late as possible [BZ #19431]
- From: Torvald Riegel <triegel at redhat dot com>
- To: Florian Weimer <fweimer at redhat dot com>
- Cc: GNU C Library <libc-alpha at sourceware dot org>
- Date: Mon, 15 Feb 2016 19:10:33 +0100
- Subject: Re: [PATCH] malloc: Run fork handler as late as possible [BZ #19431]
- Authentication-results: sourceware.org; auth=none
- References: <56BBAF3D dot 5030905 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?