218 lines
8.1 KiB
Diff
218 lines
8.1 KiB
Diff
From 6e1ea21501eac981204c3cc8212d45998f74983c Mon Sep 17 00:00:00 2001
|
|
From: Carlos O'Donell <carlos@systemhalted.org>
|
|
Date: Thu, 28 Sep 2017 11:05:18 -0600
|
|
Subject: [PATCH 47] 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.
|
|
|
|
(cherry picked from commit 1e26d35193efbb29239c710a4c46a64708643320)
|
|
---
|
|
ChangeLog | 9 ++++
|
|
malloc/Makefile | 3 ++
|
|
malloc/malloc.c | 8 +--
|
|
malloc/tst-malloc-tcache-leak.c | 112 ++++++++++++++++++++++++++++++++++++++++
|
|
4 files changed, 129 insertions(+), 3 deletions(-)
|
|
create mode 100644 malloc/tst-malloc-tcache-leak.c
|
|
|
|
diff --git a/ChangeLog b/ChangeLog
|
|
index d7a185e99d..bfcdc1ebbf 100644
|
|
--- a/ChangeLog
|
|
+++ b/ChangeLog
|
|
@@ -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-04 H.J. Lu <hongjiu.lu@intel.com>
|
|
|
|
* math/test-math-iscanonical.cc (error): Replace bool with int.
|
|
diff --git a/malloc/Makefile b/malloc/Makefile
|
|
index 3fa395b949..9e23db9343 100644
|
|
--- a/malloc/Makefile
|
|
+++ b/malloc/Makefile
|
|
@@ -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)
|
|
diff --git a/malloc/malloc.c b/malloc/malloc.c
|
|
index e3ff778113..01ec1571b9 100644
|
|
--- a/malloc/malloc.c
|
|
+++ b/malloc/malloc.c
|
|
@@ -2952,7 +2952,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
|
|
@@ -2989,8 +2989,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])
|
|
@@ -3002,8 +3006,6 @@ tcache_thread_freeres (void)
|
|
}
|
|
|
|
__libc_free (tcache_tmp);
|
|
-
|
|
- tcache_shutting_down = 1;
|
|
}
|
|
text_set_element (__libc_thread_subfreeres, tcache_thread_freeres);
|
|
|
|
diff --git a/malloc/tst-malloc-tcache-leak.c b/malloc/tst-malloc-tcache-leak.c
|
|
new file mode 100644
|
|
index 0000000000..22c679b65b
|
|
--- /dev/null
|
|
+++ b/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
|
|
+ <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>
|
|
|