From 62d0be964d03a4790a3a4a637e0248eaebf17119 Mon Sep 17 00:00:00 2001 From: Shyotl Date: Sun, 31 Jul 2011 01:51:43 -0500 Subject: [PATCH] Updated LLThread and LLMutex[Base] to prevent nested mutex locks in same thread from hardlocking. --- indra/llcommon/llthread.cpp | 113 +++++++++++++++++++++++++++-- indra/llcommon/llthread.h | 27 ++++++- indra/newview/llmeshrepository.cpp | 22 ++---- indra/newview/llmeshrepository.h | 2 +- 4 files changed, 141 insertions(+), 23 deletions(-) diff --git a/indra/llcommon/llthread.cpp b/indra/llcommon/llthread.cpp index 8bd341457..b6d35f1db 100644 --- a/indra/llcommon/llthread.cpp +++ b/indra/llcommon/llthread.cpp @@ -62,6 +62,21 @@ // //---------------------------------------------------------------------------- +#if !LL_DARWIN +U32 ll_thread_local sThreadID = 0; +#endif + +U32 LLThread::sIDIter = 0; + +LL_COMMON_API void assert_main_thread() +{ + static U32 s_thread_id = LLThread::currentID(); + if (LLThread::currentID() != s_thread_id) + { + llerrs << "Illegal execution outside main thread." << llendl; + } +} + // // Handed to the APR thread creation function // @@ -73,8 +88,10 @@ void *APR_THREAD_FUNC LLThread::staticRun(apr_thread_t *apr_threadp, void *datap LLThread *threadp = (LLThread *)datap; - // Set thread state to running - threadp->mStatus = RUNNING; +#if !LL_DARWIN + sThreadID = threadp->mID; +#endif + // Create a thread local data. AIThreadLocalData::create(threadp); @@ -98,6 +115,7 @@ LLThread::LLThread(std::string const& name) : mStatus(STOPPED), mThreadLocalData(NULL) { + mID = ++sIDIter; mRunCondition = new LLCondition; } @@ -119,7 +137,7 @@ void LLThread::shutdown() // First, set the flag that indicates that we're ready to die setQuitting(); - llinfos << "LLThread::~LLThread() Killing thread " << mName << " Status: " << mStatus << llendl; + llinfos << "LLThread::shutdown() Killing thread " << mName << " Status: " << mStatus << llendl; // Now wait a bit for the thread to exit // It's unclear whether I should even bother doing this - this destructor // should netver get called unless we're already stopped, really... @@ -142,20 +160,38 @@ void LLThread::shutdown() { // This thread just wouldn't stop, even though we gave it time llwarns << "LLThread::shutdown() exiting thread before clean exit!" << llendl; + // Put a stake in its heart. + apr_thread_exit(mAPRThreadp, -1); return; } mAPRThreadp = NULL; } delete mRunCondition; + mRunCondition = 0; } void LLThread::start() { - apr_thread_create(&mAPRThreadp, NULL, staticRun, (void *)this, tldata().mRootPool()); + llassert(isStopped()); + + // Set thread state to running + mStatus = RUNNING; - // We won't bother joining - apr_thread_detach(mAPRThreadp); + apr_status_t status = + apr_thread_create(&mAPRThreadp, NULL, staticRun, (void *)this, tldata().mRootPool()); + + if(status == APR_SUCCESS) + { + // We won't bother joining + apr_thread_detach(mAPRThreadp); + } + else + { + mStatus = STOPPED; + llwarns << "failed to start thread " << mName << llendl; + ll_apr_warn_status(status); + } } //============================================================================ @@ -318,6 +354,55 @@ AIThreadLocalData& AIThreadLocalData::tldata(void) //============================================================================ +void LLMutexBase::lock() +{ +#if LL_DARWIN + if (mLockingThread == LLThread::currentID()) +#else + if (mLockingThread == sThreadID) +#endif + { //redundant lock + mCount++; + return; + } + + apr_thread_mutex_lock(mAPRMutexp); + +#if MUTEX_DEBUG + // Have to have the lock before we can access the debug info + U32 id = LLThread::currentID(); + if (mIsLocked[id] != FALSE) + llerrs << "Already locked in Thread: " << id << llendl; + mIsLocked[id] = TRUE; +#endif + +#if LL_DARWIN + mLockingThread = LLThread::currentID(); +#else + mLockingThread = sThreadID; +#endif +} + +void LLMutexBase::unlock() +{ + if (mCount > 0) + { //not the root unlock + mCount--; + return; + } + +#if MUTEX_DEBUG + // Access the debug info while we have the lock + U32 id = LLThread::currentID(); + if (mIsLocked[id] != TRUE) + llerrs << "Not locked in Thread: " << id << llendl; + mIsLocked[id] = FALSE; +#endif + + mLockingThread = NO_THREAD; + apr_thread_mutex_unlock(mAPRMutexp); +} + bool LLMutexBase::isLocked() { if (!tryLock()) @@ -328,6 +413,11 @@ bool LLMutexBase::isLocked() return false; } +U32 LLMutexBase::lockingThread() const +{ + return mLockingThread; +} + //============================================================================ LLCondition::LLCondition(AIAPRPool& parent) : LLMutex(parent) @@ -335,14 +425,25 @@ LLCondition::LLCondition(AIAPRPool& parent) : LLMutex(parent) apr_thread_cond_create(&mAPRCondp, mPool()); } + LLCondition::~LLCondition() { apr_thread_cond_destroy(mAPRCondp); mAPRCondp = NULL; } + void LLCondition::wait() { + if (!isLocked()) + { //mAPRMutexp MUST be locked before calling apr_thread_cond_wait + apr_thread_mutex_lock(mAPRMutexp); +#if MUTEX_DEBUG + // avoid asserts on destruction in non-release builds + U32 id = LLThread::currentID(); + mIsLocked[id] = TRUE; +#endif + } apr_thread_cond_wait(mAPRCondp, mAPRMutexp); } diff --git a/indra/llcommon/llthread.h b/indra/llcommon/llthread.h index 97dac455e..7939a0d56 100644 --- a/indra/llcommon/llthread.h +++ b/indra/llcommon/llthread.h @@ -48,6 +48,12 @@ class LLThread; class LLMutex; class LLCondition; +#if LL_WINDOWS +#define ll_thread_local __declspec(thread) +#else +#define ll_thread_local __thread +#endif + class LL_COMMON_API AIThreadLocalData { private: @@ -66,6 +72,9 @@ public: class LL_COMMON_API LLThread { +private: + static U32 sIDIter; + public: typedef enum e_thread_status { @@ -106,6 +115,8 @@ public: // Return thread-local data for the current thread. static AIThreadLocalData& tldata(void) { return AIThreadLocalData::tldata(); } + U32 getID() const { return mID; } + private: BOOL mPaused; @@ -118,6 +129,7 @@ protected: apr_thread_t *mAPRThreadp; EThreadStatus mStatus; + U32 mID; friend void AIThreadLocalData::create(LLThread* threadp); AIThreadLocalData* mThreadLocalData; @@ -151,16 +163,26 @@ protected: class LL_COMMON_API LLMutexBase { public: - void lock() { apr_thread_mutex_lock(mAPRMutexp); } - void unlock() { apr_thread_mutex_unlock(mAPRMutexp); } + typedef enum + { + NO_THREAD = 0xFFFFFFFF + } e_locking_thread; + + LLMutexBase() : mLockingThread(NO_THREAD), mCount(0) {} + + void lock(); //blocks + void unlock(); // Returns true if lock was obtained successfully. bool tryLock() { return !APR_STATUS_IS_EBUSY(apr_thread_mutex_trylock(mAPRMutexp)); } bool isLocked(); // non-blocking, but does do a lock/unlock so not free + U32 lockingThread() const; //get ID of locking thread protected: // mAPRMutexp is initialized and uninitialized in the derived class. apr_thread_mutex_t* mAPRMutexp; + mutable U32 mCount; + mutable U32 mLockingThread; }; class LL_COMMON_API LLMutex : public LLMutexBase @@ -350,6 +372,7 @@ void LLThread::unlockData() mRunCondition->unlock(); } + //============================================================================ // see llmemory.h for LLPointer<> definition diff --git a/indra/newview/llmeshrepository.cpp b/indra/newview/llmeshrepository.cpp index 04011b6ae..c67249ebf 100644 --- a/indra/newview/llmeshrepository.cpp +++ b/indra/newview/llmeshrepository.cpp @@ -603,7 +603,7 @@ void LLMeshRepoThread::loadMeshPhysicsShape(const LLUUID& mesh_id) } -void LLMeshRepoThread::loadMeshLOD(const LLVolumeParams& mesh_params, S32 lod, bool do_lock) +void LLMeshRepoThread::loadMeshLOD(const LLVolumeParams& mesh_params, S32 lod) { //protected by mSignal, no locking needed here mesh_header_map::iterator iter = mMeshHeader.find(mesh_params.getSculptID()); @@ -611,11 +611,8 @@ void LLMeshRepoThread::loadMeshLOD(const LLVolumeParams& mesh_params, S32 lod, b { //if we have the header, request LOD byte range LODRequest req(mesh_params, lod); { - if(do_lock) - mMutex->lock(); + LLMutexLock lock(mMutex); mLODReqQ.push(req); - if(do_lock) - mMutex->unlock(); } } else @@ -631,12 +628,9 @@ void LLMeshRepoThread::loadMeshLOD(const LLVolumeParams& mesh_params, S32 lod, b } else { //if no header request is pending, fetch header - if(do_lock) - mMutex->lock(); + LLMutexLock lock(mMutex); mHeaderReqQ.push(req); mPendingLOD[mesh_params].push_back(lod); - if(do_lock) - mMutex->unlock(); } } } @@ -1604,10 +1598,10 @@ void LLMeshRepoThread::notifyLoadedMeshes() {//called via gMeshRepo.notifyLoadedMeshes(). mMutex already locked while (!mLoadedQ.empty()) { - //mMutex->lock(); + mMutex->lock(); LoadedMesh mesh = mLoadedQ.front(); mLoadedQ.pop(); - //mMutex->unlock(); + mMutex->unlock(); if (mesh.mVolume && mesh.mVolume->getNumVolumeFaces() > 0) { @@ -1622,10 +1616,10 @@ void LLMeshRepoThread::notifyLoadedMeshes() while (!mUnavailableQ.empty()) { - //mMutex->lock(); + mMutex->lock(); LODRequest req = mUnavailableQ.front(); mUnavailableQ.pop(); - //mMutex->unlock(); + mMutex->unlock(); gMeshRepo.notifyMeshUnavailable(req.mMeshParams, req.mLOD); } @@ -2405,7 +2399,7 @@ void LLMeshRepository::notifyLoadedMeshes() { LLFastTimer t(LLFastTimer::FTM_LOAD_MESH_LOD); LLMeshRepoThread::LODRequest& request = mPendingRequests.front(); - mThread->loadMeshLOD(request.mMeshParams, request.mLOD, false); + mThread->loadMeshLOD(request.mMeshParams, request.mLOD); mPendingRequests.erase(mPendingRequests.begin()); push_count--; } diff --git a/indra/newview/llmeshrepository.h b/indra/newview/llmeshrepository.h index fe905764d..80511a1d8 100644 --- a/indra/newview/llmeshrepository.h +++ b/indra/newview/llmeshrepository.h @@ -349,7 +349,7 @@ public: virtual void run(); - void loadMeshLOD(const LLVolumeParams& mesh_params, S32 lod, bool do_lock = true); + void loadMeshLOD(const LLVolumeParams& mesh_params, S32 lod); bool fetchMeshHeader(const LLVolumeParams& mesh_params); bool fetchMeshLOD(const LLVolumeParams& mesh_params, S32 lod); bool headerReceived(const LLVolumeParams& mesh_params, U8* data, S32 data_size);