Re: [Patch] [BZ 13939] malloc deadlock

[Jeff Law <law at redhat dot com>]

On 07/02/2012 03:29 PM, Maxim Kuvyrkov wrote:
> On 3/07/2012, at 9:21 AM, Jeff Law wrote:
>
> > On 07/02/2012 02:00 PM, Maxim Kuvyrkov wrote:
> > > I don't recommend going this way as it is error-prone for bugs from
> > > future changes.  A reasonable programmer would expect ar_ptr->mutex
> > > to lock the entirety of ar_ptr, including ar_ptr->next.
> > Precisely.  Thus if we're trying to be robust WRT future changes and avoiding problems with ar->next changing after the lock is released but before we call arena_get2 (which is what it looks like libc_memalign is trying to do), then we need to get the value from ar->next before we release the lock.
> >
> > Note that libc_malloc and libc_calloc's current implementations can't race on ar->next because the lock is still held.  Of course it's the holding of that lock which leads to the deadlock that we're trying to fix right now.
> >
> > libc_pvalloc and libc_valloc's current implementation would have problems if there are races with writes to ar->next as they unlock the arena then blindly use the current value of ar->next.
>
> I see now, you are right again.  I would appreciate you adding a comment to one of the instances of "prev = ar_ptr->next" saying something like "Get ar_ptr->next before releasing the lock.".
You asked for a header comment on reused_arena.  Done.

You asked for avoiding special casing the main_arena by passing in the 
arena to be avoided.  Done.

You asked for a comment WRT getting the arena's next pointer prior to 
releasing its lock (to avoid potential races on the arena's next 
pointer).  Done.


OK?

Thanks,
Jeff

Attachment: patch
Description: Text document