From 19e246dda6772ffc532b1762cd7870d6c3b01c12 Mon Sep 17 00:00:00 2001 From: Yabin Cui Date: Thu, 18 Dec 2014 14:22:09 -0800 Subject: Fix possible leak in pthread_detach. If pthread_detach() is called while the thread is in pthread_exit(), it takes the risk that no one can free the pthread_internal_t. So I add PTHREAD_ATTR_FLAG_ZOMBIE to detect this, maybe very rare, but both glibc and netbsd libpthread have similar function. Change-Id: Iaa15f651903b8ca07aaa7bd4de46ff14a2f93835 --- libc/bionic/pthread_detach.cpp | 33 ++++++++++++++++++--------------- libc/bionic/pthread_exit.cpp | 3 +++ libc/bionic/pthread_internal.h | 3 +++ 3 files changed, 24 insertions(+), 15 deletions(-) (limited to 'libc') diff --git a/libc/bionic/pthread_detach.cpp b/libc/bionic/pthread_detach.cpp index 715acf1..c800660 100644 --- a/libc/bionic/pthread_detach.cpp +++ b/libc/bionic/pthread_detach.cpp @@ -27,29 +27,32 @@ */ #include +#include #include "pthread_accessor.h" int pthread_detach(pthread_t t) { - pthread_accessor thread(t); - if (thread.get() == NULL) { + { + pthread_accessor thread(t); + if (thread.get() == NULL) { return ESRCH; - } + } - if (thread->attr.flags & PTHREAD_ATTR_FLAG_DETACHED) { - return EINVAL; // Already detached. - } + if (thread->attr.flags & PTHREAD_ATTR_FLAG_DETACHED) { + return EINVAL; // Already detached. + } - if (thread->attr.flags & PTHREAD_ATTR_FLAG_JOINED) { - return 0; // Already being joined; silently do nothing, like glibc. - } + if (thread->attr.flags & PTHREAD_ATTR_FLAG_JOINED) { + return 0; // Already being joined; silently do nothing, like glibc. + } - if (thread->tid == 0) { - // Already exited; clean up. - _pthread_internal_remove_locked(thread.get(), true); - return 0; + // If the thread has not exited, we can detach it safely. + if ((thread->attr.flags & PTHREAD_ATTR_FLAG_ZOMBIE) == 0) { + thread->attr.flags |= PTHREAD_ATTR_FLAG_DETACHED; + return 0; + } } - thread->attr.flags |= PTHREAD_ATTR_FLAG_DETACHED; - return 0; + // The thread is in zombie state, use pthread_join to clean it up. + return pthread_join(t, NULL); } diff --git a/libc/bionic/pthread_exit.cpp b/libc/bionic/pthread_exit.cpp index 9603a79..d0d64b0 100644 --- a/libc/bionic/pthread_exit.cpp +++ b/libc/bionic/pthread_exit.cpp @@ -99,6 +99,9 @@ void pthread_exit(void* return_value) { // pthread_internal_t is freed below with stack, not here. _pthread_internal_remove_locked(thread, false); free_mapped_space = true; + } else { + // Mark the thread as exiting without freeing pthread_internal_t. + thread->attr.flags |= PTHREAD_ATTR_FLAG_ZOMBIE; } pthread_mutex_unlock(&g_thread_list_lock); diff --git a/libc/bionic/pthread_internal.h b/libc/bionic/pthread_internal.h index 80002e9..8fbaf22 100644 --- a/libc/bionic/pthread_internal.h +++ b/libc/bionic/pthread_internal.h @@ -38,6 +38,9 @@ /* Has the thread been joined by another thread? */ #define PTHREAD_ATTR_FLAG_JOINED 0x00000002 +/* Did the thread exit without freeing pthread_internal_t? */ +#define PTHREAD_ATTR_FLAG_ZOMBIE 0x00000004 + /* Is this the main thread? */ #define PTHREAD_ATTR_FLAG_MAIN_THREAD 0x80000000 -- cgit v1.1