malloc: Fix tcache leak after thread destruction [BZ #22111]

The malloc tcache added in 2.26 will leak all of the elements remaining
in the cache and the cache structure itself when a thread exits. The
defect is that we do not set tcache_shutting_down early enough, and the
thread simply recreates the tcache and places the elements back onto a
new tcache which is subsequently lost as the thread exits (unfreed
memory). The fix is relatively simple, move the setting of
tcache_shutting_down earlier in tcache_thread_freeres. We add a test
case which uses mallinfo and some heuristics to look for unaccounted for
memory usage between the start and end of a thread start/join loop. It
is very reliable at detecting that there is a leak given the number of
iterations.  Without the fix the test will consume 122MiB of leaked
memory.
This commit is contained in:
Carlos O'Donell 2017-09-28 11:05:18 -06:00
parent d138676258
commit 1e26d35193
4 changed files with 129 additions and 3 deletions

View File

@ -1,3 +1,12 @@
2017-10-06 Carlos O'Donell <carlos@redhat.com>
[BZ #22111]
* malloc/malloc.c (tcache_shutting_down): Use bool type.
(tcache_thread_freeres): Set tcache_shutting_down before
freeing the tcache.
* malloc/Makefile (tests): Add tst-malloc-tcache-leak.
* malloc/tst-malloc-tcache-leak.c: New file.
2017-10-06 Rajalakshmi Srinivasaraghavan <raji@linux.vnet.ibm.com>
* sysdeps/powerpc/powerpc64/multiarch/memrchr-ppc64.c: Revert

View File

@ -34,6 +34,7 @@ tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \
tst-interpose-nothread \
tst-interpose-thread \
tst-alloc_buffer \
tst-malloc-tcache-leak \
tests-static := \
tst-interpose-static-nothread \
@ -242,3 +243,5 @@ tst-dynarray-fail-ENV = MALLOC_TRACE=$(objpfx)tst-dynarray-fail.mtrace
$(objpfx)tst-dynarray-fail-mem.out: $(objpfx)tst-dynarray-fail.out
$(common-objpfx)malloc/mtrace $(objpfx)tst-dynarray-fail.mtrace > $@; \
$(evaluate-test)
$(objpfx)tst-malloc-tcache-leak: $(shared-thread-library)

View File

@ -2916,7 +2916,7 @@ typedef struct tcache_perthread_struct
tcache_entry *entries[TCACHE_MAX_BINS];
} tcache_perthread_struct;
static __thread char tcache_shutting_down = 0;
static __thread bool tcache_shutting_down = false;
static __thread tcache_perthread_struct *tcache = NULL;
/* Caller must ensure that we know tc_idx is valid and there's room
@ -2953,8 +2953,12 @@ tcache_thread_freeres (void)
if (!tcache)
return;
/* Disable the tcache and prevent it from being reinitialized. */
tcache = NULL;
tcache_shutting_down = true;
/* Free all of the entries and the tcache itself back to the arena
heap for coalescing. */
for (i = 0; i < TCACHE_MAX_BINS; ++i)
{
while (tcache_tmp->entries[i])
@ -2966,8 +2970,6 @@ tcache_thread_freeres (void)
}
__libc_free (tcache_tmp);
tcache_shutting_down = 1;
}
text_set_element (__libc_thread_subfreeres, tcache_thread_freeres);

View File

@ -0,0 +1,112 @@
/* Bug 22111: Test that threads do not leak their per thread cache.
Copyright (C) 2015-2017 Free Software Foundation, Inc.
This file is part of the GNU C Library.
The GNU C Library is free software; you can redistribute it and/or
modify it under the terms of the GNU Lesser General Public
License as published by the Free Software Foundation; either
version 2.1 of the License, or (at your option) any later version.
The GNU C Library is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
Lesser General Public License for more details.
You should have received a copy of the GNU Lesser General Public
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
/* The point of this test is to start and exit a large number of
threads, while at the same time looking to see if the used
memory grows with each round of threads run. If the memory
grows above some linear bound we declare the test failed and
that the malloc implementation is leaking memory with each
thread. This is a good indicator that the thread local cache
is leaking chunks. */
#include <stdio.h>
#include <stdlib.h>
#include <malloc.h>
#include <pthread.h>
#include <assert.h>
#include <support/check.h>
#include <support/support.h>
#include <support/xthread.h>
void *
worker (void *data)
{
void *ret;
/* Allocate an arbitrary amount of memory that is known to fit into
the thread local cache (tcache). If we have at least 64 bins
(default e.g. TCACHE_MAX_BINS) we should be able to allocate 32
bytes and force malloc to fill the tcache. We are assuming tcahce
init happens at the first small alloc, but it might in the future
be deferred to some other point. Therefore to future proof this
test we include a full alloc/free/alloc cycle for the thread. We
need a compiler barrier to avoid the removal of the useless
alloc/free. We send some memory back to main to have the memory
freed after the thread dies, as just another check that the chunks
that were previously in the tcache are still OK to free after
thread death. */
ret = xmalloc (32);
__asm__ volatile ("" ::: "memory");
free (ret);
return (void *) xmalloc (32);
}
static int
do_test (void)
{
pthread_t *thread;
struct mallinfo info_before, info_after;
void *retval;
/* This is an arbitrary choice. We choose a total of THREADS
threads created and joined. This gives us enough iterations to
show a leak. */
int threads = 100000;
/* Avoid there being 0 malloc'd data at this point by allocating the
pthread_t required to run the test. */
thread = (pthread_t *) xcalloc (1, sizeof (pthread_t));
info_before = mallinfo ();
assert (info_before.uordblks != 0);
printf ("INFO: %d (bytes) are in use before starting threads.\n",
info_before.uordblks);
for (int loop = 0; loop < threads; loop++)
{
*thread = xpthread_create (NULL, worker, NULL);
retval = xpthread_join (*thread);
free (retval);
}
info_after = mallinfo ();
printf ("INFO: %d (bytes) are in use after all threads joined.\n",
info_after.uordblks);
/* We need to compare the memory in use before and the memory in use
after starting and joining THREADS threads. We almost always grow
memory slightly, but not much. Consider that if even 1-byte leaked
per thread we'd have THREADS bytes of additional memory, and in
general the in-use at the start of main is quite low. We will
always leak a full malloc chunk, and never just 1-byte, therefore
anything above "+ threads" from the start (constant offset) is a
leak. Obviously this assumes no thread-related malloc'd internal
libc data structures persist beyond the thread death, and any that
did would limit the number of times you could call pthread_create,
which is a QoI we'd want to detect and fix. */
if (info_after.uordblks > (info_before.uordblks + threads))
FAIL_EXIT1 ("Memory usage after threads is too high.\n");
/* Did not detect excessive memory usage. */
free (thread);
exit (0);
}
#include <support/test-driver.c>