2002-09-21 Robert Collins <rbtcollins@hotmail.com>

        * thread.cc: Finish the removal of the separate pthread_key
        destructor list.
        Remove all pthread_key_destructor and pthread_key_destructor_list
        references throughout.
        (pthread::exit): Call the new pthread_key interface to activate
        destructors.
        (pthread_key::keys): Change into a list.
        (pthread_key::saveAKey): New method, used via forEach.
        (pthread_key::restoreAKey): Ditto.
        (pthread_key::destroyAKey): Ditto.
        (pthread_key::fixup_before_fork): Use the List::forEach functionality.
        (pthread_key::fixup_after_fork): Ditto.
        (pthread_key::runAllDestructors): New method implementation.
        (pthread_key::pthread_key): Use List::Insert rather than custom list
        code.
        (pthread_key::~pthread_key): Use List::Remove for the same reason.
        * thread.h: Remove all pthread_key_destructor and
        pthread_key_destructor_list references throughout.
        (List): Move the interface above pthread_key in the header.
        Use atomic operations during insert and delete.
        (List::forEach): A generic interface for doing something on each node.
        (pthread_key::runAllDestructors): New method, run all destructors.
        (pthread_key::fork_buf): Make private.
        (pthread_key::run_destructor): Ditto.
        (pthread_key::saveAKey): New method for clearer source.
        (pthread_key::restoreAKey): Ditto.
        (pthread_key::destroyAKey): Ditto.
        (MTinterface::destructors): Remove.

3 files changed, 107 insertions, 148 deletions

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index 5f03faf..bd271b2 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,5 +1,36 @@
 2002-09-21  Robert Collins <rbtcollins@hotmail.com>
 
+	* thread.cc: Finish the removal of the separate pthread_key
+	destructor list.
+	Remove all pthread_key_destructor and pthread_key_destructor_list
+	references throughout.
+	(pthread::exit): Call the new pthread_key interface to activate
+	destructors.
+	(pthread_key::keys): Change into a list.
+	(pthread_key::saveAKey): New method, used via forEach.
+	(pthread_key::restoreAKey): Ditto.
+	(pthread_key::destroyAKey): Ditto.
+	(pthread_key::fixup_before_fork): Use the List::forEach functionality.
+	(pthread_key::fixup_after_fork): Ditto.
+	(pthread_key::runAllDestructors): New method implementation.
+	(pthread_key::pthread_key): Use List::Insert rather than custom list
+	code.
+	(pthread_key::~pthread_key): Use List::Remove for the same reason.
+	* thread.h: Remove all pthread_key_destructor and
+	pthread_key_destructor_list references throughout.
+	(List): Move the interface above pthread_key in the header.
+	Use atomic operations during insert and delete.
+	(List::forEach): A generic interface for doing something on each node.
+	(pthread_key::runAllDestructors): New method, run all destructors.
+	(pthread_key::fork_buf): Make private.
+	(pthread_key::run_destructor): Ditto.
+	(pthread_key::saveAKey): New method for clearer source.
+	(pthread_key::restoreAKey): Ditto.
+	(pthread_key::destroyAKey): Ditto.
+	(MTinterface::destructors): Remove.
+
+2002-09-21  Robert Collins <rbtcollins@hotmail.com>
+
 	* thread.cc: Partial refactoring of pthread_key destructor 
 	handling. Loosely based on Thomas Pfaff's work.
 	(pthread_key_destructor_list::Insert): Remove.
diff --git a/winsup/cygwin/thread.cc b/winsup/cygwin/thread.cc
index 1683b21..20a56bd 100644
--- a/winsup/cygwin/thread.cc
+++ b/winsup/cygwin/thread.cc
@@ -46,73 +46,6 @@ details. */
 
 extern int threadsafe;
 
