Compare commits

..

2 Commits

Author SHA1 Message Date
Daniel Xie
2010fb6722 Merge "Fix potential race condition on CTS TC pthread_gettid_np" into marshmallow-cts-dev 2015-11-11 18:53:16 +00:00
Junjie Hu
4f80102935 Fix potential race condition on CTS TC pthread_gettid_np
Root cause:
If start_routine thread exits before pthread_gettid_np is invokded, the "tid" field
will be cleared so that pthread_gettid_np will get "0" (which is cleared by kernel, 
due to the flag "CLONE_CHILD_CLEARTID" is set while calling clone system call inside
pthread_create).

Proposed patch:
Use a mutex to guarantee pthread_gettid_np will be invoked and returned before the
start_routine exits

Signed-off-by: Junjie Hu <junjie.hu@mediatek.com>

Change-Id: I22411f1b0f7446d76a0373cef4ccec858fac7018
2015-11-11 04:54:52 +00:00
3 changed files with 9 additions and 31 deletions

View File

@@ -130,13 +130,8 @@ __LIBC_HIDDEN__ void pthread_key_clean_all(void);
*/ */
#define PTHREAD_STACK_SIZE_DEFAULT ((1 * 1024 * 1024) - SIGSTKSZ) #define PTHREAD_STACK_SIZE_DEFAULT ((1 * 1024 * 1024) - SIGSTKSZ)
// Leave room for a guard page in the internally created signal stacks. /* Leave room for a guard page in the internally created signal stacks. */
#if defined(__LP64__)
// SIGSTKSZ is not big enough for 64-bit arch. See http://b/23041777.
#define SIGNAL_STACK_SIZE (16 * 1024 + PAGE_SIZE)
#else
#define SIGNAL_STACK_SIZE (SIGSTKSZ + PAGE_SIZE) #define SIGNAL_STACK_SIZE (SIGSTKSZ + PAGE_SIZE)
#endif
/* Needed by fork. */ /* Needed by fork. */
__LIBC_HIDDEN__ extern void __bionic_atfork_run_prepare(); __LIBC_HIDDEN__ extern void __bionic_atfork_run_prepare();

31
tests/pthread_test.cpp Normal file → Executable file
View File

@@ -27,7 +27,6 @@
#include <sys/syscall.h> #include <sys/syscall.h>
#include <time.h> #include <time.h>
#include <unistd.h> #include <unistd.h>
#include <unwind.h>
#include <atomic> #include <atomic>
#include <regex> #include <regex>
@@ -1245,8 +1244,11 @@ TEST(pthread, pthread_attr_getstack_18908062) {
} }
#if defined(__BIONIC__) #if defined(__BIONIC__)
static pthread_mutex_t gettid_mutex;
static void* pthread_gettid_np_helper(void* arg) { static void* pthread_gettid_np_helper(void* arg) {
pthread_mutex_lock(&gettid_mutex);
*reinterpret_cast<pid_t*>(arg) = gettid(); *reinterpret_cast<pid_t*>(arg) = gettid();
pthread_mutex_unlock(&gettid_mutex);
return NULL; return NULL;
} }
#endif #endif
@@ -1257,11 +1259,15 @@ TEST(pthread, pthread_gettid_np) {
pid_t t_gettid_result; pid_t t_gettid_result;
pthread_t t; pthread_t t;
pthread_mutex_init(&gettid_mutex, NULL);
pthread_mutex_lock(&gettid_mutex);
pthread_create(&t, NULL, pthread_gettid_np_helper, &t_gettid_result); pthread_create(&t, NULL, pthread_gettid_np_helper, &t_gettid_result);
pid_t t_pthread_gettid_np_result = pthread_gettid_np(t); pid_t t_pthread_gettid_np_result = pthread_gettid_np(t);
pthread_mutex_unlock(&gettid_mutex);
pthread_join(t, NULL); pthread_join(t, NULL);
pthread_mutex_destroy(&gettid_mutex);
ASSERT_EQ(t_gettid_result, t_pthread_gettid_np_result); ASSERT_EQ(t_gettid_result, t_pthread_gettid_np_result);
#else #else
@@ -1572,26 +1578,3 @@ TEST_F(pthread_DeathTest, pthread_mutex_unlock_null_64) {
GTEST_LOG_(INFO) << "This test tests bionic implementation details on 64 bit devices."; GTEST_LOG_(INFO) << "This test tests bionic implementation details on 64 bit devices.";
#endif #endif
} }
extern _Unwind_Reason_Code FrameCounter(_Unwind_Context* ctx, void* arg);
static volatile bool signal_handler_on_altstack_done;
static void SignalHandlerOnAltStack(int signo, siginfo_t*, void*) {
ASSERT_EQ(SIGUSR1, signo);
// Check if we have enough stack space for unwinding.
int count = 0;
_Unwind_Backtrace(FrameCounter, &count);
ASSERT_GT(count, 0);
// Check if we have enough stack space for logging.
std::string s(2048, '*');
GTEST_LOG_(INFO) << s;
signal_handler_on_altstack_done = true;
}
TEST(pthread, big_enough_signal_stack_for_64bit_arch) {
signal_handler_on_altstack_done = false;
ScopedSignalHandler handler(SIGUSR1, SignalHandlerOnAltStack, SA_SIGINFO | SA_ONSTACK);
kill(getpid(), SIGUSR1);
ASSERT_TRUE(signal_handler_on_altstack_done);
}

View File

@@ -34,7 +34,7 @@
#define noinline __attribute__((__noinline__)) #define noinline __attribute__((__noinline__))
#define __unused __attribute__((__unused__)) #define __unused __attribute__((__unused__))
_Unwind_Reason_Code FrameCounter(_Unwind_Context* ctx __unused, void* arg) { static _Unwind_Reason_Code FrameCounter(_Unwind_Context* ctx __unused, void* arg) {
int* count_ptr = reinterpret_cast<int*>(arg); int* count_ptr = reinterpret_cast<int*>(arg);
#if SHOW_FRAME_LOCATIONS #if SHOW_FRAME_LOCATIONS