diff options
| author | Nat Goodspeed <nat@lindenlab.com> | 2019-12-06 16:31:49 -0500 | 
|---|---|---|
| committer | Nat Goodspeed <nat@lindenlab.com> | 2020-03-25 15:28:17 -0400 | 
| commit | 5e7df752a66b2082d063d2c4a10bc7013d479f55 (patch) | |
| tree | 35d0a70687b3516a4486b001f71f571cc7c31251 /indra | |
| parent | d6baa7a8533a65174f96051c67f7d8b5b160394f (diff) | |
DRTVWR-494: Use std::thread::id for LLThread::currentID().
LLThread::currentID() used to return a U32, a distinct unsigned value
incremented by explicitly constructing LLThread or by calling LLThread::
registerThreadID() early in a thread launched by other means. The latter
imposed an unobvious requirement on new code based on std::thread. Using
std::thread::id instead delegates to the compiler/library the problem of
distinguishing threads launched by any means.
Change lots of explicit U32 declarations. Introduce LLThread::id_t typedef to
avoid having to run around fixing uses again if we later revisit this decision.
LLMutex, which stores an LLThread::id_t, wants a distinguished value meaning
NO_THREAD, and had an enum with that name. But as std::thread::id promises
that the default-constructed value is distinct from every valid value,
NO_THREAD becomes unnecessary and goes away.
Because LLMutex now stores LLThread::id_t instead of U32, make llmutex.h
#include "llthread.h" instead of the other way around. This makes LLMutex an
incomplete type within llthread.h, so move LLThread::lockData() and
unlockData() to the .cpp file. Similarly, remove llrefcount.h's #include
"llmutex.h" to break circularity; instead forward-declare LLMutex.
It turns out that a number of source files assumed that #include "llthread.h"
would get the definition for LLMutex. Sprinkle #include "llmutex.h" as needed.
In the SAFE_SSL code in llcorehttp/httpcommon.cpp, there's an ssl_thread_id()
callback that returns an unsigned long to the SSL library. When LLThread::
currentID() was U32, we could simply return that. But std::thread::id is very
deliberately opaque, and can't be reinterpret_cast to unsigned long.
Fortunately it can be hashed because std::hash is specialized with that type.
Diffstat (limited to 'indra')
| -rw-r--r-- | indra/llcommon/llmutex.cpp | 13 | ||||
| -rw-r--r-- | indra/llcommon/llmutex.h | 14 | ||||
| -rw-r--r-- | indra/llcommon/llrefcount.h | 3 | ||||
| -rw-r--r-- | indra/llcommon/llthread.cpp | 36 | ||||
| -rw-r--r-- | indra/llcommon/llthread.h | 24 | ||||
| -rw-r--r-- | indra/llcommon/lluuid.cpp | 3 | ||||
| -rw-r--r-- | indra/llcommon/llworkerthread.h | 1 | ||||
| -rw-r--r-- | indra/llcorehttp/httpcommon.cpp | 4 | ||||
| -rw-r--r-- | indra/llmessage/llbuffer.cpp | 1 | ||||
| -rw-r--r-- | indra/llmessage/llbufferstream.cpp | 1 | ||||
| -rw-r--r-- | indra/llmessage/llproxy.h | 1 | ||||
| -rw-r--r-- | indra/llplugin/llpluginmessagepipe.h | 1 | ||||
| -rw-r--r-- | indra/llvfs/llvfs.h | 1 | 
13 files changed, 49 insertions, 54 deletions
| diff --git a/indra/llcommon/llmutex.cpp b/indra/llcommon/llmutex.cpp index 75f43a4704..4d73c04d07 100644 --- a/indra/llcommon/llmutex.cpp +++ b/indra/llcommon/llmutex.cpp @@ -32,8 +32,7 @@  //============================================================================  LLMutex::LLMutex() : - mCount(0), - mLockingThread(NO_THREAD) + mCount(0)  {  } @@ -55,7 +54,7 @@ void LLMutex::lock()  #if MUTEX_DEBUG  	// Have to have the lock before we can access the debug info -	U32 id = LLThread::currentID(); +	auto id = LLThread::currentID();  	if (mIsLocked[id] != FALSE)  		LL_ERRS() << "Already locked in Thread: " << id << LL_ENDL;  	mIsLocked[id] = TRUE; @@ -74,13 +73,13 @@ void LLMutex::unlock()  #if MUTEX_DEBUG  	// Access the debug info while we have the lock -	U32 id = LLThread::currentID(); +	auto id = LLThread::currentID();  	if (mIsLocked[id] != TRUE)  		LL_ERRS() << "Not locked in Thread: " << id << LL_ENDL;	  	mIsLocked[id] = FALSE;  #endif -	mLockingThread = NO_THREAD; +	mLockingThread = LLThread::id_t();  	mMutex.unlock();  } @@ -102,7 +101,7 @@ bool LLMutex::isSelfLocked()  	return mLockingThread == LLThread::currentID();  } -U32 LLMutex::lockingThread() const +LLThread::id_t LLMutex::lockingThread() const  {  	return mLockingThread;  } @@ -122,7 +121,7 @@ bool LLMutex::trylock()  #if MUTEX_DEBUG  	// Have to have the lock before we can access the debug info -	U32 id = LLThread::currentID(); +	auto id = LLThread::currentID();  	if (mIsLocked[id] != FALSE)  		LL_ERRS() << "Already locked in Thread: " << id << LL_ENDL;  	mIsLocked[id] = TRUE; diff --git a/indra/llcommon/llmutex.h b/indra/llcommon/llmutex.h index 1a93c048b6..838d7d34c0 100644 --- a/indra/llcommon/llmutex.h +++ b/indra/llcommon/llmutex.h @@ -28,6 +28,7 @@  #define LL_LLMUTEX_H  #include "stdtypes.h" +#include "llthread.h"  #include <boost/noncopyable.hpp>  #include "mutex.h" @@ -44,11 +45,6 @@  class LL_COMMON_API LLMutex  {  public: -	typedef enum -	{ -		NO_THREAD = 0xFFFFFFFF -	} e_locking_thread; -  	LLMutex();  	virtual ~LLMutex(); @@ -57,15 +53,15 @@ public:  	void unlock();		// undefined behavior when called on mutex not being held  	bool isLocked(); 	// non-blocking, but does do a lock/unlock so not free  	bool isSelfLocked(); //return true if locked in a same thread -	U32 lockingThread() const; //get ID of locking thread -	 +	LLThread::id_t lockingThread() const; //get ID of locking thread +  protected:  	std::mutex			mMutex;  	mutable U32			mCount; -	mutable U32			mLockingThread; +	mutable LLThread::id_t	mLockingThread;  #if MUTEX_DEBUG -	std::map<U32, BOOL> mIsLocked; +	std::map<LLThread::id_t, BOOL> mIsLocked;  #endif  }; diff --git a/indra/llcommon/llrefcount.h b/indra/llcommon/llrefcount.h index fb0411d27b..7e4af6ea66 100644 --- a/indra/llcommon/llrefcount.h +++ b/indra/llcommon/llrefcount.h @@ -28,9 +28,10 @@  #include <boost/noncopyable.hpp>  #include <boost/intrusive_ptr.hpp> -#include "llmutex.h"  #include "llatomic.h" +class LLMutex; +  //----------------------------------------------------------------------------  // RefCount objects should generally only be accessed by way of LLPointer<>'s  // see llthread.h for LLThreadSafeRefCount diff --git a/indra/llcommon/llthread.cpp b/indra/llcommon/llthread.cpp index f875e4e0dc..0b9dec969c 100644 --- a/indra/llcommon/llthread.cpp +++ b/indra/llcommon/llthread.cpp @@ -92,21 +92,16 @@ void set_thread_name( DWORD dwThreadID, const char* threadName)  // }  //   //---------------------------------------------------------------------------- - -U32 LL_THREAD_LOCAL sThreadID = 0; - -U32 LLThread::sIDIter = 0; -  namespace  { -    U32 main_thread() +    LLThread::id_t main_thread()      {          // Using a function-static variable to identify the main thread          // requires that control reach here from the main thread before it          // reaches here from any other thread. We simply trust that whichever          // thread gets here first is the main thread. -        static U32 s_thread_id = LLThread::currentID(); +        static LLThread::id_t s_thread_id = LLThread::currentID();          return s_thread_id;      } @@ -128,10 +123,8 @@ LL_COMMON_API void assert_main_thread()      }  } -void LLThread::registerThreadID() -{ -    sThreadID = ++sIDIter; -} +// this function has become moot +void LLThread::registerThreadID() {}  //  // Handed to the APR thread creation function @@ -142,11 +135,12 @@ void LLThread::threadRun()      set_thread_name(-1, mName.c_str());  #endif +    // this is the first point at which we're actually running in the new thread +    mID = currentID(); +      // for now, hard code all LLThreads to report to single master thread recorder, which is known to be running on main thread      mRecorder = new LLTrace::ThreadRecorder(*LLTrace::get_master_thread_recorder()); -    sThreadID = mID; -      // Run the user supplied function      do       { @@ -188,8 +182,6 @@ LLThread::LLThread(const std::string& name, apr_pool_t *poolp) :      mStatus(STOPPED),      mRecorder(NULL)  { - -    mID = ++sIDIter;      mRunCondition = new LLCondition();      mDataLock = new LLMutex();      mLocalAPRFilePoolp = NULL ; @@ -367,9 +359,9 @@ void LLThread::setQuitting()  }  // static -U32 LLThread::currentID() +LLThread::id_t LLThread::currentID()  { -    return sThreadID; +    return std::this_thread::get_id();  }  // static @@ -396,6 +388,16 @@ void LLThread::wakeLocked()      }  } +void LLThread::lockData() +{ +    mDataLock->lock(); +} + +void LLThread::unlockData() +{ +    mDataLock->unlock(); +} +  //============================================================================  //---------------------------------------------------------------------------- diff --git a/indra/llcommon/llthread.h b/indra/llcommon/llthread.h index 37f6e66bbb..5cd0731f6c 100644 --- a/indra/llcommon/llthread.h +++ b/indra/llcommon/llthread.h @@ -30,7 +30,6 @@  #include "llapp.h"  #include "llapr.h"  #include "boost/intrusive_ptr.hpp" -#include "llmutex.h"  #include "llrefcount.h"  #include <thread> @@ -43,7 +42,6 @@ class LL_COMMON_API LLThread  {  private:      friend class LLMutex; -    static U32 sIDIter;  public:      typedef enum e_thread_status @@ -53,6 +51,7 @@ public:          QUITTING= 2,    // Someone wants this thread to quit          CRASHED = -1    // An uncaught exception was thrown by the thread      } EThreadStatus; +    typedef std::thread::id id_t;      LLThread(const std::string& name, apr_pool_t *poolp = NULL);      virtual ~LLThread(); // Warning!  You almost NEVER want to destroy a thread unless it's in the STOPPED state. @@ -62,7 +61,7 @@ public:      bool isStopped() const { return (STOPPED == mStatus) || (CRASHED == mStatus); }      bool isCrashed() const { return (CRASHED == mStatus); }  -    static U32 currentID(); // Return ID of current thread +    static id_t currentID(); // Return ID of current thread      static void yield(); // Static because it can be called by the main thread, which doesn't have an LLThread data structure.  public: @@ -86,7 +85,7 @@ public:      LLVolatileAPRPool* getLocalAPRFilePool() { return mLocalAPRFilePoolp ; } -    U32 getID() const { return mID; } +    id_t getID() const { return mID; }      // Called by threads *not* created via LLThread to register some      // internal state used by LLMutex.  You must call this once early @@ -107,7 +106,7 @@ protected:      std::thread        *mThreadp;      EThreadStatus       mStatus; -    U32                 mID; +    id_t                mID;      LLTrace::ThreadRecorder* mRecorder;      //a local apr_pool for APRFile operations in this thread. If it exists, LLAPRFile::sAPRFilePoolp should not be used. @@ -124,8 +123,8 @@ protected:      virtual bool runCondition(void);      // Lock/Unlock Run Condition -- use around modification of any variable used in runCondition() -    inline void lockData(); -    inline void unlockData(); +    void lockData(); +    void unlockData();      // This is the predicate that decides whether the thread should sleep.        // It should only be called with mDataLock locked, since the virtual runCondition() function may need to access @@ -140,17 +139,6 @@ protected:  }; -void LLThread::lockData() -{ -    mDataLock->lock(); -} - -void LLThread::unlockData() -{ -    mDataLock->unlock(); -} - -  //============================================================================  // Simple responder for self destructing callbacks diff --git a/indra/llcommon/lluuid.cpp b/indra/llcommon/lluuid.cpp index 8f33d789eb..b05630c6b5 100644 --- a/indra/llcommon/lluuid.cpp +++ b/indra/llcommon/lluuid.cpp @@ -43,6 +43,7 @@  #include "llstring.h"  #include "lltimer.h"  #include "llthread.h" +#include "llmutex.h"  const LLUUID LLUUID::null;  const LLTransactionID LLTransactionID::tnull; @@ -738,7 +739,7 @@ void LLUUID::getCurrentTime(uuid_time_t *timestamp)        getSystemTime(&time_last);        uuids_this_tick = uuids_per_tick;        init = TRUE; -	  mMutex = new LLMutex(); +      mMutex = new LLMutex();     }     uuid_time_t time_now = {0,0}; diff --git a/indra/llcommon/llworkerthread.h b/indra/llcommon/llworkerthread.h index b1a6f61360..0387e75c65 100644 --- a/indra/llcommon/llworkerthread.h +++ b/indra/llcommon/llworkerthread.h @@ -34,6 +34,7 @@  #include "llqueuedthread.h"  #include "llatomic.h" +#include "llmutex.h"  #define USE_FRAME_CALLBACK_MANAGER 0 diff --git a/indra/llcorehttp/httpcommon.cpp b/indra/llcorehttp/httpcommon.cpp index 7c93c54cdf..e37a38b05f 100644 --- a/indra/llcorehttp/httpcommon.cpp +++ b/indra/llcorehttp/httpcommon.cpp @@ -40,6 +40,7 @@  #include <sstream>  #if SAFE_SSL  #include <openssl/crypto.h> +#include <functional>               // std::hash  #endif @@ -369,7 +370,8 @@ void ssl_locking_callback(int mode, int type, const char *file, int line)  //static  unsigned long ssl_thread_id(void)  { -    return LLThread::currentID(); +    // std::thread::id is very deliberately opaque, but we can hash it +    return std::hash<LLThread::id_t>()(LLThread::currentID());  }  #endif diff --git a/indra/llmessage/llbuffer.cpp b/indra/llmessage/llbuffer.cpp index 1a0eceba0f..cfe38605ad 100644 --- a/indra/llmessage/llbuffer.cpp +++ b/indra/llmessage/llbuffer.cpp @@ -32,6 +32,7 @@  #include "llmath.h"  #include "llstl.h"  #include "llthread.h" +#include "llmutex.h"  #include <iterator>  #define ASSERT_LLBUFFERARRAY_MUTEX_LOCKED() llassert(!mMutexp || mMutexp->isSelfLocked()) diff --git a/indra/llmessage/llbufferstream.cpp b/indra/llmessage/llbufferstream.cpp index ff1c9993cc..39508c1c52 100644 --- a/indra/llmessage/llbufferstream.cpp +++ b/indra/llmessage/llbufferstream.cpp @@ -31,6 +31,7 @@  #include "llbuffer.h"  #include "llthread.h" +#include "llmutex.h"  static const S32 DEFAULT_OUTPUT_SEGMENT_SIZE = 1024 * 4; diff --git a/indra/llmessage/llproxy.h b/indra/llmessage/llproxy.h index 87891901ad..a1ffa9e5d5 100644 --- a/indra/llmessage/llproxy.h +++ b/indra/llmessage/llproxy.h @@ -32,6 +32,7 @@  #include "llmemory.h"  #include "llsingleton.h"  #include "llthread.h" +#include "llmutex.h"  #include <curl/curl.h>  #include <string> diff --git a/indra/llplugin/llpluginmessagepipe.h b/indra/llplugin/llpluginmessagepipe.h index c3498beac0..9d5835eb82 100644 --- a/indra/llplugin/llpluginmessagepipe.h +++ b/indra/llplugin/llpluginmessagepipe.h @@ -31,6 +31,7 @@  #include "lliosocket.h"  #include "llthread.h" +#include "llmutex.h"  class LLPluginMessagePipe; diff --git a/indra/llvfs/llvfs.h b/indra/llvfs/llvfs.h index dca5ff4ad5..42feafe20b 100644 --- a/indra/llvfs/llvfs.h +++ b/indra/llvfs/llvfs.h @@ -31,6 +31,7 @@  #include "lluuid.h"  #include "llassettype.h"  #include "llthread.h" +#include "llmutex.h"  enum EVFSValid   { | 
