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

	* pthread/pt-alloc.c: Don't include <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 <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-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/l4/hurd/pt-thread-halt.c (__pthread_thread_halt): Remove
	argument need_dealloc.  Move the deallocation code from this
	file...
	* sysdeps/l4/hurd/pt-thread-dealloc.c: ... to this new file.
	* sysdeps/l4/hurd/pt-sysdep.h (PTHREAD_SYSDEP_MEMBERS): Add field
	have_kernel_resources.
	* sysdeps/l4/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.

11 files changed, 174 insertions, 146 deletions

diff --git a/libpthread/ChangeLog b/libpthread/ChangeLog
index 1670b28..7bd9ee7 100644
--- a/libpthread/ChangeLog
+++ b/libpthread/ChangeLog
@@ -1,3 +1,36 @@
+2008-08-16  Neal H. Walfield  <neal@gnu.org>
+
+	* pthread/pt-alloc.c: Don't include <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 <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-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/l4/hurd/pt-thread-halt.c (__pthread_thread_halt): Remove
+	argument need_dealloc.  Move the deallocation code from this
+	file...
+	* sysdeps/l4/hurd/pt-thread-dealloc.c: ... to this new file.
+	* sysdeps/l4/hurd/pt-sysdep.h (PTHREAD_SYSDEP_MEMBERS): Add field
+	have_kernel_resources.
+	* sysdeps/l4/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.
+
 2008-08-14  Neal H. Walfield  <neal@gnu.org>
 
 	* headers.m4: Remove link from sysroot/include/bits/pthreadtypes.h
diff --git a/libpthread/pthread/pt-alloc.c b/libpthread/pthread/pt-alloc.c
index 6cf9106..6af2da9 100644
--- a/libpthread/pthread/pt-alloc.c
+++ b/libpthread/pthread/pt-alloc.c
@@ -1,5 +1,5 @@
 /* Allocate a new thread structure.
-   Copyright (C) 2000, 2002, 2005, 2007 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 <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,37 +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 (atomic_compare_and_exchange_val_acq (&__pthread_free_threads,
-					       (uintptr_t) new->next,
-					       (uintptr_t) new)
-	  == (uintptr_t) new)
+      /* 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 (atomic_compare_and_exchange_val_acq
-		    (&__pthread_free_threads,
-		     (uintptr_t) new, (uintptr_t) new->next)
-		    == (uintptr_t) new->next)
-		  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/libpthread/pthread/pt-create.c b/libpthread/pthread/pt-create.c
index 5bb9f1f..504cacc 100644
--- a/libpthread/pthread/pt-create.c
+++ b/libpthread/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/libpthread/pthread/pt-dealloc.c b/libpthread/pthread/pt-dealloc.c
index 879608b..92fe1fd 100644
--- a/libpthread/pthread/pt-dealloc.c
+++ b/libpthread/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 <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,22 +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 (atomic_compare_and_exchange_val_acq (&__pthread_free_threads,
-					       (uintptr_t) pthread,
-					       (uintptr_t) pthread->next)
-	  == (uintptr_t) pthread->next)
-	break;
-    }
+  pthread_mutex_lock (&__pthread_free_threads_lock);
+  __pthread_enqueue (&__pthread_free_threads, pthread);
+  pthread_mutex_unlock (&__pthread_free_threads_lock);
 
-  /* NOTREACHED */
+  /* 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/libpthread/pthread/pt-exit.c b/libpthread/pthread/pt-exit.c
index a8f85b1..68c56d7 100644
--- a/libpthread/pthread/pt-exit.c
+++ b/libpthread/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/libpthread/pthread/pt-internal.h b/libpthread/pthread/pt-internal.h
index 9eb84ed..986ec6b 100644
--- a/libpthread/pthread/pt-internal.h
+++ b/libpthread/pthread/pt-internal.h
@@ -38,9 +38,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
 };
 
@@ -215,22 +219,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/libpthread/pthread/pt-join.c b/libpthread/pthread/pt-join.c
index 06e9f1f..153058b 100644
--- a/libpthread/pthread/pt-join.c
+++ b/libpthread/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/libpthread/sysdeps/l4/hurd/pt-sysdep.h b/libpthread/sysdeps/l4/hurd/pt-sysdep.h
index 08bcd14..5e9fabd 100644
--- a/libpthread/sysdeps/l4/hurd/pt-sysdep.h
+++ b/libpthread/sysdeps/l4/hurd/pt-sysdep.h
@@ -42,6 +42,8 @@
   l4_thread_id_t threadid; \
   addr_t exception_area[EXCEPTION_AREA_SIZE / PAGESIZE]; \
   void *exception_area_va; \
+  /* If the above four fields are valid.  */ \
+  bool have_kernel_resources; \
   l4_word_t my_errno;
 
 extern inline struct __pthread *
diff --git a/libpthread/sysdeps/l4/hurd/pt-thread-alloc.c b/libpthread/sysdeps/l4/hurd/pt-thread-alloc.c
index ada7b3b..11af1d0 100644
--- a/libpthread/sysdeps/l4/hurd/pt-thread-alloc.c
+++ b/libpthread/sysdeps/l4/hurd/pt-thread-alloc.c
@@ -35,12 +35,15 @@ extern addr_t meta_data_activity;
 int
 __pthread_thread_alloc (struct __pthread *thread)
 {
+  if (thread->have_kernel_resources)
+    return 0;
+
+
   /* The main thread is already running of course.  */
   if (__pthread_num_threads == 1)
     {
       thread->object = __hurd_startup_data->thread;
       thread->threadid = l4_myself ();
-      return 0;
     }
   else
     {
@@ -91,5 +94,7 @@ __pthread_thread_alloc (struct __pthread *thread)
       thread->object = storage.addr;
     }
 
+  thread->have_kernel_resources = true;
+
   return 0;
 }
