diff options
author | Christopher Ferris <cferris@google.com> | 2014-11-08 15:57:11 -0800 |
---|---|---|
committer | Christopher Ferris <cferris@google.com> | 2014-11-14 10:46:39 -0800 |
commit | 3cdbfdce6a38cd23968d27d6e9e8d3ee65c3cf98 (patch) | |
tree | 29fa47b6c8aa35997d02c6b6c42f7cf7f3609449 /libbacktrace | |
parent | 6ef68b55b77b199fdcef2822750a392e1d0a4b04 (diff) | |
download | core-3cdbfdce6a38cd23968d27d6e9e8d3ee65c3cf98.tar.gz core-3cdbfdce6a38cd23968d27d6e9e8d3ee65c3cf98.tar.bz2 core-3cdbfdce6a38cd23968d27d6e9e8d3ee65c3cf98.zip |
Convert futex to cond wait.
Switch to the better supported pthread_cond to handle the Wait/Wake
functions.
Also, increase the number of simultaneous threads in the thread tests.
Bug: 18381207
(cherry picked from commit db44538387b08f367fc2419653639866f4c2fbd6)
Change-Id: Id326a7a7b92cb61573def3f761597c40f3ef2f4b
Diffstat (limited to 'libbacktrace')
-rwxr-xr-x | libbacktrace/Android.mk | 4 | ||||
-rw-r--r-- | libbacktrace/BacktraceThread.cpp | 49 | ||||
-rw-r--r-- | libbacktrace/BacktraceThread.h | 10 | ||||
-rw-r--r-- | libbacktrace/backtrace_test.cpp | 17 |
4 files changed, 56 insertions, 24 deletions
diff --git a/libbacktrace/Android.mk b/libbacktrace/Android.mk index 01b4f4d15..f2f71f93c 100755 --- a/libbacktrace/Android.mk +++ b/libbacktrace/Android.mk @@ -64,6 +64,10 @@ libbacktrace_shared_libraries_host := \ libbacktrace_static_libraries_host := \ libcutils \ +libbacktrace_ldlibs_host := \ + -lpthread \ + -lrt \ + module := libbacktrace module_tag := optional build_type := target diff --git a/libbacktrace/BacktraceThread.cpp b/libbacktrace/BacktraceThread.cpp index b47cd2ad2..439cc3bde 100644 --- a/libbacktrace/BacktraceThread.cpp +++ b/libbacktrace/BacktraceThread.cpp @@ -17,14 +17,15 @@ #include <errno.h> #include <inttypes.h> #include <limits.h> -#include <linux/futex.h> #include <pthread.h> #include <signal.h> +#include <stdlib.h> #include <string.h> #include <sys/syscall.h> #include <sys/time.h> #include <sys/types.h> #include <ucontext.h> +#include <unistd.h> #include <cutils/atomic.h> @@ -32,10 +33,6 @@ #include "BacktraceThread.h" #include "thread_utils.h" -static inline int futex(volatile int* uaddr, int op, int val, const struct timespec* ts, volatile int* uaddr2, int val3) { - return syscall(__NR_futex, uaddr, op, val, ts, uaddr2, val3); -} - //------------------------------------------------------------------------- // ThreadEntry implementation. //------------------------------------------------------------------------- @@ -45,7 +42,14 @@ pthread_mutex_t ThreadEntry::list_mutex_ = PTHREAD_MUTEX_INITIALIZER; // Assumes that ThreadEntry::list_mutex_ has already been locked before // creating a ThreadEntry object. ThreadEntry::ThreadEntry(pid_t pid, pid_t tid) - : pid_(pid), tid_(tid), futex_(0), ref_count_(1), mutex_(PTHREAD_MUTEX_INITIALIZER), next_(ThreadEntry::list_), prev_(NULL) { + : pid_(pid), tid_(tid), ref_count_(1), mutex_(PTHREAD_MUTEX_INITIALIZER), + wait_mutex_(PTHREAD_MUTEX_INITIALIZER), wait_value_(0), + next_(ThreadEntry::list_), prev_(NULL) { + pthread_condattr_t attr; + pthread_condattr_init(&attr); + pthread_condattr_setclock(&attr, CLOCK_MONOTONIC); + pthread_cond_init(&wait_cond_, &attr); + // Add ourselves to the list. if (ThreadEntry::list_) { ThreadEntry::list_->prev_ = this; @@ -99,22 +103,35 @@ ThreadEntry::~ThreadEntry() { next_ = NULL; prev_ = NULL; + + pthread_cond_destroy(&wait_cond_); } void ThreadEntry::Wait(int value) { timespec ts; - ts.tv_sec = 10; - ts.tv_nsec = 0; - errno = 0; - futex(&futex_, FUTEX_WAIT, value, &ts, NULL, 0); - if (errno != 0 && errno != EWOULDBLOCK) { - BACK_LOGW("futex wait failed, futex = %d: %s", futex_, strerror(errno)); + if (clock_gettime(CLOCK_MONOTONIC, &ts) == -1) { + BACK_LOGW("clock_gettime failed: %s", strerror(errno)); + abort(); } + ts.tv_sec += 10; + + pthread_mutex_lock(&wait_mutex_); + while (wait_value_ != value) { + int ret = pthread_cond_timedwait(&wait_cond_, &wait_mutex_, &ts); + if (ret != 0) { + BACK_LOGW("pthread_cond_timedwait failed: %s", strerror(ret)); + break; + } + } + pthread_mutex_unlock(&wait_mutex_); } void ThreadEntry::Wake() { - futex_++; - futex(&futex_, FUTEX_WAKE, INT_MAX, NULL, NULL, 0); + pthread_mutex_lock(&wait_mutex_); + wait_value_++; + pthread_mutex_unlock(&wait_mutex_); + + pthread_cond_signal(&wait_cond_); } void ThreadEntry::CopyUcontextFromSigcontext(void* sigcontext) { @@ -142,7 +159,7 @@ static void SignalHandler(int, siginfo_t*, void* sigcontext) { // Pause the thread until the unwind is complete. This avoids having // the thread run ahead causing problems. - entry->Wait(1); + entry->Wait(2); ThreadEntry::Remove(entry); } @@ -194,7 +211,7 @@ bool BacktraceThread::Unwind(size_t num_ignore_frames, ucontext_t* ucontext) { } // Wait for the thread to get the ucontext. - entry->Wait(0); + entry->Wait(1); // After the thread has received the signal, allow other unwinders to // continue. diff --git a/libbacktrace/BacktraceThread.h b/libbacktrace/BacktraceThread.h index ff3e9f3f4..99a863878 100644 --- a/libbacktrace/BacktraceThread.h +++ b/libbacktrace/BacktraceThread.h @@ -48,8 +48,10 @@ public: inline void Lock() { pthread_mutex_lock(&mutex_); - // Reset the futex value in case of multiple unwinds of the same thread. - futex_ = 0; + + // Always reset the wait value since this could be the first or nth + // time this entry is locked. + wait_value_ = 0; } inline void Unlock() { @@ -66,9 +68,11 @@ private: pid_t pid_; pid_t tid_; - int futex_; int ref_count_; pthread_mutex_t mutex_; + pthread_mutex_t wait_mutex_; + pthread_cond_t wait_cond_; + int wait_value_; ThreadEntry* next_; ThreadEntry* prev_; ucontext_t ucontext_; diff --git a/libbacktrace/backtrace_test.cpp b/libbacktrace/backtrace_test.cpp index ed6b21112..8002ed6b8 100644 --- a/libbacktrace/backtrace_test.cpp +++ b/libbacktrace/backtrace_test.cpp @@ -51,7 +51,7 @@ #define NS_PER_SEC 1000000000ULL // Number of simultaneous dumping operations to perform. -#define NUM_THREADS 20 +#define NUM_THREADS 40 // Number of simultaneous threads running in our forked process. #define NUM_PTRACE_THREADS 5 @@ -486,6 +486,13 @@ TEST(libbacktrace, thread_level_trace) { struct sigaction new_action; ASSERT_TRUE(sigaction(THREAD_SIGNAL, NULL, &new_action) == 0); EXPECT_EQ(cur_action.sa_sigaction, new_action.sa_sigaction); + // The SA_RESTORER flag gets set behind our back, so a direct comparison + // doesn't work unless we mask the value off. Mips doesn't have this + // flag, so skip this on that platform. +#ifdef SA_RESTORER + cur_action.sa_flags &= ~SA_RESTORER; + new_action.sa_flags &= ~SA_RESTORER; +#endif EXPECT_EQ(cur_action.sa_flags, new_action.sa_flags); } @@ -583,7 +590,7 @@ TEST(libbacktrace, thread_multiple_dump) { // Wait for tids to be set. for (std::vector<thread_t>::iterator it = runners.begin(); it != runners.end(); ++it) { - ASSERT_TRUE(WaitForNonZero(&it->state, 10)); + ASSERT_TRUE(WaitForNonZero(&it->state, 30)); } // Start all of the dumpers at once, they will spin until they are signalled @@ -602,7 +609,7 @@ TEST(libbacktrace, thread_multiple_dump) { android_atomic_acquire_store(1, &dump_now); for (size_t i = 0; i < NUM_THREADS; i++) { - ASSERT_TRUE(WaitForNonZero(&dumpers[i].done, 10)); + ASSERT_TRUE(WaitForNonZero(&dumpers[i].done, 30)); // Tell the runner thread to exit its infinite loop. android_atomic_acquire_store(0, &runners[i].state); @@ -625,7 +632,7 @@ TEST(libbacktrace, thread_multiple_dump_same_thread) { ASSERT_TRUE(pthread_create(&runner.threadId, &attr, ThreadMaxRun, &runner) == 0); // Wait for tids to be set. - ASSERT_TRUE(WaitForNonZero(&runner.state, 10)); + ASSERT_TRUE(WaitForNonZero(&runner.state, 30)); // Start all of the dumpers at once, they will spin until they are signalled // to begin their dump run. @@ -645,7 +652,7 @@ TEST(libbacktrace, thread_multiple_dump_same_thread) { android_atomic_acquire_store(1, &dump_now); for (size_t i = 0; i < NUM_THREADS; i++) { - ASSERT_TRUE(WaitForNonZero(&dumpers[i].done, 100)); + ASSERT_TRUE(WaitForNonZero(&dumpers[i].done, 30)); ASSERT_TRUE(dumpers[i].backtrace != NULL); VerifyMaxDump(dumpers[i].backtrace); |