Patch for broken pthread_key_delete.

[Kaz Kylheku <kaz at ashi dot footprints dot net>]

diff -urNp --exclude defs.h --exclude '*.S' --exclude='*.d' --exclude 'CVS*' --exclude='*.out' --exclude='*.o' linuxthreads-ORIG/ChangeLog linuxthreads/ChangeLog
--- linuxthreads-ORIG/ChangeLog	Wed Nov 28 14:27:20 2001
+++ linuxthreads/ChangeLog	Wed Nov 28 22:59:07 2001
@@ -1,3 +1,21 @@
+2001-11-28  Kaz Kylheku  <kaz@ashi.footprints.net>
+
+	Bugfix to pthread_key_delete. It was iterating over the thread
+	manager's linked list of threads, behind the thread manager's
+	back causing a race. The fix is to have the manager iterate over
+	the threads instead, using a new request type for doing so.
+
+	* internals.h (struct pthread_request): New manager request type
+	REQ_FOR_EACH_THREAD.
+	* manager.c (pthread_for_each_thread): New function.
+	(__pthread_manager): Handle new REQ_FOR_EACH_THREAD request.
+	* specific.c (struct pthread_key_delete_helper_args): New type.
+	(pthread_key_delete_helper): New static function.
+	(pthread_key_delete): Use the new thread manager 
+	REQ_FOR_EACH_THREAD function to iterate over the threads and set
+	the delete key slot to a null value in each thread.
+	* Examples/ex18.c: New test.
+
 2001-11-22  Wolfram Gloger  <wg@malloc.de>
 
 	* pthread.c (pthread_onexit_process): Don't call free
diff -urNp --exclude defs.h --exclude '*.S' --exclude='*.d' --exclude 'CVS*' --exclude='*.out' --exclude='*.o' linuxthreads-ORIG/Examples/ex18.c linuxthreads/Examples/ex18.c
--- linuxthreads-ORIG/Examples/ex18.c	Wed Dec 31 16:00:00 1969
+++ linuxthreads/Examples/ex18.c	Wed Nov 28 23:00:15 2001
@@ -0,0 +1,112 @@
+/*
+ * Beat up the pthread_key_create and pthread_key_delete
+ * functions.
+ */
+
+#if 0
+#define CHATTY
+#endif
+
+#include <stdio.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <pthread.h>
+
+const int beatup_iterations = 10000;
+const int num_threads = 30;
+const int max_keys = 500;
+
+struct key_list {
+  struct key_list *next;
+  pthread_key_t key;	
+};
+
+struct key_list *key_list;
+pthread_mutex_t key_lock = PTHREAD_MUTEX_INITIALIZER;
+
+/*
+ * Create a new key and put it at the tail of a linked list.
+ * If the linked list grows to a certain length, delete a key from the
+ * head of * the list. 
+ */
+
+static void 
+beat_up(void)
+{
+  struct key_list *new = malloc(sizeof *new);
+  struct key_list **iter, *old_key = 0;
+  int key_count = 0;
+
+  if (new == 0) {
+    fprintf(stderr, "malloc failed\n");
+    abort();
+  }
+
+  new->next = 0;
+
+  if (pthread_key_create(&new->key, 0) != 0) {
+    fprintf(stderr, "pthread_key_create failed\n");
+    abort();
+  }
+
+  if (pthread_getspecific(new->key) != 0) {
+    fprintf(stderr, "new pthread_key_t resolves to non-null value\n");
+    abort();
+  }
+
+  pthread_setspecific(new->key, (void *) 1);
+
+#ifdef CHATTY
+  printf("created key\n");
+#endif
+
+  pthread_mutex_lock(&key_lock);
+
+  for (iter = &key_list; *iter != 0; iter = &(*iter)->next)
+    key_count++;
+
+  *iter = new;
+
+  if (key_count > max_keys) {
+    old_key = key_list;
+    key_list = key_list->next;
+  }
+
+  pthread_mutex_unlock(&key_lock);
+
+  if (old_key != 0) {
+#ifdef CHATTY
+    printf("deleting key\n");
+#endif
+    pthread_key_delete(old_key->key);
+  }
+}
+
+static void *
+thread(void *arg)
+{
+  int i;
+  for (i = 0; i < beatup_iterations; i++) 
+    beat_up();
+  return 0;
+}
+
+int
+main(void)
+{
+  int i;
+  pthread_attr_t detached_thread;
+
+  pthread_attr_init(&detached_thread);
+  pthread_attr_setdetachstate(&detached_thread, PTHREAD_CREATE_DETACHED);
+
+  for (i = 0; i < num_threads; i++) {
+    pthread_t thread_id;
+    while (pthread_create(&thread_id, &detached_thread, thread, 0) == EAGAIN) {
+      /* let some threads die, so system can breathe. :) */
+      sleep(1);
+    }
+  }
+
+  pthread_exit(0);
+}
diff -urNp --exclude defs.h --exclude '*.S' --exclude='*.d' --exclude 'CVS*' --exclude='*.out' --exclude='*.o' linuxthreads-ORIG/Makefile linuxthreads/Makefile
--- linuxthreads-ORIG/Makefile	Tue Jul 24 14:17:50 2001
+++ linuxthreads/Makefile	Wed Nov 28 22:05:23 2001
@@ -60,7 +60,7 @@ endif
 librt-tests = ex10 ex11
 tests = ex1 ex2 ex3 ex4 ex5 ex6 ex7 ex8 ex9 $(librt-tests) ex12 ex13 joinrace \
 	tststack $(tests-nodelete-$(have-z-nodelete)) ecmutex ex14 ex15 ex16 \
