Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 9dbd7d81 authored by Glenn Kasten's avatar Glenn Kasten
Browse files

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
parent 679dd99c

include/utils/threads.h

+1 −0
Original line number Diff line number Diff line
@@ -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;

libs/utils/Threads.cpp

+5 −8
Original line number Diff line number Diff line
@@ -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<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 @@ 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);

    }