-/*pthread_key_destructor_list class: to-be threadsafe single linked list
- *FIXME: Put me in a dedicated file, or a least a tools area !
- */
-
-pthread_key_destructor *
-pthread_key_destructor::InsertAfter (pthread_key_destructor *node)
-{
-  pthread_key_destructor *temp = next;
-  next = node;
-  return temp;
-}
-
-pthread_key_destructor *
-pthread_key_destructor::UnlinkNext ()
-{
-  pthread_key_destructor *temp = next;
-  if (next)
-    next = next->Next ();
-  return temp;
-}
-
-pthread_key_destructor *
-pthread_key_destructor::Next ()
-{
-  return next;
-}
-
-  /*remove a given dataitem, wherever in the list it is */
-pthread_key_destructor *
-pthread_key_destructor_list::Remove (pthread_key *key)
-{
-  if (!key)
-    return NULL;
-  if (!head)
-    return NULL;
-  if (key == head->key)
-    return Pop ();
-  pthread_key_destructor *temp = head;
-  while (temp && temp->Next () && !(key == temp->Next ()->key))
-    {
-      temp = temp->Next ();
-    }
-  if (temp)
-    return temp->UnlinkNext ();
-  return NULL;
-}
-
-pthread_key_destructor::
-pthread_key_destructor (void (*thedestructor) (void *), pthread_key *key)
-{
-  destructor = thedestructor;
-  next = NULL;
-  this->key = key;
-}
-
-void
-pthread_key_destructor_list::IterateNull ()
-{
-  pthread_key_destructor *temp = head;
-  while (temp)
-    {
-      temp->key->run_destructor ();
-      temp = temp->Next ();
-    }
-}
-
-
 #define MT_INTERFACE user_data->threadinterface
 
 struct _reent *
@@ -438,7 +371,7 @@ pthread::exit (void *value_ptr)
   // run cleanup handlers
   pop_all_cleanup_handlers ();
 
-  MT_INTERFACE->destructors.IterateNull ();
+  pthread_key::runAllDestructors ();
 
   mutex.Lock ();
   // cleanup if thread is in detached state and not joined
@@ -1001,30 +934,42 @@ pthread_cond::fixup_after_fork ()
 
 /* pthread_key */
 /* static members */
-pthread_key *pthread_key::keys = NULL;
+List<pthread_key> pthread_key::keys;
+
+void
+pthread_key::saveAKey (pthread_key *key)
+{
+  key->saveKeyToBuffer ();
+}
 
 void
 pthread_key::fixup_before_fork ()
 {
-  pthread_key *key = keys;
-  debug_printf ("keys is %x",keys);
-  while (key)
-    {
-      key->saveKeyToBuffer ();
-      key = key->next;
-    }
+  keys.forEach (saveAKey);
+}
+
+void
+pthread_key::restoreAKey (pthread_key *key)
+{
+  key->recreateKeyFromBuffer ();
 }
 
 void
 pthread_key::fixup_after_fork ()
 {
-  pthread_key *key = keys;
-  debug_printf ("keys is %x",keys);
-  while (key)
-    {
-      key->recreateKeyFromBuffer ();
-      key = key->next;
-    }
+  keys.forEach (restoreAKey);
+}
+
+void
+pthread_key::destroyAKey (pthread_key *key)
+{
+  key->run_destructor ();
+}
+
+void
+pthread_key::runAllDestructors ()
+{
+  keys.forEach (destroyAKey);
 }
 
 bool
@@ -1042,31 +987,18 @@ pthread_key::pthread_key (void (*aDestructor) (void *)):verifyable_object (PTHRE
   dwTlsIndex = TlsAlloc ();
   if (dwTlsIndex == TLS_OUT_OF_INDEXES)
     magic = 0;
-  else if (destructor)
-    {
-      MT_INTERFACE->destructors.
-	Insert (new pthread_key_destructor (destructor, this));
-    }
-  /* threadsafe addition is easy */
-  next = (pthread_key *) InterlockedExchangePointer (&keys, this);
+  else
+    keys.Insert (this);
 }
 
 pthread_key::~pthread_key ()
 {
-  if (pthread_key_destructor *dest = MT_INTERFACE->destructors.Remove (this))
-    delete dest;
-  TlsFree (dwTlsIndex);
-
-  /* I'm not 100% sure the next bit is threadsafe. I think it is... */
-  if (keys == this)
-    InterlockedExchangePointer (keys, this->next);
-  else
+  /* We may need to make the list code lock the list during operations
+   */
+  if (magic != 0) 
     {
-      pthread_key *tempkey = keys;
-      while (tempkey->next && tempkey->next != this)
-        tempkey = tempkey->next;
-      /* but there may be a race between the loop above and this statement */
-      InterlockedExchangePointer (&tempkey->next, this->next);
+      keys.Remove (this);
+      TlsFree (dwTlsIndex);
     }
 }
 
diff --git a/winsup/cygwin/thread.h b/winsup/cygwin/thread.h
index 05bf979..3585b0f 100644
--- a/winsup/cygwin/thread.h
+++ b/winsup/cygwin/thread.h
@@ -173,49 +173,60 @@ typedef enum
 verifyable_object_state verifyable_object_isvalid (void const *, long);
 verifyable_object_state verifyable_object_isvalid (void const *, long, void *);
 
