From d9fb59bea59e62ee86925830b88f94922b06f6a9 Mon Sep 17 00:00:00 2001
From: George Bosilca <bosilca@icl.utk.edu>
Date: Thu, 2 Jun 2016 11:53:56 +0900
Subject: [PATCH] Update the synchronization primitive Add comments and make
 sure we correctly return the status of the synchronization primitive,
 especially if it was completed with error.

---
 opal/threads/wait_sync.c | 22 ++++++++++++----------
 opal/threads/wait_sync.h | 16 +++++++++++-----
 2 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/opal/threads/wait_sync.c b/opal/threads/wait_sync.c
index 355c461870..099eb82391 100644
--- a/opal/threads/wait_sync.c
+++ b/opal/threads/wait_sync.c
@@ -27,18 +27,18 @@ int sync_wait_st(ompi_wait_sync_t *sync)
     while(sync->count > 0) {
         opal_progress();
     }
-    return OPAL_SUCCESS;
+    return (0 == sync->status) ? OPAL_SUCCESS : OPAL_ERROR;
 }
 
 int sync_wait_mt(ompi_wait_sync_t *sync)
 {
     if(sync->count <= 0)
-        return OPAL_SUCCESS;
+        return (0 == sync->status) ? OPAL_SUCCESS : OPAL_ERROR;
 
     /* lock so nobody can signal us during the list updating */
     pthread_mutex_lock(&sync->lock);
 
-    /* Insert sync to the list */
+    /* Insert sync on the list of pending synchronization constructs */
     OPAL_THREAD_LOCK(&wait_sync_lock);
     if( NULL == wait_sync_list ) {
         sync->next = sync->prev = sync;
@@ -52,32 +52,34 @@ int sync_wait_mt(ompi_wait_sync_t *sync)
     OPAL_THREAD_UNLOCK(&wait_sync_lock);
 
     /**
-     * If we are not responsible for progresing, let's go silent until something worth noticing happen:
+     * If we are not responsible for progresing, go silent until something worth noticing happen:
      *  - this thread has been promoted to take care of the progress
      *  - our sync has been triggered.
      */
+ check_status:
     if( sync != wait_sync_list ) {
         pthread_cond_wait(&sync->condition, &sync->lock);
 
         /**
-         * At this point either the sync was completed in which case we should remove it from the wait
-         * list, or/and I was promoted as the progress manager.
+         * At this point either the sync was completed in which case
+         * we should remove it from the wait list, or/and I was
+         * promoted as the progress manager.
          */
 
         if( sync->count <= 0 ) {  /* Completed? */
             pthread_mutex_unlock(&sync->lock);
             goto i_am_done;
         }
-        /* promoted ! */
-        assert(sync == wait_sync_list);
+        /* either promoted, or spurious wakeup ! */
+        goto check_status;
     }
 
     pthread_mutex_unlock(&sync->lock);
     while(sync->count > 0) {  /* progress till completion */
         opal_progress();  /* don't progress with the sync lock locked or you'll deadlock */
     }
-
     assert(sync == wait_sync_list);
+
  i_am_done:
     /* My sync is now complete. Trim the list: remove self, wake next */
     OPAL_THREAD_LOCK(&wait_sync_lock);
@@ -91,5 +93,5 @@ int sync_wait_mt(ompi_wait_sync_t *sync)
     }
     OPAL_THREAD_UNLOCK(&wait_sync_lock);
 
-    return OPAL_SUCCESS;
+    return (0 == sync->status) ? OPAL_SUCCESS : OPAL_ERROR;
 }
diff --git a/opal/threads/wait_sync.h b/opal/threads/wait_sync.h
index 0c396f8b0a..da77003a1f 100644
--- a/opal/threads/wait_sync.h
+++ b/opal/threads/wait_sync.h
@@ -73,17 +73,23 @@ OPAL_DECLSPEC int sync_wait_st(ompi_wait_sync_t *sync);
        PTHREAD_MUTEX_INIT(&(sync)->lock, NULL);       \
     } while(0)
 
-static inline void wait_sync_update(ompi_wait_sync_t *sync, int updates, int req_status)
+/**
+ * Update the status of the synchronization primitive. If an error is
+ * reported the synchronization is completed and the signal
+ * triggered. The status of the synchronization will be reported to
+ * the waiting threads.
+ */
+static inline void wait_sync_update(ompi_wait_sync_t *sync, int updates, int status)
 {
-    if( OPAL_LIKELY(OPAL_SUCCESS == req_status) ) {
-        if( 0 == (OPAL_ATOMIC_ADD_32(&sync->count, -updates)) ) {
-            WAIT_SYNC_SIGNAL(sync);
+    if( OPAL_LIKELY(OPAL_SUCCESS == status) ) {
+        if( 0 != (OPAL_ATOMIC_ADD_32(&sync->count, -updates)) ) {
+            return;
         }
     } else {
         OPAL_ATOMIC_CMPSET_32(&(sync->count), 0, 0);
         sync->status = -1;
-        WAIT_SYNC_SIGNAL(sync);
     }
+    WAIT_SYNC_SIGNAL(sync);
 }
 
 END_C_DECLS