diff options
author | Josh Gao <jmgao@google.com> | 2017-01-22 18:22:52 -0800 |
---|---|---|
committer | Josh Gao <jmgao@google.com> | 2017-01-23 11:34:49 -0800 |
commit | b64dd85c94ee21684d4ccbb0683d628eb1eb6121 (patch) | |
tree | b42a3d28031b895231cd289e2449282d5c8038d8 | |
parent | 13c15e05d0b6f35399800902eb8ea1943172db79 (diff) | |
download | system_core-b64dd85c94ee21684d4ccbb0683d628eb1eb6121.tar.gz system_core-b64dd85c94ee21684d4ccbb0683d628eb1eb6121.tar.bz2 system_core-b64dd85c94ee21684d4ccbb0683d628eb1eb6121.zip |
debuggerd_handler: actually wait for pseudothread to exit.
Occasionally, the pseudothread wouldn't exit in time after unlocking
the mutex to get crash_dump to proceed, resulting in spurious error
messages. Instead of using a mutex to emulate pthread_join, just
implement it correctly.
Bug: http://b/34472671
Test: debuggerd_test
Change-Id: I5c2658a84e9407ed8cc0ef2ad0fb648c388b7ad1
-rw-r--r-- | debuggerd/handler/debuggerd_handler.cpp | 22 |
1 files changed, 13 insertions, 9 deletions
diff --git a/debuggerd/handler/debuggerd_handler.cpp b/debuggerd/handler/debuggerd_handler.cpp index 6033a6b53..83db3a745 100644 --- a/debuggerd/handler/debuggerd_handler.cpp +++ b/debuggerd/handler/debuggerd_handler.cpp @@ -31,6 +31,7 @@ #include <errno.h> #include <fcntl.h> #include <inttypes.h> +#include <linux/futex.h> #include <pthread.h> #include <sched.h> #include <signal.h> @@ -46,6 +47,7 @@ #include <sys/wait.h> #include <unistd.h> +#include "private/bionic_futex.h" #include "private/libc_logging.h" // see man(2) prctl, specifically the section about PR_GET_NAME @@ -151,7 +153,6 @@ static bool have_siginfo(int signum) { } struct debugger_thread_info { - pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER; bool crash_dump_started = false; pid_t crashing_tid; pid_t pseudothread_tid; @@ -228,7 +229,7 @@ static int debuggerd_dispatch_pseudothread(void* arg) { } } - pthread_mutex_unlock(&thread_info->mutex); + syscall(__NR_exit, 0); return 0; } @@ -267,23 +268,26 @@ static void debuggerd_signal_handler(int signal_number, siginfo_t* info, void*) } debugger_thread_info thread_info = { + .pseudothread_tid = -1, .crashing_tid = gettid(), .signal_number = signal_number, .info = info }; - pthread_mutex_lock(&thread_info.mutex); // Essentially pthread_create without CLONE_FILES (see debuggerd_dispatch_pseudothread). - pid_t child_pid = clone(debuggerd_dispatch_pseudothread, pseudothread_stack, - CLONE_THREAD | CLONE_SIGHAND | CLONE_VM | CLONE_CHILD_SETTID, - &thread_info, nullptr, nullptr, &thread_info.pseudothread_tid); + pid_t child_pid = + clone(debuggerd_dispatch_pseudothread, pseudothread_stack, + CLONE_THREAD | CLONE_SIGHAND | CLONE_VM | CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID, + &thread_info, nullptr, nullptr, &thread_info.pseudothread_tid); if (child_pid == -1) { fatal("failed to spawn debuggerd dispatch thread: %s", strerror(errno)); } - // Wait for the child to finish and unlock the mutex. - // This relies on bionic behavior that isn't guaranteed by the standard. - pthread_mutex_lock(&thread_info.mutex); + // Wait for the child to start... + __futex_wait(&thread_info.pseudothread_tid, -1, nullptr); + + // and then wait for it to finish. + __futex_wait(&thread_info.pseudothread_tid, child_pid, nullptr); // Signals can either be fatal or nonfatal. // For fatal signals, crash_dump will PTRACE_CONT us with the signal we |