Don't corrupt the thread list in static executables.
Several previous changes conspired to make a mess of the thread list
in static binaries. This was most obvious when trying to call
pthread_key_delete(3) on the main thread.
Bug: http://code.google.com/p/android/issues/detail?id=36893
Change-Id: I2a2f553114d8fb40533c481252b410c10656da2e
diff --git a/libc/bionic/libc_init_common.c b/libc/bionic/libc_init_common.c
index 4ce4db6..6508c0b 100644
--- a/libc/bionic/libc_init_common.c
+++ b/libc/bionic/libc_init_common.c
@@ -76,7 +76,7 @@
pthread_attr_init(&thread_attr);
pthread_attr_setstack(&thread_attr, (void*)stackbottom, stacksize);
- _init_thread(&thread, gettid(), &thread_attr, (void*)stackbottom);
+ _init_thread(&thread, gettid(), &thread_attr, (void*)stackbottom, false);
__init_tls(tls_area, &thread);
tls_area[TLS_SLOT_BIONIC_PREINIT] = elfdata;
diff --git a/libc/bionic/libc_init_static.c b/libc/bionic/libc_init_static.c
index 97156e8..a73bb71 100644
--- a/libc/bionic/libc_init_static.c
+++ b/libc/bionic/libc_init_static.c
@@ -67,9 +67,6 @@
__libc_init_tls(NULL);
- /* get the initial thread from TLS and add it to gThreadList */
- _pthread_internal_add(__get_thread());
-
/* Initialize the C runtime environment */
__libc_init_common(elfdata);
diff --git a/libc/bionic/pthread.c b/libc/bionic/pthread.c
index 40a09ba..14d2fec 100644
--- a/libc/bionic/pthread.c
+++ b/libc/bionic/pthread.c
@@ -117,8 +117,8 @@
static void
_pthread_internal_remove_locked( pthread_internal_t* thread )
{
- thread->next->pref = thread->pref;
- thread->pref[0] = thread->next;
+ thread->next->prev = thread->prev;
+ thread->prev[0] = thread->next;
}
static void
@@ -130,14 +130,17 @@
}
__LIBC_ABI_PRIVATE__ void
-_pthread_internal_add( pthread_internal_t* thread )
+_pthread_internal_add(pthread_internal_t* thread)
{
pthread_mutex_lock(&gThreadListLock);
- thread->pref = &gThreadList;
- thread->next = thread->pref[0];
- if (thread->next)
- thread->next->pref = &thread->next;
- thread->pref[0] = thread;
+
+ thread->prev = &gThreadList;
+ thread->next = *(thread->prev);
+ if (thread->next != NULL) {
+ thread->next->prev = &thread->next;
+ }
+ *(thread->prev) = thread;
+
pthread_mutex_unlock(&gThreadListLock);
}
@@ -203,7 +206,8 @@
}
__LIBC_ABI_PRIVATE__
-int _init_thread(pthread_internal_t * thread, pid_t kernel_id, pthread_attr_t * attr, void * stack_base)
+int _init_thread(pthread_internal_t* thread, pid_t kernel_id, pthread_attr_t* attr,
+ void* stack_base, bool add_to_thread_list)
{
int error = 0;
@@ -231,10 +235,12 @@
pthread_cond_init(&thread->join_cond, NULL);
thread->join_count = 0;
-
thread->cleanup_stack = NULL;
- _pthread_internal_add(thread);
+ if (add_to_thread_list) {
+ _pthread_internal_add(thread);
+ }
+
return error;
}
@@ -348,7 +354,7 @@
return clone_errno;
}
- int init_errno = _init_thread(thread, tid, (pthread_attr_t*) attr, stack);
+ int init_errno = _init_thread(thread, tid, (pthread_attr_t*) attr, stack, true);
if (init_errno != 0) {
// Mark the thread detached and let its __thread_entry run to
// completion. (It'll just exit immediately, cleaning up its resources.)
@@ -1919,8 +1925,8 @@
/* This deletes a pthread_key_t. note that the standard mandates that this does
* not call the destructor of non-NULL key values. Instead, it is the
- * responsability of the caller to properly dispose of the corresponding data
- * and resources, using any mean it finds suitable.
+ * responsibility of the caller to properly dispose of the corresponding data
+ * and resources, using any means it finds suitable.
*
* On the other hand, this function will clear the corresponding key data
* values in all known threads. this prevents later (invalid) calls to
@@ -2184,7 +2190,7 @@
for (;;) {
/* Try to atomically set the INITIALIZING flag.
* This requires a cmpxchg loop, and we may need
- * to exit prematurely if we detect that
+ * to exit prematurely if we detect that
* COMPLETED is now set.
*/
int32_t oldval, newval;
diff --git a/libc/bionic/pthread_internal.h b/libc/bionic/pthread_internal.h
index 2bd110c..58a809a 100644
--- a/libc/bionic/pthread_internal.h
+++ b/libc/bionic/pthread_internal.h
@@ -29,13 +29,14 @@
#define _PTHREAD_INTERNAL_H_
#include <pthread.h>
+#include <stdbool.h>
__BEGIN_DECLS
typedef struct pthread_internal_t
{
struct pthread_internal_t* next;
- struct pthread_internal_t** pref;
+ struct pthread_internal_t** prev;
pthread_attr_t attr;
pid_t kernel_id;
pthread_cond_t join_cond;
@@ -46,7 +47,8 @@
void** tls; /* thread-local storage area */
} pthread_internal_t;
-extern int _init_thread(pthread_internal_t * thread, pid_t kernel_id, pthread_attr_t * attr, void * stack_base);
+int _init_thread(pthread_internal_t* thread, pid_t kernel_id, pthread_attr_t* attr,
+ void* stack_base, bool add_to_thread_list);
void _pthread_internal_add( pthread_internal_t* thread );
pthread_internal_t* __get_thread(void);
@@ -100,9 +102,9 @@
return 0;
}
-extern int __pthread_cond_timedwait(pthread_cond_t*,
+extern int __pthread_cond_timedwait(pthread_cond_t*,
pthread_mutex_t*,
- const struct timespec*,
+ const struct timespec*,
clockid_t);
extern int __pthread_cond_timedwait_relative(pthread_cond_t*,
diff --git a/tests/Android.mk b/tests/Android.mk
index 1219afd..ae57d9b 100644
--- a/tests/Android.mk
+++ b/tests/Android.mk
@@ -20,9 +20,10 @@
test_src_files = \
getcwd_test.cpp \
+ pthread_test.cpp \
regex_test.cpp \
-# Build for the device (with bionic). Run with:
+# Build for the device (with bionic's .so). Run with:
# adb shell /data/nativetest/bionic-unit-tests/bionic-unit-tests
include $(CLEAR_VARS)
LOCAL_MODULE := bionic-unit-tests
@@ -30,6 +31,16 @@
LOCAL_SRC_FILES := $(test_src_files)
include $(BUILD_NATIVE_TEST)
+# Build for the device (with bionic's .a). Run with:
+# adb shell /data/nativetest/bionic-unit-tests-static/bionic-unit-tests-static
+include $(CLEAR_VARS)
+LOCAL_FORCE_STATIC_EXECUTABLE := true
+LOCAL_STATIC_LIBRARIES += libstlport_static libstdc++ libm libc
+LOCAL_MODULE := bionic-unit-tests-static
+LOCAL_ADDITIONAL_DEPENDENCIES := $(LOCAL_PATH)/Android.mk
+LOCAL_SRC_FILES := $(test_src_files)
+include $(BUILD_NATIVE_TEST)
+
# Build for the host (with glibc).
# Note that this will build against glibc, so it's not useful for testing
# bionic's implementation, but it does let you use glibc as a reference
@@ -37,6 +48,7 @@
include $(CLEAR_VARS)
LOCAL_MODULE := bionic-unit-tests-glibc
LOCAL_ADDITIONAL_DEPENDENCIES := $(LOCAL_PATH)/Android.mk
+LOCAL_LDFLAGS += -lpthread
LOCAL_SRC_FILES := $(test_src_files)
include $(BUILD_HOST_NATIVE_TEST)
diff --git a/tests/pthread_test.cpp b/tests/pthread_test.cpp
new file mode 100644
index 0000000..04975df
--- /dev/null
+++ b/tests/pthread_test.cpp
@@ -0,0 +1,28 @@
+/*
+ * Copyright (C) 2012 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <gtest/gtest.h>
+
+#include <errno.h>
+#include <pthread.h>
+
+TEST(pthread, pthread_key_create) {
+ pthread_key_t key;
+ ASSERT_EQ(0, pthread_key_create(&key, NULL));
+ ASSERT_EQ(0, pthread_key_delete(key));
+ // Can't delete a key that's already been deleted.
+ ASSERT_EQ(EINVAL, pthread_key_delete(key));
+}