This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH][BZ 13690] Do not violate mutex destruction requirements.
- From: OndÅej BÃlka <neleai at seznam dot cz>
- To: Torvald Riegel <triegel at redhat dot com>
- Cc: GLIBC Devel <libc-alpha at sourceware dot org>, Carlos O'Donell <carlos at redhat dot com>, David Miller <davem at davemloft dot net>
- Date: Tue, 21 Jul 2015 19:14:47 +0200
- Subject: Re: [PATCH][BZ 13690] Do not violate mutex destruction requirements.
- Authentication-results: sourceware.org; auth=none
- References: <1436905273 dot 19451 dot 14 dot camel at localhost dot localdomain> <20150721121831 dot GB3415 at domone> <1437497027 dot 18846 dot 6 dot camel at localhost dot localdomain>
On Tue, Jul 21, 2015 at 06:43:47PM +0200, Torvald Riegel wrote:
> On Tue, 2015-07-21 at 14:18 +0200, OndÅej BÃlka wrote:
> > On Tue, Jul 14, 2015 at 10:21:13PM +0200, Torvald Riegel wrote:
> > > POSIX and C++11 require that a thread can destroy a mutex if no thread
> > > owns the mutex, is blocked on the mutex, or will try to acquire it in
> > > the future. After destroying the mutex, it can reuse or unmap the
> > > underlying memory. Thus, we must not access a mutex' memory after
> > > releasing it. Currently, we can load the private flag after releasing
> > > the mutex, which is fixed by this patch.
> > >
> > > See https://sourceware.org/bugzilla/show_bug.cgi?id=13690 for more
> > > background.
> > >
> > > We need to call futex_wake on the lock after releasing it, however.
> > > This is by design, and can lead to spurious wake-ups on unrelated futex
> > > words (e.g., when the mutex memory is reused for another mutex). This
> > > behavior is documented in the glibc-internal futex API and in recent
> > > drafts of the Linux kernel's futex documentation (see the draft_futex
> > > branch of git://git.kernel.org/pub/scm/docs/man-pages/man-pages.git).
> > >
> > > Carlos, do you want to consider this for 2.22, or should this rather
> > > target 2.23? The actual change is simple.
> > >
> > > Not tested. Could someone test this on a non-x86-linux machine (I don't
> > > have one handy). Dave, could you test on sparc, please?
> > >
> > >
> > > 2015-07-14 Torvald Riegel <triegel@redhat.com>
> > >
> > > [BZ #13690]
> > > * sysdeps/nptl/lowlevellock.h (__lll_unlock): Do not access the lock
> > > after releasing it.
> > > (__lll_robust_unlock): Likewise.
> > > * nptl/pthread_mutex_unlock.c (__pthread_mutex_unlock_full): Likewise.
> > > * sysdeps/unix/sysv/linux/sparc/lowlevellock.h (lll_unlock): Likewise.
> > > (lll_robust_unlock): Likewise.
> > >
> >
> > So this is equivalent of my patch from 2013
>
> I don't remember such a patch, and you didn't gave a citation.
>
> I thought we had fixed this already, saw that we didn't when browsing
> through the concurrency-related bugs, and thus proposed the patch.
>
> > where you said that you need
> > to ask posix with fixed bitrot?
>
> I don't understand this, sorry.
>
As for bitrot my patch isn't valid as we meanwhile removed duplicate
lowlevellock.h
Then you said here following:
https://sourceware.org/ml/libc-ports/2013-12/msg00029.html
This needs clarification in POSIX; see my comment on #13690
(https://sourceware.org/bugzilla/show_bug.cgi?id=13690#c24). Depending
on how the Austin Group decides, this is either not a bug, or we have to
fix the pending load AND investigate whether the pending futex_wake call
is harmless. The latter might be the case for normal mutexes, but I
wouldn't be surprised to find out that PI or robust mutexes aren't as
simple and need a more complex fix.
Therefore, I think we should wait for the POSIX clarification and then
decide what the next steps should be.