[PATCH] malloc: Fix attached thread reference count handling [BZ #19243]

[Florian Weimer]

On 12/16/2015 05:51 AM, Carlos O'Donell wrote:

> The code in reused_arena walks all arenas starting with the main arena
> and therefore may pick an arena that is in the sequence of arenas listed
> via the free_list next_free pointer chain.
> 
> This can happen concurrently and there

Your sentence just broke off.

> If only get_free_list removes an arena from the free_list chain, then what
> prevents reused_arena from attaching threads to all of those arenas and
> rendering the free_list useless?
>
> Is it simply the expectation that arena_get2 calls get_free_list *first*
> and therefore you are more likely to get an unattached arena? Probably.

Yes, that's what I assumed.

We could perhaps increment a global counter and restart the arena
selection if another arena is put on the free list.  The “right” fix
from a concurrency perspective is to acquire a lock during arena
selection which is also acquired by arena_thread_freeres.  This would
ensure that the invariant is actually preserved.  But I don't think this
is worth it because it would prevent threads from exiting.  Sacrificing
the invariant seems better.

> This is some really painful logic. One really wants a thread to thread
> handoff of a free arena, but that's quiet a rewrite.

With the deadlock avoidance patch posted separately, it becomes clear
that we can switch to a lockless approach for free list management anyway.

I think the future lies with per-core arenas anyway, and then the issue
of arena selection only comes up in case of a CPU hotplug event.

>> +/* This thread spawns a number of outer threads, equal to the arena
>> +   limit.  The outer threads run a loop which start and join two
>> +   different kinds of threads: the first kind allocates (attaching an
>> +   arena to the thread) and waits, the second kind waits and
>> +   allocates.  The hope is that this will exhibit races in thread
>> +   termination and arena management.  */
> 
> You should expand a bit and describe what things we are looking for
> in this test.

This test also covers the original cyclic free list issue, but it
currently can't check for it because I first need to export the
definition of struct malloc_state (and a few of chunk header decoding
macros).

I'm attaching what I've committed.

Florian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-malloc-Fix-attached-thread-reference-count-handling-.patch
Type: text/x-patch
Size: 10393 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/libc-alpha/attachments/20151216/10876843/attachment.bin>