-	ex17 tst-cancel tst-context
+	ex17 ex18 tst-cancel tst-context
 
 ifeq (yes,$(build-shared))
 tests-nodelete-yes = unload
diff -urNp --exclude defs.h --exclude '*.S' --exclude='*.d' --exclude 'CVS*' --exclude='*.out' --exclude='*.o' linuxthreads-ORIG/internals.h linuxthreads/internals.h
--- linuxthreads-ORIG/internals.h	Wed Aug 29 19:12:08 2001
+++ linuxthreads/internals.h	Wed Nov 28 18:22:02 2001
@@ -208,7 +208,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_POST, REQ_DEBUG, REQ_KICK, REQ_FOR_EACH_THREAD
   } req_kind;
   union {                       /* Arguments for request */
     struct {                    /* For REQ_CREATE: */
@@ -224,6 +224,10 @@ struct pthread_request {
       int code;                 /*   exit status */
     } exit;
     void * post;                /* For REQ_POST: the semaphore */
+    struct {			/* For REQ_FOR_EACH_THREAD: callback */
+      void (*fn)(void *, pthread_descr);
+      void *arg;
+    } for_each;
   } req_args;
 };
 
diff -urNp --exclude defs.h --exclude '*.S' --exclude='*.d' --exclude 'CVS*' --exclude='*.out' --exclude='*.o' linuxthreads-ORIG/manager.c linuxthreads/manager.c
--- linuxthreads-ORIG/manager.c	Wed Nov 28 14:26:19 2001
+++ linuxthreads/manager.c	Wed Nov 28 18:37:31 2001
@@ -100,6 +100,8 @@ static void pthread_handle_exit(pthread_
      __attribute__ ((noreturn));
 static void pthread_reap_children(void);
 static void pthread_kill_all_threads(int sig, int main_thread_also);
+static void pthread_for_each_thread(void *arg, 
+    void (*fn)(void *, pthread_descr));
 
 /* The server thread managing requests for thread creation and termination */
 
@@ -211,6 +213,11 @@ __pthread_manager(void *arg)
 	/* This is just a prod to get the manager to reap some
 	   threads right away, avoiding a potential delay at shutdown. */
 	break;
+      case REQ_FOR_EACH_THREAD:
+	pthread_for_each_thread(request.req_args.for_each.arg,
+	                        request.req_args.for_each.fn);
+	restart(request.req_thread);
+	break;
       }
     }
   }
@@ -902,6 +909,20 @@ static void pthread_kill_all_threads(int
   }
 }
 
+static void pthread_for_each_thread(void *arg, 
+    void (*fn)(void *, pthread_descr))
+{
+  pthread_descr th;
+
+  for (th = __pthread_main_thread->p_nextlive;
+       th != __pthread_main_thread;
+       th = th->p_nextlive) {
+    fn(arg, th);
+  }
+
+  fn(arg, __pthread_main_thread);
+}
+
 /* Process-wide exit() */
 
 static void pthread_handle_exit(pthread_descr issuing_thread, int exitcode)
