2017-10-06 Carlos O'Donell [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. Index: glibc-2.26/malloc/Makefile =================================================================== --- glibc-2.26.orig/malloc/Makefile +++ glibc-2.26/malloc/Makefile @@ -34,6 +34,7 @@ tests := mallocbug tst-malloc tst-valloc 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=$(o $(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) Index: glibc-2.26/malloc/malloc.c =================================================================== --- glibc-2.26.orig/malloc/malloc.c +++ glibc-2.26/malloc/malloc.c @@ -2940,7 +2940,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 @@ -2977,8 +2977,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]) @@ -2990,8 +2994,6 @@ tcache_thread_freeres (void) } __libc_free (tcache_tmp); - - tcache_shutting_down = 1; } text_set_element (__libc_thread_subfreeres, tcache_thread_freeres); Index: glibc-2.26/malloc/tst-malloc-tcache-leak.c =================================================================== --- /dev/null +++ glibc-2.26/malloc/tst-malloc-tcache-leak.c @@ -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 + . */ + +/* 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 +#include +#include +#include +#include + +#include +#include +#include + +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