Clean up pthread_internal_t.

Bug: 11755300
Change-Id: Ib509e8c5ec6b23513aa78b5ac5141d7c34ce2dc8

diff --git a/libc/bionic/libc_init_common.cpp b/libc/bionic/libc_init_common.cpp
index 130c287..1cfaf50 100644 (file)
--- a/libc/bionic/libc_init_common.cpp
+++ b/libc/bionic/libc_init_common.cpp
@@ -101,9 +101,9 @@ void __libc_init_tls(KernelArgumentBlock& args) {
   // environment variables with global scope live on it).
   pthread_attr_init(&main_thread.attr);
   pthread_attr_setstack(&main_thread.attr, (void*) stack_bottom, stack_size);
-  main_thread.attr.flags = PTHREAD_ATTR_FLAG_USER_ALLOCATED_STACK;
+  main_thread.attr.flags = PTHREAD_ATTR_FLAG_USER_ALLOCATED_STACK | PTHREAD_ATTR_FLAG_MAIN_THREAD;
 
-  _init_thread(&main_thread, false);
+  __init_thread(&main_thread, false);
   __init_tls(&main_thread);
   __set_tls(main_thread.tls);
   tls[TLS_SLOT_BIONIC_PREINIT] = &args;
@@ -124,7 +124,6 @@ void __libc_init_common(KernelArgumentBlock& args) {
 
   // Get the main thread from TLS and add it to the thread list.
   pthread_internal_t* main_thread = __get_thread();
-  main_thread->allocated_on_heap = false;
   _pthread_internal_add(main_thread);
 
   __system_properties_init(); // Requires 'environ'.

diff --git a/libc/bionic/pthread_create.cpp b/libc/bionic/pthread_create.cpp
index dde5ed7..19d19b0 100644 (file)
--- a/libc/bionic/pthread_create.cpp
+++ b/libc/bionic/pthread_create.cpp
@@ -80,7 +80,7 @@ void __init_alternate_signal_stack(pthread_internal_t* thread) {
   }
 }
 
-int _init_thread(pthread_internal_t* thread, bool add_to_thread_list) {
+int __init_thread(pthread_internal_t* thread, bool add_to_thread_list) {
   int error = 0;
 
   // Set the scheduling policy/priority of the thread.
@@ -148,16 +148,19 @@ static int __pthread_start(void* arg) {
 
   __init_alternate_signal_stack(thread);
 
-  if ((thread->internal_flags & PTHREAD_INTERNAL_FLAG_THREAD_INIT_FAILED) != 0) {
-    pthread_exit(NULL);
-  }
-
   void* result = thread->start_routine(thread->start_routine_arg);
   pthread_exit(result);
 
   return 0;
 }
 
+// A dummy start routine for pthread_create failures where we've created a thread but aren't
+// going to run user code on it. We swap out the user's start routine for this and take advantage
+// of the regular thread teardown to free up resources.
+static void* __do_nothing(void*) {
+  return NULL;
+}
+
 int pthread_create(pthread_t* thread_out, pthread_attr_t const* attr,
                    void* (*start_routine)(void*), void* arg) {
   ErrnoRestorer errno_restorer;
@@ -174,7 +177,6 @@ int pthread_create(pthread_t* thread_out, pthread_attr_t const* attr,
     __libc_format_log(ANDROID_LOG_WARN, "libc", "pthread_create failed: couldn't allocate thread");
     return EAGAIN;
   }
-  thread->allocated_on_heap = true;
 
   if (attr == NULL) {
     pthread_attr_init(&thread->attr);
@@ -238,12 +240,13 @@ int pthread_create(pthread_t* thread_out, pthread_attr_t const* attr,
     return clone_errno;
   }
 
-  int init_errno = _init_thread(thread, true);
+  int init_errno = __init_thread(thread, true);
   if (init_errno != 0) {
-    // Mark the thread detached and let its __pthread_start run to completion.
-    // It'll check this flag and exit immediately, cleaning up its resources.
-    thread->internal_flags |= PTHREAD_INTERNAL_FLAG_THREAD_INIT_FAILED;
+    // Mark the thread detached and replace its start_routine with a no-op.
+    // Letting the thread run is the easiest way to clean up its resources.
     thread->attr.flags |= PTHREAD_ATTR_FLAG_DETACHED;
+    thread->start_routine = __do_nothing;
+    pthread_mutex_unlock(start_mutex);
     return init_errno;
   }
 

diff --git a/libc/bionic/pthread_internal.h b/libc/bionic/pthread_internal.h
index bcd9b71..31ed07c 100644 (file)
--- a/libc/bionic/pthread_internal.h
+++ b/libc/bionic/pthread_internal.h
@@ -39,8 +39,6 @@ struct pthread_internal_t {
   void** tls;
 
   pthread_attr_t attr;
-  bool allocated_on_heap; /* TODO: move this into attr.flags? */
-  int internal_flags; /* TODO: move this into attr.flags? */
 
   __pthread_cleanup_t* cleanup_stack;
 
@@ -58,7 +56,7 @@ struct pthread_internal_t {
   char dlerror_buffer[__BIONIC_DLERROR_BUFFER_SIZE];
 };
 
-__LIBC_HIDDEN__ int _init_thread(pthread_internal_t* thread, bool add_to_thread_list);
+__LIBC_HIDDEN__ int __init_thread(pthread_internal_t* thread, bool add_to_thread_list);
 __LIBC_HIDDEN__ void __init_tls(pthread_internal_t* thread);
 __LIBC_HIDDEN__ void __init_alternate_signal_stack(pthread_internal_t*);
 __LIBC_HIDDEN__ void _pthread_internal_add(pthread_internal_t* thread);
@@ -76,7 +74,8 @@ __LIBC_HIDDEN__ void _pthread_internal_remove_locked(pthread_internal_t* thread)
 /* Has the thread been joined by another thread? */
 #define PTHREAD_ATTR_FLAG_JOINED 0x00000004
 
-#define PTHREAD_INTERNAL_FLAG_THREAD_INIT_FAILED 1
+/* Is this the main thread? */
+#define PTHREAD_ATTR_FLAG_MAIN_THREAD 0x80000000
 
 /*
  * Traditionally we give threads a 1MiB stack. When we started

diff --git a/libc/bionic/pthread_internals.cpp b/libc/bionic/pthread_internals.cpp
index f8afeaf..09c48dc 100644 (file)
--- a/libc/bionic/pthread_internals.cpp
+++ b/libc/bionic/pthread_internals.cpp
@@ -48,7 +48,7 @@ void _pthread_internal_remove_locked(pthread_internal_t* thread) {
 
   // The main thread is not heap-allocated. See __libc_init_tls for the declaration,
   // and __libc_init_common for the point where it's added to the thread list.
-  if (thread->allocated_on_heap) {
+  if ((thread->attr.flags & PTHREAD_ATTR_FLAG_MAIN_THREAD) == 0) {
     free(thread);
   }
 }