From 3ea9a014232bc51eac4c5257e8502f8e633abd77 Mon Sep 17 00:00:00 2001 From: Nicky Date: Fri, 6 Apr 2018 09:49:32 +0200 Subject: [PATCH] Change pointer to member where possible and do a bit of cleanup in lerror (move the mutexex in there and us the LLMutextTryLock rather than a custom class). --- indra/llcommon/llapr.cpp | 13 ------ indra/llcommon/llapr.h | 3 -- indra/llcommon/llerror.cpp | 70 +++++----------------------- indra/llcommon/llmutex.cpp | 35 ++++---------- indra/llcommon/llmutex.h | 24 ++++++++-- indra/llcommon/llthread.cpp | 4 +- indra/llcommon/llthread.h | 6 +-- indra/llplugin/slplugin/slplugin.cpp | 1 - 8 files changed, 48 insertions(+), 108 deletions(-) diff --git a/indra/llcommon/llapr.cpp b/indra/llcommon/llapr.cpp index 33c3d5db20..ec2d322885 100644 --- a/indra/llcommon/llapr.cpp +++ b/indra/llcommon/llapr.cpp @@ -33,8 +33,6 @@ apr_pool_t *gAPRPoolp = NULL; // Global APR memory pool LLVolatileAPRPool *LLAPRFile::sAPRFilePoolp = NULL ; //global volatile APR memory pool. -std::mutex *gLogMutexp = nullptr; -std::mutex *gCallStacksLogMutexp = nullptr; const S32 FULL_VOLATILE_APR_POOL = 1024 ; //number of references to LLVolatileAPRPool @@ -46,17 +44,10 @@ void ll_init_apr() apr_initialize(); if (!gAPRPoolp) - { apr_pool_create(&gAPRPoolp, NULL); - - gLogMutexp = new std::mutex(); - gCallStacksLogMutexp = new std::mutex(); - } if(!LLAPRFile::sAPRFilePoolp) - { LLAPRFile::sAPRFilePoolp = new LLVolatileAPRPool(FALSE) ; - } LLThreadLocalPointerBase::initAllThreadLocalStorage(); gAPRInitialized = true; @@ -76,10 +67,6 @@ void ll_cleanup_apr() // Clean up the logging mutex // All other threads NEED to be done before we clean up APR, so this is okay. - delete gLogMutexp; - gLogMutexp = nullptr; - delete gCallStacksLogMutexp; - gCallStacksLogMutexp = nullptr; LLThreadLocalPointerBase::destroyAllThreadLocalStorage(); diff --git a/indra/llcommon/llapr.h b/indra/llcommon/llapr.h index 9ca47f6d66..cafa771c22 100644 --- a/indra/llcommon/llapr.h +++ b/indra/llcommon/llapr.h @@ -53,9 +53,6 @@ #include "llstring.h" -extern LL_COMMON_API std::mutex* gLogMutexp; -extern std::mutex* gCallStacksLogMutexp; - struct apr_dso_handle_t; /** * @brief Function which appropriately logs error or remains quiet on diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index fdea42d08a..2367b5702b 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -52,6 +52,9 @@ #include "lltimer.h" namespace { + LLMutex gLogMutex; + LLMutex gCallStacksLogMutex ; + #if LL_WINDOWS void debugger_print(const std::string& s) { @@ -412,7 +415,7 @@ namespace namespace LLError { - class SettingsConfig : public LLRefCount + class SettingsConfig: public LLRefCount { friend class Settings; @@ -994,63 +997,14 @@ namespace { } return found_level; } - - class LogLock - { - public: - LogLock(); - ~LogLock(); - bool ok() const { return mOK; } - private: - bool mLocked; - bool mOK; - }; - - LogLock::LogLock() - : mLocked(false), mOK(false) - { - if (!gLogMutexp) - { - mOK = true; - return; - } - - const int MAX_RETRIES = 5; - for (int attempts = 0; attempts < MAX_RETRIES; ++attempts) - { - if( gLogMutexp->try_lock() ) - { - mLocked = true; - mOK = true; - return; - } - - ms_sleep(1); - //apr_thread_yield(); - // Just yielding won't necessarily work, I had problems with - // this on Linux - doug 12/02/04 - } - - // We're hosed, we can't get the mutex. Blah. - std::cerr << "LogLock::LogLock: failed to get mutex for log" - << std::endl; - } - - LogLock::~LogLock() - { - if (mLocked) - { - gLogMutexp->unlock(); - } - } } namespace LLError { bool Log::shouldLog(CallSite& site) { - LogLock lock; - if (!lock.ok()) + LLMutexTrylock lock( &gLogMutex,5); + if (!lock.isLocked() ) { return false; } @@ -1100,11 +1054,11 @@ namespace LLError std::ostringstream* Log::out() { - LogLock lock; + LLMutexTrylock lock(&gLogMutex,5); // If we hit a logging request very late during shutdown processing, // when either of the relevant LLSingletons has already been deleted, // DO NOT resurrect them. - if (lock.ok() && ! (Settings::wasDeleted() || Globals::wasDeleted())) + if (lock.isLocked() && ! (Settings::wasDeleted() || Globals::wasDeleted())) { Globals* g = Globals::getInstance(); @@ -1120,8 +1074,8 @@ namespace LLError void Log::flush(std::ostringstream* out, char* message) { - LogLock lock; - if (!lock.ok()) + LLMutexTrylock lock(&gLogMutex,5); + if (!lock.isLocked()) { return; } @@ -1160,8 +1114,8 @@ namespace LLError void Log::flush(std::ostringstream* out, const CallSite& site) { - LogLock lock; - if (!lock.ok()) + LLMutexTrylock lock(&gLogMutex,5); + if (!lock.isLocked()) { return; } diff --git a/indra/llcommon/llmutex.cpp b/indra/llcommon/llmutex.cpp index 1090acd9fd..7457204aa3 100644 --- a/indra/llcommon/llmutex.cpp +++ b/indra/llcommon/llmutex.cpp @@ -36,8 +36,6 @@ LLMutex::LLMutex() : mCount(0), mLockingThread(NO_THREAD) { - - mMutexp = new std::mutex(); } @@ -47,8 +45,6 @@ LLMutex::~LLMutex() //bad assertion, the subclass LLSignal might be "locked", and that's OK //llassert_always(!isLocked()); // better not be locked! #endif - delete mMutexp; - mMutexp = nullptr; } @@ -60,7 +56,7 @@ void LLMutex::lock() return; } - mMutexp->lock(); + mMutex.lock(); #if MUTEX_DEBUG // Have to have the lock before we can access the debug info @@ -90,18 +86,16 @@ void LLMutex::unlock() #endif mLockingThread = NO_THREAD; - mMutexp->unlock(); + mMutex.unlock(); } bool LLMutex::isLocked() { - if (!mMutexp->try_lock()) - { + if (!mMutex.try_lock()) return true; - } else { - mMutexp->unlock(); + mMutex.unlock(); return false; } } @@ -124,11 +118,9 @@ bool LLMutex::trylock() return true; } - if (!mMutexp->try_lock()) - { + if (!mMutex.try_lock()) return false; - } - + #if MUTEX_DEBUG // Have to have the lock before we can access the debug info U32 id = LLThread::currentID(); @@ -144,34 +136,27 @@ bool LLMutex::trylock() //============================================================================ LLCondition::LLCondition() - { - // base class (LLMutex) has already ensured that mAPRPoolp is set up. - mCondp = new std::condition_variable(); } - LLCondition::~LLCondition() { - delete mCondp; - mCondp = nullptr; } - void LLCondition::wait() { - std::unique_lock< std::mutex > lock( *mMutexp ); - mCondp->wait( lock ); + std::unique_lock< std::mutex > lock( mMutex ); + mCond.wait( lock ); } void LLCondition::signal() { - mCondp->notify_one() ; + mCond.notify_one() ; } void LLCondition::broadcast() { - mCondp->notify_all(); + mCond.notify_all(); } diff --git a/indra/llcommon/llmutex.h b/indra/llcommon/llmutex.h index 566f37c66a..0cdf33e62f 100644 --- a/indra/llcommon/llmutex.h +++ b/indra/llcommon/llmutex.h @@ -28,6 +28,7 @@ #define LL_LLMUTEX_H #include "stdtypes.h" +#include "lltimer.h" #if LL_WINDOWS #pragma warning(disable:4265) @@ -67,11 +68,11 @@ public: U32 lockingThread() const; //get ID of locking thread protected: - std::mutex *mMutexp; + std::mutex mMutex; mutable U32 mCount; mutable U32 mLockingThread; - BOOL mIsLocalPool; + bool mIsLocalPool; #if MUTEX_DEBUG std::map mIsLocked; @@ -90,7 +91,7 @@ public: void broadcast(); protected: - std::condition_variable* mCondp; + std::condition_variable mCond; }; class LLMutexLock @@ -132,6 +133,23 @@ public: if (mMutex) mLocked = mMutex->trylock(); } + LLMutexTrylock( LLMutex* mutex, U32 aTries ) + : mMutex( mutex ), + mLocked( false ) + { + if( !mMutex ) + return; + + U32 i = 0; + while( i < aTries ) + { + mLocked = mMutex->trylock(); + if( mLocked ) + break; + ++i; + ms_sleep( 10 ); + } + } ~LLMutexTrylock() { diff --git a/indra/llcommon/llthread.cpp b/indra/llcommon/llthread.cpp index e1564339f7..7cb745855e 100644 --- a/indra/llcommon/llthread.cpp +++ b/indra/llcommon/llthread.cpp @@ -116,7 +116,7 @@ void LLThread::registerThreadID() // // Handed to the APR thread creation function // -void LLThread::staticRun() +void LLThread::threadRun() { #ifdef LL_WINDOWS set_thread_name( -1, mName.c_str() ); @@ -268,7 +268,7 @@ void LLThread::start() try { - mThreadp = new std::thread( std::bind( &LLThread::staticRun, this ) ); + mThreadp = new std::thread( std::bind( &LLThread::threadRun, this ) ); //mThreadp->detach(); } catch( std::system_error& ex ) diff --git a/indra/llcommon/llthread.h b/indra/llcommon/llthread.h index 9c24875db2..818b53e4a4 100644 --- a/indra/llcommon/llthread.h +++ b/indra/llcommon/llthread.h @@ -98,16 +98,16 @@ public: static void registerThreadID(); private: - BOOL mPaused; + bool mPaused; - void staticRun( ); + void threadRun( ); protected: std::string mName; class LLCondition* mRunCondition; LLMutex* mDataLock; - std::thread *mThreadp; + std::thread *mThreadp; EThreadStatus mStatus; U32 mID; LLTrace::ThreadRecorder* mRecorder; diff --git a/indra/llplugin/slplugin/slplugin.cpp b/indra/llplugin/slplugin/slplugin.cpp index 5c54705c71..38a6e1d468 100644 --- a/indra/llplugin/slplugin/slplugin.cpp +++ b/indra/llplugin/slplugin/slplugin.cpp @@ -155,7 +155,6 @@ int APIENTRY WinMain( HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdL int main(int argc, char **argv) #endif { - ll_init_apr(); // Set up llerror logging