[gnuastro-commits] master ad70428 2/2: pthread_barrier_t re-usable after pthread_barrier_destroy

[Mohammad Akhlaghi]

branch: master
commit ad704286459b8ea7619c2d8a0ccbf22e320bd01f
Author: Mohammad Akhlaghi <address@hidden>
Commit: Mohammad Akhlaghi <address@hidden>

    pthread_barrier_t re-usable after pthread_barrier_destroy
    
    Barriers can greatly simplify the process of spinning off threads and
    waiting for them to finish. They allow us to define detached threads (where
    no communication is needed between the threads). However, they are a
    high-level pthreads construct and are thus optional in POSIX. Therefore
    they are not present in some OSs. So in Gnuastro we had an internal
    implementation.
    
    It was reported by Alan Lefor that on one of his Mac OS X systems, `make
    check' hangs and doens't finish. Since it could only be reproduced on his
    system, it took us several days of successive tests to narrow down the
    problem and find the cause.
    
    In our implementation of pthread_barrier, (which was inspired from another
    webpage), there was one hidden assumption: we won't be using the barrier
    any more. So the `pthread_barrier_destroy' function would just destroy the
    mutex and condition variable when the `pthread_barrier_wait' of the main
    thread-spinner function was freed. However, all the separate thread
    functions are also waiting behind the `pthread_barrier_wait' functions and
    there is no particular order to distinguish the thread-spinner's wait
    function from the separate threads. So it was possible that the
    thread-spinner's wait would be freed before the threads. Therefore, those
    threads that would get freed after the thread-spinner would hit an
    un-initialized mutex and condition variable.
    
    To fix this problem, a new element was added to the pthread_barrier
    function to count how many condition variables have been freed (are no
    longer needed). The `pthread_barrier_destroy' function (which is only
    called once by the thread-spinner) now waits (using the `sleep' function)
    until no more threads need the condition variables and only after they are
    all done, will it destroy the condition variable and mutex. The wait is
    1000 nano-seconds or 1 micro-second, and is only necessary when the
    thread-spinner gets to destroy, so its effect on performance can be
    negligible.
    
    This fixes bug #49049.
---
 lib/gnuastro/threads.h |   14 ++++++---
 lib/threads.c          |   75 ++++++++++++++++++++++++++++++++++--------------
 2 files changed, 64 insertions(+), 25 deletions(-)

diff --git a/lib/gnuastro/threads.h b/lib/gnuastro/threads.h
index d0b276b..3a1db59 100644
--- a/lib/gnuastro/threads.h
+++ b/lib/gnuastro/threads.h
@@ -49,6 +49,10 @@ along with Gnuastro. If not, see 
<http://www.gnu.org/licenses/>.
 /*****************************************************************/
 #ifndef HAVE_PTHREAD_BARRIER
 
+/* Integer number of nano-seconds that `pthread_barrier_destroy' should
+   wait for a check to see if all barriers have been reached. */
+#define GAL_THREADS_BARRIER_DESTROY_NANOSECS 1000
+
 typedef int pthread_barrierattr_t;
 
 typedef struct
@@ -56,18 +60,20 @@ typedef struct
   pthread_mutex_t mutex;
   pthread_cond_t   cond;
   size_t          count;
-  size_t      tripCount;
+  size_t          limit;
+  size_t   condfinished;
 } pthread_barrier_t;
 
 int
 pthread_barrier_init(pthread_barrier_t *b, pthread_barrierattr_t *attr,
-                     unsigned int count);
-int
-pthread_barrier_destroy(pthread_barrier_t *b);
+                     unsigned int limit);
 
 int
 pthread_barrier_wait(pthread_barrier_t *b);
 
+int
+pthread_barrier_destroy(pthread_barrier_t *b);
+
 #endif
 
 
diff --git a/lib/threads.c b/lib/threads.c
index 3d8ca26..89ff98b 100644
--- a/lib/threads.c
+++ b/lib/threads.c
@@ -22,6 +22,7 @@ along with Gnuastro. If not, see 
<http://www.gnu.org/licenses/>.
 **********************************************************************/
 #include <config.h>
 
