diff --git a/libc/bionic/pthread.c b/libc/bionic/pthread.c index c7612f75a..791a772b0 100644 --- a/libc/bionic/pthread.c +++ b/libc/bionic/pthread.c @@ -582,7 +582,7 @@ void pthread_exit(void * retval) pthread_key_clean_all(); // if the thread is detached, destroy the pthread_internal_t - // otherwise, keep it in memory and signal any joiners + // otherwise, keep it in memory and signal any joiners. if (thread->attr.flags & PTHREAD_ATTR_FLAG_DETACHED) { _pthread_internal_remove(thread); _pthread_internal_free(thread); @@ -668,8 +668,9 @@ FoundIt: pthread_cond_wait( &thread->join_cond, &gThreadListLock ); count = --thread->join_count; } - if (ret_val) + if (ret_val) { *ret_val = thread->return_value; + } /* remove thread descriptor when we're the last joiner or when the * thread was already a zombie. @@ -686,28 +687,30 @@ int pthread_detach( pthread_t thid ) { pthread_internal_t* thread; int result = 0; - int flags; pthread_mutex_lock(&gThreadListLock); - for (thread = gThreadList; thread != NULL; thread = thread->next) - if (thread == (pthread_internal_t*)thid) + for (thread = gThreadList; thread != NULL; thread = thread->next) { + if (thread == (pthread_internal_t*)thid) { goto FoundIt; + } + } result = ESRCH; goto Exit; FoundIt: - do { - flags = thread->attr.flags; - - if ( flags & PTHREAD_ATTR_FLAG_DETACHED ) { - /* thread is not joinable ! */ - result = EINVAL; - goto Exit; - } + if (thread->attr.flags & PTHREAD_ATTR_FLAG_DETACHED) { + result = EINVAL; // Already detached. + goto Exit; } - while ( __bionic_cmpxchg( flags, flags | PTHREAD_ATTR_FLAG_DETACHED, - (volatile int*)&thread->attr.flags ) != 0 ); + + if (thread->join_count > 0) { + result = 0; // Already being joined; silently do nothing, like glibc. + goto Exit; + } + + thread->attr.flags |= PTHREAD_ATTR_FLAG_DETACHED; + Exit: pthread_mutex_unlock(&gThreadListLock); return result; diff --git a/tests/pthread_test.cpp b/tests/pthread_test.cpp index 479c4622f..e400b84ab 100644 --- a/tests/pthread_test.cpp +++ b/tests/pthread_test.cpp @@ -37,10 +37,26 @@ static void* SleepFn(void* arg) { return NULL; } +static void* SpinFn(void* arg) { + volatile bool* b = reinterpret_cast(arg); + while (!*b) { + } + return NULL; +} + static void* JoinFn(void* arg) { return reinterpret_cast(pthread_join(reinterpret_cast(arg), NULL)); } +static void AssertDetached(pthread_t t, bool is_detached) { + pthread_attr_t attr; + ASSERT_EQ(0, pthread_getattr_np(t, &attr)); + int detach_state; + ASSERT_EQ(0, pthread_attr_getdetachstate(&attr, &detach_state)); + pthread_attr_destroy(&attr); + ASSERT_EQ(is_detached, (detach_state == PTHREAD_CREATE_DETACHED)); +} + TEST(pthread, pthread_create) { void* expected_result = reinterpret_cast(123); // Can we create a thread? @@ -58,6 +74,7 @@ TEST(pthread, pthread_no_join_after_detach) { // After a pthread_detach... ASSERT_EQ(0, pthread_detach(t1)); + AssertDetached(t1, true); // ...pthread_join should fail. void* result; @@ -65,20 +82,27 @@ TEST(pthread, pthread_no_join_after_detach) { } TEST(pthread, pthread_no_op_detach_after_join) { + bool done = false; + pthread_t t1; - ASSERT_EQ(0, pthread_create(&t1, NULL, SleepFn, reinterpret_cast(1))); + ASSERT_EQ(0, pthread_create(&t1, NULL, SpinFn, &done)); // If thread 2 is already waiting to join thread 1... pthread_t t2; ASSERT_EQ(0, pthread_create(&t2, NULL, JoinFn, reinterpret_cast(t1))); - // ...a call to pthread_detach on thread 1 will "succeed"... - ASSERT_EQ(0, pthread_detach(t1)); + sleep(1); // (Give t2 a chance to call pthread_join.) - // ...but the join still goes ahead. + // ...a call to pthread_detach on thread 1 will "succeed" (silently fail)... + ASSERT_EQ(0, pthread_detach(t1)); + AssertDetached(t1, false); + + done = true; + + // ...but t2's join on t1 still goes ahead (which we can tell because our join on t2 finishes). void* join_result; ASSERT_EQ(0, pthread_join(t2, &join_result)); - ASSERT_EQ(EINVAL, reinterpret_cast(join_result)); + ASSERT_EQ(0, reinterpret_cast(join_result)); } TEST(pthread, pthread_join_self) {