Fix deadlock/timeout in thread unwinding.
When malloc debug is enabled, using libbacktrace to unwind can
result in a deadlock. This happens when an unwind of a thread
is occuring which triggers a signal to be sent to that thread. If
that thread is interrupted while a malloc debug function is
executing and owns a lock, that thread is then stuck in the signal
handler. Then the original unwinding thread attempts to do an
allocation and gets stuck waiting for the same malloc debug lock.
This is not a complete deadlock since the unwinder has timeouts,
but it results in truncated unwinds that take at least five
seconds to complete.
Only the backtrace signals needs to be blocked because it is the only
known signal that will result in a thread being paused in a signal
handler.
Also, added a named signal in the reserved signal list for the
special bionic backtrace signal.
Bug: 150833265
Test: New unit tests pass with fix, fail without fix.
Change-Id: If3e41f092ebd40ce62a59ef51d636a91bc31ed80
diff --git a/libc/malloc_debug/malloc_debug.cpp b/libc/malloc_debug/malloc_debug.cpp
index 6eaac7d..609f030 100644
--- a/libc/malloc_debug/malloc_debug.cpp
+++ b/libc/malloc_debug/malloc_debug.cpp
@@ -30,11 +30,13 @@
#include <inttypes.h>
#include <malloc.h>
#include <pthread.h>
+#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/cdefs.h>
#include <sys/param.h>
+#include <sys/syscall.h>
#include <unistd.h>
#include <mutex>
@@ -44,8 +46,9 @@
#include <android-base/properties.h>
#include <android-base/stringprintf.h>
#include <bionic/malloc_tagged_pointers.h>
-#include <private/bionic_malloc_dispatch.h>
+#include <platform/bionic/reserved_signals.h>
#include <private/MallocXmlElem.h>
+#include <private/bionic_malloc_dispatch.h>
#include "Config.h"
#include "DebugData.h"
@@ -131,6 +134,40 @@
};
pthread_rwlock_t ScopedConcurrentLock::lock_;
+// Use this because the sigprocmask* functions filter out the reserved bionic
+// signals including the signal this code blocks.
+static inline int __rt_sigprocmask(int how, const sigset64_t* new_set, sigset64_t* old_set,
+ size_t sigset_size) {
+ return syscall(SYS_rt_sigprocmask, how, new_set, old_set, sigset_size);
+}
+
+// Need to block the backtrace signal while in malloc debug routines
+// otherwise there is a chance of a deadlock and timeout when unwinding.
+// This can occur if a thread is paused while owning a malloc debug
+// internal lock.
+class ScopedBacktraceSignalBlocker {
+ public:
+ ScopedBacktraceSignalBlocker() {
+ sigemptyset64(&backtrace_set_);
+ sigaddset64(&backtrace_set_, BIONIC_SIGNAL_BACKTRACE);
+ sigset64_t old_set;
+ __rt_sigprocmask(SIG_BLOCK, &backtrace_set_, &old_set, sizeof(backtrace_set_));
+ if (sigismember64(&old_set, BIONIC_SIGNAL_BACKTRACE)) {
+ unblock_ = false;
+ }
+ }
+
+ ~ScopedBacktraceSignalBlocker() {
+ if (unblock_) {
+ __rt_sigprocmask(SIG_UNBLOCK, &backtrace_set_, nullptr, sizeof(backtrace_set_));
+ }
+ }
+
+ private:
+ bool unblock_ = true;
+ sigset64_t backtrace_set_;
+};
+
static void InitAtfork() {
static pthread_once_t atfork_init = PTHREAD_ONCE_INIT;
pthread_once(&atfork_init, []() {
@@ -334,8 +371,8 @@
void debug_get_malloc_leak_info(uint8_t** info, size_t* overall_size, size_t* info_size,
size_t* total_memory, size_t* backtrace_size) {
ScopedConcurrentLock lock;
-
ScopedDisableDebugCalls disable;
+ ScopedBacktraceSignalBlocker blocked;
// Verify the arguments.
if (info == nullptr || overall_size == nullptr || info_size == nullptr || total_memory == nullptr ||
@@ -370,6 +407,7 @@
}
ScopedConcurrentLock lock;
ScopedDisableDebugCalls disable;
+ ScopedBacktraceSignalBlocker blocked;
if (!VerifyPointer(pointer, "malloc_usable_size")) {
return 0;
@@ -434,6 +472,7 @@
}
ScopedConcurrentLock lock;
ScopedDisableDebugCalls disable;
+ ScopedBacktraceSignalBlocker blocked;
void* pointer = InternalMalloc(size);
@@ -510,6 +549,7 @@
}
ScopedConcurrentLock lock;
ScopedDisableDebugCalls disable;
+ ScopedBacktraceSignalBlocker blocked;
if (g_debug->config().options() & RECORD_ALLOCS) {
g_debug->record->AddEntry(new FreeEntry(pointer));
@@ -528,6 +568,7 @@
}
ScopedConcurrentLock lock;
ScopedDisableDebugCalls disable;
+ ScopedBacktraceSignalBlocker blocked;
if (bytes == 0) {
bytes = 1;
@@ -607,6 +648,7 @@
}
ScopedConcurrentLock lock;
ScopedDisableDebugCalls disable;
+ ScopedBacktraceSignalBlocker blocked;
if (pointer == nullptr) {
pointer = InternalMalloc(bytes);
@@ -726,6 +768,7 @@
}
ScopedConcurrentLock lock;
ScopedDisableDebugCalls disable;
+ ScopedBacktraceSignalBlocker blocked;
size_t size;
if (__builtin_mul_overflow(nmemb, bytes, &size)) {
@@ -792,6 +835,7 @@
ScopedConcurrentLock lock;
ScopedDisableDebugCalls disable;
+ ScopedBacktraceSignalBlocker blocked;
// Avoid any issues where allocations are made that will be freed
// in the fclose.
@@ -880,6 +924,7 @@
}
ScopedConcurrentLock lock;
ScopedDisableDebugCalls disable;
+ ScopedBacktraceSignalBlocker blocked;
if (!(g_debug->config().options() & BACKTRACE)) {
return 0;
@@ -938,6 +983,7 @@
ScopedConcurrentLock lock;
ScopedDisableDebugCalls disable;
+ ScopedBacktraceSignalBlocker blocked;
std::lock_guard<std::mutex> guard(g_dump_lock);
@@ -953,6 +999,7 @@
void debug_dump_heap(const char* file_name) {
ScopedConcurrentLock lock;
ScopedDisableDebugCalls disable;
+ ScopedBacktraceSignalBlocker blocked;
std::lock_guard<std::mutex> guard(g_dump_lock);