From cda2cdda511eb2f7a58e284db2c852fd4a3808ae Mon Sep 17 00:00:00 2001 From: Richard Linden Date: Thu, 3 Jan 2013 00:30:54 -0800 Subject: SH-3406 WIP convert fast timers to lltrace system made fast timer stack thread local added LLThreadLocalSingleton made LLThreadLocalPointer obey pointer rules for const added LLThreadLocalSingletonPointer for fast thread local pointers --- indra/llcommon/llsingleton.h | 52 ++++++++++++++++++++++++-------------------- 1 file changed, 29 insertions(+), 23 deletions(-) (limited to 'indra/llcommon/llsingleton.h') diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 49d99f2cd0..f6b0a7194b 100644 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -90,7 +90,7 @@ template class LLSingleton : private boost::noncopyable { -private: +protected: typedef enum e_init_state { UNINITIALIZED, @@ -124,7 +124,7 @@ private: public: virtual ~LLSingleton() { - SingletonInstanceData& data = getData(); + SingletonInstanceData& data = getSingletonData(); data.mSingletonInstance = NULL; data.mInitState = DELETED; } @@ -151,29 +151,15 @@ public: */ static void deleteSingleton() { - delete getData().mSingletonInstance; - getData().mSingletonInstance = NULL; - getData().mInitState = DELETED; - } - - static SingletonInstanceData& getData() - { - // this is static to cache the lookup results - static void * & registry = LLSingletonRegistry::get(); - - // *TODO - look into making this threadsafe - if(NULL == registry) - { - static SingletonInstanceData data; - registry = &data; - } - - return *static_cast(registry); + SingletonInstanceData& data = getSingletonData(); + delete data.mSingletonInstance; + data.mSingletonInstance = NULL; + data.mInitState = DELETED; } static DERIVED_TYPE* getInstance() { - SingletonInstanceData& data = getData(); + SingletonInstanceData& data = getSingletonData(); if (data.mInitState == CONSTRUCTING) { @@ -197,6 +183,12 @@ public: return data.mSingletonInstance; } + static DERIVED_TYPE* getIfExists() + { + SingletonInstanceData& data = getSingletonData(); + return data.mSingletonInstance; + } + // Reference version of getInstance() // Preferred over getInstance() as it disallows checking for NULL static DERIVED_TYPE& instance() @@ -208,17 +200,31 @@ public: // Use this to avoid accessing singletons before the can safely be constructed static bool instanceExists() { - return getData().mInitState == INITIALIZED; + return getSingletonData().mInitState == INITIALIZED; } // Has this singleton already been deleted? // Use this to avoid accessing singletons from a static object's destructor static bool destroyed() { - return getData().mInitState == DELETED; + return getSingletonData().mInitState == DELETED; } private: + static SingletonInstanceData& getSingletonData() + { + // this is static to cache the lookup results + static void * & registry = LLSingletonRegistry::get(); + + // *TODO - look into making this threadsafe + if(NULL == registry) + { + static SingletonInstanceData data; + registry = &data; + } + + return *static_cast(registry); + } virtual void initSingleton() {} }; -- cgit v1.2.3 From 84af0e9852486231b5ef0cde7ad1704d41689a3a Mon Sep 17 00:00:00 2001 From: Richard Linden Date: Wed, 24 Apr 2013 14:13:45 -0700 Subject: SH-4080 WIP interesting: random crash on Mac potential fix for crasher cleaned up llsingleton --- indra/llcommon/llsingleton.h | 74 +++++++++++--------------------------------- 1 file changed, 18 insertions(+), 56 deletions(-) (limited to 'indra/llcommon/llsingleton.h') diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index f6b0a7194b..697d1b042a 100644 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -30,38 +30,6 @@ #include #include -/// @brief A global registry of all singletons to prevent duplicate allocations -/// across shared library boundaries -class LL_COMMON_API LLSingletonRegistry { - private: - typedef std::map TypeMap; - static TypeMap * sSingletonMap; - - static void checkInit() - { - if(sSingletonMap == NULL) - { - sSingletonMap = new TypeMap(); - } - } - - public: - template static void * & get() - { - std::string name(typeid(T).name()); - - checkInit(); - - // the first entry of the pair returned by insert will be either the existing - // iterator matching our key, or the newly inserted NULL initialized entry - // see "Insert element" in http://www.sgi.com/tech/stl/UniqueAssociativeContainer.html - TypeMap::iterator result = - sSingletonMap->insert(std::make_pair(name, (void*)NULL)).first; - - return result->second; - } -}; - // LLSingleton implements the getInstance() method part of the Singleton // pattern. It can't make the derived class constructors protected, though, so // you have to do that yourself. @@ -90,10 +58,9 @@ template class LLSingleton : private boost::noncopyable { -protected: +private: typedef enum e_init_state { - UNINITIALIZED, CONSTRUCTING, INITIALIZING, INITIALIZED, @@ -109,8 +76,11 @@ protected: SingletonInstanceData() : mSingletonInstance(NULL), - mInitState(UNINITIALIZED) - {} + mInitState(CONSTRUCTING) + { + mSingletonInstance = new DERIVED_TYPE(); + mInitState = INITIALIZING; + } ~SingletonInstanceData() { @@ -151,12 +121,12 @@ public: */ static void deleteSingleton() { - SingletonInstanceData& data = getSingletonData(); - delete data.mSingletonInstance; - data.mSingletonInstance = NULL; - data.mInitState = DELETED; + delete getSingletonData().mSingletonInstance; + getSingletonData().mSingletonInstance = NULL; + getSingletonData().mInitState = DELETED; } + static DERIVED_TYPE* getInstance() { SingletonInstanceData& data = getSingletonData(); @@ -171,13 +141,11 @@ public: llwarns << "Trying to access deleted singleton " << typeid(DERIVED_TYPE).name() << " creating new instance" << llendl; } - if (!data.mSingletonInstance) + if (data.mInitState == INITIALIZING) { - data.mInitState = CONSTRUCTING; - data.mSingletonInstance = new DERIVED_TYPE(); - data.mInitState = INITIALIZING; - data.mSingletonInstance->initSingleton(); + // go ahead and flag ourselves as initialized so we can be reentrant during initialization data.mInitState = INITIALIZED; + data.mSingletonInstance->initSingleton(); } return data.mSingletonInstance; @@ -185,7 +153,7 @@ public: static DERIVED_TYPE* getIfExists() { - SingletonInstanceData& data = getSingletonData(); + SingletonInstanceData& data = getData(); return data.mSingletonInstance; } @@ -211,20 +179,14 @@ public: } private: + static SingletonInstanceData& getSingletonData() { // this is static to cache the lookup results - static void * & registry = LLSingletonRegistry::get(); - - // *TODO - look into making this threadsafe - if(NULL == registry) - { - static SingletonInstanceData data; - registry = &data; - } - - return *static_cast(registry); + static SingletonInstanceData sData; + return sData; } + virtual void initSingleton() {} }; -- cgit v1.2.3 From 5622a47403443fb7b4b459fac1b0206deb2eadb3 Mon Sep 17 00:00:00 2001 From: Richard Linden Date: Wed, 24 Apr 2013 17:44:34 -0700 Subject: BUILDFIX: method name was wrong --- indra/llcommon/llsingleton.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/llcommon/llsingleton.h') diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 697d1b042a..e3b614cf86 100644 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -153,7 +153,7 @@ public: static DERIVED_TYPE* getIfExists() { - SingletonInstanceData& data = getData(); + SingletonInstanceData& data = getSingletonData(); return data.mSingletonInstance; } -- cgit v1.2.3 From 1a01542e22a7c602b4e2733a42d933600c5e6609 Mon Sep 17 00:00:00 2001 From: Richard Linden Date: Wed, 24 Apr 2013 21:04:51 -0700 Subject: BUILDFIX: singleton unit test could not resurrect singleton --- indra/llcommon/llsingleton.h | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'indra/llcommon/llsingleton.h') diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index e3b614cf86..1cbefb1cd0 100644 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -77,6 +77,11 @@ private: SingletonInstanceData() : mSingletonInstance(NULL), mInitState(CONSTRUCTING) + { + construct(); + } + + void construct() { mSingletonInstance = new DERIVED_TYPE(); mInitState = INITIALIZING; @@ -139,6 +144,7 @@ public: if (data.mInitState == DELETED) { llwarns << "Trying to access deleted singleton " << typeid(DERIVED_TYPE).name() << " creating new instance" << llendl; + data.construct(); } if (data.mInitState == INITIALIZING) -- cgit v1.2.3 From 81291381f876e7a2c01889ddc3d849d88520af41 Mon Sep 17 00:00:00 2001 From: Richard Linden Date: Fri, 26 Apr 2013 11:21:16 -0700 Subject: SH-4080 WIP interesting: random crash on Mac fixed Mac crash related to non-reentrant singleton constructor --- indra/llcommon/llsingleton.h | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'indra/llcommon/llsingleton.h') diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 1cbefb1cd0..165344ed19 100644 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -83,8 +83,10 @@ private: void construct() { + sReentrantConstructorGuard = true; mSingletonInstance = new DERIVED_TYPE(); mInitState = INITIALIZING; + sReentrantConstructorGuard = false; } ~SingletonInstanceData() @@ -174,7 +176,7 @@ public: // Use this to avoid accessing singletons before the can safely be constructed static bool instanceExists() { - return getSingletonData().mInitState == INITIALIZED; + return sReentrantConstructorGuard || getSingletonData().mInitState == INITIALIZED; } // Has this singleton already been deleted? @@ -194,6 +196,11 @@ private: } virtual void initSingleton() {} + + static bool sReentrantConstructorGuard; }; +template +bool LLSingleton::sReentrantConstructorGuard = false; + #endif -- cgit v1.2.3 From a4e53da0b0d8f227865303a785d3d65848cd4ade Mon Sep 17 00:00:00 2001 From: Richard Linden Date: Fri, 26 Apr 2013 16:14:44 -0700 Subject: SH-4080 WIP interesting: random crash on Mac fixed Mac crash related to non-reentrant singleton constructor --- indra/llcommon/llsingleton.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'indra/llcommon/llsingleton.h') diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 165344ed19..829c7e4192 100644 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -176,7 +176,7 @@ public: // Use this to avoid accessing singletons before the can safely be constructed static bool instanceExists() { - return sReentrantConstructorGuard || getSingletonData().mInitState == INITIALIZED; + return !sReentrantConstructorGuard && getSingletonData().mInitState == INITIALIZED; } // Has this singleton already been deleted? -- cgit v1.2.3 From 215612bde86969e49ff9577d1d64390696280b3e Mon Sep 17 00:00:00 2001 From: Richard Linden Date: Sat, 27 Apr 2013 07:42:32 -0700 Subject: SH-4080 WIP interesting: random crash on Mac more singleton cleanup to eliminate crashes on startup/exit --- indra/llcommon/llsingleton.h | 95 +++++++++++++++++++++----------------------- 1 file changed, 45 insertions(+), 50 deletions(-) (limited to 'indra/llcommon/llsingleton.h') diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 829c7e4192..7558bf4fae 100644 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -61,6 +61,7 @@ class LLSingleton : private boost::noncopyable private: typedef enum e_init_state { + UNINITIALIZED, CONSTRUCTING, INITIALIZING, INITIALIZED, @@ -68,30 +69,23 @@ private: } EInitState; // stores pointer to singleton instance - // and tracks initialization state of singleton - struct SingletonInstanceData + struct SingletonLifetimeManager { - EInitState mInitState; - DERIVED_TYPE* mSingletonInstance; - - SingletonInstanceData() - : mSingletonInstance(NULL), - mInitState(CONSTRUCTING) + SingletonLifetimeManager() { construct(); } - void construct() + static void construct() { - sReentrantConstructorGuard = true; - mSingletonInstance = new DERIVED_TYPE(); - mInitState = INITIALIZING; - sReentrantConstructorGuard = false; + sData.mInitState = CONSTRUCTING; + sData.mInstance = new DERIVED_TYPE(); + sData.mInitState = INITIALIZING; } - ~SingletonInstanceData() + ~SingletonLifetimeManager() { - if (mInitState != DELETED) + if (sData.mInitState != DELETED) { deleteSingleton(); } @@ -101,9 +95,8 @@ private: public: virtual ~LLSingleton() { - SingletonInstanceData& data = getSingletonData(); - data.mSingletonInstance = NULL; - data.mInitState = DELETED; + sData.mInstance = NULL; + sData.mInitState = DELETED; } /** @@ -128,41 +121,45 @@ public: */ static void deleteSingleton() { - delete getSingletonData().mSingletonInstance; - getSingletonData().mSingletonInstance = NULL; - getSingletonData().mInitState = DELETED; + delete sData.mInstance; + sData.mInstance = NULL; + sData.mInitState = DELETED; } static DERIVED_TYPE* getInstance() { - SingletonInstanceData& data = getSingletonData(); + static SingletonLifetimeManager sLifeTimeMgr; - if (data.mInitState == CONSTRUCTING) + switch (sData.mInitState) { + case UNINITIALIZED: + // should never be uninitialized at this point + llassert(false); + break; + case CONSTRUCTING: llerrs << "Tried to access singleton " << typeid(DERIVED_TYPE).name() << " from singleton constructor!" << llendl; - } - - if (data.mInitState == DELETED) - { - llwarns << "Trying to access deleted singleton " << typeid(DERIVED_TYPE).name() << " creating new instance" << llendl; - data.construct(); - } - - if (data.mInitState == INITIALIZING) - { + break; + case INITIALIZING: // go ahead and flag ourselves as initialized so we can be reentrant during initialization - data.mInitState = INITIALIZED; - data.mSingletonInstance->initSingleton(); + sData.mInitState = INITIALIZED; + sData.mInstance->initSingleton(); + return sData.mInstance; + break; + case INITIALIZED: + return sData.mInstance; + case DELETED: + llwarns << "Trying to access deleted singleton " << typeid(DERIVED_TYPE).name() << " creating new instance" << llendl; + SingletonLifetimeManager::construct(); + return sData.mInstance; } - - return data.mSingletonInstance; + + return NULL; } static DERIVED_TYPE* getIfExists() { - SingletonInstanceData& data = getSingletonData(); - return data.mSingletonInstance; + return sData.mInstance; } // Reference version of getInstance() @@ -176,31 +173,29 @@ public: // Use this to avoid accessing singletons before the can safely be constructed static bool instanceExists() { - return !sReentrantConstructorGuard && getSingletonData().mInitState == INITIALIZED; + return sData.mInitState == INITIALIZED; } // Has this singleton already been deleted? // Use this to avoid accessing singletons from a static object's destructor static bool destroyed() { - return getSingletonData().mInitState == DELETED; + return sData.mInitState == DELETED; } private: - static SingletonInstanceData& getSingletonData() - { - // this is static to cache the lookup results - static SingletonInstanceData sData; - return sData; - } - virtual void initSingleton() {} - static bool sReentrantConstructorGuard; + struct SingletonData + { + EInitState mInitState; + DERIVED_TYPE* mInstance; + }; + static SingletonData sData; }; template -bool LLSingleton::sReentrantConstructorGuard = false; +typename LLSingleton::SingletonData LLSingleton::sData; #endif -- cgit v1.2.3 From 41e5bf346eaa0a43646058691cc8090ddfe498e9 Mon Sep 17 00:00:00 2001 From: Richard Linden Date: Mon, 29 Apr 2013 10:58:37 -0700 Subject: SH-4080 WIP interesting: random crash on Mac fixed singleton unit test resurrecting a singleton now properly calls initSingleton() --- indra/llcommon/llsingleton.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'indra/llcommon/llsingleton.h') diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 7558bf4fae..40002313f1 100644 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -136,21 +136,22 @@ public: case UNINITIALIZED: // should never be uninitialized at this point llassert(false); - break; + return NULL; case CONSTRUCTING: llerrs << "Tried to access singleton " << typeid(DERIVED_TYPE).name() << " from singleton constructor!" << llendl; - break; + return NULL; case INITIALIZING: // go ahead and flag ourselves as initialized so we can be reentrant during initialization sData.mInitState = INITIALIZED; sData.mInstance->initSingleton(); return sData.mInstance; - break; case INITIALIZED: return sData.mInstance; case DELETED: llwarns << "Trying to access deleted singleton " << typeid(DERIVED_TYPE).name() << " creating new instance" << llendl; SingletonLifetimeManager::construct(); + sData.mInitState = INITIALIZED; + sData.mInstance->initSingleton(); return sData.mInstance; } -- cgit v1.2.3 From dd07e240538ae8eaed07526396069dae82448576 Mon Sep 17 00:00:00 2001 From: Richard Linden Date: Thu, 2 May 2013 19:38:35 -0700 Subject: SH-4080 WIP interesting: random crash on Mac added comments to llsingleton.h --- indra/llcommon/llsingleton.h | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'indra/llcommon/llsingleton.h') diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 40002313f1..1e87d9bd7b 100644 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -143,6 +143,8 @@ public: case INITIALIZING: // go ahead and flag ourselves as initialized so we can be reentrant during initialization sData.mInitState = INITIALIZED; + // initialize singleton after constructing it so that it can reference other singletons which in turn depend on it, + // thus breaking cyclic dependencies sData.mInstance->initSingleton(); return sData.mInstance; case INITIALIZED: @@ -150,6 +152,7 @@ public: case DELETED: llwarns << "Trying to access deleted singleton " << typeid(DERIVED_TYPE).name() << " creating new instance" << llendl; SingletonLifetimeManager::construct(); + // same as first time construction sData.mInitState = INITIALIZED; sData.mInstance->initSingleton(); return sData.mInstance; @@ -190,6 +193,8 @@ private: struct SingletonData { + // explicitly has a default constructor so that member variables are zero initialized in BSS + // and only changed by singleton logic, not constructor running during startup EInitState mInitState; DERIVED_TYPE* mInstance; }; -- cgit v1.2.3 From 13f43fdc5bd046f7857f06254c84b8993bdcc50a Mon Sep 17 00:00:00 2001 From: Richard Linden Date: Mon, 20 May 2013 18:56:40 -0700 Subject: BUILDFIX: mac gcc fix --- indra/llcommon/llsingleton.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'indra/llcommon/llsingleton.h') diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 1e87d9bd7b..b9cb8e3d41 100644 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -67,6 +67,11 @@ private: INITIALIZED, DELETED } EInitState; + + static DERIVED_TYPE* constructSingleton() + { + return new DERIVED_TYPE(); + } // stores pointer to singleton instance struct SingletonLifetimeManager @@ -79,7 +84,7 @@ private: static void construct() { sData.mInitState = CONSTRUCTING; - sData.mInstance = new DERIVED_TYPE(); + sData.mInstance = constructSingleton(); sData.mInitState = INITIALIZING; } -- cgit v1.2.3