[PATCH] [Issue #857] Add assert and guard to prevent the next round of barrier-waits from starting before the previous round of barrier-waits completes

['rean' via OSv Development]

Signed-off-by: Rean Griffith <r...@caa.columbia.edu>
---
 include/osv/latch.hh    |  3 +++
 libc/pthread_barrier.cc | 25 ++++++++++++++++---------
 2 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/include/osv/latch.hh b/include/osv/latch.hh
index 0fc47ba..d8a90a9 100644
--- a/include/osv/latch.hh
+++ b/include/osv/latch.hh
@@ -10,6 +10,7 @@
 #include <condition_variable>
 #include <mutex>
 #include <atomic>
+#include <assert.h>
 
 class latch
 {
@@ -29,6 +30,8 @@ public:
         if (_count.fetch_add(-1, std::memory_order_release) == 1) {
             _condvar.notify_all();
         }
+        // Sanity check
+        assert(_count.load(std::memory_order_acquire) >= 0);
     }
 
     bool is_released()
diff --git a/libc/pthread_barrier.cc b/libc/pthread_barrier.cc
index 2437d73..0fbbb63 100644
--- a/libc/pthread_barrier.cc
+++ b/libc/pthread_barrier.cc
@@ -15,6 +15,7 @@
 // pthread_barrierattr_t
 typedef struct
 {
+    unsigned int in;
     unsigned int out;
     unsigned int count;
     latch *ltch;
@@ -47,6 +48,7 @@ int pthread_barrier_init(pthread_barrier_t *barrier_opq,
     // threads can manipulate the memory area associated with the
     // pthread_barrier_t so it doesn't matter what the value of pshared is set 
to
     barrier->count = count;
+    barrier->in = 0;
     barrier->out = 0;
     barrier->ltch = new latch(count);
     barrier->mtx = new pthread_mutex_t;
@@ -67,7 +69,19 @@ int pthread_barrier_wait(pthread_barrier_t *barrier_opq)
     int retval = 0;
     pthread_mutex_t *mtx = barrier->mtx;
 
+    // Check whether we are about to start another round of waiting on the
+    // barrier before the previous round completes (i.e., the 'in' counter
+    // on the barrier plus 1 from the current thread entering is greater
+    // than barrier->count). If the round isn't complete then we should
+    // sleep until the previous round is finished and the 'in' counter is
+    // reset by the last thread exiting from the previous round
+    while(barrier->in + 1 > barrier->count) {
+        usleep(5);
+    }
+
     pthread_mutex_lock(mtx);
+    // Increment the incoming count
+    barrier->in++;
     pthread_mutex_unlock(mtx);
 
     latch *l  = barrier->ltch;
@@ -75,15 +89,6 @@ int pthread_barrier_wait(pthread_barrier_t *barrier_opq)
     // All threads stuck here until we get at least 'count' waiters
     l->await();
 
-    // If the last thread (thread x) to wait on the barrier is descheduled here
-    // (immediately after being the count'th thread crossing the barrier)
-    // the barrier remains open (a new waiting thread will cross) until
-    // the barrier is reset below (when thread x is rescheduled), which doesn't
-    // seem technically incorrect. Only one of the crossing threads will get a
-    // retval of PTHREAD_BARRIER_SERIAL_THREAD, when
-    // barrier->out == barrier->count.
-    // All other crossing threads will get a retval of 0.
-
     pthread_mutex_lock(mtx);
     barrier->out++;
     // Make the last thread out responsible for resetting the barrier's latch.
@@ -99,6 +104,8 @@ int pthread_barrier_wait(pthread_barrier_t *barrier_opq)
         // Reset the 'out' counter so that the equality check above works 
across
         // multiple rounds of threads waiting on the barrier
         barrier->out = 0;
+        // Reset the 'in' counter so that the next round of waits can start
+        barrier->in = 0;
     }
     pthread_mutex_unlock(mtx);
     return retval;
-- 
2.7.4

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.