Bug 2479: Fixed unexpected failure of atomic operation tests. 05/13405/1
authorShigeru Yasuda <s-yasuda@da.jp.nec.com>
Fri, 5 Dec 2014 09:49:47 +0000 (18:49 +0900)
committerShigeru Yasuda <s-yasuda@da.jp.nec.com>
Fri, 5 Dec 2014 09:49:47 +0000 (18:49 +0900)
Race condition tests for atomic operations need to ensure that all the
test threads have been dispatched.

Change-Id: I5a973060800bfff0df17f03def88f3e47b689ff4
Signed-off-by: Shigeru Yasuda <s-yasuda@da.jp.nec.com>
coordinator/core/test/libs/libpfc_util/test_atomic.cc
coordinator/core/test/libs/libpfc_util/thread_subr.cc
coordinator/core/test/libs/libpfc_util/thread_subr.hh

index 74d5094eda32295c82ead8d706a3946206d6b4e7..1925ace707ad28dad5e21d98eb67ba8c157ba5fa 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2013 NEC Corporation
+ * Copyright (c) 2010-2014 NEC Corporation
  * All rights reserved.
  * 
  * This program and the accompanying materials are made available under the
@@ -338,13 +338,8 @@ public:
     bool
     checkState(void)
     {
-        test_state_t   state;
-
-        pfc_mutex_lock(&_mutex);
-        state = _state;
-        pfc_mutex_unlock(&_mutex);
-
-        return (state == ATS_RUNNING) ? true : false;
+        PfcMutex  m(_mutex);
+        return (_state == ATS_RUNNING) ? true : false;
     }
 
     void
@@ -383,9 +378,7 @@ private:
 bool
 AtomicThreadEnv::waitForStart(void)
 {
-    bool       ret(true);
-
-    pfc_mutex_lock(&_mutex);
+    PfcMutex  m(_mutex);
 
     for (;;) {
         test_state_t   state(_state);
@@ -396,16 +389,13 @@ AtomicThreadEnv::waitForStart(void)
 
         if (PFC_EXPECT_FALSE(state == ATS_FINI)) {
             // Test was cancelled.
-            ret = false;
-            break;
+            return false;
         }
 
         pfc_cond_wait(&_cond, &_mutex);
     }
 
-    pfc_mutex_unlock(&_mutex);
-
-    return ret;
+    return true;
 }
 
 void
@@ -418,12 +408,11 @@ AtomicThreadEnv::initCommon(void)
 void
 AtomicThreadEnv::setState(test_state_t state)
 {
-    pfc_mutex_lock(&_mutex);
+    PfcMutex  m(_mutex);
     if (_state != ATS_FINI) {
         _state = state;
         pfc_cond_broadcast(&_cond);
     }
-    pfc_mutex_unlock(&_mutex);
 }
 
 /*
@@ -519,6 +508,7 @@ class AtomicThread
 {
 public:
     static const uint32_t      QUANTUM = 10;
+    static const uint32_t      MIN_COUNT = 20;
 
     AtomicThread(AtomicThreadEnv &env)
         : _env(&env), _succeeded(false), _count(0), _quantum(0)
@@ -543,7 +533,7 @@ public:
     bool
     checkState(void)
     {
-        return _env->checkState();
+        return (_env->checkState() || _count < MIN_COUNT);
     }
 
     uint32_t
@@ -603,27 +593,27 @@ AtomicThreadEnv::invoke(AtomicThread **threads, int nthreads, uint32_t &count)
 
     pfc_timespec_t  ts = {ATOMIC_RACE_DURATION, ATOMIC_RACE_DURATION_NSEC};
     pfc_timespec_t  abs;
-    pfc_mutex_lock(&_mutex);
 
-    ASSERT_EQ(0, pfc_clock_abstime(&abs, &ts));
-    for (;;) {
-        test_state_t   state(_state);
-        if (PFC_EXPECT_FALSE(state != ATS_RUNNING)) {
-            pfc_mutex_unlock(&_mutex);
-            return;
-        }
+    {
+        PfcMutex  m(_mutex);
+        ASSERT_EQ(0, pfc_clock_abstime(&abs, &ts));
 
-        int    err(pfc_cond_timedwait_abs(&_cond, &_mutex, &abs));
-        if (err == ETIMEDOUT) {
-            // Terminate all test threads.
-            _state = ATS_FINI;
-            pfc_cond_broadcast(&_cond);
-            break;
+        for (;;) {
+            test_state_t       state(_state);
+            if (PFC_EXPECT_FALSE(state != ATS_RUNNING)) {
+                return;
+            }
+
+            int  err(pfc_cond_timedwait_abs(&_cond, &_mutex, &abs));
+            if (err == ETIMEDOUT) {
+                // Terminate all test threads.
+                _state = ATS_FINI;
+                pfc_cond_broadcast(&_cond);
+                break;
+            }
         }
     }
 
-    pfc_mutex_unlock(&_mutex);
-
     uint32_t   cnt(0);
     for (int i(0); i < nthreads; i++) {
         ASSERT_EQ(0, threads[i]->join());
index 2a5f5ea7f8f81a4e0b0043438f8ef6987e9b0587..31b809d41dfa7aaa72a3caa5b7f5fd032abdd364 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2013 NEC Corporation
+ * Copyright (c) 2010-2014 NEC Corporation
  * All rights reserved.
  * 
  * This program and the accompanying materials are made available under the
 
 #include <sstream>
 #include <cstring>
+#include <pfc/atomic.h>
 #include "thread_subr.hh"
 
+#define        JOIN_TIMEOUT    30
+
+/*
+ * TSD key used to call destructor.
+ */
+static pthread_key_t   thread_dtor_key;
+
+/*
+ * Set 1 if the thread utility is initialized.
+ */
+static uint8_t         thread_initialized = 0;
+
 /*
  * TempThread::TempThread()
  *     Constructor of temporary thread instance.
  */
 TempThread::TempThread()
