* thread.cc (pthread_rwlock::add_reader): Remove mx parameter for

List_insert call.
(pthread::prepare): Ensure race safeness when adding function
pointers to atfork lists by using List_insert.
* thread.h (List_insert): Use InterlockedCompareExchangePointer to
ensure race safeness without using a mutex.
(List_remove): Use InterlockedCompareExchangePointer to
ensure race safeness with List_insert.
(List::insert): Remove mx parameter for	List_insert call.

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index 1030e4b57476b9eb153fac30da205408f746cab3..a4d21b23536dc7a446e88fe6f9223a062d885129 100644 (file)
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,3 +1,15 @@
+2003-12-01  Thomas Pfaff  <tpfaff@gmx.net>
+
+       * thread.cc (pthread_rwlock::add_reader): Remove mx parameter for
+       List_insert call.
+       (pthread::prepare): Ensure race safeness when adding function
+       pointers to atfork lists by using List_insert.
+       * thread.h (List_insert): Use InterlockedCompareExchangePointer to
+       ensure race safeness without using a mutex.
+       (List_remove): Use InterlockedCompareExchangePointer to
+       ensure race safeness with List_insert.
+       (List::insert): Remove mx parameter for List_insert call.
+
 2003-12-01  Corinna Vinschen  <corinna@vinschen.de>
 
        * Makefile.in (OBSOLETE_FUNCTIONS): Add fcntl.

diff --git a/winsup/cygwin/thread.cc b/winsup/cygwin/thread.cc
index db12e5cb716257caaa0ed32fd0f8134e54bba474..cb99be031929c7c69d3623471804ed81ca57ad62 100644 (file)
--- a/winsup/cygwin/thread.cc
+++ b/winsup/cygwin/thread.cc
@@ -1259,7 +1259,7 @@ pthread_rwlock::unlock ()
 void
 pthread_rwlock::add_reader (struct RWLOCK_READER *rd)
 {
-  List_insert (readers_mx, readers, rd);
+  List_insert (readers, rd);
 }
 
 void
@@ -1998,22 +1998,6 @@ pthread::cancel (pthread_t thread)
   return thread->cancel ();
 }
 
-/* Races in pthread_atfork:
-   We are race safe in that any additions to the lists are made via
-   InterlockedExchangePointer.
-   However, if the user application doesn't perform syncronisation of some sort
-   It's not guaranteed that a near simultaneous call to pthread_atfork and fork
-   will result in the new atfork handlers being calls.
-   More rigorous internal syncronisation isn't needed as the user program isn't
-   guaranteeing their own state.
-
-   as far as multiple calls to pthread_atfork, the worst case is simultaneous calls
-   will result in an indeterminate order for parent and child calls (what gets inserted
-   first isn't guaranteed.)
-
-   There is one potential race... Does the result of InterlockedExchangePointer
-   get committed to the return location _before_ any context switches can occur?
-   If yes, we're safe, if no, we're not.  */
 void
 pthread::atforkprepare (void)
 {
@@ -2090,7 +2074,7 @@ pthread::atfork (void (*prepare)(void), void (*parent)(void), void (*child)(void
   if (prepcb)
   {
     prepcb->cb = prepare;
-    prepcb->next = (callback *) InterlockedExchangePointer ((LONG *) &MT_INTERFACE->pthread_prepare, (long int) prepcb);
+    List_insert (MT_INTERFACE->pthread_prepare, prepcb);
   }
   if (parentcb)
   {
@@ -2099,7 +2083,7 @@ pthread::atfork (void (*prepare)(void), void (*parent)(void), void (*child)(void
     while (*t)
       t = &(*t)->next;
     /* t = pointer to last next in the list */
-    parentcb->next = (callback *) InterlockedExchangePointer ((LONG *) t, (long int) parentcb);
+    List_insert (*t, parentcb);
   }
   if (childcb)
   {
@@ -2108,7 +2092,7 @@ pthread::atfork (void (*prepare)(void), void (*parent)(void), void (*child)(void
     while (*t)
       t = &(*t)->next;
     /* t = pointer to last next in the list */
-    childcb->next = (callback *) InterlockedExchangePointer ((LONG *) t, (long int) childcb);
+    List_insert (*t, childcb);
   }
   return 0;
 }

diff --git a/winsup/cygwin/thread.h b/winsup/cygwin/thread.h
index 5dd9857fbb5f2e92bb18304736f612abc6a1edbf..0cba6cd487c1cbe6d32207411748691219950b98 100644 (file)
--- a/winsup/cygwin/thread.h
+++ b/winsup/cygwin/thread.h
@@ -224,14 +224,13 @@ verifyable_object_state verifyable_object_isvalid (void const *, long);
 verifyable_object_state verifyable_object_isvalid (void const *, long, void *);
 
 template <class list_node> inline void
-List_insert (fast_mutex &mx, list_node *&head, list_node *node)
+List_insert (list_node *&head, list_node *node)
 {
   if (!node)
     return;
-  mx.lock ();
-  node->next = head;
-  head = node;
-  mx.unlock ();
+  do
+    node->next = head;
+  while (InterlockedCompareExchangePointer (&head, node, node->next) != node->next);
 }
 
 template <class list_node> inline void
@@ -240,16 +239,17 @@ List_remove (fast_mutex &mx, list_node *&head, list_node *node)
   if (!node)
     return;
   mx.lock ();
-  if (node == head)
-    head = head->next;
-  else if (head)
+  if (head)
     {
-      list_node *cur = head;
-
-      while (cur->next && node != cur->next)
-        cur = cur->next;
-      if (node == cur->next)
-        cur->next = cur->next->next;
+      if (InterlockedCompareExchangePointer (&head, node->next, node) != node)
+        {
+          list_node *cur = head;
+
+          while (cur->next && node != cur->next)
+            cur = cur->next;
+          if (node == cur->next)
+            cur->next = cur->next->next;
+        }
     }
   mx.unlock ();
 }
@@ -274,7 +274,7 @@ template <class list_node> class List
 
   void insert (list_node *node)
   {
-    List_insert (mx, head, node);
+    List_insert (head, node);
   }
 
   void remove (list_node *node)