From cef3faec0ea40fdfe58e425fd0be64f00de6a26d Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Tue, 19 Nov 2013 16:52:24 -0800 Subject: [PATCH] Clean up pthread_internal_t. Bug: 11755300 Change-Id: Ib509e8c5ec6b23513aa78b5ac5141d7c34ce2dc8 --- libc/bionic/libc_init_common.cpp | 5 ++--- libc/bionic/pthread_create.cpp | 23 +++++++++++++---------- libc/bionic/pthread_internal.h | 7 +++---- libc/bionic/pthread_internals.cpp | 2 +- 4 files changed, 19 insertions(+), 18 deletions(-) diff --git a/libc/bionic/libc_init_common.cpp b/libc/bionic/libc_init_common.cpp index 130c2875c..1cfaf507a 100644 --- 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 dde5ed760..19d19b04b 100644 --- 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 bcd9b7104..31ed07c21 100644 --- 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 f8afeaf47..09c48dcd3 100644 --- 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); } }