commit 10ce96944eaea4c459392952652fdb24742c9c29
author Sergey Melnikov <sergey.melnikov@intel.com> Fri Oct 26 14:06:43 2012 +0400
committer Elliott Hughes <enh@google.com> Mon Oct 29 11:31:26 2012 -0700
tree 2691b3629da1399e527f9719bdebea6d14837cbb
parent 4d36b0bd38559ae0376b61a0ac2f5fa8329ec56d

Fix bug in pthread_join, pthread_exit, pthread_detach

pthread_no_op_detach_after_join test from bionic-unit-tests hangs
on x86 emulator. There is a race in the pthread_join, pthread_exit,
pthread_detach functions:
- pthread_join waits for the non-detached thread
- pthread_detach sets the detached flag on that thread
- the thread executes pthread_exit which just kills the now-detached
thread, without sending the join notification.

This patch improves the test so it fails on ARM too, and modifies
pthread_detach to behave more like glibc, not setting the detach state if
called on a thread that's already being joined (but not returning an error).

Change-Id: I87dc688221ce979ef5178753dd63d01ac0b108e6
Signed-off-by: Sergey Melnikov <sergey.melnikov@intel.com>

libc/bionic/pthread.c

diff --git a/libc/bionic/pthread.c b/libc/bionic/pthread.c
index c7612f7..791a772 100644
--- a/libc/bionic/pthread.c
+++ b/libc/bionic/pthread.c
@@ -582,7 +582,7 @@
     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 @@
         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 @@
 {
     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;