Fix pthread_detach for already-exited threads.

Change-Id: I2bf7f41234d93b226132a4c51705f4186f4961c3
Reported-by: Paresh Nakhe <pnakhe@codeaurora.org>

diff --git a/libc/bionic/pthread_detach.cpp b/libc/bionic/pthread_detach.cpp
index 95f11ac..a8608e3 100644 (file)
--- a/libc/bionic/pthread_detach.cpp
+++ b/libc/bionic/pthread_detach.cpp
@@ -44,6 +44,12 @@ int pthread_detach(pthread_t t) {
     return 0; // Already being joined; silently do nothing, like glibc.
   }
 
+  if (thread->tid == 0) {
+    // Already exited; clean up.
+    _pthread_internal_remove_locked(thread.get());
+    return 0;
+  }
+
   thread->attr.flags |= PTHREAD_ATTR_FLAG_DETACHED;
   return 0;
 }

diff --git a/tests/pthread_test.cpp b/tests/pthread_test.cpp
index 1e85a54..9e7a9bc 100644 (file)
--- a/tests/pthread_test.cpp
+++ b/tests/pthread_test.cpp
@@ -19,6 +19,7 @@
 #include <errno.h>
 #include <inttypes.h>
 #include <limits.h>
+#include <malloc.h>
 #include <pthread.h>
 #include <signal.h>
 #include <sys/mman.h>
@@ -350,6 +351,35 @@ TEST(pthread, pthread_detach__no_such_thread) {
   ASSERT_EQ(ESRCH, pthread_detach(dead_thread));
 }
 
+TEST(pthread, pthread_detach__leak) {
+  size_t initial_bytes = mallinfo().uordblks;
+
+  pthread_attr_t attr;
+  ASSERT_EQ(0, pthread_attr_init(&attr));
+  ASSERT_EQ(0, pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_JOINABLE));
+
+  std::vector<pthread_t> threads;
+  for (size_t i = 0; i < 32; ++i) {
+    pthread_t t;
+    ASSERT_EQ(0, pthread_create(&t, &attr, IdFn, NULL));
+    threads.push_back(t);
+  }
+
+  sleep(1);
+
+  for (size_t i = 0; i < 32; ++i) {
+    ASSERT_EQ(0, pthread_detach(threads[i])) << i;
+  }
+
+  size_t final_bytes = mallinfo().uordblks;
+
+  int leaked_bytes = (final_bytes - initial_bytes);
+
+  // User code (like this test) doesn't know how large pthread_internal_t is.
+  // We can be pretty sure it's more than 128 bytes.
+  ASSERT_LT(leaked_bytes, 32 /*threads*/ * 128 /*bytes*/);
+}
+
 TEST(pthread, pthread_getcpuclockid__clock_gettime) {
   pthread_t t;
   ASSERT_EQ(0, pthread_create(&t, NULL, SleepFn, reinterpret_cast<void*>(5)));