From 94d2416049fd158471da45380df829ffce203671 Mon Sep 17 00:00:00 2001 From: Thomas Pfaff Date: Mon, 1 Dec 2003 22:10:57 +0000 Subject: [PATCH] * 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. --- winsup/cygwin/ChangeLog | 12 ++++++++++++ winsup/cygwin/thread.cc | 24 ++++-------------------- winsup/cygwin/thread.h | 30 +++++++++++++++--------------- 3 files changed, 31 insertions(+), 35 deletions(-) diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog index 1030e4b57..a4d21b235 100644 --- a/winsup/cygwin/ChangeLog +++ b/winsup/cygwin/ChangeLog @@ -1,3 +1,15 @@ +2003-12-01 Thomas Pfaff + + * 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 * Makefile.in (OBSOLETE_FUNCTIONS): Add fcntl. diff --git a/winsup/cygwin/thread.cc b/winsup/cygwin/thread.cc index db12e5cb7..cb99be031 100644 --- 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 5dd9857fb..0cba6cd48 100644 --- 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 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 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 void insert (list_node *node) { - List_insert (mx, head, node); + List_insert (head, node); } void remove (list_node *node) -- 2.43.5