-    : _state(THR_INIT)
+    : _state(THR_INIT), _done(false)
 {
+    if (pfc_atomic_swap_uint8(&thread_initialized, 0U) == 0U) {
+        PFC_VERIFY_INT(pthread_key_create(&thread_dtor_key, TempThread::dtor),
+                       0);
+    }
+
     PFC_MUTEX_INIT(&_mutex);
+    PFC_MUTEX_INIT(&_join_mutex);
     pfc_cond_init(&_cond);
+    pfc_cond_init(&_join_cond);
 }
 
 /*
@@ -46,12 +66,14 @@ TempThread::~TempThread()
         pthread_t      t(_thread);
 
         if (t != PFC_PTHREAD_INVALID_ID) {
-            (void)pthread_join(_thread, NULL);
+            (void)timedJoin();
         }
     }
 
     pfc_cond_destroy(&_cond);
+    pfc_cond_destroy(&_join_cond);
     pfc_mutex_destroy(&_mutex);
+    pfc_mutex_destroy(&_join_mutex);
 }
 
 /*
@@ -109,7 +131,7 @@ TempThread::join(void)
     condSignal();
     unlock();
 
-    int        err(pthread_join(_thread, NULL));
+    int        err(timedJoin());
     _thread = PFC_PTHREAD_INVALID_ID;
 
     return err;
@@ -215,6 +237,9 @@ TempThread::entry(void *arg)
 {
     TempThread *thr(reinterpret_cast<TempThread *>(arg));
 
+    // Set TempThread instance into TSD.
+    PFC_VERIFY_INT(pthread_setspecific(thread_dtor_key, thr), 0);
+
     // Disable cancellation.
     (void)pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, NULL);
 
@@ -222,3 +247,54 @@ TempThread::entry(void *arg)
 
     return NULL;
 }
+
+/*
+ * void
+ * TempThread::dtor(void *arg)
+ *     Destructor of the thread bound to a TempThread instance.
+ */
+void
+TempThread::dtor(void *arg)
+{
+    TempThread *thr(reinterpret_cast<TempThread *>(arg));
+
+    PfcMutex  m(thr->_join_mutex);
+    thr->_done = true;
+    pfc_cond_broadcast(&thr->_join_cond);
+}
+
+/*
+ * int
+ * TempThread::timedJoin(void)
+ *     Wait for the thread to terminate within the timeout.
+ *
+ * Calling/Exit State:
+ *     Upon successful completion, zero is returned.
+ *     Otherwise error number which indicates the cause of error is returned.
+ */
+int
+TempThread::timedJoin(void)
+{
+    pfc_timespec_t  timeout = {JOIN_TIMEOUT, 0};
+    pfc_timespec_t  abs;
+    int  err(pfc_clock_abstime(&abs, &timeout));
+
+    if (PFC_EXPECT_FALSE(err != 0)) {
+        return err;
+    }
+
+    {
+        PfcMutex  m(_join_mutex);
+        while (!_done) {
+            err = pfc_cond_timedwait_abs(&_join_cond, &_join_mutex, &abs);
+            if (PFC_EXPECT_FALSE(err != 0 && err != EINTR)) {
+                if (_done) {
+                    break;
+                }
+                return err;
+            }
+        }
+    }
+
+    return pthread_join(_thread, NULL);
+}
index f9bb6f695eef485989fd18a5390580950d3e475d..529e1b3203d397df79fa43298fa5946b5e556aaf 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2013 NEC Corporation
+ * Copyright (c) 2010-2014 NEC Corporation
  * All rights reserved.
  * 
  * This program and the accompanying materials are made available under the
 #include <string>
 #include <stdint.h>
 #include <pthread.h>
+#include <boost/noncopyable.hpp>
 #include <pfc/synch.h>
 #include <pfc/clock.h>
+#include <pfc/debug.h>
 
 /*
  * Thread's exit state that means the thread is stopped by force.
@@ -82,6 +84,9 @@ protected:
 
 private:
     static void        *entry(void *arg);
+    static void        dtor(void *arg);
+
+    int                timedJoin(void);
 
     /* Thread state. */
     volatile tstate_t  _state;
@@ -97,6 +102,35 @@ private:
 
     /* Error message. */
     std::string                _message;
+
+    /* Set true when the thread returns from run(). */
+    bool               _done;
+
+    /* Mutex lock and condition variable for timed join. */
+    pfc_mutex_t                _join_mutex;
+    pfc_cond_t         _join_cond;
+};
+
+/*
+ * Scoped mutex which uses pfc_mutex_t.
+ */
+class PfcMutex
+    : boost::noncopyable
+{
+public:
+    PfcMutex(pfc_mutex_t &mutex)
+        : _mutex(&mutex)
+    {
+        PFC_VERIFY_INT(pfc_mutex_lock(_mutex), 0);
+    }
+
+    ~PfcMutex()
+    {
+        PFC_VERIFY_INT(pfc_mutex_unlock(_mutex), 0);
+    }
+
+private:
+    pfc_mutex_t    *_mutex;
 };
 
 #endif /* !_TEST_THREAD_SUBR_HH */