[PATCH] Fix linuxthreads memory leak

[PATCH] Fix linuxthreads memory leak

[Jakub Jelinek]

Hi!

We need to call _dl_deallocate_tls even for threads created with user
supplied stacks.  Also one failure path in __pthread_initialize_manager
was missing _dl_deallocate_tls.
linuxthreads still doesn't free all its memory on exit (particularly
manager thread's stack, tcb and dtv), not sure what to do about it
if anything.  Perhaps telling malloc code to do nothing in free and
free things anyway, so that mtrace can print things out.

2003-12-17  Jakub Jelinek  <jakub@redhat.com>

	* manager.c (pthread_free): Call _dl_deallocate_tls even for
	p_userstack threads.
	* pthread.c (__pthread_initialize_manager): Call _dl_deallocate_tls
	on error.
	(pthread_onexit_process): Update comment.
	* Makefile (tests): Add tst-stack1.  Depend on $(objpfx)tst-stack1-mem.
	(generated): Add tst-stack1.mtrace and tst-stack1-mem.
	(tst-stack1-ENV): Set.
	($(objpfx)tst-stack1-mem): New.
	* tst-stack1.c: New test.

--- libc/linuxthreads/manager.c.jj	2003-10-29 00:18:42.000000000 +0100
+++ libc/linuxthreads/manager.c	2003-12-17 12:39:30.000000000 +0100
@@ -903,13 +903,14 @@ static void pthread_free(pthread_descr t
       /* Unmap the stack.  */
       munmap(guardaddr, stacksize + guardsize);
 
+    }
+
 #ifdef USE_TLS
 # if TLS_DTV_AT_TP
-      th = (pthread_descr) ((char *) th + TLS_PRE_TCB_SIZE);
+  th = (pthread_descr) ((char *) th + TLS_PRE_TCB_SIZE);
 # endif
-      _dl_deallocate_tls (th, true);
+  _dl_deallocate_tls (th, true);
 #endif
-    }
 }
 
 /* Handle threads that have exited */
--- libc/linuxthreads/pthread.c.jj	2003-10-29 00:18:42.000000000 +0100
+++ libc/linuxthreads/pthread.c	2003-12-17 12:56:28.000000000 +0100
@@ -654,7 +654,7 @@ int __pthread_initialize_manager(void)
 
 #ifdef USE_TLS
   /* Allocate memory for the thread descriptor and the dtv.  */
-  tcbp  = _dl_allocate_tls (NULL);
+  tcbp = _dl_allocate_tls (NULL);
   if (tcbp == NULL) {
     free(__pthread_manager_thread_bos);
     __libc_close(manager_pipe[0]);
@@ -783,6 +783,9 @@ int __pthread_initialize_manager(void)
 #endif
     }
   if (__builtin_expect (pid, 0) == -1) {
+#ifdef USE_TLS
+    _dl_deallocate_tls (tcbp, true);
+#endif
     free(__pthread_manager_thread_bos);
     __libc_close(manager_pipe[0]);
     __libc_close(manager_pipe[1]);
@@ -1014,8 +1017,16 @@ static void pthread_onexit_process(int r
 	waitpid(__pthread_manager_thread.p_pid, NULL, __WCLONE);
 #endif
 	/* Since all threads have been asynchronously terminated
-           (possibly holding locks), free cannot be used any more.  */
-	/*free (__pthread_manager_thread_bos);*/
+           (possibly holding locks), free cannot be used any more.
+           For mtrace, we'd like to print something though.  */
+	/* #ifdef USE_TLS
+	   tcbhead_t *tcbp = (tcbhead_t *) manager_thread;
+	   # if TLS_DTV_AT_TP
+	   tcbp = (tcbhead_t) ((char *) tcbp + TLS_PRE_TCB_SIZE);
+	   # endif
+	   _dl_deallocate_tls (tcbp, true);
+	   #endif
+	   free (__pthread_manager_thread_bos); */
 	__pthread_manager_thread_bos = __pthread_manager_thread_tos = NULL;
       }
   }
--- libc/linuxthreads/Makefile.jj	2003-12-03 21:44:57.000000000 +0100
+++ libc/linuxthreads/Makefile	2003-12-17 13:57:46.000000000 +0100
@@ -109,11 +109,14 @@ tests = ex1 ex2 ex3 ex4 ex5 ex6 ex7 ex8 
 	tststack $(tests-nodelete-$(have-z-nodelete)) ecmutex ex14 ex15 ex16 \
 	ex17 ex18 tst-cancel tst-context bug-sleep \
 	tst-cancel1 tst-cancel2 tst-cancel3 tst-cancel4 tst-cancel5 \
