Merge changes I604610b9,I6093c376,I118cc621,I3f7c3482,Iae4ba2a1
* changes:
Added filtered velocity to the output and removed unnecesary comment after gyro fix.
Dedup comment in joystick_reader.
Send out 0 for the gyro while zeroing.
Added drivetrain_action back in.
Sped up pickup.
diff --git a/aos/build/build.py b/aos/build/build.py
index 5be0950..725de33 100755
--- a/aos/build/build.py
+++ b/aos/build/build.py
@@ -445,8 +445,6 @@
OTHER_SYSROOT = '/usr/lib/llvm-3.5/'
SYMBOLIZER_PATH = OTHER_SYSROOT + 'bin/llvm-symbolizer'
r = {}
- if self.compiler() == 'clang' or self.compiler() == 'gcc_4.8':
- r['LD_LIBRARY_PATH'] = OTHER_SYSROOT + 'lib'
if self.sanitizer() == 'address':
r['ASAN_SYMBOLIZER_PATH'] = SYMBOLIZER_PATH
r['ASAN_OPTIONS'] = \
@@ -457,6 +455,11 @@
r['MSAN_SYMBOLIZER_PATH'] = SYMBOLIZER_PATH
elif self.sanitizer() == 'thread':
r['TSAN_OPTIONS'] = 'external_symbolizer_path=' + SYMBOLIZER_PATH
+ # This is apparently the default for newer versions, which disagrees
+ # with documentation, so just turn it on explicitly.
+ r['TSAN_OPTIONS'] += ':detect_deadlocks=1'
+ # Print more useful stacks for mutex locking order problems.
+ r['TSAN_OPTIONS'] += ':second_deadlock_stack=1'
r['CCACHE_COMPRESS'] = 'yes'
r['CCACHE_DIR'] = os.path.abspath(os.path.join(aos_path(), '..', 'output',
@@ -663,6 +666,7 @@
clean: Remove all the built output.
tests: Build and then run tests.
deploy: Build and then download.
+ environment: Dump the environment for building.
platform What variants of the code to build.
Defaults to something reasonable.
See below for details.
@@ -740,7 +744,7 @@
print_help(1, 'Not enough arguments')
args.processor = sys.argv.pop(0)
args.main_gyp = sys.argv.pop(0)
- VALID_ACTIONS = ['build', 'clean', 'deploy', 'tests']
+ VALID_ACTIONS = ['build', 'clean', 'deploy', 'tests', 'environment']
while sys.argv:
arg = sys.argv.pop(0)
if arg == '-j' or arg == '--jobs':
@@ -874,6 +878,11 @@
user_output('Building %s (%d/%d)...' % (platform, num, len(platforms)))
if args.action_name == 'clean':
shutil.rmtree(platform.outdir(), onerror=handle_clean_error)
+ elif args.action_name == 'environment':
+ user_output('Environment for building <<END')
+ for name, value in env(platform).items():
+ print('%s=%s' % (name, value))
+ print('END')
else:
if need_to_run_gyp(platform):
user_output('Running gyp...')
diff --git a/aos/common/actions/action_test.cc b/aos/common/actions/action_test.cc
index 59d8d79..1e94b32 100644
--- a/aos/common/actions/action_test.cc
+++ b/aos/common/actions/action_test.cc
@@ -136,7 +136,7 @@
ASSERT_FALSE(actions::test_action.status.FetchLatest());
::std::thread init_thread([&nop_act]() { nop_act.Initialize(); });
- ::aos::time::SleepFor(::aos::time::Time::InSeconds(0.01));
+ ::aos::time::SleepFor(::aos::time::Time::InSeconds(0.1));
ASSERT_TRUE(actions::test_action.goal.MakeWithBuilder().run(1).Send());
init_thread.join();
ASSERT_TRUE(actions::test_action.status.FetchLatest());
diff --git a/aos/common/condition_test.cc b/aos/common/condition_test.cc
index 05990e5..2310273 100644
--- a/aos/common/condition_test.cc
+++ b/aos/common/condition_test.cc
@@ -307,7 +307,7 @@
Settle();
shared_->condition.Signal();
EXPECT_FALSE(child.Hung());
- EXPECT_EQ(Mutex::State::kUnlocked, shared_->mutex.TryLock());
+ EXPECT_EQ(Mutex::State::kLockFailed, shared_->mutex.TryLock());
}
// Tests that Signal() stops exactly 1 Wait()er.
diff --git a/aos/common/libc/aos_strerror.cc b/aos/common/libc/aos_strerror.cc
index 4bca500..2c9735d 100644
--- a/aos/common/libc/aos_strerror.cc
+++ b/aos/common/libc/aos_strerror.cc
@@ -5,8 +5,6 @@
#include <string.h>
#include <stdio.h>
-#include "aos/linux_code/thread_local.h"
-
// This code uses an overloaded function to handle the result from either
// version of strerror_r correctly without needing a way to get the choice out
// of the compiler/glibc/whatever explicitly.
@@ -35,7 +33,7 @@
} // namespace
const char *aos_strerror(int error) {
- static AOS_THREAD_LOCAL char buffer[kBufferSize];
+ static thread_local char buffer[kBufferSize];
// Call the overload for whichever version we're using.
return aos_strerror_handle_result(
diff --git a/aos/common/libc/aos_strsignal.cc b/aos/common/libc/aos_strsignal.cc
index 5fae327..2321a07 100644
--- a/aos/common/libc/aos_strsignal.cc
+++ b/aos/common/libc/aos_strsignal.cc
@@ -2,11 +2,10 @@
#include <signal.h>
-#include "aos/linux_code/thread_local.h"
#include "aos/common/logging/logging.h"
const char *aos_strsignal(int signal) {
- static AOS_THREAD_LOCAL char buffer[512];
+ static thread_local char buffer[512];
if (signal >= SIGRTMIN && signal <= SIGRTMAX) {
CHECK(snprintf(buffer, sizeof(buffer), "Real-time signal %d",
diff --git a/aos/common/mutex.h b/aos/common/mutex.h
index db1b012..ca35b10 100644
--- a/aos/common/mutex.h
+++ b/aos/common/mutex.h
@@ -20,8 +20,12 @@
// Otherwise, the destructor will LOG(FATAL).
class Mutex {
public:
+ // States that signify the result of TryLock.
enum class State {
- kLocked, kUnlocked
+ // The mutex was acquired successfully.
+ kLocked,
+ // TryLock tried to grab the mutex and failed.
+ kLockFailed
};
// Creates an unlocked mutex.
@@ -43,6 +47,10 @@
// Doesn't wait for the mutex to be unlocked if it is locked.
State TryLock() __attribute__((warn_unused_result));
+ // Returns true iff the current task has this mutex locked.
+ // This is mainly for IPCRecursiveMutexLocker to use.
+ bool OwnedBySelf() const;
+
private:
aos_mutex impl_;
@@ -101,6 +109,36 @@
DISALLOW_COPY_AND_ASSIGN(IPCMutexLocker);
};
+// A version of IPCMutexLocker which only locks (and unlocks) the mutex if the
+// current task does not already hold it.
+class IPCRecursiveMutexLocker {
+ public:
+ explicit IPCRecursiveMutexLocker(Mutex *mutex)
+ : mutex_(mutex),
+ locked_(!mutex_->OwnedBySelf()),
+ owner_died_(locked_ ? mutex_->Lock() : false) {}
+ ~IPCRecursiveMutexLocker() {
+ if (__builtin_expect(!owner_died_checked_, false)) {
+ ::aos::Die("nobody checked if the previous owner of mutex %p died", this);
+ }
+ if (locked_) mutex_->Unlock();
+ }
+
+ // Whether or not the previous owner died. If this is not called at least
+ // once, the destructor will ::aos::Die.
+ __attribute__((warn_unused_result)) bool owner_died() {
+ owner_died_checked_ = true;
+ return __builtin_expect(owner_died_, false);
+ }
+
+ private:
+ Mutex *const mutex_;
+ const bool locked_, owner_died_;
+ bool owner_died_checked_ = false;
+
+ DISALLOW_COPY_AND_ASSIGN(IPCRecursiveMutexLocker);
+};
+
} // namespace aos
#endif // AOS_COMMON_MUTEX_H_
diff --git a/aos/common/mutex_test.cc b/aos/common/mutex_test.cc
index 56f7772..6614d5c 100644
--- a/aos/common/mutex_test.cc
+++ b/aos/common/mutex_test.cc
@@ -21,7 +21,7 @@
class MutexTest : public ::testing::Test {
public:
- Mutex test_mutex;
+ Mutex test_mutex_;
protected:
void SetUp() override {
@@ -35,39 +35,40 @@
typedef MutexTest MutexLockerDeathTest;
typedef MutexTest IPCMutexLockerTest;
typedef MutexTest IPCMutexLockerDeathTest;
+typedef MutexTest IPCRecursiveMutexLockerTest;
TEST_F(MutexTest, TryLock) {
- EXPECT_EQ(Mutex::State::kLocked, test_mutex.TryLock());
- EXPECT_EQ(Mutex::State::kUnlocked, test_mutex.TryLock());
+ EXPECT_EQ(Mutex::State::kLocked, test_mutex_.TryLock());
+ EXPECT_EQ(Mutex::State::kLockFailed, test_mutex_.TryLock());
- test_mutex.Unlock();
+ test_mutex_.Unlock();
}
TEST_F(MutexTest, Lock) {
- ASSERT_FALSE(test_mutex.Lock());
- EXPECT_EQ(Mutex::State::kUnlocked, test_mutex.TryLock());
+ ASSERT_FALSE(test_mutex_.Lock());
+ EXPECT_EQ(Mutex::State::kLockFailed, test_mutex_.TryLock());
- test_mutex.Unlock();
+ test_mutex_.Unlock();
}
TEST_F(MutexTest, Unlock) {
- ASSERT_FALSE(test_mutex.Lock());
- EXPECT_EQ(Mutex::State::kUnlocked, test_mutex.TryLock());
- test_mutex.Unlock();
- EXPECT_EQ(Mutex::State::kLocked, test_mutex.TryLock());
+ ASSERT_FALSE(test_mutex_.Lock());
+ EXPECT_EQ(Mutex::State::kLockFailed, test_mutex_.TryLock());
+ test_mutex_.Unlock();
+ EXPECT_EQ(Mutex::State::kLocked, test_mutex_.TryLock());
- test_mutex.Unlock();
+ test_mutex_.Unlock();
}
// Sees what happens with multiple unlocks.
TEST_F(MutexDeathTest, RepeatUnlock) {
logging::Init();
- ASSERT_FALSE(test_mutex.Lock());
- test_mutex.Unlock();
+ ASSERT_FALSE(test_mutex_.Lock());
+ test_mutex_.Unlock();
EXPECT_DEATH(
{
logging::AddImplementation(new util::DeathTestLogImplementation());
- test_mutex.Unlock();
+ test_mutex_.Unlock();
},
".*multiple unlock.*");
}
@@ -78,7 +79,7 @@
EXPECT_DEATH(
{
logging::AddImplementation(new util::DeathTestLogImplementation());
- test_mutex.Unlock();
+ test_mutex_.Unlock();
},
".*multiple unlock.*");
}
@@ -88,8 +89,8 @@
EXPECT_DEATH(
{
logging::AddImplementation(new util::DeathTestLogImplementation());
- ASSERT_FALSE(test_mutex.Lock());
- ASSERT_FALSE(test_mutex.Lock());
+ ASSERT_FALSE(test_mutex_.Lock());
+ ASSERT_FALSE(test_mutex_.Lock());
},
".*multiple lock.*");
}
@@ -134,9 +135,9 @@
TEST_F(MutexTest, ThreadSanitizerContended) {
int counter = 0;
AdderThread threads[2]{
- {&counter, &test_mutex, ::aos::time::Time::InSeconds(0.2),
+ {&counter, &test_mutex_, ::aos::time::Time::InSeconds(0.2),
::aos::time::Time::InSeconds(0)},
- {&counter, &test_mutex, ::aos::time::Time::InSeconds(0),
+ {&counter, &test_mutex_, ::aos::time::Time::InSeconds(0),
::aos::time::Time::InSeconds(0)}, };
for (auto &c : threads) {
c.Start();
@@ -153,12 +154,12 @@
int counter = 0;
::std::thread thread([&counter, this]() {
for (int i = 0; i < 1000; ++i) {
- MutexLocker locker(&test_mutex);
+ MutexLocker locker(&test_mutex_);
++counter;
}
});
for (int i = 0; i < 1000; ++i) {
- MutexLocker locker(&test_mutex);
+ MutexLocker locker(&test_mutex_);
--counter;
}
thread.join();
@@ -170,9 +171,9 @@
TEST_F(MutexTest, ThreadSanitizerUncontended) {
int counter = 0;
AdderThread threads[2]{
- {&counter, &test_mutex, ::aos::time::Time::InSeconds(0.2),
+ {&counter, &test_mutex_, ::aos::time::Time::InSeconds(0.2),
::aos::time::Time::InSeconds(0)},
- {&counter, &test_mutex, ::aos::time::Time::InSeconds(0),
+ {&counter, &test_mutex_, ::aos::time::Time::InSeconds(0),
::aos::time::Time::InSeconds(0)}, };
for (auto &c : threads) {
c.Start();
@@ -183,31 +184,90 @@
EXPECT_EQ(2, counter);
}
+namespace {
+
+class LockerThread : public util::Thread {
+ public:
+ LockerThread(Mutex *mutex, bool lock, bool unlock)
+ : mutex_(mutex), lock_(lock), unlock_(unlock) {}
+
+ private:
+ virtual void Run() override {
+ if (lock_) ASSERT_FALSE(mutex_->Lock());
+ if (unlock_) mutex_->Unlock();
+ }
+
+ Mutex *const mutex_;
+ const bool lock_, unlock_;
+};
+
+} // namespace
+
+// Makes sure that we don't SIGSEGV or something with multiple threads.
+TEST_F(MutexTest, MultiThreadedLock) {
+ LockerThread t(&test_mutex_, true, true);
+ t.Start();
+ ASSERT_FALSE(test_mutex_.Lock());
+ test_mutex_.Unlock();
+ t.Join();
+}
+
TEST_F(MutexLockerTest, Basic) {
{
- aos::MutexLocker locker(&test_mutex);
- EXPECT_EQ(Mutex::State::kUnlocked, test_mutex.TryLock());
+ aos::MutexLocker locker(&test_mutex_);
+ EXPECT_EQ(Mutex::State::kLockFailed, test_mutex_.TryLock());
}
- EXPECT_EQ(Mutex::State::kLocked, test_mutex.TryLock());
+ EXPECT_EQ(Mutex::State::kLocked, test_mutex_.TryLock());
- test_mutex.Unlock();
+ test_mutex_.Unlock();
}
TEST_F(IPCMutexLockerTest, Basic) {
{
- aos::IPCMutexLocker locker(&test_mutex);
- EXPECT_EQ(Mutex::State::kUnlocked, test_mutex.TryLock());
+ aos::IPCMutexLocker locker(&test_mutex_);
+ EXPECT_EQ(Mutex::State::kLockFailed, test_mutex_.TryLock());
EXPECT_FALSE(locker.owner_died());
}
- EXPECT_EQ(Mutex::State::kLocked, test_mutex.TryLock());
+ EXPECT_EQ(Mutex::State::kLocked, test_mutex_.TryLock());
- test_mutex.Unlock();
+ test_mutex_.Unlock();
}
+// Tests what happens when the caller doesn't check if the previous owner died
+// with an IPCMutexLocker.
TEST_F(IPCMutexLockerDeathTest, NoCheckOwnerDied) {
- EXPECT_DEATH({ aos::IPCMutexLocker locker(&test_mutex); },
+ EXPECT_DEATH({ aos::IPCMutexLocker locker(&test_mutex_); },
"nobody checked if the previous owner of mutex [^ ]+ died.*");
}
+TEST_F(IPCRecursiveMutexLockerTest, Basic) {
+ {
+ aos::IPCRecursiveMutexLocker locker(&test_mutex_);
+ EXPECT_EQ(Mutex::State::kLockFailed, test_mutex_.TryLock());
+ EXPECT_FALSE(locker.owner_died());
+ }
+ EXPECT_EQ(Mutex::State::kLocked, test_mutex_.TryLock());
+
+ test_mutex_.Unlock();
+}
+
+// Tests actually locking a mutex recursively with IPCRecursiveMutexLocker.
+TEST_F(IPCRecursiveMutexLockerTest, RecursiveLock) {
+ {
+ aos::IPCRecursiveMutexLocker locker(&test_mutex_);
+ EXPECT_EQ(Mutex::State::kLockFailed, test_mutex_.TryLock());
+ {
+ aos::IPCRecursiveMutexLocker locker(&test_mutex_);
+ EXPECT_EQ(Mutex::State::kLockFailed, test_mutex_.TryLock());
+ EXPECT_FALSE(locker.owner_died());
+ }
+ EXPECT_EQ(Mutex::State::kLockFailed, test_mutex_.TryLock());
+ EXPECT_FALSE(locker.owner_died());
+ }
+ EXPECT_EQ(Mutex::State::kLocked, test_mutex_.TryLock());
+
+ test_mutex_.Unlock();
+}
+
} // namespace testing
} // namespace aos
diff --git a/aos/common/queue_test.cc b/aos/common/queue_test.cc
index 6d5dda6..310301d 100644
--- a/aos/common/queue_test.cc
+++ b/aos/common/queue_test.cc
@@ -78,6 +78,37 @@
EXPECT_EQ(true, my_test_queue.IsNewerThanMS(10000));
}
+// Tests that multiple queue instances don't break each other.
+TEST_F(QueueTest, MultipleQueues) {
+ my_test_queue.MakeWithBuilder().test_bool(true).test_int(0x971).Send();
+ ASSERT_TRUE(my_test_queue.FetchLatest());
+ EXPECT_TRUE(my_test_queue.get());
+
+ {
+ ::aos::Queue<TestingMessage> my_other_test_queue(
+ ".aos.common.testing.queue_name");
+ my_other_test_queue.MakeMessage();
+ EXPECT_FALSE(my_other_test_queue.FetchLatest());
+ EXPECT_FALSE(my_test_queue.FetchLatest());
+ }
+
+ EXPECT_TRUE(my_test_queue.get());
+}
+
+// Tests that using a queue from multiple threads works correctly.
+TEST_F(QueueTest, MultipleThreads) {
+ my_test_queue.MakeWithBuilder().test_bool(true).test_int(0x971).Send();
+ ASSERT_TRUE(my_test_queue.FetchLatest());
+ my_test_queue.MakeWithBuilder().test_bool(true).test_int(0x254).Send();
+ EXPECT_EQ(0x971, my_test_queue->test_int);
+
+ ::aos::util::FunctionThread::RunInOtherThread([this]() {
+ ASSERT_TRUE(my_test_queue.FetchLatest());
+ EXPECT_EQ(0x254, my_test_queue->test_int);
+ });
+ EXPECT_EQ(0x254, my_test_queue->test_int);
+}
+
// Makes sure that MakeWithBuilder zeros the message initially.
// This might randomly succeed sometimes, but it will fail with asan if it
// doesn't.
diff --git a/aos/linux_code/ipc_lib/aos_sync.cc b/aos/linux_code/ipc_lib/aos_sync.cc
index 311d88d..1275bbf 100644
--- a/aos/linux_code/ipc_lib/aos_sync.cc
+++ b/aos/linux_code/ipc_lib/aos_sync.cc
@@ -21,7 +21,6 @@
#include <algorithm>
#include "aos/common/logging/logging.h"
-#include "aos/linux_code/thread_local.h"
#include "aos/common/once.h"
#include "aos/common/macros.h"
@@ -182,7 +181,7 @@
// Starts off at 0 in each new thread (because that's what it gets initialized
// to in most of them or it gets to reset to 0 after a fork by atfork_child()).
-AOS_THREAD_LOCAL pid_t my_tid = 0;
+thread_local pid_t my_tid = 0;
// Gets called before the fork(2) wrapper function returns in the child.
void atfork_child() {
diff --git a/aos/linux_code/ipc_lib/mutex.cc b/aos/linux_code/ipc_lib/mutex.cc
index 9e270c9..c5b8652 100644
--- a/aos/linux_code/ipc_lib/mutex.cc
+++ b/aos/linux_code/ipc_lib/mutex.cc
@@ -15,7 +15,7 @@
}
Mutex::~Mutex() {
- if (__builtin_expect(mutex_islocked(&impl_), 0)) {
+ if (__builtin_expect(mutex_islocked(&impl_), false)) {
LOG(FATAL, "destroying locked mutex %p (aka %p)\n",
this, &impl_);
}
@@ -44,11 +44,15 @@
case 0:
return State::kLocked;
case 4:
- return State::kUnlocked;
+ return State::kLockFailed;
default:
LOG(FATAL, "mutex_trylock(%p(=%" PRIu32 ")) failed with %d\n",
&impl_, impl_.futex, ret);
}
}
+bool Mutex::OwnedBySelf() const {
+ return mutex_islocked(&impl_);
+}
+
} // namespace aos
diff --git a/aos/linux_code/logging/linux_interface.cc b/aos/linux_code/logging/linux_interface.cc
index 626b2ee..b0a5c1b 100644
--- a/aos/linux_code/logging/linux_interface.cc
+++ b/aos/linux_code/logging/linux_interface.cc
@@ -3,7 +3,6 @@
#include <sys/prctl.h>
#include "aos/linux_code/complex_thread_local.h"
-#include "aos/linux_code/thread_local.h"
#include "aos/common/die.h"
namespace aos {
@@ -47,7 +46,7 @@
// reason for doing this instead of just deleting them is that tsan (at least)
// doesn't like it when pthread_atfork handlers do complicated stuff and it's
// not a great idea anyways.
-AOS_THREAD_LOCAL bool delete_current_context(false);
+thread_local bool delete_current_context(false);
} // namespace
diff --git a/aos/linux_code/thread_local.h b/aos/linux_code/thread_local.h
deleted file mode 100644
index 503e18c..0000000
--- a/aos/linux_code/thread_local.h
+++ /dev/null
@@ -1,13 +0,0 @@
-#ifndef AOS_LINUX_CODE_THREAD_LOCAL_H_
-#define AOS_LINUX_CODE_THREAD_LOCAL_H_
-
-// The storage class to use when declaring thread-local variables. This provides
-// a single place to change it if/when we want to switch to something standard.
-//
-// Example: AOS_THREAD_LOCAL void *bla; // at namespace (aka global) scope
-//
-// C++11 has thread_local, but it's not clear whether Clang supports that as of
-// 12/18/12.
-#define AOS_THREAD_LOCAL __thread
-
-#endif // AOS_LINUX_CODE_THREAD_LOCAL_H_