[waffle] [PATCH v2 3/3] wcore_error_unittest: Fix race condition leading to forever loop

[Pauli Nieminen]

Reading non-volatile value in loop without function calls allows C
compiler to generate forever loop without checking the memory value. To
avoid the problem locking or atomic operations could be used.

But there is better solution of using pthread_barrier_t that test case
was trying to simulate with mutex and conditional. Barrier promises that
all threads are released when requested number of threads are blocked in
pthread_barrier_wait.

Signed-off-by: Pauli Nieminen <pauli.nieminen at linux.intel.com>
---
 tests/unittests/waffle/core/wcore_error_unittest.c |   37 ++++----------------
 1 file changed, 7 insertions(+), 30 deletions(-)

diff --git a/tests/unittests/waffle/core/wcore_error_unittest.c b/tests/unittests/waffle/core/wcore_error_unittest.c
index 2a1f5c1..0098450 100644
--- a/tests/unittests/waffle/core/wcore_error_unittest.c
+++ b/tests/unittests/waffle/core/wcore_error_unittest.c
@@ -131,13 +131,7 @@ struct thread_arg {
     int thread_id;
 
     /// Protects `num_threads_waiting` and `cond`.
-    pthread_mutex_t *mutex;
-
-    /// Satisfied when `num_threads_waiting == TOTAL_THREADS`.
-    pthread_cond_t *cond;
-
-    /// Number of threads waiting on `cond`.
-    int *num_threads_waiting;
+    pthread_barrier_t *barrier;
 };
 
 /// The start routine given to threads in test wcore_error.thread_local.
@@ -159,13 +153,7 @@ thread_start(struct thread_arg *a)
     // Each thread sets its error code.
     wcore_error(error_code);
 
-    // Wait for all threads to set their error codes, thus giving
-    // the threads opportunity to clobber each other's codes.
-    pthread_mutex_lock(a->mutex); {
-        *a->num_threads_waiting += 1;
-        pthread_cond_wait(a->cond, a->mutex);
-        pthread_mutex_unlock(a->mutex);
-    }
+    pthread_barrier_wait(a->barrier);
 
     // Verify that the threads did not clobber each other's
     // error codes.
@@ -177,23 +165,18 @@ thread_start(struct thread_arg *a)
 // Test that threads do not clobber each other's error codes.
 TEST(wcore_error, thread_local)
 {
-    pthread_mutex_t mutex;
-    pthread_cond_t cond;
-    int num_threads_waiting = 0;
+    pthread_barrier_t barrier;
 
     pthread_t threads[NUM_THREADS];
     struct thread_arg thread_args[NUM_THREADS];
     bool exit_codes[NUM_THREADS];
 
-    pthread_mutex_init(&mutex, NULL);
-    pthread_cond_init(&cond, NULL);
+    pthread_barrier_init(&barrier, NULL, NUM_THREADS + 1);
 
     for (intptr_t i = 0; i < NUM_THREADS; ++i) {
         struct thread_arg *a = &thread_args[i];
         a->thread_id = i;
-        a->mutex = &mutex;
-        a->cond = &cond;
-        a->num_threads_waiting = &num_threads_waiting;
+        a->barrier = &barrier;
 
         pthread_create(&threads[i], NULL,
                       (void* (*)(void*)) thread_start,
@@ -202,20 +185,14 @@ TEST(wcore_error, thread_local)
 
     // Wait for all threads to set their error codes, thus giving
     // the threads opportunity to clobber each other's codes.
-    while (num_threads_waiting < NUM_THREADS)
-        ;;
-    pthread_mutex_lock(&mutex); {
-        pthread_cond_broadcast(&cond);
-        pthread_mutex_unlock(&mutex);
-    }
+    pthread_barrier_wait(&barrier);
 
     for (int i = 0; i < NUM_THREADS; ++i) {
         pthread_join(threads[i], (void**) &exit_codes[i]);
         EXPECT_TRUE(exit_codes[i] == true);
     }
 
-    pthread_cond_destroy(&cond);
-    pthread_mutex_destroy(&mutex);
+    pthread_barrier_destroy(&barrier);
 }
 
 void
-- 
1.7.9.5