GNU C Library master sources branch master updated. glibc-2.26-538-g1e26d35

[carlos at sourceware dot org]

This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, master has been updated
       via  1e26d35193efbb29239c710a4c46a64708643320 (commit)
      from  d13867625894fda6c6a5034dadfa8ff86983ea12 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
http://sourceware.org/git/gitweb.cgi?p=glibc.git;a=commitdiff;h=1e26d35193efbb29239c710a4c46a64708643320

commit 1e26d35193efbb29239c710a4c46a64708643320
Author: Carlos O'Donell <carlos@systemhalted.org>
Date:   Thu Sep 28 11:05:18 2017 -0600

    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.

diff --git a/ChangeLog b/ChangeLog
index ac0f188..bbd80b1 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-06  Rajalakshmi Srinivasaraghavan  <raji@linux.vnet.ibm.com>
 
 	* sysdeps/powerpc/powerpc64/multiarch/memrchr-ppc64.c: Revert
diff --git a/malloc/Makefile b/malloc/Makefile
index 50b487e..6cf78e1 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 1c2a0b0..d3fcadd 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -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);
 
diff --git a/malloc/tst-malloc-tcache-leak.c b/malloc/tst-malloc-tcache-leak.c
new file mode 100644
index 0000000..22c679b
--- /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>

-----------------------------------------------------------------------

Summary of changes:
 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


hooks/post-receive
-- 
GNU C Library master sources