added more multithreaded tests that are useful with tsan
Also made various small cleanups while writing and checking these tests.
diff --git a/aos/common/common.gyp b/aos/common/common.gyp
index 186599a..0a4bf62 100644
--- a/aos/common/common.gyp
+++ b/aos/common/common.gyp
@@ -276,7 +276,7 @@
'target_name': 'mutex_test',
'type': 'executable',
'sources': [
- 'mutex_test.cpp',
+ 'mutex_test.cc',
],
'dependencies': [
'<(EXTERNALS):gtest',
@@ -284,6 +284,8 @@
'die',
'<(AOS)/build/aos.gyp:logging',
'<(AOS)/common/util/util.gyp:death_test_log_implementation',
+ '<(AOS)/common/util/util.gyp:thread',
+ '<(AOS)/common/common.gyp:time',
],
},
{
diff --git a/aos/common/mutex_test.cc b/aos/common/mutex_test.cc
new file mode 100644
index 0000000..f55d15d
--- /dev/null
+++ b/aos/common/mutex_test.cc
@@ -0,0 +1,151 @@
+#include "aos/common/mutex.h"
+
+#include <sched.h>
+#include <math.h>
+#include <pthread.h>
+
+#include "gtest/gtest.h"
+
+#include "aos/linux_code/ipc_lib/aos_sync.h"
+#include "aos/common/die.h"
+#include "aos/common/util/death_test_log_implementation.h"
+#include "aos/common/util/thread.h"
+#include "aos/common/time.h"
+
+namespace aos {
+namespace testing {
+
+class MutexTest : public ::testing::Test {
+ public:
+ Mutex test_mutex;
+
+ protected:
+ void SetUp() override {
+ SetDieTestMode(true);
+ }
+};
+
+typedef MutexTest MutexDeathTest;
+
+TEST_F(MutexTest, TryLock) {
+ EXPECT_TRUE(test_mutex.TryLock());
+ EXPECT_FALSE(test_mutex.TryLock());
+}
+
+TEST_F(MutexTest, Lock) {
+ test_mutex.Lock();
+ EXPECT_FALSE(test_mutex.TryLock());
+}
+
+TEST_F(MutexTest, Unlock) {
+ test_mutex.Lock();
+ EXPECT_FALSE(test_mutex.TryLock());
+ test_mutex.Unlock();
+ EXPECT_TRUE(test_mutex.TryLock());
+}
+
+// Sees what happens with multiple unlocks.
+TEST_F(MutexDeathTest, RepeatUnlock) {
+ test_mutex.Lock();
+ test_mutex.Unlock();
+ EXPECT_DEATH(
+ {
+ logging::AddImplementation(new util::DeathTestLogImplementation());
+ test_mutex.Unlock();
+ },
+ ".*multiple unlock.*");
+}
+
+// Sees what happens if you unlock without ever locking (or unlocking) it.
+TEST_F(MutexDeathTest, NeverLock) {
+ EXPECT_DEATH(
+ {
+ logging::AddImplementation(new util::DeathTestLogImplementation());
+ test_mutex.Unlock();
+ },
+ ".*multiple unlock.*");
+}
+
+TEST_F(MutexTest, MutexLocker) {
+ {
+ aos::MutexLocker locker(&test_mutex);
+ EXPECT_FALSE(test_mutex.TryLock());
+ }
+ EXPECT_TRUE(test_mutex.TryLock());
+}
+
+TEST_F(MutexTest, MutexUnlocker) {
+ test_mutex.Lock();
+ {
+ aos::MutexUnlocker unlocker(&test_mutex);
+ // If this fails, then something weird is going on and the next line might
+ // hang, so fail immediately.
+ ASSERT_TRUE(test_mutex.TryLock());
+ test_mutex.Unlock();
+ }
+ EXPECT_FALSE(test_mutex.TryLock());
+}
+
+namespace {
+
+class AdderThread : public ::aos::util::Thread {
+ public:
+ AdderThread(int *counter, Mutex *mutex, ::aos::time::Time sleep_before_time,
+ ::aos::time::Time sleep_after_time)
+ : counter_(counter),
+ mutex_(mutex),
+ sleep_before_time_(sleep_before_time),
+ sleep_after_time_(sleep_after_time) {}
+ virtual void Run() override {
+ ::aos::time::SleepFor(sleep_before_time_);
+ MutexLocker locker(mutex_);
+ ++(*counter_);
+ ::aos::time::SleepFor(sleep_after_time_);
+ }
+
+ private:
+ int *const counter_;
+ Mutex *const mutex_;
+ const ::aos::time::Time sleep_before_time_, sleep_after_time_;
+};
+
+} // namespace
+
+// Verifies that ThreadSanitizer understands that a contended mutex establishes
+// a happens-before relationship.
+TEST_F(MutexTest, ThreadSanitizerContended) {
+ int counter = 0;
+ AdderThread threads[2]{
+ {&counter, &test_mutex, ::aos::time::Time::InSeconds(1),
+ ::aos::time::Time::InSeconds(0)},
+ {&counter, &test_mutex, ::aos::time::Time::InSeconds(0),
+ ::aos::time::Time::InSeconds(0)}, };
+ for (auto &c : threads) {
+ c.Start();
+ }
+ for (auto &c : threads) {
+ c.WaitUntilDone();
+ }
+ EXPECT_EQ(2, counter);
+}
+
+// Verifies that ThreadSanitizer understands that an uncontended mutex
+// establishes a happens-before relationship.
+TEST_F(MutexTest, ThreadSanitizerUncontended) {
+ int counter = 0;
+ AdderThread threads[2]{
+ {&counter, &test_mutex, ::aos::time::Time::InSeconds(1),
+ ::aos::time::Time::InSeconds(0)},
+ {&counter, &test_mutex, ::aos::time::Time::InSeconds(0),
+ ::aos::time::Time::InSeconds(0)}, };
+ for (auto &c : threads) {
+ c.Start();
+ }
+ for (auto &c : threads) {
+ c.WaitUntilDone();
+ }
+ EXPECT_EQ(2, counter);
+}
+
+} // namespace testing
+} // namespace aos
diff --git a/aos/common/mutex_test.cpp b/aos/common/mutex_test.cpp
deleted file mode 100644
index 2ab3e5c..0000000
--- a/aos/common/mutex_test.cpp
+++ /dev/null
@@ -1,93 +0,0 @@
-#include "aos/common/mutex.h"
-
-#include <sched.h>
-#include <math.h>
-#include <pthread.h>
-#ifdef __VXWORKS__
-#include <taskLib.h>
-#endif
-
-#include "gtest/gtest.h"
-
-#include "aos/linux_code/ipc_lib/aos_sync.h"
-#include "aos/common/die.h"
-#include "aos/common/util/death_test_log_implementation.h"
-
-namespace aos {
-namespace testing {
-
-class MutexTest : public ::testing::Test {
- public:
- Mutex test_mutex;
-
- protected:
- void SetUp() override {
- SetDieTestMode(true);
- }
-};
-
-typedef MutexTest MutexDeathTest;
-
-TEST_F(MutexTest, TryLock) {
- EXPECT_TRUE(test_mutex.TryLock());
- EXPECT_FALSE(test_mutex.TryLock());
-}
-
-TEST_F(MutexTest, Lock) {
- test_mutex.Lock();
- EXPECT_FALSE(test_mutex.TryLock());
-}
-
-TEST_F(MutexTest, Unlock) {
- test_mutex.Lock();
- EXPECT_FALSE(test_mutex.TryLock());
- test_mutex.Unlock();
- EXPECT_TRUE(test_mutex.TryLock());
-}
-
-#ifndef __VXWORKS__
-// Sees what happens with multiple unlocks.
-TEST_F(MutexDeathTest, RepeatUnlock) {
- test_mutex.Lock();
- test_mutex.Unlock();
- EXPECT_DEATH(
- {
- logging::AddImplementation(new util::DeathTestLogImplementation());
- test_mutex.Unlock();
- },
- ".*multiple unlock.*");
-}
-
-// Sees what happens if you unlock without ever locking (or unlocking) it.
-TEST_F(MutexDeathTest, NeverLock) {
- EXPECT_DEATH(
- {
- logging::AddImplementation(new util::DeathTestLogImplementation());
- test_mutex.Unlock();
- },
- ".*multiple unlock.*");
-}
-#endif
-
-TEST_F(MutexTest, MutexLocker) {
- {
- aos::MutexLocker locker(&test_mutex);
- EXPECT_FALSE(test_mutex.TryLock());
- }
- EXPECT_TRUE(test_mutex.TryLock());
-}
-
-TEST_F(MutexTest, MutexUnlocker) {
- test_mutex.Lock();
- {
- aos::MutexUnlocker unlocker(&test_mutex);
- // If this fails, then something weird is going on and the next line might
- // hang, so fail immediately.
- ASSERT_TRUE(test_mutex.TryLock());
- test_mutex.Unlock();
- }
- EXPECT_FALSE(test_mutex.TryLock());
-}
-
-} // namespace testing
-} // namespace aos
diff --git a/aos/common/queue_test.cc b/aos/common/queue_test.cc
index dfafe6a..dfd18a2 100644
--- a/aos/common/queue_test.cc
+++ b/aos/common/queue_test.cc
@@ -53,7 +53,7 @@
usleep(50000);
my_test_queue.MakeWithBuilder().test_bool(true).test_int(0x971).Send();
t.Join();
- EXPECT_LE(t.threaded_test_queue.Age(), time::Time::InMS(55));
+ EXPECT_LE(t.threaded_test_queue.Age(), time::Time::InMS(57));
}
// Tests that we can send a message with the message pointer and get it back.
diff --git a/aos/common/util/run_command_test.cc b/aos/common/util/run_command_test.cc
index b440e47..daff3c5 100644
--- a/aos/common/util/run_command_test.cc
+++ b/aos/common/util/run_command_test.cc
@@ -2,6 +2,8 @@
#include "gtest/gtest.h"
+#include "aos/common/util/thread.h"
+
namespace aos {
namespace util {
namespace testing {
@@ -34,6 +36,26 @@
EXPECT_EQ(SIGQUIT, WTERMSIG(result));
}
+TEST(RunCommandTest, MultipleThreads) {
+ int result1, result2;
+ util::FunctionThread t1([&result1](util::Thread *) {
+ result1 = RunCommand("true");
+ });
+ util::FunctionThread t2([&result2](util::Thread *) {
+ result2 = RunCommand("true");
+ });
+ t1.Start();
+ t2.Start();
+ t1.WaitUntilDone();
+ t2.WaitUntilDone();
+ ASSERT_NE(-1, result1);
+ ASSERT_NE(-1, result2);
+ ASSERT_TRUE(WIFEXITED(result1));
+ ASSERT_TRUE(WIFEXITED(result2));
+ EXPECT_EQ(0, WEXITSTATUS(result1));
+ EXPECT_EQ(0, WEXITSTATUS(result2));
+}
+
} // namespace testing
} // namespace util
} // namespace aos
diff --git a/aos/common/util/thread.cc b/aos/common/util/thread.cc
index fab62eb..dbb638e 100644
--- a/aos/common/util/thread.cc
+++ b/aos/common/util/thread.cc
@@ -1,7 +1,8 @@
#include "aos/common/util/thread.h"
#include <pthread.h>
-#include <assert.h>
+
+#include "aos/common/logging/logging.h"
namespace aos {
namespace util {
@@ -10,24 +11,30 @@
Thread::~Thread() {
if (started_ && !joined_) {
- assert(false);
+ CHECK(false);
}
}
void Thread::Start() {
- assert(!started_);
+ CHECK(!started_);
started_ = true;
- assert(pthread_create(&thread_, NULL, &Thread::StaticRun, this) == 0);
+ CHECK(pthread_create(&thread_, NULL, &Thread::StaticRun, this) == 0);
}
void Thread::Join() {
- assert(!joined_ && started_);
+ CHECK(!joined_ && started_);
joined_ = true;
{
MutexLocker locker(&should_terminate_mutex_);
should_terminate_ = true;
}
- assert(pthread_join(thread_, NULL) == 0);
+ CHECK(pthread_join(thread_, NULL) == 0);
+}
+
+void Thread::WaitUntilDone() {
+ CHECK(!joined_ && started_);
+ joined_ = true;
+ CHECK(pthread_join(thread_, NULL) == 0);
}
void *Thread::StaticRun(void *self) {
diff --git a/aos/common/util/thread.h b/aos/common/util/thread.h
index ab6f09c..97123d5 100644
--- a/aos/common/util/thread.h
+++ b/aos/common/util/thread.h
@@ -1,7 +1,10 @@
#ifndef AOS_COMMON_UTIL_THREAD_H_
#define AOS_COMMON_UTIL_THREAD_H_
+#include <functional>
+
#include "aos/common/mutex.h"
+#include "aos/common/macros.h"
namespace aos {
namespace util {
@@ -20,6 +23,9 @@
// Asks the code to stop and then waits until it has done so.
void Join();
+ // Waits until the code has stopped. Does not ask it to do so.
+ void WaitUntilDone();
+
protected:
// Subclasses need to call this periodically if they are going to loop to
// check whether they have been asked to stop.
@@ -42,6 +48,21 @@
bool joined_;
bool should_terminate_;
Mutex should_terminate_mutex_;
+
+ DISALLOW_COPY_AND_ASSIGN(Thread);
+};
+
+class FunctionThread : public Thread {
+ public:
+ FunctionThread(::std::function<void(FunctionThread *)> function)
+ : function_(function) {}
+
+ private:
+ virtual void Run() override {
+ function_(this);
+ }
+
+ const ::std::function<void(FunctionThread *)> function_;
};
} // namespace util
diff --git a/aos/common/util/util.gyp b/aos/common/util/util.gyp
index 7c0adf3..fb29d7f 100644
--- a/aos/common/util/util.gyp
+++ b/aos/common/util/util.gyp
@@ -20,6 +20,7 @@
'run_command',
'<(EXTERNALS):gtest',
'<(AOS)/build/aos.gyp:logging',
+ 'thread',
],
},
{