Skip to content
Snippets Groups Projects
Commit db445383 authored by Christopher Ferris's avatar Christopher Ferris
Browse files

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
Change-Id: I63240208e8b7f641b3f35a3fc0bb1acf80dc796e
parent eb6036ac
Branches
Tags
No related merge requests found
...@@ -64,6 +64,10 @@ libbacktrace_shared_libraries_host := \ ...@@ -64,6 +64,10 @@ libbacktrace_shared_libraries_host := \
libbacktrace_static_libraries_host := \ libbacktrace_static_libraries_host := \
libcutils \ libcutils \
libbacktrace_ldlibs_host := \
-lpthread \
-lrt \
module := libbacktrace module := libbacktrace
module_tag := optional module_tag := optional
build_type := target build_type := target
...@@ -105,6 +109,10 @@ libbacktrace_libc++_shared_libraries_host := \ ...@@ -105,6 +109,10 @@ libbacktrace_libc++_shared_libraries_host := \
libbacktrace_libc++_static_libraries_host := \ libbacktrace_libc++_static_libraries_host := \
libcutils \ libcutils \
libbacktrace_libc++_ldlibs_host := \
-lpthread \
-lrt \
libbacktrace_libc++_libc++ := true libbacktrace_libc++_libc++ := true
module := libbacktrace_libc++ module := libbacktrace_libc++
......
...@@ -17,14 +17,15 @@ ...@@ -17,14 +17,15 @@
#include <errno.h> #include <errno.h>
#include <inttypes.h> #include <inttypes.h>
#include <limits.h> #include <limits.h>
#include <linux/futex.h>
#include <pthread.h> #include <pthread.h>
#include <signal.h> #include <signal.h>
#include <stdlib.h>
#include <string.h> #include <string.h>
#include <sys/syscall.h> #include <sys/syscall.h>
#include <sys/time.h> #include <sys/time.h>
#include <sys/types.h> #include <sys/types.h>
#include <ucontext.h> #include <ucontext.h>
#include <unistd.h>
#include <cutils/atomic.h> #include <cutils/atomic.h>
...@@ -32,10 +33,6 @@ ...@@ -32,10 +33,6 @@
#include "BacktraceThread.h" #include "BacktraceThread.h"
#include "thread_utils.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. // ThreadEntry implementation.
//------------------------------------------------------------------------- //-------------------------------------------------------------------------
...@@ -45,7 +42,14 @@ pthread_mutex_t ThreadEntry::list_mutex_ = PTHREAD_MUTEX_INITIALIZER; ...@@ -45,7 +42,14 @@ pthread_mutex_t ThreadEntry::list_mutex_ = PTHREAD_MUTEX_INITIALIZER;
// Assumes that ThreadEntry::list_mutex_ has already been locked before // Assumes that ThreadEntry::list_mutex_ has already been locked before
// creating a ThreadEntry object. // creating a ThreadEntry object.
ThreadEntry::ThreadEntry(pid_t pid, pid_t tid) 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. // Add ourselves to the list.
if (ThreadEntry::list_) { if (ThreadEntry::list_) {
ThreadEntry::list_->prev_ = this; ThreadEntry::list_->prev_ = this;
...@@ -99,22 +103,35 @@ ThreadEntry::~ThreadEntry() { ...@@ -99,22 +103,35 @@ ThreadEntry::~ThreadEntry() {
next_ = NULL; next_ = NULL;
prev_ = NULL; prev_ = NULL;
pthread_cond_destroy(&wait_cond_);
} }
void ThreadEntry::Wait(int value) { void ThreadEntry::Wait(int value) {
timespec ts; timespec ts;
ts.tv_sec = 10; if (clock_gettime(CLOCK_MONOTONIC, &ts) == -1) {
ts.tv_nsec = 0; BACK_LOGW("clock_gettime failed: %s", strerror(errno));
errno = 0; abort();
futex(&futex_, FUTEX_WAIT, value, &ts, NULL, 0);
if (errno != 0 && errno != EWOULDBLOCK) {
BACK_LOGW("futex wait failed, futex = %d: %s", futex_, strerror(errno));
} }
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() { void ThreadEntry::Wake() {
futex_++; pthread_mutex_lock(&wait_mutex_);
futex(&futex_, FUTEX_WAKE, INT_MAX, NULL, NULL, 0); wait_value_++;
pthread_mutex_unlock(&wait_mutex_);
pthread_cond_signal(&wait_cond_);
} }
void ThreadEntry::CopyUcontextFromSigcontext(void* sigcontext) { void ThreadEntry::CopyUcontextFromSigcontext(void* sigcontext) {
...@@ -142,7 +159,7 @@ static void SignalHandler(int, siginfo_t*, 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 // Pause the thread until the unwind is complete. This avoids having
// the thread run ahead causing problems. // the thread run ahead causing problems.
entry->Wait(1); entry->Wait(2);
ThreadEntry::Remove(entry); ThreadEntry::Remove(entry);
} }
...@@ -194,7 +211,7 @@ bool BacktraceThread::Unwind(size_t num_ignore_frames, ucontext_t* ucontext) { ...@@ -194,7 +211,7 @@ bool BacktraceThread::Unwind(size_t num_ignore_frames, ucontext_t* ucontext) {
} }
// Wait for the thread to get the 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 // After the thread has received the signal, allow other unwinders to
// continue. // continue.
......
...@@ -48,8 +48,10 @@ public: ...@@ -48,8 +48,10 @@ public:
inline void Lock() { inline void Lock() {
pthread_mutex_lock(&mutex_); 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() { inline void Unlock() {
...@@ -66,9 +68,11 @@ private: ...@@ -66,9 +68,11 @@ private:
pid_t pid_; pid_t pid_;
pid_t tid_; pid_t tid_;
int futex_;
int ref_count_; int ref_count_;
pthread_mutex_t mutex_; pthread_mutex_t mutex_;
pthread_mutex_t wait_mutex_;
pthread_cond_t wait_cond_;
int wait_value_;
ThreadEntry* next_; ThreadEntry* next_;
ThreadEntry* prev_; ThreadEntry* prev_;
ucontext_t ucontext_; ucontext_t ucontext_;
......
...@@ -51,7 +51,7 @@ ...@@ -51,7 +51,7 @@
#define NS_PER_SEC 1000000000ULL #define NS_PER_SEC 1000000000ULL
// Number of simultaneous dumping operations to perform. // Number of simultaneous dumping operations to perform.
#define NUM_THREADS 20 #define NUM_THREADS 40
// Number of simultaneous threads running in our forked process. // Number of simultaneous threads running in our forked process.
#define NUM_PTRACE_THREADS 5 #define NUM_PTRACE_THREADS 5
...@@ -486,6 +486,13 @@ TEST(libbacktrace, thread_level_trace) { ...@@ -486,6 +486,13 @@ TEST(libbacktrace, thread_level_trace) {
struct sigaction new_action; struct sigaction new_action;
ASSERT_TRUE(sigaction(THREAD_SIGNAL, NULL, &new_action) == 0); ASSERT_TRUE(sigaction(THREAD_SIGNAL, NULL, &new_action) == 0);
EXPECT_EQ(cur_action.sa_sigaction, new_action.sa_sigaction); 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); EXPECT_EQ(cur_action.sa_flags, new_action.sa_flags);
} }
...@@ -583,7 +590,7 @@ TEST(libbacktrace, thread_multiple_dump) { ...@@ -583,7 +590,7 @@ TEST(libbacktrace, thread_multiple_dump) {
// Wait for tids to be set. // Wait for tids to be set.
for (std::vector<thread_t>::iterator it = runners.begin(); it != runners.end(); ++it) { 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 // Start all of the dumpers at once, they will spin until they are signalled
...@@ -602,7 +609,7 @@ TEST(libbacktrace, thread_multiple_dump) { ...@@ -602,7 +609,7 @@ TEST(libbacktrace, thread_multiple_dump) {
android_atomic_acquire_store(1, &dump_now); android_atomic_acquire_store(1, &dump_now);
for (size_t i = 0; i < NUM_THREADS; i++) { 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. // Tell the runner thread to exit its infinite loop.
android_atomic_acquire_store(0, &runners[i].state); android_atomic_acquire_store(0, &runners[i].state);
...@@ -625,7 +632,7 @@ TEST(libbacktrace, thread_multiple_dump_same_thread) { ...@@ -625,7 +632,7 @@ TEST(libbacktrace, thread_multiple_dump_same_thread) {
ASSERT_TRUE(pthread_create(&runner.threadId, &attr, ThreadMaxRun, &runner) == 0); ASSERT_TRUE(pthread_create(&runner.threadId, &attr, ThreadMaxRun, &runner) == 0);
// Wait for tids to be set. // 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 // Start all of the dumpers at once, they will spin until they are signalled
// to begin their dump run. // to begin their dump run.
...@@ -645,7 +652,7 @@ TEST(libbacktrace, thread_multiple_dump_same_thread) { ...@@ -645,7 +652,7 @@ TEST(libbacktrace, thread_multiple_dump_same_thread) {
android_atomic_acquire_store(1, &dump_now); android_atomic_acquire_store(1, &dump_now);
for (size_t i = 0; i < NUM_THREADS; i++) { 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); ASSERT_TRUE(dumpers[i].backtrace != NULL);
VerifyMaxDump(dumpers[i].backtrace); VerifyMaxDump(dumpers[i].backtrace);
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please to comment