Merge "Fix race when frees after main thread finishes." am: 48e981556e am: 80232a7145
Original change: https://android-review.googlesource.com/c/platform/bionic/+/1755319
Change-Id: I14f71e5cc388cb8c6aa257e0f8cee198d3e1d869
diff --git a/libc/malloc_debug/malloc_debug.cpp b/libc/malloc_debug/malloc_debug.cpp
index 609f030..d23ab15 100644
--- a/libc/malloc_debug/malloc_debug.cpp
+++ b/libc/malloc_debug/malloc_debug.cpp
@@ -362,10 +362,9 @@
backtrace_shutdown();
- delete g_debug;
- g_debug = nullptr;
-
- DebugDisableFinalize();
+ // In order to prevent any issues of threads freeing previous pointers
+ // after the main thread calls this code, simply leak the g_debug pointer
+ // and do not destroy the debug disable pthread key.
}
void debug_get_malloc_leak_info(uint8_t** info, size_t* overall_size, size_t* info_size,
diff --git a/libc/malloc_debug/tests/malloc_debug_system_tests.cpp b/libc/malloc_debug/tests/malloc_debug_system_tests.cpp
index 1bfe61e..68b3a1e 100644
--- a/libc/malloc_debug/tests/malloc_debug_system_tests.cpp
+++ b/libc/malloc_debug/tests/malloc_debug_system_tests.cpp
@@ -30,6 +30,7 @@
#include <fcntl.h>
#include <poll.h>
#include <signal.h>
+#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/types.h>
@@ -449,12 +450,12 @@
}
static constexpr int kExpectedExitCode = 30;
+static constexpr size_t kMaxThreads = sizeof(uint32_t) * 8;
TEST(MallocTests, DISABLED_exit_while_threads_allocating) {
- std::atomic_uint32_t thread_mask;
- thread_mask = 0;
+ std::atomic_uint32_t thread_mask = {};
- for (size_t i = 0; i < 32; i++) {
+ for (size_t i = 0; i < kMaxThreads; i++) {
std::thread malloc_thread([&thread_mask, i] {
while (true) {
void* ptr = malloc(100);
@@ -469,7 +470,7 @@
}
// Wait until each thread has done at least one allocation.
- while (thread_mask.load() != 0xffffffff)
+ while (thread_mask.load() != UINT32_MAX)
;
exit(kExpectedExitCode);
}
@@ -492,6 +493,59 @@
}
}
+TEST(MallocTests, DISABLED_exit_while_threads_freeing_allocs_with_header) {
+ static constexpr size_t kMaxAllocsPerThread = 1000;
+ std::atomic_uint32_t thread_mask = {};
+ std::atomic_bool run;
+
+ std::vector<std::vector<void*>> allocs(kMaxThreads);
+ // Pre-allocate a bunch of memory so that we can try to trigger
+ // the frees after the main thread finishes.
+ for (size_t i = 0; i < kMaxThreads; i++) {
+ for (size_t j = 0; j < kMaxAllocsPerThread; j++) {
+ void* ptr = malloc(8);
+ ASSERT_TRUE(ptr != nullptr);
+ allocs[i].push_back(ptr);
+ }
+ }
+
+ for (size_t i = 0; i < kMaxThreads; i++) {
+ std::thread malloc_thread([&thread_mask, &run, &allocs, i] {
+ thread_mask.fetch_or(1 << i);
+ while (!run)
+ ;
+ for (auto ptr : allocs[i]) {
+ free(ptr);
+ }
+ });
+ malloc_thread.detach();
+ }
+
+ // Wait until all threads are running.
+ while (thread_mask.load() != UINT32_MAX)
+ ;
+ run = true;
+ exit(kExpectedExitCode);
+}
+
+TEST(MallocDebugSystemTest, exit_while_threads_freeing_allocs_with_header) {
+ for (size_t i = 0; i < 50; i++) {
+ SCOPED_TRACE(::testing::Message() << "Run " << i);
+ pid_t pid;
+ // Enable guard to force the use of a header.
+ ASSERT_NO_FATAL_FAILURE(
+ Exec("MallocTests.DISABLED_exit_while_threads_freeing_allocs_with_header", "verbose guard",
+ &pid, kExpectedExitCode));
+
+ ASSERT_NO_FATAL_FAILURE(FindStrings(pid, std::vector<const char*>{"malloc debug enabled"}));
+
+ std::string log_str;
+ GetLogStr(pid, &log_str, LOG_ID_CRASH);
+ ASSERT_TRUE(log_str.find("Fatal signal") == std::string::npos)
+ << "Found crash in log.\nLog message: " << log_str;
+ }
+}
+
TEST(MallocTests, DISABLED_write_leak_info) {
TemporaryFile tf;
ASSERT_TRUE(tf.fd != -1);
@@ -555,8 +609,11 @@
static constexpr size_t kNumUnwinds = 1000;
for (size_t i = 0; i < kNumUnwinds; i++) {
std::unique_ptr<Backtrace> backtrace(Backtrace::Create(getpid(), tid));
+ // Only verify that there is at least one frame in the unwind.
+ // This is not a test of the unwinder and clang for arm seems to
+ // produces an increasing number of code that does not have unwind
+ // information.
ASSERT_TRUE(backtrace->Unwind(0)) << "Failed on unwind " << i;
- ASSERT_LT(1, backtrace->NumFrames()) << "Failed on unwind " << i;
}
running = false;
thread.join();