commit 673b15e4ee2c6d99b150aedddc0f389e29f98e1b
author Yabin Cui <yabinc@google.com> Thu Mar 19 14:19:19 2015 -0700
committer Yabin Cui <yabinc@google.com> Mon Mar 23 19:03:49 2015 -0700
tree edc101b54c689960bdf1262e6378b21082d3e277
parent 4149dc944bbe3c5f1c14fc998106975e68bdd6bf

Let g_thread_list_lock only protect g_thread_list.

As glibc/netbsd don't protect access to thread struct members by a global
lock, we don't want to do it either. This change reduces the
responsibility of g_thread_list_lock to only protect g_thread_list.

Bug: 19636317
Change-Id: I897890710653dac165d8fa4452c7ecf74abdbf2b

libc/bionic/pthread_join.cpp

diff --git a/libc/bionic/pthread_join.cpp b/libc/bionic/pthread_join.cpp
index 15543b4..4d852cb 100644
--- a/libc/bionic/pthread_join.cpp
+++ b/libc/bionic/pthread_join.cpp
@@ -29,35 +29,31 @@
 #include <errno.h>
 
 #include "private/bionic_futex.h"
-#include "pthread_accessor.h"
+#include "pthread_internal.h"
 
 int pthread_join(pthread_t t, void** return_value) {
   if (t == pthread_self()) {
     return EDEADLK;
   }
 
-  pid_t tid;
-  volatile int* tid_ptr;
-  {
-    pthread_accessor thread(t);
-    if (thread.get() == NULL) {
-      return ESRCH;
-    }
-
-    ThreadJoinState old_state = THREAD_NOT_JOINED;
-    while ((old_state == THREAD_NOT_JOINED || old_state == THREAD_EXITED_NOT_JOINED) &&
-           !atomic_compare_exchange_weak(&thread->join_state, &old_state, THREAD_JOINED)) {
-    }
-
-    if (old_state == THREAD_DETACHED || old_state == THREAD_JOINED) {
-      return EINVAL;
-    }
-
-    tid = thread->tid;
-    tid_ptr = &thread->tid;
+  pthread_internal_t* thread = __pthread_internal_find(t);
+  if (thread == NULL) {
+    return ESRCH;
   }
 
-  // We set the PTHREAD_ATTR_FLAG_JOINED flag with the lock held,
+  ThreadJoinState old_state = THREAD_NOT_JOINED;
+  while ((old_state == THREAD_NOT_JOINED || old_state == THREAD_EXITED_NOT_JOINED) &&
+         !atomic_compare_exchange_weak(&thread->join_state, &old_state, THREAD_JOINED)) {
+  }
+
+  if (old_state == THREAD_DETACHED || old_state == THREAD_JOINED) {
+    return EINVAL;
+  }
+
+  pid_t tid = thread->tid;
+  volatile int* tid_ptr = &thread->tid;
+
+  // We set thread->join_state to THREAD_JOINED with atomic operation,
   // so no one is going to remove this thread except us.
 
   // Wait for the thread to actually exit, if it hasn't already.
@@ -65,14 +61,10 @@
     __futex_wait(tid_ptr, tid, NULL);
   }
 
-  // Take the lock again so we can pull the thread's return value
-  // and remove the thread from the list.
-  pthread_accessor thread(t);
-
   if (return_value) {
     *return_value = thread->return_value;
   }
 
-  _pthread_internal_remove_locked(thread.get(), true);
+  __pthread_internal_remove_and_free(thread);
   return 0;
 }