+#include <time.h>
 #include <stdio.h>
 #include <errno.h>
 #include <error.h>
@@ -42,17 +43,20 @@ along with Gnuastro. If not, see 
<http://www.gnu.org/licenses/>.
 http://blog.albertarmea.com/post/47089939939/using-pthread-barrier-on-mac-os-x
  */
 #ifndef HAVE_PTHREAD_BARRIER
+
+/* Initialize the barrier structure. A barrier is a high-level way to wait
+   until several threads have finished. */
 int
 pthread_barrier_init(pthread_barrier_t *b, pthread_barrierattr_t *attr,
-                     unsigned int count)
+                     unsigned int limit)
 {
   int err;
 
   /* Sanity check: */
-  if(count==0)
+  if(limit==0)
     {
       errno = EINVAL;
-      error(EXIT_FAILURE, errno, "in pthread_barrier_init, count is zero");
+      error(EXIT_FAILURE, errno, "in pthread_barrier_init, limit is zero");
     }
 
   /* Initialize the mutex: */
@@ -69,21 +73,9 @@ pthread_barrier_init(pthread_barrier_t *b, 
pthread_barrierattr_t *attr,
     }
 
   /* set the values: */
-  b->tripCount=count;
-  b->count=0;
-
-  return 0;
-}
-
-
-
+  b->limit=limit;
+  b->condfinished=b->count=0;
 
-
-int
-pthread_barrier_destroy(pthread_barrier_t *b)
-{
-  pthread_cond_destroy(&b->cond);
-  pthread_mutex_destroy(&b->mutex);
   return 0;
 }
 
@@ -91,25 +83,66 @@ pthread_barrier_destroy(pthread_barrier_t *b)
 
 
 
+/* Suspend the calling thread (tell it to wait), until the limiting number
+   of barriers is reached by the other threads. When the number isn't
+   reached yet (process goes into the `else'), then we use the
+   `pthread_cond_wait' function, which will wait until a broadcast is
+   announced by another thread that succeeds the `if'. After the thread no
+   longer needs the condition variable, we increment `b->condfinished' so
+   `pthread_barrier_destroy' can know if it should wait (sleep) or
+   continue.*/
 int
 pthread_barrier_wait(pthread_barrier_t *b)
 {
   pthread_mutex_lock(&b->mutex);
-  ++(b->count);
-  if(b->count >= b->tripCount)
+  ++b->count;
+  if(b->count >= b->limit)
     {
-      b->count = 0;
       pthread_cond_broadcast(&b->cond);
+      ++b->condfinished;
       pthread_mutex_unlock(&b->mutex);
       return 1;
     }
   else
     {
-      pthread_cond_wait(&b->cond, &(b->mutex));
+      /* Initially b->count is smaller than b->limit, otherwise control
+         would never have reached here. However, when it get to
+         `pthread_cond_wait', this thread goes into a suspended period and
+         is only awaken when a broad-cast is made. But we only want this
+         thread to finish when b->count >= b->limit, so we have this while
+         loop. */
+      while(b->count < b->limit)
+        pthread_cond_wait(&b->cond, &b->mutex);
+      ++b->condfinished;
       pthread_mutex_unlock(&b->mutex);
       return 0;
     }
 }
+
+
+
+
+
+/* Wait until all the threads that were blocked behind this barrier have
+   finished (don't need the mutex and condition variable anymore. Then
+   destroy the two. After this function, you can safely re-use the
+   pthread_barrier_t structure. */
+int
+pthread_barrier_destroy(pthread_barrier_t *b)
+{
+  struct timespec request, remaining;
+
+  /* Wait until no more threads need the barrier. */
+  request.tv_sec=0;
+  request.tv_nsec=GAL_THREADS_BARRIER_DESTROY_NANOSECS;
+  while( b->condfinished < b->limit )
+    nanosleep(&request, &remaining);
+
+  /* Destroy the condition variable and mutex */
+  pthread_cond_destroy(&b->cond);
+  pthread_mutex_destroy(&b->mutex);
+  return 0;
+}
 #endif