[PATCH] Fix linuxthreads with pthread_attr_setstack{,addr}

[Jakub Jelinek <jakub at redhat dot com>]

Hi!

Despite what the bugzilla says, I believe the problem is not related
to TLS.  After pthread_join successfully returns, the user thread stack
may be still in use (both by the thread manager (thread descriptor,
i.e. !TLS only) and by the actual thread being joined (both the thread
descriptor and stack being in use)).
The following patch for p_userstack threads waits in pthread_join
till thread manager tells it will not use the thread stack any longer.

2004-04-29  Jakub Jelinek  <jakub@redhat.com>

	[BZ #110]
	* descr.h (struct _pthread_descr_struct): Add p_userstack_joining
	field.
	* internals.h (struct pthread_request): Add REQ_FREE_AND_NOTIFY
	request type.
	* join.c (pthread_join): Use REQ_FREE_AND_NOTIFY if th->p_userstack
	instead of REQ_FREE and suspend till thread manager awakens us.
	* manager.c (pthread_handle_free): Add req_thread argument.  If
	non-NULL, restart that thread if freed thread is already dead or
	set p_userstack_joining.
	(__pthread_manager): Adjust caller.  Handle REQ_FREE_AND_NOTIFY.
	(pthread_exited): Restart p_userstack_joining thread.

	* Makefile (tests): Add tst-stack2.
	* tst-stack2.c: New test.

--- libc/linuxthreads/join.c.jj	2002-12-18 02:14:09.000000000 +0100
+++ libc/linuxthreads/join.c	2004-04-29 17:49:25.384460978 +0200
@@ -173,9 +173,13 @@ int pthread_join(pthread_t thread_id, vo
   if (__pthread_manager_request >= 0) {
     request.req_thread = self;
     request.req_kind = REQ_FREE;
+    if (th->p_userstack)
+      request.req_kind = REQ_FREE_AND_NOTIFY;
     request.req_args.free.thread_id = thread_id;
     TEMP_FAILURE_RETRY(__libc_write(__pthread_manager_request,
 				    (char *) &request, sizeof(request)));
+    if (request.req_kind == REQ_FREE_AND_NOTIFY)
+      suspend (self);
   }
   return 0;
 }
--- libc/linuxthreads/descr.h.jj	2003-09-17 13:42:30.000000000 +0200
+++ libc/linuxthreads/descr.h	2004-04-29 17:48:26.765966175 +0200
@@ -189,6 +189,8 @@ struct _pthread_descr_struct
 #endif
   size_t p_alloca_cutoff;	/* Maximum size which should be allocated
 				   using alloca() instead of malloc().  */
+  pthread_descr p_userstack_joining; /* Thread waiting until thread stack
+					can be freed.  */
   /* New elements must be added at the end.  */
 } __attribute__ ((aligned(32))); /* We need to align the structure so that
 				    doubles are aligned properly.  This is 8
--- libc/linuxthreads/internals.h.jj	2003-09-17 13:42:30.000000000 +0200
+++ libc/linuxthreads/internals.h	2004-04-29 11:13:00.000000000 +0200
@@ -81,7 +81,7 @@ struct pthread_request {
   pthread_descr req_thread;     /* Thread doing the request */
   enum {                        /* Request kind */
     REQ_CREATE, REQ_FREE, REQ_PROCESS_EXIT, REQ_MAIN_THREAD_EXIT,
-    REQ_POST, REQ_DEBUG, REQ_KICK, REQ_FOR_EACH_THREAD
+    REQ_POST, REQ_DEBUG, REQ_KICK, REQ_FOR_EACH_THREAD, REQ_FREE_AND_NOTIFY
   } req_kind;
   union {                       /* Arguments for request */
     struct {                    /* For REQ_CREATE: */
@@ -90,7 +90,7 @@ struct pthread_request {
       void * arg;               /*   argument to start function */
       sigset_t mask;            /*   signal mask */
     } create;
-    struct {                    /* For REQ_FREE: */
+    struct {                    /* For REQ_FREE and REQ_FREE_AND_NOTIFY: */
       pthread_t thread_id;      /*   identifier of thread to free */
     } free;
     struct {                    /* For REQ_PROCESS_EXIT: */
--- libc/linuxthreads/manager.c.jj	2003-12-19 17:57:04.000000000 +0100
+++ libc/linuxthreads/manager.c	2004-04-29 17:57:58.221553893 +0200
@@ -101,7 +101,7 @@ static int pthread_handle_create(pthread
                                  sigset_t *mask, int father_pid,
 				 int report_events,
 				 td_thr_events_t *event_maskp);
-static void pthread_handle_free(pthread_t th_id);
+static void pthread_handle_free(pthread_t th_id, pthread_descr req_thread);
 static void pthread_handle_exit(pthread_descr issuing_thread, int exitcode)
      __attribute__ ((noreturn));
 static void pthread_reap_children(void);
@@ -187,8 +187,11 @@ __pthread_manager(void *arg)
         restart(request.req_thread);
         break;
       case REQ_FREE:
-	pthread_handle_free(request.req_args.free.thread_id);
+	pthread_handle_free(request.req_args.free.thread_id, NULL);
         break;
+      case REQ_FREE_AND_NOTIFY:
+	pthread_handle_free(request.req_args.free.thread_id, request.req_thread);
+	break;
       case REQ_PROCESS_EXIT:
         pthread_handle_exit(request.req_thread,
                             request.req_args.exit.code);
@@ -917,7 +920,7 @@ static void pthread_free(pthread_descr t
 
 static void pthread_exited(pid_t pid)
 {
-  pthread_descr th;
+  pthread_descr th, userstack_joining;
   int detached;
   /* Find thread with that pid */
   for (th = __pthread_main_thread->p_nextlive;
@@ -950,9 +953,14 @@ static void pthread_exited(pid_t pid)
 	    }
 	}
       detached = th->p_detached;
+      userstack_joining = th->p_userstack_joining;
       __pthread_unlock(th->p_lock);
       if (detached)
-	pthread_free(th);
+	{
+	  pthread_free(th);
+	  if (userstack_joining)
+	    restart (userstack_joining);
+	}
       break;
     }
   }
@@ -984,7 +992,7 @@ static void pthread_reap_children(void)
 /* Try to free the resources of a thread when requested by pthread_join
    or pthread_detach on a terminated thread. */
 
-static void pthread_handle_free(pthread_t th_id)
+static void pthread_handle_free(pthread_t th_id, pthread_descr req_thread)
 {
   pthread_handle handle = thread_handle(th_id);
   pthread_descr th;
@@ -994,17 +1002,22 @@ static void pthread_handle_free(pthread_
     /* pthread_reap_children has deallocated the thread already,
        nothing needs to be done */
     __pthread_unlock(&handle->h_lock);
+    if (req_thread)
+      restart (req_thread);
     return;
   }
   th = handle->h_descr;
   if (th->p_exited) {
     __pthread_unlock(&handle->h_lock);
     pthread_free(th);
+    if (req_thread)
+      restart (req_thread);
   } else {
     /* The Unix process of the thread is still running.
        Mark the thread as detached so that the thread manager will
        deallocate its resources when the Unix process exits. */
     th->p_detached = 1;
+    th->p_userstack_joining = req_thread;
     __pthread_unlock(&handle->h_lock);
   }
 }
--- libc/linuxthreads/Makefile.jj	2004-02-23 09:46:19.000000000 +0100
+++ libc/linuxthreads/Makefile	2004-04-29 11:31:57.164316902 +0200
@@ -111,7 +111,7 @@ tests = ex1 ex2 ex3 ex4 ex5 ex6 ex7 ex8 
 	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-stack1
+	tst-stack1 tst-stack2
 test-srcs = tst-signal
 # These tests are linked with libc before libpthread
 tests-reverse += tst-cancel5
--- libc/linuxthreads/tst-stack2.c.jj	2004-04-29 15:34:20.398979901 +0200
+++ libc/linuxthreads/tst-stack2.c	2004-04-29 17:58:36.314727101 +0200
@@ -0,0 +1,96 @@
+/* Copyright (C) 2004 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+   Contributed by Jakub Jelinek <jakub@redhat.com>, 2004.
+
+   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.  */
+
+#include <limits.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)
+{
+  int result = 0;
+  for (int i = 0; i < 128; ++i)
+    {
+      void *stack;
+      int res = posix_memalign (&stack, getpagesize (), 16 * PTHREAD_STACK_MIN);
+      if (res)
+	{
+	  printf ("malloc failed %s\n", strerror (res));
+	  return 1;
+	}
+
+      pthread_attr_t attr;
+      pthread_attr_init (&attr);
+
+      res = pthread_attr_setstack (&attr, stack, 16 * PTHREAD_STACK_MIN);
+      if (res)
+	{
+	  printf ("pthread_attr_setstack failed %d\n", res);
+	  result = 1;
+	}
+
+      /* Create the thread.  */
+      pthread_t th;
+      res = pthread_create (&th, &attr, tf, NULL);
+      if (res)
+	{
+	  printf ("pthread_create failed %d\n", res);
+	  result = 1;
+	  break;
+	}
+      else
+	{
+	  res = pthread_join (th, NULL);
+	  if (res)
+	    {
+	      printf ("pthread_join failed %d\n", res);
+	      result = 1;
+	      break;
+	    }
+	}
+
+      free (stack);
+      pthread_attr_destroy (&attr);
+
+    }
+
+  if (seen != 128)
+    {
+      printf ("seen %d != 128\n", seen);
+      result = 1;
+    }
+
+  return result;
+}
+
+#define TIMEOUT 10
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"

	Jakub