SH-3275 WIP Run viewer metrics for object update messages

fixed various issues related to unit tests and LLThreadLocalPtr initialization and teardown

9 files changed, 108 insertions, 77 deletions

diff --git a/indra/llcommon/llapr.cpp b/indra/llcommon/llapr.cpp
index 76749f8a91..e9930c10f7 100644
--- a/indra/llcommon/llapr.cpp
+++ b/indra/llcommon/llapr.cpp
@@ -52,7 +52,7 @@ void ll_init_apr()
 
 	if(!LLAPRFile::sAPRFilePoolp)
 	{
-		LLAPRFile::sAPRFilePoolp = new LLVolatileAPRPool(FALSE) ;
+		LLAPRFile::sAPRFilePoolp = new LLVolatileAPRPool(FALSE);
 	}
 
 	LLThreadLocalPtrBase::initAllThreadLocalStorage();
@@ -79,6 +79,9 @@ void ll_cleanup_apr()
 		apr_thread_mutex_destroy(gCallStacksLogMutexp);
 		gCallStacksLogMutexp = NULL;
 	}
+
+	LLThreadLocalPtrBase::destroyAllThreadLocalStorage();
+
 	if (gAPRPoolp)
 	{
 		apr_pool_destroy(gAPRPoolp);
@@ -86,7 +89,7 @@ void ll_cleanup_apr()
 	}
 	if (LLAPRFile::sAPRFilePoolp)
 	{
-		delete LLAPRFile::sAPRFilePoolp ;
+		delete LLAPRFile::sAPRFilePoolp ;	
 		LLAPRFile::sAPRFilePoolp = NULL ;
 	}
 	apr_terminate();
@@ -483,9 +486,8 @@ S32 LLAPRFile::seek(apr_seek_where_t where, S32 offset)
 //
 bool LLThreadLocalPtrBase::sInitialized = false;
 