-	tst-cancel6 tst-cancel7 tst-cancel8 tst-popen tst-popen2 tst-attr1
+	tst-cancel6 tst-cancel7 tst-cancel8 tst-popen tst-popen2 tst-attr1 \
+	tst-stack1
 test-srcs = tst-signal
 # These tests are linked with libc before libpthread
 tests-reverse += tst-cancel5
 
+generated += tst-stack1.mtrace tst-stack1-mem
+
 ifeq ($(build-static),yes)
 tests += tststatic tst-static-locale tst-cancel-static
 tests-static += tststatic tst-static-locale tst-cancel-static
@@ -139,6 +142,15 @@ tst-tls1modd.so-no-z-defs = yes
 tst-tls1mode.so-no-z-defs = yes
 tst-tls1modf.so-no-z-defs = yes
 
+tests: $(objpfx)tst-stack1-mem
+tst-stack1-ENV = MALLOC_TRACE=$(objpfx)tst-stack1.mtrace
+
+# There are still up to 3 objects unfreed:
+# manager thread's stack, tls block and dtv
+$(objpfx)tst-stack1-mem: $(objpfx)tst-stack1.out
+	$(common-objpfx)malloc/mtrace $(objpfx)tst-stack1.mtrace > $@ \
+	|| [ `grep ^0 $@ | wc -l` -le 3 ]
+
 $(test-modules): $(objpfx)%.so: $(objpfx)%.os $(common-objpfx)shlib.lds
 	$(build-module)
 
--- libc/linuxthreads/tst-stack1.c.jj	2003-12-17 00:00:56.000000000 +0100
+++ libc/linuxthreads/tst-stack1.c	2003-12-17 12:31:57.000000000 +0100
@@ -0,0 +1,98 @@
+/* Copyright (C) 2003 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+   Contributed by Jakub Jelinek <jakub@redhat.com>, 2003.
+
+   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, write to the Free
+   Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+   02111-1307 USA.  */
+
+/* Test whether pthread_create/pthread_join with user defined stacks
+   doesn't leak memory.  */
+
+#include <limits.h>
+#include <mcheck.h>
+#include <pthread.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+
+static int seen;
+
+static void *
+tf (void *p)
+{
+  ++seen;
+  return NULL;
+}
+
+static int
+do_test (void)
+{
+  mtrace ();
+
+  void *stack;
+  int res = posix_memalign (&stack, getpagesize (), 4 * PTHREAD_STACK_MIN);
+  if (res)
+    {
+      printf ("malloc failed %s\n", strerror (res));
+      return 1;
+    }
+
+  pthread_attr_t attr;
+  pthread_attr_init (&attr);
+
+  int result = 0;
+  res = pthread_attr_setstack (&attr, stack, 4 * PTHREAD_STACK_MIN);
+  if (res)
+    {
+      printf ("pthread_attr_setstack failed %d\n", res);
+      result = 1;
+    }
+
+  for (int i = 0; i < 16; ++i)
+    {
+      /* Create the thread.  */
+      pthread_t th;
+      res = pthread_create (&th, &attr, tf, NULL);
+      if (res)
+	{
+	  printf ("pthread_create failed %d\n", res);
+	  result = 1;
+	}
+      else
+	{
+	  res = pthread_join (th, NULL);
+	  if (res)
+	    {
+	      printf ("pthread_join failed %d\n", res);
+	      result = 1;
+	    }
+	}
+    }
+
+  pthread_attr_destroy (&attr);
+
+  if (seen != 16)
+    {
+      printf ("seen %d != 16\n", seen);
+      result = 1;
+    }
+
+  free (stack);
+  return result;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"

	Jakub

Re: [PATCH] Fix linuxthreads memory leak

[Ulrich Drepper]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Jakub Jelinek wrote:

> 	* manager.c (pthread_free): Call _dl_deallocate_tls even for
> 	p_userstack threads.
> 	* pthread.c (__pthread_initialize_manager): Call _dl_deallocate_tls
> 	on error.
> 	(pthread_onexit_process): Update comment.
> 	* Makefile (tests): Add tst-stack1.  Depend on $(objpfx)tst-stack1-mem.
> 	(generated): Add tst-stack1.mtrace and tst-stack1-mem.
> 	(tst-stack1-ENV): Set.
> 	($(objpfx)tst-stack1-mem): New.
> 	* tst-stack1.c: New test.

Applied.  Thanks,

- -- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.3 (GNU/Linux)

iD8DBQE/4Or/2ijCOnn/RHQRAkFRAKDJQ4lzo1HiBqzHF1crwv9XAS3vagCfbOnw
wvh9WfP+B0VJy53Kct4cD2E=
=f+ER
-----END PGP SIGNATURE-----