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: Florian Weimer <fweimer at redhat dot com>
- To: Torvald Riegel <triegel at redhat dot com>
- Cc: GNU C Library <libc-alpha at sourceware dot org>
- Date: Thu, 18 Feb 2016 17:15:46 +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> <1455559833 dot 20971 dot 11 dot camel at localhost dot localdomain>
On 02/15/2016 07:10 PM, Torvald Riegel wrote:
>> 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?
Fork handlers used by the implementation should be invisible to
applications. The old one wasn't, and this was a bug.
> 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.
Do you mean something like this inside the fork implementation?
/* Acquire malloc locks. This needs to come last because fork
handlers may use malloc, and the libio list lock has an indirect
malloc dependency as well (via the getdelim function). */
__malloc_fork_lock_parent ();
>> 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?
Okay, I'm going to do that. GCC uses names of the form âpr33880.câ,
maybe that's where I got the idea.
>> 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?
We also have libio/libioP.h, and include/malloc.h. But I think a
separate header makes sense if its contents is truly internal.
What are the preferences here?
(I hope we will eventually stop build test cases against the internal
headers in include/.)
Florian