cleaned up lots of stuff (no functional changes)
ipc_stress_test runs all of the tests successfully now. In order to make
that work, I had to fix raw_queue_test so it didn't count the time spent
waiting for the fork() to complete as part of the timeout, and I cleaned
up various other parts at the same time.
Also improved some documentation and removed the mutex fairness tester
because it was slow, prone to random failures, and it's always been kind
of a fishy test.
diff --git a/aos/common/common.gyp b/aos/common/common.gyp
index eee1d05..f3b4e69 100644
--- a/aos/common/common.gyp
+++ b/aos/common/common.gyp
@@ -24,7 +24,11 @@
'<(AOS)/build/aos.gyp:logging',
'once',
'<(EXTERNALS):gtest',
+ '<(AOS)/atom_code/ipc_lib/ipc_lib.gyp:shared_mem',
],
+ 'export_dependent_settings': [
+ '<(AOS)/atom_code/ipc_lib/ipc_lib.gyp:shared_mem',
+ ],
},
{
'target_name': 'time',
diff --git a/aos/common/condition.h b/aos/common/condition.h
index 346ae54..c407070 100644
--- a/aos/common/condition.h
+++ b/aos/common/condition.h
@@ -7,8 +7,38 @@
namespace aos {
// A condition variable (IPC mechanism where 1 process/task can notify all
-// others that are waiting for something to happen).
-// This implementation will LOG(FATAL) if anything weird happens.
+// others that are waiting for something to happen) without the race condition
+// where a notification is sent after some process has checked if the thing has
+// happened but before it has started listening for notifications.
+//
+// This implementation will print debugging information and abort the process
+// if anything weird happens.
+//
+// A simple example of the use of a condition variable (adapted from
+// pthread_cond(3)):
+//
+// int x, y;
+// Mutex mutex;
+// Condition condition(&mutex);
+//
+// // Waiting until x is greater than y:
+// {
+// MutexLocker locker(&mutex);
+// while (!(x > y)) condition.Wait();
+// // do whatever
+// }
+//
+// // Modifying x and/or y:
+// {
+// MutexLocker locker(&mutex);
+// // modify x and y
+// if (x > y) condition.Broadcast();
+// }
+//
+// Notice the loop around the Wait(). This is very important because some other
+// process can lock the mutex and modify the shared state (possibly undoing
+// whatever the Wait()er was waiting for) in between the Broadcast()er unlocking
+// the mutex and the Wait()er(s) relocking it.
//
// Multiple condition variables may be associated with the same mutex but
// exactly 1 mutex must be associated with each condition variable.
@@ -18,9 +48,12 @@
// object will hold on to a reference to it but does not take ownership.
explicit Condition(Mutex *m);
- // Waits for the condition variable to be signalled, atomically unlocking m at
- // the same time. The mutex associated with this condition variable must be
- // locked when this is called and will be locked when this method returns.
+ // Waits for the condition variable to be signalled, atomically unlocking the
+ // mutex associated with this condition variable at the same time. The mutex
+ // associated with this condition variable must be locked when this is called
+ // and will be locked when this method returns.
+ // NOTE: The relocking of the mutex is not performed atomically with waking
+ // up.
void Wait();
// Signals at most 1 other process currently Wait()ing on this condition
diff --git a/aos/common/condition_test.cc b/aos/common/condition_test.cc
index 49112bc..28b4adc 100644
--- a/aos/common/condition_test.cc
+++ b/aos/common/condition_test.cc
@@ -76,6 +76,7 @@
child_ = fork();
if (child_ == 0) { // in child
+ ::aos::common::testing::PreventExit();
Run();
exit(EXIT_SUCCESS);
} else { // in parent
@@ -131,11 +132,11 @@
ready.Lock();
}
- bool started;
- bool delayed;
+ volatile bool started;
+ volatile bool delayed;
Mutex done_delaying;
::Time start_time;
- bool finished;
+ volatile bool finished;
Mutex ready;
};
static_assert(shm_ok<Shared>::value,
diff --git a/aos/common/mutex_test.cpp b/aos/common/mutex_test.cpp
index 6ab7ed6..652cd9e 100644
--- a/aos/common/mutex_test.cpp
+++ b/aos/common/mutex_test.cpp
@@ -72,105 +72,5 @@
EXPECT_FALSE(test_mutex.TryLock());
}
-// A worker thread for testing the fairness of the mutex implementation.
-class MutexFairnessWorkerThread {
- public:
- MutexFairnessWorkerThread(int *cycles, int index,
- Mutex *in_mutex, mutex *start)
- : cycles_(cycles), index_(index), mutex_(in_mutex), start_(start) {}
-
- static void *RunStatic(void *self_in) {
- MutexFairnessWorkerThread *self =
- static_cast<MutexFairnessWorkerThread *>(self_in);
- self->Run();
- delete self;
- return NULL;
- }
-
- static void Reset(int cycles) {
- cyclesRun = 0;
- totalCycles = cycles;
- }
-
- private:
- void Run() {
- cycles_[index_] = 0;
- ASSERT_EQ(futex_wait(start_), 0);
- while (cyclesRun < totalCycles) {
- {
- MutexLocker locker(mutex_);
- ++cyclesRun;
- }
- ++cycles_[index_];
- // Otherwise the fitpc implementation tends to just relock in the same
- // thread.
- sched_yield();
- }
-
-#ifdef __VXWORKS__
- // Without this, all of the "task ... deleted ..." messages come out at
- // once, and it looks weird and triggers an socat bug (at least for
- // Squeeze's version 1.7.1.3-1) which locks it up and is very annoying.
- taskDelay(index_);
-#endif
- }
-
- int *cycles_;
- int index_;
- Mutex *mutex_;
- mutex *start_;
- static int cyclesRun, totalCycles;
-};
-int MutexFairnessWorkerThread::cyclesRun;
-int MutexFairnessWorkerThread::totalCycles;
-
-// Tests the fairness of the implementation. It does this by repeatedly locking
-// and unlocking a mutex in multiple threads and then checking the standard
-// deviation of the number of times each one locks.
-//
-// It is safe to do this with threads because this is the test so it can change
-// if the implementations ever change to not support that. Fitpc logging calls
-// are not thread-safe, but it doesn't really matter because the only logging
-// call that would get made would be a LOG(FATAL) that would still terminate the
-// process.
-TEST_F(MutexTest, Fairness) {
- static const int kThreads = 13;
-#ifdef __VXWORKS__
- static const int kWarmupCycles = 1000, kRunCycles = 60000, kMaxDeviation = 20;
-#else
- static const int kWarmupCycles = 30000, kRunCycles = 3000000, kMaxDeviation = 20000;
-#endif
-
- int cycles[kThreads];
- pthread_t workers[kThreads];
- mutex start = 0;
-
- for (int repeats = 0; repeats < 2; ++repeats) {
- futex_unset(&start);
- MutexFairnessWorkerThread::Reset(repeats ? kRunCycles : kWarmupCycles);
- for (int i = 0; i < kThreads; ++i) {
- MutexFairnessWorkerThread *c = new MutexFairnessWorkerThread(cycles, i,
- &test_mutex,
- &start);
- ASSERT_EQ(0, pthread_create(&workers[i], NULL,
- MutexFairnessWorkerThread::RunStatic, c));
- }
- futex_set(&start);
- for (int i = 0; i < kThreads; ++i) {
- ASSERT_EQ(0, pthread_join(workers[i], NULL));
- }
- }
-
- double variance = 0;
- int expected = kRunCycles / kThreads;
- for (int i = 0; i < kThreads; ++i) {
- variance += (cycles[i] - expected) * (cycles[i] - expected);
- }
- ASSERT_GT(variance, 0);
- double deviation = sqrt(variance / kThreads);
- ASSERT_GT(deviation, 0);
- EXPECT_LT(deviation, kMaxDeviation);
-}
-
} // namespace testing
} // namespace aos
diff --git a/aos/common/queue_testutils.cc b/aos/common/queue_testutils.cc
index 629c9be..8d195c5 100644
--- a/aos/common/queue_testutils.cc
+++ b/aos/common/queue_testutils.cc
@@ -2,6 +2,9 @@
#include <string.h>
#include <sys/mman.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <assert.h>
#include "gtest/gtest.h"
@@ -113,13 +116,18 @@
const size_t kCoreSize = 0x100000;
+void TerminateExitHandler() {
+ _exit(EXIT_SUCCESS);
+}
+
} // namespace
GlobalCoreInstance::GlobalCoreInstance() {
global_core = &global_core_data_;
- global_core->owner = 1;
- // Use mmap(2) manually so that we can pass MAP_SHARED so that forked
- // processes can still communicate using the "shared" memory.
+ global_core->owner = true;
+ // Use mmap(2) manually instead of through malloc(3) so that we can pass
+ // MAP_SHARED which allows forked processes to communicate using the
+ // "shared" memory.
void *memory = mmap(NULL, kCoreSize, PROT_READ | PROT_WRITE,
MAP_SHARED | MAP_ANONYMOUS, -1, 0);
assert(memory != MAP_FAILED);
@@ -138,6 +146,10 @@
enable_test_logging_once.Get();
}
+void PreventExit() {
+ assert(atexit(TerminateExitHandler) == 0);
+}
+
} // namespace testing
} // namespace common
} // namespace aos
diff --git a/aos/common/queue_testutils.h b/aos/common/queue_testutils.h
index aabdd2d..2d26262 100644
--- a/aos/common/queue_testutils.h
+++ b/aos/common/queue_testutils.h
@@ -1,11 +1,20 @@
-#include "aos/common/queue.h"
+#ifndef AOS_COMMON_QUEUE_TESTUTILS_H_
+#define AOS_COMMON_QUEUE_TESTUTILS_H_
+
+#include "aos/atom_code/ipc_lib/shared_mem.h"
+
+// This file has some general helper functions for dealing with testing things
+// that use shared memory etc.
namespace aos {
namespace common {
namespace testing {
+// Manages creating and cleaning up "shared memory" which works within this
+// process and any that it fork(2)s.
class GlobalCoreInstance {
public:
+ // Calls EnableTestLogging().
GlobalCoreInstance();
~GlobalCoreInstance();
@@ -20,6 +29,14 @@
// initialized), however it can be called more than that.
void EnableTestLogging();
+// Registers an exit handler (using atexit(3)) which will call _exit(2).
+// Intended to be called in a freshly fork(2)ed process where it will run before
+// any other exit handlers that were already registered and prevent them from
+// being run.
+void PreventExit();
+
} // namespace testing
} // namespace common
} // namespace aos
+
+#endif // AOS_COMMON_QUEUE_TESTUTILS_H_