+/* interface */
+template <class ListNode> class List {
+public:
+  List();
+  void Insert (ListNode *aNode);
+  ListNode *Remove ( ListNode *aNode);
+  ListNode *Pop ();
+  void forEach (void (*)(ListNode *aNode));
+protected:
+  ListNode *head;
+};
+
 class pthread_key:public verifyable_object
 {
 public:
   static bool isGoodObject (pthread_key_t const *);
+  static void runAllDestructors ();
 
   DWORD dwTlsIndex;
-  void *fork_buf;
-  class pthread_key *next;
 
   int set (const void *);
   void *get () const;
-  void run_destructor () const;
 
   pthread_key (void (*)(void *));
    ~pthread_key ();
   static void fixup_before_fork();
   static void fixup_after_fork();
+
+  /* List support calls */
+  class pthread_key *next;
 private:
   // lists of objects. USE THREADSAFE INSERTS AND DELETES.
-  static pthread_key * keys;
+  static List<pthread_key> keys;
+  static void saveAKey (pthread_key *);
+  static void restoreAKey (pthread_key *);
+  static void destroyAKey (pthread_key *);
   void saveKeyToBuffer ();
   void recreateKeyFromBuffer ();
   void (*destructor) (void *);
+  void run_destructor () const;
+  void *fork_buf;
 };
 
-/* interface */
-template <class ListNode> class List {
-public:
-  void Insert (ListNode *aNode);
-  ListNode *Remove ( ListNode *aNode);
-  ListNode *Pop ();
-protected:
-  ListNode *head;
-};
 /* implementation */
+template <class ListNode>
+List<ListNode>::List<ListNode> () : head(NULL)
+{
+}
 template <class ListNode> void
 List<ListNode>::Insert (ListNode *aNode)
 {
   if (!aNode)
     return;
-  head = aNode->InsertAfter (head);
-  if (!head)
-    head = aNode;                /*first node special case */
+  aNode->next = (ListNode *) InterlockedExchangePointer (&head, aNode);
 }
 template <class ListNode> ListNode *
 List<ListNode>::Remove ( ListNode *aNode)
@@ -227,42 +238,28 @@ List<ListNode>::Remove ( ListNode *aNode)
   if (aNode == head)
     return Pop ();
   ListNode *resultPrev = head;
-  while (resultPrev && resultPrev->Next() && !(aNode == resultPrev->Next()))
-    resultPrev = resultprev->Next();
+  while (resultPrev && resultPrev->next && !(aNode == resultPrev->next))
+    resultPrev = resultPrev->next;
   if (resultPrev)
-    return resultPrev->UnlinkNext ();
+    return (ListNode *)InterlockedExchangePointer (&resultPrev->next, resultPrev->next->next);
   return NULL;
 }
 template <class ListNode> ListNode *
 List<ListNode>::Pop ()
 {
-  ListNode *result = head;
-  head = head->Next();
-  return result;
+  return (ListNode *) InterlockedExchangePointer (&head, head->next);
 }
-
-
-/* FIXME: test using multiple inheritance and merging key_destructor into pthread_key
- * for efficiency */
-class pthread_key_destructor
-{
-public:
-  void (*destructor) (void *);
-  pthread_key_destructor *InsertAfter (pthread_key_destructor * node);
-  pthread_key_destructor *UnlinkNext ();
-  pthread_key_destructor *Next ();
-
-    pthread_key_destructor (void (*thedestructor) (void *), pthread_key * key);
-  pthread_key_destructor *next;
-  pthread_key *key;
-};
-
-class pthread_key_destructor_list : public List<pthread_key_destructor>
+/* poor mans generic programming. */
+template <class ListNode> void
+List<ListNode>::forEach (void (*callback)(ListNode *))
 {
-public:
-  pthread_key_destructor *Remove (pthread_key * key);
-  void IterateNull ();
-};
+  ListNode *aNode = head;
+  while (aNode) 
+    {
+      callback (aNode);
+      aNode = aNode->next;
+    }
+}
 
 class pthread_attr:public verifyable_object
 {
@@ -478,7 +475,6 @@ public:
   struct _winsup_t winsup_reent;
   pthread mainthread;
 
-  pthread_key_destructor_list destructors;
   callback *pthread_prepare;
   callback *pthread_child;
   callback *pthread_parent;