[newlib-cygwin] Cygwin: pthreads: iterate over key destructors per POSIX

[newlib-cygwin] Cygwin: pthreads: iterate over key destructors per POSIX

[Corinna Vinschen]

https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=b9ad0fbf286ada6cdb6b86f351196369cf703c73

commit b9ad0fbf286ada6cdb6b86f351196369cf703c73
Author: Corinna Vinschen <corinna@vinschen.de>
Date:   Sat Aug 29 21:08:31 2020 +0200

    Cygwin: pthreads: iterate over key destructors per POSIX
    
    POSIX requires that key destructors are called in a loop
    for each key with a non-NULL value until all values are
    NULL, or until all destructors for non-NULL values
    have been called at least PTHREAD_DESTRUCTOR_ITERATIONS
    (per POSIX: 4) times.
    
    Cygwinonly called all destructors with non-NULL values
    exactly once.  This patch fixes Cygwin to follow POSIX.
    
    Signed-off-by: Corinna Vinschen <corinna@vinschen.de>

Diff:
---
 winsup/cygwin/include/limits.h |  7 +------
 winsup/cygwin/thread.cc        |  4 ++++
 winsup/cygwin/thread.h         | 20 +++++++++++++++++++-
 3 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/winsup/cygwin/include/limits.h b/winsup/cygwin/include/limits.h
index 524a469e4..6a55578f3 100644
--- a/winsup/cygwin/include/limits.h
+++ b/winsup/cygwin/include/limits.h
@@ -225,13 +225,8 @@ details. */
 
 /* Maximum number of attempts made to destroy a thread's thread-specific
    data values on thread exit. */
-/* FIXME: I really don't understand this value.  Why should multiple
-   attempts be necessary to destroy thread-specific data?!?  Anyway, the
-   current value here is 1, taken originally from our pthread.h file,
-   where it was mistakenly defined first.  Unfortunately this value is
-   lower than the POSIX defined minimum value, which is 4. */
 #undef PTHREAD_DESTRUCTOR_ITERATIONS
-#define PTHREAD_DESTRUCTOR_ITERATIONS 1
+#define PTHREAD_DESTRUCTOR_ITERATIONS 4
 
 /* Maximum number of data keys that can be created by a process. */
 /* Tls has 1088 items - and we don't want to use them all :] */
diff --git a/winsup/cygwin/thread.cc b/winsup/cygwin/thread.cc
index e09507e07..617be57c2 100644
--- a/winsup/cygwin/thread.cc
+++ b/winsup/cygwin/thread.cc
@@ -1713,6 +1713,8 @@ pthread_key::_fixup_after_fork ()
   set (fork_buf);
 }
 
+bool pthread_key::iterate_dtors_once_more;
+
 void
 pthread_key::run_destructor ()
 {
@@ -1723,6 +1725,8 @@ pthread_key::run_destructor ()
 	{
 	  set (NULL);
 	  destructor (oldValue);
+	  if (get ())
+	    iterate_dtors_once_more = true;
 	}
     }
 }
diff --git a/winsup/cygwin/thread.h b/winsup/cygwin/thread.h
index 4c6557aef..c574a3915 100644
--- a/winsup/cygwin/thread.h
+++ b/winsup/cygwin/thread.h
@@ -197,6 +197,7 @@ protected:
 class pthread_key: public verifyable_object
 {
   DWORD tls_index;
+  static bool iterate_dtors_once_more;
 public:
   static bool is_good_object (pthread_key_t const *);
 
@@ -218,7 +219,24 @@ public:
 
   static void run_all_destructors ()
   {
-    keys.for_each (&pthread_key::run_destructor);
+    /* POSIX requires at least four iterations of running destructors:
+
+       If, after all the destructors have been called for all non-NULL
+       values with associated destructors, there are still some non-NULL
+       values with associated destructors, then the process is repeated.
+       If, after at least {PTHREAD_DESTRUCTOR_ITERATIONS} iterations of
+       destructor calls for outstanding non-NULL values, there are still
+       some non-NULL values with associated destructors, implementations
+       may stop calling destructors, or they may continue calling
+       destructors until no non-NULL values with associated destructors
+       exist, even though this might result in an infinite loop. */
+    for (int i = 0; i < PTHREAD_DESTRUCTOR_ITERATIONS; ++i)
+      {
+	iterate_dtors_once_more = false;
+	keys.for_each (&pthread_key::run_destructor);
+	if (!iterate_dtors_once_more)
+	  break;
+      }
   }
 
   /* List support calls */