diff --git a/libpthread/sysdeps/l4/hurd/pt-thread-dealloc.c b/libpthread/sysdeps/l4/hurd/pt-thread-dealloc.c
new file mode 100644
index 0000000..71b3d96
--- /dev/null
+++ b/libpthread/sysdeps/l4/hurd/pt-thread-dealloc.c
@@ -0,0 +1,55 @@
+/* Deallocate a thread.  Viengoos version.
+   Copyright (C) 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
+   modify it under the terms of the GNU Library General Public License as
+   published by the Free Software Foundation; either version 2 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
+   Library General Public License for more details.
+
+   You should have received a copy of the GNU Library General Public
+   License along with the GNU C Library; see the file COPYING.LIB.  If not,
+   write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+   Boston, MA 02111-1307, USA.  */
+
+#include <assert.h>
+#include <errno.h>
+
+#include <pt-internal.h>
+
+#include <hurd/exceptions.h>
+#include <hurd/mutex.h>
+#include <hurd/as.h>
+#include <hurd/addr.h>
+
+void
+__pthread_thread_dealloc (struct __pthread *thread)
+{
+  assert (thread != _pthread_self ());
+
+  /* Clean up the exception page.  */
+  exception_page_cleanup
+    (ADDR_TO_PTR (addr_extend (thread->exception_area[EXCEPTION_PAGE],
+			       0, PAGESIZE_LOG2)));
+
+  /* Free the storage.  */
+  int i;
+  for (i = 0; i < EXCEPTION_AREA_SIZE / PAGESIZE; i ++)
+    {
+      assert (! ADDR_IS_VOID (thread->exception_area[i]));
+      storage_free (thread->exception_area[i], false);
+    }
+
+  /* And the address space.  */
+  as_free (addr_chop (PTR_TO_ADDR (thread->exception_area_va),
+		      EXCEPTION_AREA_SIZE_LOG2), false);
+
+  storage_free (thread->object, false);
+
+  thread->have_kernel_resources = 0;
+}
diff --git a/libpthread/sysdeps/l4/hurd/pt-thread-halt.c b/libpthread/sysdeps/l4/hurd/pt-thread-halt.c
index 98fefaa..aef1395 100644
--- a/libpthread/sysdeps/l4/hurd/pt-thread-halt.c
+++ b/libpthread/sysdeps/l4/hurd/pt-thread-halt.c
@@ -22,83 +22,11 @@
 
 #include <pt-internal.h>
 
-#include <hurd/exceptions.h>
-#include <hurd/mutex.h>
-#include <hurd/as.h>
-#include <hurd/addr.h>
-
-/* If we try to deallocate our self, we will end up causing a
-   deadlock.  Thus, when a thread tries to free itself, we add it
-   here.  The next thread to free a thread will free it.  */
-ss_mutex_t saved_object_lock;
-static addr_t saved_object;
+#include <hurd/thread.h>
 
 void
-__pthread_thread_halt (struct __pthread *thread, int need_dealloc)
+__pthread_thread_halt (struct __pthread *thread)
 {
-  /* We may deallocate THREAD.  First save any data we need.  */
-
-  addr_t exception_area[EXCEPTION_AREA_SIZE / PAGESIZE];
-  memcpy (exception_area, thread->exception_area,
-	  sizeof (thread->exception_area));
-  memset (thread->exception_area, 0, sizeof (thread->exception_area));
-
-  void *va = thread->exception_area_va;
-
-  addr_t object = thread->object;
-  l4_thread_id_t tid = thread->threadid;
-
-  if (need_dealloc)
-    __pthread_dealloc (thread);
-
-  /* The THREAD data structure is no longer valid.  */
-  thread = NULL;
-
-  /* Deallocate any saved object.  */
-  ss_mutex_lock (&saved_object_lock);
-  if (! ADDR_IS_VOID (saved_object))
-    {
-      storage_free (saved_object, false);
-      saved_object = ADDR_VOID;
-    }
-  ss_mutex_unlock (&saved_object_lock);
-
-  /* Free the exception area.  */
-
-  /* Clean up the exception page.  */
-  exception_page_cleanup
-    (ADDR_TO_PTR (addr_extend (exception_area[EXCEPTION_PAGE],
-			       0, PAGESIZE_LOG2)));
-
-  /* Free the storage.  */
-  int i;
-  for (i = 0; i < EXCEPTION_AREA_SIZE / PAGESIZE; i ++)
-    {
-      assert (! ADDR_IS_VOID (exception_area[i]));
-      storage_free (exception_area[i], false);
-    }
-
-  /* And the address space.  */
-  as_free (addr_chop (PTR_TO_ADDR (va), EXCEPTION_AREA_SIZE_LOG2), false);
-
-  if (tid == l4_myself ())
-    /* If we try to storage_free (storage.addr), we will freeze in the
-       middle.  That's no good.  We set SAVED_OBJECT to our thread
-       object and the next thread in will free us.  */
-    {
-      ss_mutex_lock (&saved_object_lock);
-      saved_object = object;
-      ss_mutex_unlock (&saved_object_lock);
-    }
-  else
-    storage_free (object, false);
-
-  if (tid == l4_myself ())
-    {
-      l4_send_timeout (l4_myself (), L4_NEVER);
-      panic ("Failed to stop thread %x.%x!",
-	     l4_thread_no (l4_myself ()), l4_version (l4_myself ()));
-    }
-  else
-    thread_stop (object);
+  if (thread->have_kernel_resources)
+    thread_stop (thread->object);
 }