Fix deadlock/timeout in thread unwinding. am: 73ca781f43

Change-Id: I3fb995a7cf99bba28c8fc4e6ad43f50db01ca2c7

4 files changed, 98 insertions, 2 deletions

diff --git a/libc/malloc_debug/Android.bp b/libc/malloc_debug/Android.bp
index b04cfe1b2..760039b9c 100644
--- a/libc/malloc_debug/Android.bp
+++ b/libc/malloc_debug/Android.bp
@@ -173,6 +173,7 @@ cc_test {
 
     shared_libs: [
         "libbase",
+        "libbacktrace",
         "liblog",
         "libunwindstack",
     ],
diff --git a/libc/malloc_debug/malloc_debug.cpp b/libc/malloc_debug/malloc_debug.cpp
index 6eaac7de8..609f030bf 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 @@ class ScopedConcurrentLock {
 };
 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_finalize() {
 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 @@ size_t debug_malloc_usable_size(void* pointer) {
   }
   ScopedConcurrentLock lock;
   ScopedDisableDebugCalls disable;
+  ScopedBacktraceSignalBlocker blocked;
 
   if (!VerifyPointer(pointer, "malloc_usable_size")) {
     return 0;
@@ -434,6 +472,7 @@ void* debug_malloc(size_t size) {
   }
   ScopedConcurrentLock lock;
   ScopedDisableDebugCalls disable;
+  ScopedBacktraceSignalBlocker blocked;
 
   void* pointer = InternalMalloc(size);
 
@@ -510,6 +549,7 @@ void debug_free(void* pointer) {
   }
   ScopedConcurrentLock lock;
   ScopedDisableDebugCalls disable;
+  ScopedBacktraceSignalBlocker blocked;
 
   if (g_debug->config().options() & RECORD_ALLOCS) {
     g_debug->record->AddEntry(new FreeEntry(pointer));
@@ -528,6 +568,7 @@ void* debug_memalign(size_t alignment, size_t bytes) {
   }
   ScopedConcurrentLock lock;
   ScopedDisableDebugCalls disable;
+  ScopedBacktraceSignalBlocker blocked;
 
   if (bytes == 0) {
     bytes = 1;
@@ -607,6 +648,7 @@ void* debug_realloc(void* pointer, size_t bytes) {
   }
   ScopedConcurrentLock lock;
   ScopedDisableDebugCalls disable;
+  ScopedBacktraceSignalBlocker blocked;
 
   if (pointer == nullptr) {
     pointer = InternalMalloc(bytes);
@@ -726,6 +768,7 @@ void* debug_calloc(size_t nmemb, size_t bytes) {
   }
   ScopedConcurrentLock lock;
   ScopedDisableDebugCalls disable;
+  ScopedBacktraceSignalBlocker blocked;
 
   size_t size;
   if (__builtin_mul_overflow(nmemb, bytes, &size)) {
@@ -792,6 +835,7 @@ int debug_malloc_info(int options, FILE* fp) {
 
   ScopedConcurrentLock lock;
   ScopedDisableDebugCalls disable;
+  ScopedBacktraceSignalBlocker blocked;
 
   // Avoid any issues where allocations are made that will be freed
   // in the fclose.
@@ -880,6 +924,7 @@ ssize_t debug_malloc_backtrace(void* pointer, uintptr_t* frames, size_t max_fram
   }
   ScopedConcurrentLock lock;
   ScopedDisableDebugCalls disable;
+  ScopedBacktraceSignalBlocker blocked;
 
   if (!(g_debug->config().options() & BACKTRACE)) {
     return 0;
@@ -938,6 +983,7 @@ bool debug_write_malloc_leak_info(FILE* fp) {
 
   ScopedConcurrentLock lock;
   ScopedDisableDebugCalls disable;
+  ScopedBacktraceSignalBlocker blocked;
 
   std::lock_guard<std::mutex> guard(g_dump_lock);
 
@@ -953,6 +999,7 @@ bool debug_write_malloc_leak_info(FILE* fp) {
 void debug_dump_heap(const char* file_name) {
   ScopedConcurrentLock lock;
   ScopedDisableDebugCalls disable;
+  ScopedBacktraceSignalBlocker blocked;
 
   std::lock_guard<std::mutex> guard(g_dump_lock);
 
diff --git a/libc/malloc_debug/tests/malloc_debug_system_tests.cpp b/libc/malloc_debug/tests/malloc_debug_system_tests.cpp
index 9e612f03b..f260db71a 100644
--- a/libc/malloc_debug/tests/malloc_debug_system_tests.cpp
+++ b/libc/malloc_debug/tests/malloc_debug_system_tests.cpp
@@ -42,12 +42,20 @@
 #include <gtest/gtest.h>
 #include <log/log.h>
 
+#include <atomic>
 #include <string>
 #include <thread>
 #include <vector>
 
+#include <backtrace/Backtrace.h>
+#include <backtrace/BacktraceMap.h>
+
 #include <bionic/malloc.h>
 
+// All DISABLED_ tests are designed to be executed after malloc debug
+// is enabled. These tests don't run be default, and are executed
+// by wrappers that will enable various malloc debug features.
+
 static constexpr time_t kTimeoutSeconds = 10;
 
 extern "C" bool GetInitialArgs(const char*** args, size_t* num_args) {
@@ -525,3 +533,42 @@ TEST(MallocDebugSystemTest, write_leak_info_header) {
   ASSERT_NO_FATAL_FAILURE(FindStrings(pid, std::vector<const char*>{"malloc debug enabled"},
                           std::vector<const char*>{" HAS INVALID TAG ", "USED AFTER FREE ", "UNKNOWN POINTER "}));
 }
+
+TEST(MallocTests, DISABLED_malloc_and_backtrace_deadlock) {
+  std::atomic_bool running(false);
+  pid_t tid;
+  std::thread thread([&tid, &running] {
+    tid = gettid();
+    running = true;
+    while (running) {
+      void* ptr = malloc(200);
+      if (ptr == nullptr) {
+        return;
+      }
+      free(ptr);
+    }
+  });
+
+  while (!running) {
+  }
+
+  static constexpr size_t kNumUnwinds = 1000;
+  for (size_t i = 0; i < kNumUnwinds; i++) {
+    std::unique_ptr<Backtrace> backtrace(Backtrace::Create(getpid(), tid));
+    ASSERT_TRUE(backtrace->Unwind(0)) << "Failed on unwind " << i;
+    ASSERT_LT(1, backtrace->NumFrames()) << "Failed on unwind " << i;
+  }
+  running = false;
+  thread.join();
+}
+
+TEST(MallocDebugSystemTest, malloc_and_backtrace_deadlock) {
+  pid_t pid;
+  ASSERT_NO_FATAL_FAILURE(Exec("MallocTests.DISABLED_malloc_and_backtrace_deadlock",
+                               "verbose verify_pointers", &pid, 0, 180));
+
+  // Make sure that malloc debug is enabled and that no timeouts occur during
+  // unwinds.
+  ASSERT_NO_FATAL_FAILURE(FindStrings(pid, std::vector<const char*>{"malloc debug enabled"},
+                                      std::vector<const char*>{"Timed out waiting for "}));
+}
diff --git a/libc/platform/bionic/reserved_signals.h b/libc/platform/bionic/reserved_signals.h
index c90fc0698..e8e517e20 100644
--- a/libc/platform/bionic/reserved_signals.h
+++ b/libc/platform/bionic/reserved_signals.h
@@ -48,6 +48,7 @@
 // in <android/legacy_signal_inlines.h> to match.
 
 #define BIONIC_SIGNAL_POSIX_TIMERS (__SIGRTMIN + 0)
+#define BIONIC_SIGNAL_BACKTRACE (__SIGRTMIN + 1)
 #define BIONIC_SIGNAL_DEBUGGER (__SIGRTMIN + 3)
 #define BIONIC_SIGNAL_PROFILER (__SIGRTMIN + 4)
 #define BIONIC_SIGNAL_ART_PROFILER (__SIGRTMIN + 6)