From 5b8ceff5f87889e781c13305767e140afd28eb76 Mon Sep 17 00:00:00 2001 From: Nicolas Geoffray Date: Wed, 26 Nov 2014 11:53:44 +0000 Subject: [PATCH] Revert "Use mmap to create the pthread_internal_t." Unfortunately, this change provokes random crashes for ART, and I have seen libc crashes on the device that might be related to it. Reverting it fixes the ART crashes. there is unfortunately no stack trace for the crashes, but just a "Segmentation fault" message. This reverts commit cc5f6543e3f91385b9a912438965b7e8265df54a. Change-Id: I68dca8e1e9b9edcce7eb84596e8db619e40e8052 --- libc/bionic/pthread_create.cpp | 7 ++++--- libc/bionic/pthread_exit.cpp | 10 ++++++---- libc/bionic/pthread_internal.h | 3 --- libc/bionic/pthread_internals.cpp | 25 +------------------------ 4 files changed, 11 insertions(+), 34 deletions(-) diff --git a/libc/bionic/pthread_create.cpp b/libc/bionic/pthread_create.cpp index 8bb1be9d9..fc8afa2ff 100644 --- a/libc/bionic/pthread_create.cpp +++ b/libc/bionic/pthread_create.cpp @@ -158,8 +158,9 @@ int pthread_create(pthread_t* thread_out, pthread_attr_t const* attr, // Inform the rest of the C library that at least one thread was created. __isthreaded = 1; - pthread_internal_t* thread = __create_thread_struct(); + pthread_internal_t* thread = reinterpret_cast(calloc(sizeof(*thread), 1)); if (thread == NULL) { + __libc_format_log(ANDROID_LOG_WARN, "libc", "pthread_create failed: couldn't allocate thread"); return EAGAIN; } @@ -178,7 +179,7 @@ int pthread_create(pthread_t* thread_out, pthread_attr_t const* attr, // The caller didn't provide a stack, so allocate one. thread->attr.stack_base = __create_thread_stack(thread); if (thread->attr.stack_base == NULL) { - __free_thread_struct(thread); + free(thread); return EAGAIN; } } else { @@ -229,7 +230,7 @@ int pthread_create(pthread_t* thread_out, pthread_attr_t const* attr, if (!thread->user_allocated_stack()) { munmap(thread->attr.stack_base, thread->attr.stack_size); } - __free_thread_struct(thread); + free(thread); __libc_format_log(ANDROID_LOG_WARN, "libc", "pthread_create failed: clone failed: %s", strerror(errno)); return clone_errno; } diff --git a/libc/bionic/pthread_exit.cpp b/libc/bionic/pthread_exit.cpp index 479088032..6cd5311ed 100644 --- a/libc/bionic/pthread_exit.cpp +++ b/libc/bionic/pthread_exit.cpp @@ -37,7 +37,6 @@ extern "C" __noreturn void _exit_with_stack_teardown(void*, size_t); extern "C" __noreturn void __exit(int); extern "C" int __set_tid_address(int*); -extern "C" void __set_tls(void*); /* CAVEAT: our implementation of pthread_cleanup_push/pop doesn't support C++ exceptions * and thread cancelation @@ -76,9 +75,6 @@ void pthread_exit(void* return_value) { // space (see pthread_key_delete). pthread_key_clean_all(); - // Clear tls to prevent further use of __get_tls(), see b/16847284. - __set_tls(NULL); - if (thread->alternate_signal_stack != NULL) { // Tell the kernel to stop using the alternate signal stack. stack_t ss; @@ -116,6 +112,12 @@ void pthread_exit(void* return_value) { } pthread_mutex_unlock(&g_thread_list_lock); + // Perform a second key cleanup. When using jemalloc, a call to free from + // _pthread_internal_remove_locked causes the memory associated with a key + // to be reallocated. + // TODO: When b/16847284 is fixed this call can be removed. + pthread_key_clean_all(); + if (user_allocated_stack) { // Cleaning up this thread's stack is the creator's responsibility, not ours. __exit(0); diff --git a/libc/bionic/pthread_internal.h b/libc/bionic/pthread_internal.h index 4308a9814..392e781e4 100644 --- a/libc/bionic/pthread_internal.h +++ b/libc/bionic/pthread_internal.h @@ -94,9 +94,6 @@ struct pthread_internal_t { char dlerror_buffer[__BIONIC_DLERROR_BUFFER_SIZE]; }; -__LIBC_HIDDEN__ pthread_internal_t* __create_thread_struct(); -__LIBC_HIDDEN__ void __free_thread_struct(pthread_internal_t*); - __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*); diff --git a/libc/bionic/pthread_internals.cpp b/libc/bionic/pthread_internals.cpp index 33cddd74e..2270d96f3 100644 --- a/libc/bionic/pthread_internals.cpp +++ b/libc/bionic/pthread_internals.cpp @@ -28,38 +28,15 @@ #include "pthread_internal.h" -#include #include -#include -#include #include "private/bionic_futex.h" #include "private/bionic_tls.h" -#include "private/libc_logging.h" #include "private/ScopedPthreadMutexLocker.h" pthread_internal_t* g_thread_list = NULL; pthread_mutex_t g_thread_list_lock = PTHREAD_MUTEX_INITIALIZER; -pthread_internal_t* __create_thread_struct() { - void* result = mmap(NULL, sizeof(pthread_internal_t), PROT_READ | PROT_WRITE, - MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE, -1, 0); - if (result == MAP_FAILED) { - __libc_format_log(ANDROID_LOG_WARN, "libc", - "__create_thread_struct() failed: %s", strerror(errno)); - return NULL; - } - return reinterpret_cast(result); -} - -void __free_thread_struct(pthread_internal_t* thread) { - int result = munmap(thread, sizeof(pthread_internal_t)); - if (result != 0) { - __libc_format_log(ANDROID_LOG_WARN, "libc", - "__free_thread_struct() failed: %s", strerror(errno)); - } -} - void _pthread_internal_remove_locked(pthread_internal_t* thread) { if (thread->next != NULL) { thread->next->prev = thread->prev; @@ -73,7 +50,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->attr.flags & PTHREAD_ATTR_FLAG_MAIN_THREAD) == 0) { - __free_thread_struct(thread); + free(thread); } } -- 2.11.0