2008-08-16 Neal H. Walfield <neal@gnu.org>

	* pthread/pt-alloc.c: Don't include <bits/atomic.h>.
	(__pthread_free_threads): Change to a struct __pthread *.
	(__pthread_free_threads_lock): New variable.
	(__pthread_alloc): When looking for a TCB to reuse, iterate over
	__pthread_free_threads taking the first for which the STATE field
	is PTHREAD_TERMINATED.  When reusing a TCB, first call
	__pthread_thread_halt on it.
	* pthread/pt-dealloc.c: Don't include <bits/atomic.h>.
	(__pthread_free_threads): Change to a struct __pthread *.
	(__pthread_free_threads_lock): New declaration.
	(__pthread_dealloc): Enqueue PTHREAD on __PTHREAD_FREE_THREADS.
	Set PTHREAD->STATE to PTHREAD_TERMINATED after everything else.
	* pthread/pt-join.c (pthread_join): Call __pthread_thread_halt
	before destroying the thread.  When destroying the thread, call
	__pthread_thread_dealloc on it.
	* pthread/pt-detach.c (pthread_detach): If destroying the thread,
	call __pthread_thread_halt before deallocating the stack.  In this
	case, also call __pthread_thread_dealloc on the tcb.
	* pthread/pt-exit.c (pthread_exit): Call __pthread_dealloc only if
	the thread is detached and then as the last thing we do before
	calling __pthread_thread_halt.
	* pthread/pt-internal.h (__pthread_thread_halt): Remove argument
	NEED_DEALLOC.  Update users.
	* sysdeps/mach/pt-thread-halt.c (__pthread_thread_halt): Remove
	argument need_dealloc.
	* sysdeps/mach/hurd/pt-sysdep.h (PTHREAD_SYSDEP_MEMBERS): Add field
	have_kernel_resources.
	* sysdeps/mach/hurd/pt-thread-alloc.c (__pthread_thread_alloc): If
	THREAD->HAVE_KERNEL_RESOURCES is true, just return.  After
	allocating the resources, set THREAD->HAVE_KERNEL_RESOURCES to
	true.

10 files changed, 95 insertions, 74 deletions

diff --git a/pthread/pt-alloc.c b/pthread/pt-alloc.c
index 30dcede0..6af2da92 100644
--- a/pthread/pt-alloc.c
+++ b/pthread/pt-alloc.c
@@ -1,5 +1,5 @@
 /* Allocate a new thread structure.
-   Copyright (C) 2000, 2002, 2005 Free Software Foundation, Inc.
+   Copyright (C) 2000, 2002, 2005, 2007, 2008 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
@@ -25,8 +25,6 @@
 
 #include <pt-internal.h>
 
-#include <bits/atomic.h>
-
 /* This braindamage is necessary because the standard says that some
    of the threads functions "shall fail" if "No thread could be found
    corresponding to that specified by the given thread ID."  */
@@ -44,9 +42,9 @@ int __pthread_num_threads;
 /* A lock for the table, and the other variables above.  */
 pthread_rwlock_t __pthread_threads_lock;
 
-
 /* List of thread structures corresponding to free thread IDs.  */
-__atomicptr_t __pthread_free_threads;
+struct __pthread *__pthread_free_threads;
+pthread_mutex_t __pthread_free_threads_lock;
 
 static inline error_t
 initialize_pthread (struct __pthread *new, int recycling)
@@ -94,33 +92,35 @@ __pthread_alloc (struct __pthread **pthread)
   int max_threads;
   int new_max_threads;
 
-  /* Try to re-use a thread structure before creating a new one.  */
-  while ((new = (struct __pthread *)__pthread_free_threads))
+  pthread_mutex_lock (&__pthread_free_threads_lock);
+  for (new = __pthread_free_threads; new; new = new->next)
     {
-      if (__atomicptr_compare_and_swap (&__pthread_free_threads,
-					new, new->next))
+      /* There is no need to take NEW->STATE_LOCK: if NEW is on this
+	 list, then it is protected by __PTHREAD_FREE_THREADS_LOCK
+	 except in __pthread_dealloc where after it is added to the
+	 list (with the lock held), it drops the lock and then sets
+	 NEW->STATE and immediately stops using NEW.  */
+      if (new->state == PTHREAD_TERMINATED)
 	{
-	  /* Yes, we managed to get one.  The thread number in the
-             thread structure still refers to the correct slot.  */
-	  err = initialize_pthread (new, 1);
-	  if (err)
-	    /* An error occured, however, we cannot just free NEW as
-	       there may be resources attached to it.  We must return
-	       it to the free list.  */
-	    while (1)
-	      {
-		new->next = (struct __pthread *)__pthread_free_threads;
-		if (__atomicptr_compare_and_swap (&__pthread_free_threads,
-						  new->next, new))
-		  break;
-	      }
-
-	  if (! err)
-	    *pthread = new;
-
-	  return err;
+	  __pthread_dequeue (new);
+	  break;
 	}
     }
