A deadlock was fixed that was caused by a bug in the memory allocation debugging library.
Summary: The process would deadlock when trying to obtain backtraces of unreachable memory by running dumpsys -t 60 meminfo -a --unreachable process_name to dump memory information with malloc debug being enabled.
Test: Successful Build on master branch
Change-Id: Id9aa64f54d95543f79732a17386620971f893cf9
Signed-off-by: Abdelrahman Daim <adaim@meta.com>
diff --git a/libc/malloc_debug/malloc_debug.cpp b/libc/malloc_debug/malloc_debug.cpp
index c183897..a662529 100644
--- a/libc/malloc_debug/malloc_debug.cpp
+++ b/libc/malloc_debug/malloc_debug.cpp
@@ -1091,18 +1091,20 @@
void debug_malloc_disable() {
ScopedConcurrentLock lock;
- g_dispatch->malloc_disable();
if (g_debug->pointer) {
+ // Acquire the pointer locks first, otherwise, the code can be holding
+ // the allocation lock and deadlock trying to acquire a pointer lock.
g_debug->pointer->PrepareFork();
}
+ g_dispatch->malloc_disable();
}
void debug_malloc_enable() {
ScopedConcurrentLock lock;
+ g_dispatch->malloc_enable();
if (g_debug->pointer) {
g_debug->pointer->PostForkParent();
}
- g_dispatch->malloc_enable();
}
ssize_t debug_malloc_backtrace(void* pointer, uintptr_t* frames, size_t max_frames) {
diff --git a/libc/malloc_debug/tests/malloc_debug_system_tests.cpp b/libc/malloc_debug/tests/malloc_debug_system_tests.cpp
index d7a7a4f..080242c 100644
--- a/libc/malloc_debug/tests/malloc_debug_system_tests.cpp
+++ b/libc/malloc_debug/tests/malloc_debug_system_tests.cpp
@@ -57,6 +57,12 @@
#include <bionic/malloc.h>
#include <tests/utils.h>
+// exported from bionic
+__BEGIN_DECLS
+extern void malloc_disable();
+extern void malloc_enable();
+__END_DECLS
+
// 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.
@@ -788,3 +794,34 @@
unexpected_log_strings_.push_back("Timed out waiting for ");
Exec("MallocTests.DISABLED_malloc_and_backtrace_deadlock", "verbose verify_pointers", 0);
}
+
+// Creates two threads: one that calls malloc_disable() and malloc_enable() in a loop and
+// the other that performs a bunch of allocations.
+TEST(MallocTests, DISABLED_malloc_disable_deadlock) {
+ std::atomic_bool running(true);
+
+ std::thread t1([&] {
+ while (running) {
+ malloc_disable();
+ malloc_enable();
+ }
+ });
+
+ std::thread t2([&] {
+ while (running) {
+ void* p = malloc(100);
+ free(p);
+ }
+ });
+
+ // let the threads run for a while, then tell them to stop and wait for shutdown
+ std::this_thread::sleep_for(std::chrono::seconds(5));
+
+ running = false;
+ t1.join();
+ t2.join();
+}
+
+TEST_F(MallocDebugSystemTest, malloc_disable_deadlock) {
+ Exec("MallocTests.DISABLED_malloc_disable_deadlock", "verbose backtrace");
+}