diff --git a/include/utils/threads.h b/include/utils/threads.h index 41e5766a0..0bd69cf37 100644 --- a/include/utils/threads.h +++ b/include/utils/threads.h @@ -526,6 +526,7 @@ private: Thread& operator=(const Thread&); static int _threadLoop(void* user); const bool mCanCallJava; + // always hold mLock when reading or writing thread_id_t mThread; mutable Mutex mLock; Condition mThreadExitedCondition; diff --git a/libs/utils/Threads.cpp b/libs/utils/Threads.cpp index 8b5da0e58..c74822866 100644 --- a/libs/utils/Threads.cpp +++ b/libs/utils/Threads.cpp @@ -168,6 +168,9 @@ int androidCreateRawThreadEtc(android_thread_func_t entryFunction, 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 @@ status_t Thread::run(const char* name, int32_t priority, size_t stack) 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 @@ int Thread::_threadLoop(void* user) { Thread* const self = static_cast(user); - // force a memory barrier before reading any fields, in particular mHoldSelf - { - Mutex::Autolock _l(self->mLock); - } - sp strong(self->mHoldSelf); wp weak(strong); self->mHoldSelf.clear(); @@ -816,6 +813,7 @@ void Thread::requestExit() 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 @@ status_t Thread::requestExitAndWait() return WOULD_BLOCK; } - requestExit(); + mExitPending = true; - Mutex::Autolock _l(mLock); while (mRunning == true) { mThreadExitedCondition.wait(mLock); }