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: Wed, 13 Apr 2016 16:29:55 +0200
- 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> <56C5EE32 dot 1020605 at redhat dot com> <1460485015 dot 3869 dot 267 dot camel at localhost dot localdomain> <570D4944 dot 7070501 at redhat dot com> <1460552111 dot 3869 dot 362 dot camel at localhost dot localdomain> <570E5362 dot 7070300 at redhat dot com>
On Wed, 2016-04-13 at 16:10 +0200, Florian Weimer wrote:
> On 04/13/2016 02:55 PM, Torvald Riegel wrote:
> > On Tue, 2016-04-12 at 21:15 +0200, Florian Weimer wrote:
> >> On 04/12/2016 08:16 PM, Torvald Riegel wrote:
> >>> On Thu, 2016-02-18 at 17:15 +0100, Florian Weimer wrote:
> >>>> 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.
> >>>
> >>> But have you documented this and your understanding of the
> >>> synchronization scheme anywhere? Your explanation in the email with the
> >>> patch seems more detailed than the comments in the code.
> >>
> >> That's because I'm talking mainly about removed code and explaining a
> >> bug. I'm not sure how this information will be useful to future
> >> developers once the bug is gone. We have a regression test, which
> >> should avoid reintroducing precisely the same bug. For similar issues,
> >> we need to rely on code review and collective memory.
> >
> > But you do have a new scheme, right, or are doing things in such a way
> > that what was formerly a bug now doesn't matter? You arrived through
> > some information on the conclusion that the new scheme works correctly;
> > isn't this worth documenting? It doesn't seem to be obvious. For
> > example, what about saying that fork handlers should be invisible to the
> > application or otherwise you'll get problem X?
>
> I have expanded the comments somewhat. The key point is to make the
> malloc subsystem available to fork handlers, so it's not just about
> synchronization.
LGTM.