+  pthread_mutex_unlock (&__pthread_free_threads_lock);
+
+  if (new)
+    {
+      /* The thread may still be running.  Make sure it is stopped.
+	 If this is the case, then the thread is either at the end of
+	 __pthread_dealloc or in __pthread_thread_halt.  In both
+	 cases, we are interrupt it.  */
+      __pthread_thread_halt (new);
+
+      err = initialize_pthread (new, 1);
+      if (! err)
+	*pthread = new;
+      return err;
+    }
 
   /* Allocate a new thread structure.  */
   new = malloc (sizeof (struct __pthread));
diff --git a/pthread/pt-create.c b/pthread/pt-create.c
index 210f9672..8f62b78e 100644
--- a/pthread/pt-create.c
+++ b/pthread/pt-create.c
@@ -195,7 +195,7 @@ __pthread_create_internal (struct __pthread **thread,
   __pthread_sigstate_destroy (pthread);
  failed_setup:
   __pthread_thread_dealloc (pthread);
-  __pthread_thread_halt (pthread, 0);
+  __pthread_thread_halt (pthread);
  failed_thread_alloc:
   __pthread_stack_dealloc (pthread->stackaddr, pthread->stacksize);
   pthread->stack = 0;
diff --git a/pthread/pt-dealloc.c b/pthread/pt-dealloc.c
index 1fc7a7b3..92fe1fda 100644
--- a/pthread/pt-dealloc.c
+++ b/pthread/pt-dealloc.c
@@ -1,5 +1,5 @@
 /* Deallocate a thread structure.
-   Copyright (C) 2000 Free Software Foundation, Inc.
+   Copyright (C) 2000, 2008 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
@@ -23,13 +23,12 @@
 
 #include <pt-internal.h>
 
-#include <bits/atomic.h>
-
 /* List of thread structures corresponding to free thread IDs.  */
-extern __atomicptr_t __pthread_free_threads;
+extern struct __pthread *__pthread_free_threads;
+extern pthread_mutex_t __pthread_free_threads_lock;
+
 
-/* Deallocate the thread structure for PTHREAD and the resources
-   associated with it.  */
+/* Deallocate the thread structure for PTHREAD.  */
 void
 __pthread_dealloc (struct __pthread *pthread)
 {
@@ -44,20 +43,22 @@ __pthread_dealloc (struct __pthread *pthread)
      pthread_join is completely bogus, but unfortunately allowed
      by the standards.  */
   __pthread_mutex_lock (&pthread->state_lock);
-  pthread->state = PTHREAD_TERMINATED;
   if (pthread->state != PTHREAD_EXITED)
     pthread_cond_broadcast (&pthread->state_cond);
   __pthread_mutex_unlock (&pthread->state_lock);
 
   /* We do not actually deallocate the thread structure, but add it to
      a list of re-usable thread structures.  */
-  while (1)
-    {
-      pthread->next = (struct __pthread *)__pthread_free_threads;
-      if (__atomicptr_compare_and_swap (&__pthread_free_threads,
-					pthread->next, pthread))
-	return;
-    }
-
-  /* NOTREACHED */
+  pthread_mutex_lock (&__pthread_free_threads_lock);
+  __pthread_enqueue (&__pthread_free_threads, pthread);
+  pthread_mutex_unlock (&__pthread_free_threads_lock);
+
+  /* Setting PTHREAD->STATE to PTHREAD_TERMINATED makes this TCB
+     available for reuse.  After that point, we can no longer assume
+     that PTHREAD is valid.
+
+     Note that it is safe to not lock this update to PTHREAD->STATE:
+     the only way that it can now be accessed is in __pthread_alloc,
+     which reads this variable.  */
+  pthread->state = PTHREAD_TERMINATED;
 }
diff --git a/pthread/pt-detach.c b/pthread/pt-detach.c
index 42a84080..1e42c452 100644
--- a/pthread/pt-detach.c
+++ b/pthread/pt-detach.c
@@ -58,10 +58,20 @@ pthread_detach (pthread_t thread)
 
       __pthread_mutex_unlock (&pthread->state_lock);
 
+      /* Make sure the thread is not running before we remove its
+         stack.  (The only possibility is that it is in a call to
+         __pthread_thread_halt itself, but that is enough to cause a
+         sigsegv.)  */
+      __pthread_thread_halt (pthread);
+
+      /* Destroy the stack, the kernel resources and the control
+	 block.  */
       assert (pthread->stack);
       __pthread_stack_dealloc (pthread->stackaddr, pthread->stacksize);
       pthread->stack = 0;
 
+      __pthread_thread_dealloc (pthread);
+
       __pthread_dealloc (pthread);
       break;
 
diff --git a/pthread/pt-exit.c b/pthread/pt-exit.c
index 7484ffd1..5fe0ba86 100644
--- a/pthread/pt-exit.c
+++ b/pthread/pt-exit.c
@@ -28,7 +28,7 @@
 
 
 /* Terminate the current thread and make STATUS available to any
-   thread that might join us.  */
+   thread that might join it.  */
 void
 pthread_exit (void *status)
 {
@@ -70,8 +70,6 @@ pthread_exit (void *status)
   if (self->cancel_state == PTHREAD_CANCEL_ENABLE && self->cancel_pending)
     status = PTHREAD_CANCELED;
 
-  __pthread_thread_dealloc (self);
-
   switch (self->state)
     {
     default:
@@ -86,7 +84,8 @@ pthread_exit (void *status)
          deallocate our own stack.  However, it will eventually be
          reused when this thread structure is recycled.  */
       __pthread_mutex_unlock (&self->state_lock);
-      need_dealloc = 1;
+
+      __pthread_dealloc (self);
 
       break;
 
@@ -103,7 +102,6 @@ pthread_exit (void *status)
          waiting to join us.  */
       pthread_cond_broadcast (&self->state_cond);
       __pthread_mutex_unlock (&self->state_lock);
-      need_dealloc = 0;
 
       break;
     }
@@ -113,7 +111,7 @@ pthread_exit (void *status)
      This means that before freeing any resources, such a thread
      should make sure that this thread is really halted.  */
   
-  __pthread_thread_halt (self, need_dealloc);
+  __pthread_thread_halt (self);
 
   /* NOTREACHED */
   abort ();
diff --git a/pthread/pt-internal.h b/pthread/pt-internal.h
index 0fd7920f..cb441d09 100644
--- a/pthread/pt-internal.h
+++ b/pthread/pt-internal.h
@@ -36,9 +36,13 @@
 /* Thread state.  */
 enum pthread_state
 {
+  /* The thread is running and joinable.  */
   PTHREAD_JOINABLE = 0,
+  /* The thread is running and detached.  */
   PTHREAD_DETACHED,
+  /* A joinable thread exited and its return code is available.  */
   PTHREAD_EXITED,
+  /* The thread structure is unallocated and available for reuse.  */
   PTHREAD_TERMINATED
 };
 
@@ -207,22 +211,22 @@ extern int __pthread_setup (struct __pthread *__restrict thread,
    resources) for THREAD; it must not be placed on the run queue.  */
 extern int __pthread_thread_alloc (struct __pthread *thread);
 
-/* Deallocate any kernel resources associated with THREAD except don't
-   halt the thread itself.  On return, the thread will be marked as
-   dead and __pthread_halt will be called.  */
+/* Deallocate any kernel resources associated with THREAD.  The thread
+   must not be running (that is, if __pthread_thread_start was called,
+   __pthread_thread_halt must first be called).  This function will
+   never be called by a thread on itself.  In the case that a thread
+   exits, its thread structure will be cached and cleaned up
+   later.  */
 extern void __pthread_thread_dealloc (struct __pthread *thread);
 
 /* Start THREAD making it eligible to run.  */
 extern int __pthread_thread_start (struct __pthread *thread);
 
-/* Stop the kernel thread associated with THREAD.  If NEED_DEALLOC is
-   true, the function must call __pthread_dealloc on THREAD.
-
-   NB: The thread executing this function may be the thread which is
-   being halted, thus the last action should be halting the thread
-   itself.  */
-extern void __pthread_thread_halt (struct __pthread *thread,
-				   int need_dealloc);
+/* Stop the kernel thread associated with THREAD.  This function may
+   be called by two threads in parallel.  In particular, by the thread
+   itself and another thread trying to join it.  This function must be
+   implemented such that this is safe.  */
+extern void __pthread_thread_halt (struct __pthread *thread);
 
 
 /* Called by a thread just before it calls the provided start
diff --git a/pthread/pt-join.c b/pthread/pt-join.c
index 06e9f1f7..153058b5 100644
--- a/pthread/pt-join.c
+++ b/pthread/pt-join.c
@@ -54,12 +54,20 @@ pthread_join (pthread_t thread, void **status)
       if (status)
 	*status = pthread->status;
 
-      /* Make sure nobody can reference it anymore, and mark it as
-         terminated.  */
+      /* Make sure the thread is not running before we remove its
+         stack.  (The only possibility is that it is in a call to
+         __pthread_thread_halt itself, but that is enough to cause a
+         sigsegv.)  */
+      __pthread_thread_halt (pthread);
+
+      /* Destroy the stack, the kernel resources and the control
+	 block.  */
       assert (pthread->stack);
       __pthread_stack_dealloc (pthread->stackaddr, pthread->stacksize);
       pthread->stack = 0;
 
+      __pthread_thread_dealloc (pthread);
+
       __pthread_dealloc (pthread);
       break;
 
diff --git a/sysdeps/mach/hurd/pt-sysdep.h b/sysdeps/mach/hurd/pt-sysdep.h
index 83bad963..f14a1366 100644
--- a/sysdeps/mach/hurd/pt-sysdep.h
+++ b/sysdeps/mach/hurd/pt-sysdep.h
@@ -1,5 +1,5 @@
 /* Internal defenitions for pthreads library.
-   Copyright (C) 2000, 2002 Free Software Foundation, Inc.
+   Copyright (C) 2000, 2002, 2008 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
@@ -32,7 +32,8 @@
 
 #define PTHREAD_SYSDEP_MEMBERS \
   thread_t kernel_thread;      \
-  mach_msg_header_t wakeupmsg;
+  mach_msg_header_t wakeupmsg; \
+  int have_kernel_resources;
 
 #define _HURD_THREADVAR_THREAD _HURD_THREADVAR_DYNAMIC_USER
 
diff --git a/sysdeps/mach/pt-thread-alloc.c b/sysdeps/mach/pt-thread-alloc.c
index 1acba98a..7706a2bd 100644
--- a/sysdeps/mach/pt-thread-alloc.c
+++ b/sysdeps/mach/pt-thread-alloc.c
@@ -63,6 +63,9 @@ create_wakeupmsg (struct __pthread *thread)
 int
 __pthread_thread_alloc (struct __pthread *thread)
 {
+  if (thread->have_kernel_resources)
+    return 0;
+
   error_t err;
 
   err = create_wakeupmsg (thread);
@@ -97,5 +100,7 @@ __pthread_thread_alloc (struct __pthread *thread)
 	return EAGAIN;
     }
 
+  thread->have_kernel_resources = true;
+
   return 0;
 }
diff --git a/sysdeps/mach/pt-thread-halt.c b/sysdeps/mach/pt-thread-halt.c
index 9f860247..973cde1e 100644
--- a/sysdeps/mach/pt-thread-halt.c
+++ b/sysdeps/mach/pt-thread-halt.c
@@ -30,14 +30,8 @@
    being halted, thus the last action should be halting the thread
    itself.  */
 void
-__pthread_thread_halt (struct __pthread *thread, int need_dealloc)
+__pthread_thread_halt (struct __pthread *thread)
 {
-  error_t err;
-  thread_t tid = thread->kernel_thread;
-
-  if (need_dealloc)
-    __pthread_dealloc (thread);
-
-  err = __thread_terminate (tid);
+  error_t err = __thread_terminate (thread->kernel_thread);
   assert_perror (err);
 }