-LLThreadLocalPtrBase::LLThreadLocalPtrBase(void (*cleanup_func)(void*))
-:	mCleanupFunc(cleanup_func),
-	mThreadKey(NULL)
+LLThreadLocalPtrBase::LLThreadLocalPtrBase()
+:	mThreadKey(NULL)
 {
 	if (sInitialized)
 	{
@@ -494,8 +496,7 @@ LLThreadLocalPtrBase::LLThreadLocalPtrBase(void (*cleanup_func)(void*))
 }
 
 LLThreadLocalPtrBase::LLThreadLocalPtrBase( const LLThreadLocalPtrBase& other)
-:	mCleanupFunc(other.mCleanupFunc),
-	mThreadKey(NULL)
+:	mThreadKey(NULL)
 {
 	if (sInitialized)
 	{
@@ -522,7 +523,7 @@ void LLThreadLocalPtrBase::set( void* value )
 
 void LLThreadLocalPtrBase::initStorage( )
 {
-	apr_status_t result = apr_threadkey_private_create(&mThreadKey, mCleanupFunc, gAPRPoolp);
+	apr_status_t result = apr_threadkey_private_create(&mThreadKey, NULL, gAPRPoolp);
 	if (result != APR_SUCCESS)
 	{
 		ll_apr_warn_status(result);
@@ -532,13 +533,16 @@ void LLThreadLocalPtrBase::initStorage( )
 
 void LLThreadLocalPtrBase::destroyStorage()
 {
-	if (mThreadKey)
+	if (sInitialized)
 	{
-		apr_status_t result = apr_threadkey_private_delete(mThreadKey);
-		if (result != APR_SUCCESS)
+		if (mThreadKey)
 		{
-			ll_apr_warn_status(result);
-			llerrs << "Failed to delete thread local data" << llendl;
+			apr_status_t result = apr_threadkey_private_delete(mThreadKey);
+			if (result != APR_SUCCESS)
+			{
+				ll_apr_warn_status(result);
+				llerrs << "Failed to delete thread local data" << llendl;
+			}
 		}
 	}
 }
@@ -547,16 +551,29 @@ void LLThreadLocalPtrBase::initAllThreadLocalStorage()
 {
 	if (!sInitialized)
 	{
-		sInitialized = true;
 		for (LLInstanceTracker<LLThreadLocalPtrBase>::instance_iter it = beginInstances(), end_it = endInstances();
 			it != end_it;
 			++it)
 		{
 			(*it).initStorage();
 		}
+		sInitialized = true;
 	}
 }
 
+void LLThreadLocalPtrBase::destroyAllThreadLocalStorage()
+{
+	if (sInitialized)
+	{
+		for (LLInstanceTracker<LLThreadLocalPtrBase>::instance_iter it = beginInstances(), end_it = endInstances();
+			it != end_it;
+			++it)
+		{
+			(*it).destroyStorage();
+		}
+		sInitialized = false;
+	}
+}
 
 //
 //*******************************************************************************************************************************
diff --git a/indra/llcommon/llapr.h b/indra/llcommon/llapr.h
index eb0bf627a0..830e0a33fa 100644
--- a/indra/llcommon/llapr.h
+++ b/indra/llcommon/llapr.h
@@ -259,16 +259,19 @@ public:
 class LLThreadLocalPtrBase : LLInstanceTracker<LLThreadLocalPtrBase>
 {
 public:
-	LLThreadLocalPtrBase(void (*cleanup_func)(void*) );
+	LLThreadLocalPtrBase();
 	LLThreadLocalPtrBase(const LLThreadLocalPtrBase& other);
 	~LLThreadLocalPtrBase();
 
+	static void initAllThreadLocalStorage();
+	static void destroyAllThreadLocalStorage();
+
 protected:
-	friend void LL_COMMON_API ll_init_apr();
 	void set(void* value);
 
 	LL_FORCE_INLINE void* get()
 	{
+		llassert(sInitialized);
 		void* ptr;
 		//apr_status_t result =
 		apr_threadkey_private_get(&ptr, mThreadKey);
@@ -294,13 +297,9 @@ protected:
 	}
 
 	void initStorage();
-
 	void destroyStorage();
 
-	static void initAllThreadLocalStorage();
-
-private:
-	void (*mCleanupFunc)(void*);
+protected:
 	apr_threadkey_t* mThreadKey;
 	static bool		sInitialized;
 };
@@ -311,10 +310,10 @@ class LLThreadLocalPtr : public LLThreadLocalPtrBase
 public:
 
 	LLThreadLocalPtr()
-	:	LLThreadLocalPtrBase(&cleanup)
+	:	LLThreadLocalPtrBase()
 	{}
 
-	LLThreadLocalPtr(T* value)
+	explicit LLThreadLocalPtr(T* value)
 		:	LLThreadLocalPtrBase(&cleanup)
 	{
 		set(value);
@@ -327,7 +326,7 @@ public:
 		set(other.get());		
 	}
 
-	T* get()
+	LL_FORCE_INLINE T* get()
 	{
 		return (T*)LLThreadLocalPtrBase::get();
 	}
@@ -363,13 +362,11 @@ public:
 		return *this;
 	}
 
-private:
-
-	static void cleanup(void* ptr)
+	bool operator ==(T* other)
 	{
-		delete reinterpret_cast<T*>(ptr);
+		if (!sInitialized) return false;
+		return get() == other;
 	}
-
 };
 
 /**
diff --git a/indra/llcommon/llcommon.cpp b/indra/llcommon/llcommon.cpp
index 512e7da840..c149a1fe5c 100644
--- a/indra/llcommon/llcommon.cpp
+++ b/indra/llcommon/llcommon.cpp
@@ -45,15 +45,12 @@ void LLCommon::initClass()
 	LLTimer::initClass();
 	LLThreadSafeRefCount::initThreadSafeRefCount();
 	LLTrace::init();
-// 	LLWorkerThread::initClass();
-// 	LLFrameCallbackManager::initClass();
 }
 
 //static
 void LLCommon::cleanupClass()
 {
-// 	LLFrameCallbackManager::cleanupClass();
-// 	LLWorkerThread::cleanupClass();
+	LLTrace::cleanup();
 	LLThreadSafeRefCount::cleanupThreadSafeRefCount();
 	LLTimer::cleanupClass();
 	if (sAprInitialized)
@@ -62,5 +59,4 @@ void LLCommon::cleanupClass()
 		sAprInitialized = FALSE;
 	}
 	LLMemory::cleanupClass();
-	LLTrace::cleanup();
 }
diff --git a/indra/llcommon/llthread.cpp b/indra/llcommon/llthread.cpp
index de1f0801a1..2e6eb085dc 100644
--- a/indra/llcommon/llthread.cpp
+++ b/indra/llcommon/llthread.cpp
@@ -66,7 +66,7 @@ U32 __thread LLThread::sThreadID = 0;
 #endif
 
 U32 LLThread::sIDIter = 0;
-LLThreadLocalPtr<LLTrace::ThreadTraceData> LLThread::sTraceData;
+LLThreadLocalPtr<LLTrace::SlaveThreadTrace> LLThread::sTraceData;
 
 
 LL_COMMON_API void assert_main_thread()
@@ -99,6 +99,8 @@ void *APR_THREAD_FUNC LLThread::staticRun(apr_thread_t *apr_threadp, void *datap
 	// We're done with the run function, this thread is done executing now.
 	threadp->mStatus = STOPPED;
 
+	delete sTraceData.get();
+
 	return NULL;
 }
 
@@ -108,6 +110,7 @@ LLThread::LLThread(const std::string& name, apr_pool_t *poolp) :
 	mAPRThreadp(NULL),
 	mStatus(STOPPED)
 {
+
 	mID = ++sIDIter;
 
 	// Thread creation probably CAN be paranoid about APR being initialized, if necessary
diff --git a/indra/llcommon/llthread.h b/indra/llcommon/llthread.h
index fed111b0e4..6889fab2c1 100644
--- a/indra/llcommon/llthread.h
+++ b/indra/llcommon/llthread.h
@@ -105,7 +105,7 @@ protected:
 	EThreadStatus		mStatus;
 	U32					mID;
 
-	static LLThreadLocalPtr<LLTrace::ThreadTraceData> sTraceData;
+	static LLThreadLocalPtr<LLTrace::SlaveThreadTrace> sTraceData;
 
 	//a local apr_pool for APRFile operations in this thread. If it exists, LLAPRFile::sAPRFilePoolp should not be used.
 	//Note: this pool is used by APRFile ONLY, do NOT use it for any other purposes.
diff --git a/indra/llcommon/lltrace.cpp b/indra/llcommon/lltrace.cpp
index 501414ebf3..8cedcafd10 100644
--- a/indra/llcommon/lltrace.cpp
+++ b/indra/llcommon/lltrace.cpp
@@ -31,27 +31,37 @@
 namespace LLTrace
 {
 
-BlockTimer::Recorder::StackEntry BlockTimer::sCurRecorder;
-
-MasterThreadTrace *gMasterThreadTrace = NULL;
-LLThreadLocalPtr<ThreadTraceData> gCurThreadTrace;
+static MasterThreadTrace* gMasterThreadTrace = NULL;
 
 void init()
 {
 	gMasterThreadTrace = new MasterThreadTrace();
-	gCurThreadTrace = gMasterThreadTrace;
 }
 
 void cleanup()
 {
 	delete gMasterThreadTrace;
+	gMasterThreadTrace = NULL;
 }
 
 
+BlockTimer::Recorder::StackEntry BlockTimer::sCurRecorder;
+
+
+
+MasterThreadTrace& getMasterThreadTrace()
+{
+	llassert(gMasterThreadTrace != NULL);
+	return *gMasterThreadTrace;
+}
+
+
+
 ///////////////////////////////////////////////////////////////////////
 // Sampler
 ///////////////////////////////////////////////////////////////////////
 
+
 void Sampler::stop()
 {
 	getThreadTrace()->deactivate(this);
@@ -71,15 +81,15 @@ class ThreadTraceData* Sampler::getThreadTrace()
 // MasterThreadTrace
 ///////////////////////////////////////////////////////////////////////
 
-void MasterThreadTrace::pullFromWorkerThreads()
+void MasterThreadTrace::pullFromSlaveThreads()
 {
 	LLMutexLock lock(&mSlaveListMutex);
 
-	for (worker_thread_trace_list_t::iterator it = mSlaveThreadTraces.begin(), end_it = mSlaveThreadTraces.end();
+	for (slave_thread_trace_list_t::iterator it = mSlaveThreadTraces.begin(), end_it = mSlaveThreadTraces.end();
 		it != end_it;
 		++it)
 	{
-		it->mWorkerTrace->mSharedData.copyTo(it->mSamplerStorage);
+		it->mSlaveTrace->mSharedData.copyTo(it->mSamplerStorage);
 	}
 }
 
@@ -87,18 +97,18 @@ void MasterThreadTrace::addSlaveThread( class SlaveThreadTrace* child )
 {
 	LLMutexLock lock(&mSlaveListMutex);
 
-	mSlaveThreadTraces.push_back(WorkerThreadTraceProxy(child));
+	mSlaveThreadTraces.push_back(SlaveThreadTraceProxy(child));
 }
 
 void MasterThreadTrace::removeSlaveThread( class SlaveThreadTrace* child )
 {
 	LLMutexLock lock(&mSlaveListMutex);
 
-	for (worker_thread_trace_list_t::iterator it = mSlaveThreadTraces.begin(), end_it = mSlaveThreadTraces.end();
+	for (slave_thread_trace_list_t::iterator it = mSlaveThreadTraces.begin(), end_it = mSlaveThreadTraces.end();
 		it != end_it;
 		++it)
 	{
-		if (it->mWorkerTrace == child)
+		if (it->mSlaveTrace == child)
 		{
 			mSlaveThreadTraces.erase(it);
 			break;
@@ -107,3 +117,4 @@ void MasterThreadTrace::removeSlaveThread( class SlaveThreadTrace* child )
 }
 
 }
+
diff --git a/indra/llcommon/lltrace.h b/indra/llcommon/lltrace.h
index 3af05d67f9..e4bec0a644 100644
--- a/indra/llcommon/lltrace.h
+++ b/indra/llcommon/lltrace.h
@@ -44,8 +44,7 @@ namespace LLTrace
 	void init();
 	void cleanup();
 
-	extern class MasterThreadTrace *gMasterThreadTrace;
-	extern LLThreadLocalPtr<class ThreadTraceData> gCurThreadTrace;
+	class MasterThreadTrace& getMasterThreadTrace();
 
 	// one per thread per type
 	template<typename ACCUMULATOR>
@@ -59,8 +58,8 @@ namespace LLTrace
 		:	mStorageSize(64),
 			mNextStorageSlot(0),
 			mStorage(new ACCUMULATOR[DEFAULT_ACCUMULATOR_BUFFER_SIZE])
-		{
-		}
+		{}
+
 	public:
 
 		AccumulatorBuffer(const AccumulatorBuffer& other = getDefaultBuffer())
@@ -69,6 +68,15 @@ namespace LLTrace
 			mNextStorageSlot(other.mNextStorageSlot)
 		{}
 
+		~AccumulatorBuffer()
+		{
+			if (sPrimaryStorage == mStorage)
+			{
+				//TODO pick another primary?
+				sPrimaryStorage = NULL;
+			}
+		}
+
 		LL_FORCE_INLINE ACCUMULATOR& operator[](size_t index) 
 		{ 
 			return mStorage[index]; 
@@ -353,7 +361,8 @@ namespace LLTrace
 			mTimers(other.mTimers)
 		{}
 
-		~Sampler() {}
+		~Sampler()
+		{}
 
 		void makePrimary()
 		{
@@ -453,21 +462,21 @@ namespace LLTrace
 		void addSlaveThread(class SlaveThreadTrace* child);
 		void removeSlaveThread(class SlaveThreadTrace* child);
 
-		// call this periodically to gather stats data from worker threads
-		void pullFromWorkerThreads();
+		// call this periodically to gather stats data from slave threads
+		void pullFromSlaveThreads();
 
 	private:
-		struct WorkerThreadTraceProxy
+		struct SlaveThreadTraceProxy
 		{
-			WorkerThreadTraceProxy(class SlaveThreadTrace* trace)
-				:	mWorkerTrace(trace)
+			SlaveThreadTraceProxy(class SlaveThreadTrace* trace)
+				:	mSlaveTrace(trace)
 			{}
-			class SlaveThreadTrace*	mWorkerTrace;
+			class SlaveThreadTrace*	mSlaveTrace;
 			Sampler				mSamplerStorage;
 		};
-		typedef std::list<WorkerThreadTraceProxy> worker_thread_trace_list_t;
+		typedef std::list<SlaveThreadTraceProxy> slave_thread_trace_list_t;
 
-		worker_thread_trace_list_t		mSlaveThreadTraces;
+		slave_thread_trace_list_t		mSlaveThreadTraces;
 		LLMutex							mSlaveListMutex;
 	};
 
@@ -475,17 +484,16 @@ namespace LLTrace
 	{
 	public:
 		explicit 
-		SlaveThreadTrace(MasterThreadTrace& master_trace = *gMasterThreadTrace)
-		:	mMaster(master_trace),
-			ThreadTraceData(master_trace),
+		SlaveThreadTrace()
+		:	ThreadTraceData(getMasterThreadTrace()),
 			mSharedData(mPrimarySampler)
 		{
-			master_trace.addSlaveThread(this);
+			getMasterThreadTrace().addSlaveThread(this);
 		}
 
 		~SlaveThreadTrace()
 		{
-			mMaster.removeSlaveThread(this);
+			getMasterThreadTrace().removeSlaveThread(this);
 		}
 
 		// call this periodically to gather stats data for master thread to consume
@@ -494,7 +502,7 @@ namespace LLTrace
 			mSharedData.copyFrom(mPrimarySampler);
 		}
 
-		MasterThreadTrace& 	mMaster;
+		MasterThreadTrace* 	mMaster;
 
 		// this data is accessed by other threads, so give it a 64 byte alignment
 		// to avoid false sharing on most x86 processors
@@ -529,7 +537,6 @@ namespace LLTrace
 		SharedData					mSharedData;
 	};
 
-
 	class TimeInterval 
 	{
 	public:
diff --git a/indra/llimage/tests/llimageworker_test.cpp b/indra/llimage/tests/llimageworker_test.cpp
index e255d65b43..be7aae4eb7 100644
--- a/indra/llimage/tests/llimageworker_test.cpp
+++ b/indra/llimage/tests/llimageworker_test.cpp
@@ -114,11 +114,13 @@ namespace tut
 		// Constructor and destructor of the test wrapper
 		imagedecodethread_test()
 		{
+			LLTrace::init();
 			mThread = NULL;
 		}
 		~imagedecodethread_test()
 		{
 			delete mThread;
+			LLTrace::cleanup();
 		}
 	};
 
@@ -136,6 +138,8 @@ namespace tut
 		imagerequest_test()
 		{
 			done = false;
+			LLTrace::init();
+
 			mRequest = new LLImageDecodeThread::ImageRequest(0, 0,
 											 LLQueuedThread::PRIORITY_NORMAL, 0, FALSE,
 											 new responder_test(&done));
@@ -145,6 +149,7 @@ namespace tut
 			// We should delete the object *but*, because its destructor is protected, that cannot be
 			// done from outside an LLImageDecodeThread instance... So we leak memory here... It's fine...
 			//delete mRequest;
+			LLTrace::cleanup();
 		}
 	};
 
diff --git a/indra/test/test.cpp b/indra/test/test.cpp
index dc8580fe69..2b66c6aa26 100644
--- a/indra/test/test.cpp
+++ b/indra/test/test.cpp
@@ -512,15 +512,10 @@ int main(int argc, char **argv)
 	ctype_workaround();
 #endif
 
-	apr_initialize();
-	apr_pool_t* pool = NULL;
-	if(APR_SUCCESS != apr_pool_create(&pool, NULL))
-	{
-		std::cerr << "Unable to initialize pool" << std::endl;
-		return 1;
-	}
+	ll_init_apr();
+
 	apr_getopt_t* os = NULL;
-	if(APR_SUCCESS != apr_getopt_init(&os, pool, argc, argv))
+	if(APR_SUCCESS != apr_getopt_init(&os, gAPRPoolp, argc, argv))
 	{
 		std::cerr << "apr_getopt_init() failed" << std::endl;
 		return 1;
@@ -602,7 +597,7 @@ int main(int argc, char **argv)
 	if (LOGFAIL)
 	{
 		LLError::ELevel level = LLError::decodeLevel(LOGFAIL);
-		replayer.reset(new LLReplayLogReal(level, pool));
+		replayer.reset(new LLReplayLogReal(level, gAPRPoolp));
 	}
 	else
 	{
@@ -646,7 +641,7 @@ int main(int argc, char **argv)
 		s.close();
 	}
 
-	apr_terminate();
+	ll_cleanup_apr();
 
 	int retval = (success ? 0 : 1);
 	return retval;