1 files changed, 111 insertions, 46 deletions

diff --git a/ptw32_callUserDestroyRoutines.c b/ptw32_callUserDestroyRoutines.c
index 226c1d8..cdec87e 100644
--- a/ptw32_callUserDestroyRoutines.c
+++ b/ptw32_callUserDestroyRoutines.c
@@ -64,86 +64,151 @@ ptw32_callUserDestroyRoutines (pthread_t thread)
       * -------------------------------------------------------------------
       */
 {
-  ThreadKeyAssoc * next;
   ThreadKeyAssoc * assoc;
 
   if (thread.p != NULL)
     {
+      int assocsRemaining;
+      int iterations = 0;
       ptw32_thread_t * sp = (ptw32_thread_t *) thread.p;
 
       /*
        * Run through all Thread<-->Key associations
        * for the current thread.
+       *
+       * Do this process at most PTHREAD_DESTRUCTOR_ITERATIONS times.
+       *
+       * First we need to serialise with pthread_key_delete by locking
+       * both assoc guards, but in the reverse order to normal, and so
+       * we must be careful to avoid deadlock.
        */
-      assoc = next = (ThreadKeyAssoc *) sp->keys;
-
-      while (assoc != NULL)
+      do
+	{
+	  assocsRemaining = 0;
+	  iterations++;
+
+	  (void) pthread_mutex_lock(&(sp->threadLock));
+	  /*
+	   * The next assoc pointer is stored in the thread struct so that
+	   * the assoc destructor in pthread_key_delete can adjust it
+	   * if it deletes this assoc. This can happen if we fail to acquire
+	   * both locks below, and are forced to release all of our locks,
+	   * leaving open the opportunity.
+	   */
+	  sp->nextAssoc = sp->keys;
+	  (void) pthread_mutex_unlock(&(sp->threadLock));
+
+      for (;;)
 	{
+	  void * value;
 	  pthread_key_t k;
+	  void (*destructor) (void *);
+
+	  (void) pthread_mutex_lock(&(sp->threadLock));
 
-	  if ((k = assoc->key) != NULL
-	      && pthread_mutex_lock(&(k->keyLock)) == 0)
+	  if ((assoc = (ThreadKeyAssoc *)sp->nextAssoc) == NULL)
+	    {
+	      /* Finished */
+	      pthread_mutex_unlock(&(sp->threadLock));
+	      break;
+	    }
+	  else
 	    {
 	      /*
-	       * Key still active; pthread_key_delete
-	       * will block on this same mutex before
-	       * it can release actual key; therefore,
-	       * key is valid and we can call the destroy
-	       * routine;
+	       * assoc->key must be valid because assoc can't change or be
+	       * removed from our chain while we hold at least one lock. If
+	       * the assoc was on our key chain then the key has not been
+	       * deleted yet.
+	       *
+	       * Try to acquire both locks without deadlocking.
 	       */
-	      void * value = NULL;
-
-	      value = pthread_getspecific (k);
-	      if (value != NULL && k->destructor != NULL)
+	      if (pthread_mutex_trylock(&(assoc->key->keyLock)) == EBUSY)
 		{
+		  pthread_mutex_unlock(&(sp->threadLock));
+		  Sleep(1); // Ugly.
+		  /*
+		   * Go around again.
+		   * If pthread_key_delete has removed this assoc in the meantime,
+		   * sp->keys will point to a new assoc.
+		   */
+		  continue;
+		}
+	    }
 
-#ifdef __cplusplus
+	  /* We now hold both locks */
 
-		  try
-		    {
-		      /*
-		       * Run the caller's cleanup routine.
-		       */
-		      (*(k->destructor)) (value);
-		    }
-		  catch (...)
-		    {
-		      /*
-		       * A system unexpected exception has occurred
-		       * running the user's destructor.
-		       * We get control back within this block in case
-		       * the application has set up it's own terminate
-		       * handler. Since we are leaving the thread we
-		       * should not get any internal pthreads
-		       * exceptions.
-		       */
-		      (void) pthread_mutex_unlock(&(k->keyLock));
-		      terminate ();
-		    }
+	   sp->nextAssoc = assoc->nextKey;
 
-#else /* __cplusplus */
+	  /*
+	   * Key still active; pthread_key_delete
+	   * will block on these same mutexes before
+	   * it can release actual key; therefore,
+	   * key is valid and we can call the destroy
+	   * routine;
+	   */
+	  k = assoc->key;
+	  destructor = k->destructor;
+	  value = TlsGetValue(k->key);
+	  TlsSetValue (k->key, NULL);
+
+	  // Every assoc->key exists and has a destructor
+	  if (value != NULL && iterations <= PTHREAD_DESTRUCTOR_ITERATIONS)
+	    {
+	      /*
+	       * Unlock both locks before the destructor runs.
+	       * POSIX says pthread_key_delete can be run from destructors,
+	       * and that probably includes with this key as target.
+	       */
+	      (void) pthread_mutex_unlock(&(sp->threadLock));
+	      (void) pthread_mutex_unlock(&(k->keyLock));
 
+	      assocsRemaining++;
+
+#ifdef __cplusplus
+
+	      try
+		{
 		  /*
 		   * Run the caller's cleanup routine.
 		   */
-		  (*(k->destructor)) (value);
+		  destructor (value);
+		}
+	      catch (...)
+		{
+		  /*
+		   * A system unexpected exception has occurred
+		   * running the user's destructor.
+		   * We get control back within this block in case
+		   * the application has set up it's own terminate
+		   * handler. Since we are leaving the thread we
+		   * should not get any internal pthreads
+		   * exceptions.
+		   */
+		  terminate ();
+		}
+
+#else /* __cplusplus */
+
+	      /*
+	       * Run the caller's cleanup routine.
+	       */
+	      destructor (value);
 
 #endif /* __cplusplus */
-		}
 
+	    }
+	  else
+	    {
 	      /*
 	       * Remove association from both the key and thread chains
+	       * and reclaim it's memory resources.
 	       */
-	      (void) pthread_mutex_lock(&(sp->threadLock));
-	      next = assoc->nextKey;
 	      ptw32_tkAssocDestroy (assoc);
 	      (void) pthread_mutex_unlock(&(sp->threadLock));
-
-	      assoc = next;
-
 	      (void) pthread_mutex_unlock(&(k->keyLock));
 	    }
 	}
+	}
+      while (assocsRemaining);
     }
-
 }				/* ptw32_callUserDestroyRoutines */