unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
blob d6a58ebd7dc6b52dc4ce1f948f5bdfb7a1ea383e 8025 bytes (raw)
name: gnu/packages/patches/glibc-2-26-0047.patch 	 # note: path name is non-authoritative(*)

  1
  2
  3
  4
  5
  6
  7
  8
  9
 10
 11
 12
 13
 14
 15
 16
 17
 18
 19
 20
 21
 22
 23
 24
 25
 26
 27
 28
 29
 30
 31
 32
 33
 34
 35
 36
 37
 38
 39
 40
 41
 42
 43
 44
 45
 46
 47
 48
 49
 50
 51
 52
 53
 54
 55
 56
 57
 58
 59
 60
 61
 62
 63
 64
 65
 66
 67
 68
 69
 70
 71
 72
 73
 74
 75
 76
 77
 78
 79
 80
 81
 82
 83
 84
 85
 86
 87
 88
 89
 90
 91
 92
 93
 94
 95
 96
 97
 98
 99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
 
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/90] 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)

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>

debug log:

solving d6a58ebd7 ...
found d6a58ebd7 in https://yhetil.org/guix-patches/87ine0pjiu.fsf@fastmail.com/ ||
	https://yhetil.org/guix-patches/87d148pe57.fsf@fastmail.com/

applying [1/1] https://yhetil.org/guix-patches/87ine0pjiu.fsf@fastmail.com/
diff --git a/gnu/packages/patches/glibc-2-26-0047.patch b/gnu/packages/patches/glibc-2-26-0047.patch
new file mode 100644
index 000000000..d6a58ebd7

1:43: trailing whitespace.
 
1:44: space before tab in indent.
 	* math/test-math-iscanonical.cc (error): Replace bool with int.
1:50: space before tab in indent.
 	 tst-interpose-nothread \
1:51: space before tab in indent.
 	 tst-interpose-thread \
1:52: space before tab in indent.
 	 tst-alloc_buffer \
Checking patch gnu/packages/patches/glibc-2-26-0047.patch...
Applied patch gnu/packages/patches/glibc-2-26-0047.patch cleanly.
warning: squelched 10 whitespace errors
warning: 15 lines add whitespace errors.

skipping https://yhetil.org/guix-patches/87d148pe57.fsf@fastmail.com/ for d6a58ebd7
index at:
100644 d6a58ebd7dc6b52dc4ce1f948f5bdfb7a1ea383e	gnu/packages/patches/glibc-2-26-0047.patch

(*) Git path names are given by the tree(s) the blob belongs to.
    Blobs themselves have no identifier aside from the hash of its contents.^

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).