Remove redundant memory barrier
pthread_create already includes the necessary memory barriers:
- parent at pthread_create : pthread_mutex_unlock(start_mutex)
- child at __thread_entry : pthread_mutex_lock(start_mutex)
Add lock around uses of mThread.
Added comments:
- uses of mThread require lock
- androidCreateRawThreadEtc returned ID is not safe for direct use from non-parent threads.
Change-Id: I18cb296b41ddaf64cf127b57aab31154319b5970
diff --git a/libs/utils/Threads.cpp b/libs/utils/Threads.cpp
index 8b5da0e..c748228 100644
--- a/libs/utils/Threads.cpp
+++ b/libs/utils/Threads.cpp
@@ -168,6 +168,9 @@
return 0;
}
+ // Note that *threadID is directly available to the parent only, as it is
+ // assigned after the child starts. Use memory barrier / lock if the child
+ // or other threads also need access.
if (threadId != NULL) {
*threadId = (android_thread_id_t)thread; // XXX: this is not portable
}
@@ -718,7 +721,6 @@
res = androidCreateRawThreadEtc(_threadLoop,
this, name, priority, stack, &mThread);
}
- // The new thread wakes up at _threadLoop, but immediately blocks on mLock
if (res == false) {
mStatus = UNKNOWN_ERROR; // something happened!
@@ -742,11 +744,6 @@
{
Thread* const self = static_cast<Thread*>(user);
- // force a memory barrier before reading any fields, in particular mHoldSelf
- {
- Mutex::Autolock _l(self->mLock);
- }
-
sp<Thread> strong(self->mHoldSelf);
wp<Thread> weak(strong);
self->mHoldSelf.clear();
@@ -816,6 +813,7 @@
status_t Thread::requestExitAndWait()
{
+ Mutex::Autolock _l(mLock);
if (mThread == getThreadId()) {
LOGW(
"Thread (this=%p): don't call waitForExit() from this "
@@ -825,9 +823,8 @@
return WOULD_BLOCK;
}
- requestExit();
+ mExitPending = true;
- Mutex::Autolock _l(mLock);
while (mRunning == true) {
mThreadExitedCondition.wait(mLock);
}