diff -urNp --exclude defs.h --exclude '*.S' --exclude='*.d' --exclude 'CVS*' --exclude='*.out' --exclude='*.o' linuxthreads-ORIG/specific.c linuxthreads/specific.c
--- linuxthreads-ORIG/specific.c	Wed Dec 27 09:16:24 2000
+++ linuxthreads/specific.c	Wed Nov 28 18:45:51 2001
@@ -20,6 +20,7 @@
 #include "pthread.h"
 #include "internals.h"
 #include "spinlock.h"
+#include "restart.h"
 #include <bits/libc-lock.h>
 
 
@@ -58,13 +59,38 @@ int __pthread_key_create(pthread_key_t *
 }
 strong_alias (__pthread_key_create, pthread_key_create)
 
-/* Delete a key */
+/* Reset deleted key's value to NULL in each live thread.
+ * NOTE: this executes in the context of the thread manager! */
+
+struct pthread_key_delete_helper_args {
+  /* Damn, we need lexical closures in C! ;) */
+  unsigned int idx1st, idx2nd;
+  pthread_descr self;
+};
+
+static void pthread_key_delete_helper(void *arg, pthread_descr th)
+{
+  struct pthread_key_delete_helper_args *args = arg;
+  unsigned int idx1st = args->idx1st;
+  unsigned int idx2nd = args->idx2nd;
+  pthread_descr self = args->self;
+
+  if (self == 0)
+    self = args->self = thread_self();
+
+  if (!th->p_terminated) {
+    /* pthread_exit() may try to free th->p_specific[idx1st] concurrently. */
+    __pthread_lock(THREAD_GETMEM(th, p_lock), self);
+    if (th->p_specific[idx1st] != NULL)
+      th->p_specific[idx1st][idx2nd] = NULL;
+    __pthread_unlock(THREAD_GETMEM(th, p_lock));
+  }
+}
 
+/* Delete a key */
 int pthread_key_delete(pthread_key_t key)
 {
   pthread_descr self = thread_self();
-  pthread_descr th;
-  unsigned int idx1st, idx2nd;
 
   pthread_mutex_lock(&pthread_keys_mutex);
   if (key >= PTHREAD_KEYS_MAX || !pthread_keys[key].in_use) {
@@ -73,23 +99,29 @@ int pthread_key_delete(pthread_key_t key
   }
   pthread_keys[key].in_use = 0;
   pthread_keys[key].destr = NULL;
+
   /* Set the value of the key to NULL in all running threads, so
      that if the key is reallocated later by pthread_key_create, its
      associated values will be NULL in all threads. */
-  idx1st = key / PTHREAD_KEY_2NDLEVEL_SIZE;
-  idx2nd = key % PTHREAD_KEY_2NDLEVEL_SIZE;
-  th = self;
-  do {
-    /* If the thread already is terminated don't modify the memory.  */
-    if (!th->p_terminated) {
-      /* pthread_exit() may try to free th->p_specific[idx1st] concurrently. */
-      __pthread_lock(THREAD_GETMEM(th, p_lock), self);
-      if (th->p_specific[idx1st] != NULL)
-	th->p_specific[idx1st][idx2nd] = NULL;
-      __pthread_unlock(THREAD_GETMEM(th, p_lock));
-    }
-    th = th->p_nextlive;
-  } while (th != self);
+
+  {
+    struct pthread_key_delete_helper_args args;
+    struct pthread_request request;
+
+    args.idx1st = key / PTHREAD_KEY_2NDLEVEL_SIZE;
+    args.idx2nd = key % PTHREAD_KEY_2NDLEVEL_SIZE;
+    args.self = 0;
+
+    request.req_thread = self;
+    request.req_kind = REQ_FOR_EACH_THREAD;
+    request.req_args.for_each.arg = &args;
+    request.req_args.for_each.fn = pthread_key_delete_helper;
+
+    TEMP_FAILURE_RETRY(__libc_write(__pthread_manager_request,
+				    (char *) &request, sizeof(request)));
+    suspend(self);
+  }
+
   pthread_mutex_unlock(&pthread_keys_mutex);
   return 0;
 }