ADHOC/replicant-frameworks_native
3
0
Fork 0
Code Issues 7 Pull Requests Releases Wiki Activity

Remove redundant memory barrier

Browse Source 
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
This commit is contained in:
Glenn Kasten 2011-06-02 08:59:28 -07:00
parent 53df9ca28f
commit d9e1bb76fe
2 changed files with 6 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
include/utils/threads.h
View File

@ -526,6 +526,7 @@ private:
Thread& operator=(const Thread&); Thread& operator=(const Thread&);
static int _threadLoop(void* user); static int _threadLoop(void* user);
const bool mCanCallJava; const bool mCanCallJava;
// always hold mLock when reading or writing
thread_id_t mThread; thread_id_t mThread;
mutable Mutex mLock; mutable Mutex mLock;
Condition mThreadExitedCondition; Condition mThreadExitedCondition;

13
libs/utils/Threads.cpp
View File

@ -168,6 +168,9 @@ int androidCreateRawThreadEtc(android_thread_func_t entryFunction,
return 0; 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) { if (threadId != NULL) {
*threadId = (android_thread_id_t)thread; // XXX: this is not portable *threadId = (android_thread_id_t)thread; // XXX: this is not portable
} }
@ -718,7 +721,6 @@ status_t Thread::run(const char* name, int32_t priority, size_t stack)
res = androidCreateRawThreadEtc(_threadLoop, res = androidCreateRawThreadEtc(_threadLoop,
this, name, priority, stack, &mThread); this, name, priority, stack, &mThread);
} }
// The new thread wakes up at _threadLoop, but immediately blocks on mLock
if (res == false) { if (res == false) {
mStatus = UNKNOWN_ERROR; // something happened! mStatus = UNKNOWN_ERROR; // something happened!
@ -742,11 +744,6 @@ int Thread::_threadLoop(void* user)
{ {
Thread* const self = static_cast<Thread*>(user); 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); sp<Thread> strong(self->mHoldSelf);
wp<Thread> weak(strong); wp<Thread> weak(strong);
self->mHoldSelf.clear(); self->mHoldSelf.clear();
@ -816,6 +813,7 @@ void Thread::requestExit()
status_t Thread::requestExitAndWait() status_t Thread::requestExitAndWait()
{ {
Mutex::Autolock _l(mLock);
if (mThread == getThreadId()) { if (mThread == getThreadId()) {
LOGW( LOGW(
"Thread (this=%p): don't call waitForExit() from this " "Thread (this=%p): don't call waitForExit() from this "
@ -825,9 +823,8 @@ status_t Thread::requestExitAndWait()
return WOULD_BLOCK; return WOULD_BLOCK;
} }
requestExit(); mExitPending = true;
Mutex::Autolock _l(mLock);
while (mRunning == true) { while (mRunning == true) {
mThreadExitedCondition.wait(mLock); mThreadExitedCondition.wait(mLock);
} }