From 331e932857e1156a68b6d39d3ea2d8c1f39ec7ae Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 22 May 2015 14:02:24 -0400 Subject: MAINT-5232: Clean up some dubious LLSingleton methods. Remove evil getIfExists() method, used by no one. Remove evil destroyed() method, used in exactly three places -- one of which is a test. Replace with equally evil instanceExists() method, which is used EVERYWHERE -- sigh. --- indra/llcommon/llsingleton.h | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) (limited to 'indra/llcommon/llsingleton.h') diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 6e6291a165..a4877eed1f 100755 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -166,31 +166,19 @@ public: return NULL; } - static DERIVED_TYPE* getIfExists() - { - return sData.mInstance; - } - // Reference version of getInstance() // Preferred over getInstance() as it disallows checking for NULL static DERIVED_TYPE& instance() { return *getInstance(); } - - // Has this singleton been created uet? - // Use this to avoid accessing singletons before the can safely be constructed + + // Has this singleton been created yet? + // Use this to avoid accessing singletons before they can safely be constructed. static bool instanceExists() { 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 sData.mInitState == DELETED; - } private: -- cgit v1.3 From df8da8c013dfa7fc1c51b483208001cdd97269ba Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 28 May 2015 17:03:30 -0400 Subject: MAINT-5232: Stop documenting deprecated alternative LLSingleton usage. --- indra/llcommon/llsingleton.h | 38 ++++++++++++++------------------------ 1 file changed, 14 insertions(+), 24 deletions(-) (limited to 'indra/llcommon/llsingleton.h') diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index a4877eed1f..5d2a26cae5 100755 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -30,30 +30,20 @@ #include #include -// 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. -// -// There are two ways to use LLSingleton. The first way is to inherit from it -// while using the typename that you'd like to be static as the template -// parameter, like so: -// -// class Foo: public LLSingleton{}; -// -// Foo& instance = Foo::instance(); -// -// The second way is to use the singleton class directly, without inheritance: -// -// typedef LLSingleton FooSingleton; -// -// Foo& instance = FooSingleton::instance(); -// -// In this case, the class being managed as a singleton needs to provide an -// initSingleton() method since the LLSingleton virtual method won't be -// available -// -// As currently written, it is not thread-safe. - +/** + * 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. + * + * Derive your class from LLSingleton, passing your subclass name as + * LLSingleton's template parameter, like so: + * + * class Foo: public LLSingleton{}; + * + * Foo& instance = Foo::instance(); + * + * As currently written, LLSingleton is not thread-safe. + */ template class LLSingleton : private boost::noncopyable { -- cgit v1.3 From d792baf9f7220a374788b35789335a7ae2e62369 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 24 Jun 2015 21:14:55 -0400 Subject: MAINT-5232: Introduce inter-LLSingleton dependency tracking. Introduce LLSingleton::cleanupSingleton() canonical method as the place to put any subclass cleanup logic that might take nontrivial realtime or throw an exception. Neither is appropriate in a destructor. Track all extant LLSingleton subclass instances on a master list, which permits adding LLSingletonBase::cleanupAll() and deleteAll() methods. Also notice when any LLSingleton subclass constructor (or initSingleton() method) calls instance() or getInstance() for another LLSingleton, and capture that other LLSingleton instance as a dependency of the first. This permits cleanupAll() and deleteAll() to perform a dependency sort on the master list, thus cleaning up (or deleting) leaf LLSingletons AFTER the LLSingletons that depend on them. Make C++ runtime's final static destructor call LLSingletonBase::deleteAll() instead of deleting individual LLSingleton instances in arbitrary order. Eliminate "llerror.h" from llsingleton.h, a longstanding TODO. --- indra/llcommon/llsingleton.cpp | 290 ++++++++++++++++++++++++- indra/llcommon/llsingleton.h | 480 ++++++++++++++++++++++++++++++----------- 2 files changed, 639 insertions(+), 131 deletions(-) (limited to 'indra/llcommon/llsingleton.h') diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index 9b49e52377..204c0d24d0 100755 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -25,7 +25,295 @@ */ #include "linden_common.h" - #include "llsingleton.h" +#include "llerror.h" +#include "lldependencies.h" +#include +#include +#include +#include + +// Our master list of all LLSingletons is itself an LLSingleton. We used to +// store it in a function-local static, but that could get destroyed before +// the last of the LLSingletons -- and ~LLSingletonBase() definitely wants to +// remove itself from the master list. Since the whole point of this master +// list is to help track inter-LLSingleton dependencies, and since we have +// this implicit dependency from every LLSingleton to the master list, make it +// an LLSingleton. +class LLSingletonBase::MasterList: + public LLSingleton +{ +private: + friend class LLSingleton; + +public: + // No need to make this private with accessors; nobody outside this source + // file can see it. + LLSingletonBase::list_t mList; +}; + +//static +LLSingletonBase::list_t& LLSingletonBase::get_master() +{ + return LLSingletonBase::MasterList::instance().mList; +} + +void LLSingletonBase::add_master() +{ + // As each new LLSingleton is constructed, add to the master list. + get_master().push_back(this); +} + +void LLSingletonBase::remove_master() +{ + // When an LLSingleton is destroyed, remove from master list. + // add_master() used to capture the iterator to the newly-added list item + // so we could directly erase() it from the master list. Unfortunately + // that runs afoul of destruction-dependency order problems. So search the + // master list, and remove this item IF FOUND. We have few enough + // LLSingletons, and they are so rarely destroyed (once per run), that the + // cost of a linear search should not be an issue. + get_master().remove(this); +} + +// Wrapping our initializing list in a static method ensures that it will be +// constructed on demand. This list doesn't also need to be in an LLSingleton +// because (a) it should be empty by program shutdown and (b) none of our +// destructors reference it. +//static +LLSingletonBase::list_t& LLSingletonBase::get_initializing() +{ + static list_t sList; + return sList; +} + +LLSingletonBase::LLSingletonBase(): + mCleaned(false), + mDeleteSingleton(NULL) +{ + // Make this the currently-initializing LLSingleton. + push_initializing(); +} + +LLSingletonBase::~LLSingletonBase() {} + +void LLSingletonBase::push_initializing() +{ + get_initializing().push_back(this); +} + +void LLSingletonBase::pop_initializing() +{ + list_t& list(get_initializing()); + if (list.empty()) + { + LL_ERRS() << "Underflow in stack of currently-initializing LLSingletons at " + << typeid(*this).name() << "::getInstance()" << LL_ENDL; + } + if (list.back() != this) + { + LL_ERRS() << "Push/pop mismatch in stack of currently-initializing LLSingletons: " + << typeid(*this).name() << "::getInstance() trying to pop " + << typeid(*list.back()).name() << LL_ENDL; + } + // Here we're sure that list.back() == this. Whew, pop it. + list.pop_back(); +} + +void LLSingletonBase::capture_dependency() +{ + // Did this getInstance() call come from another LLSingleton, or from + // vanilla application code? Note that although this is a nontrivial + // method, the vast majority of its calls arrive here with initializing + // empty(). + list_t& initializing(get_initializing()); + if (! initializing.empty()) + { + // getInstance() is being called by some other LLSingleton. But -- is + // this a circularity? That is, does 'this' already appear in the + // initializing stack? + // For what it's worth, normally 'initializing' should contain very + // few elements. + list_t::const_iterator found = + std::find(initializing.begin(), initializing.end(), this); + if (found != initializing.end()) + { + // Report the circularity. Requiring the coder to dig through the + // logic to diagnose exactly how we got here is less than helpful. + std::ostringstream out; + for ( ; found != initializing.end(); ++found) + { + // 'found' is an iterator; *found is an LLSingletonBase*; **found + // is the actual LLSingletonBase instance. + out << typeid(**found).name() << " -> "; + } + // DEBUGGING: Initially, make this crump. We want to know how bad + // the problem is before we add it to the long, sad list of + // ominous warnings that everyone always ignores. + LL_ERRS() << "LLSingleton circularity: " << out.str() + << typeid(*this).name() << LL_ENDL; + } + else + { + // Here 'this' is NOT already in the 'initializing' stack. Great! + // Record the dependency. + // initializing.back() is the LLSingletonBase* currently being + // initialized. Store 'this' in its mDepends set. + initializing.back()->mDepends.insert(this); + } + } +} + +//static +LLSingletonBase::vec_t LLSingletonBase::dep_sort() +{ + // While it would theoretically be possible to maintain a static + // SingletonDeps through the life of the program, dynamically adding and + // removing LLSingletons as they are created and destroyed, in practice + // it's less messy to construct it on demand. The overhead of doing so + // should happen basically twice: once for cleanupAll(), once for + // deleteAll(). + typedef LLDependencies SingletonDeps; + SingletonDeps sdeps; + list_t& master(get_master()); + BOOST_FOREACH(LLSingletonBase* sp, master) + { + // Build the SingletonDeps structure by adding, for each + // LLSingletonBase* sp in the master list, sp itself. It has no + // associated value type in our SingletonDeps, hence the 0. We don't + // record the LLSingletons it must follow; rather, we record the ones + // it must precede. Copy its mDepends to a KeyList to express that. + sdeps.add(sp, 0, + SingletonDeps::KeyList(), + SingletonDeps::KeyList(sp->mDepends.begin(), sp->mDepends.end())); + } + vec_t ret; + ret.reserve(master.size()); + // We should be able to effect this with a transform_iterator that + // extracts just the first (key) element from each sorted_iterator, then + // uses vec_t's range constructor... but frankly this is more + // straightforward, as long as we remember the above reserve() call! + BOOST_FOREACH(SingletonDeps::sorted_iterator::value_type pair, sdeps.sort()) + { + ret.push_back(pair.first); + } + // The master list is not itself pushed onto the master list. Add it as + // the very last entry -- it is the LLSingleton on which ALL others + // depend! -- so our caller will process it. + ret.push_back(MasterList::getInstance()); + return ret; +} + +//static +void LLSingletonBase::cleanupAll() +{ + // It's essential to traverse these in dependency order. + BOOST_FOREACH(LLSingletonBase* sp, dep_sort()) + { + // Call cleanupSingleton() only if we haven't already done so for this + // instance. + if (! sp->mCleaned) + { + sp->mCleaned = true; + + try + { + sp->cleanupSingleton(); + } + catch (const std::exception& e) + { + LL_WARNS() << "Exception in " << typeid(*sp).name() + << "::cleanupSingleton(): " << e.what() << LL_ENDL; + } + catch (...) + { + LL_WARNS() << "Unknown exception in " << typeid(*sp).name() + << "::cleanupSingleton()" << LL_ENDL; + } + } + } +} + +//static +void LLSingletonBase::deleteAll() +{ + // It's essential to traverse these in dependency order. + BOOST_FOREACH(LLSingletonBase* sp, dep_sort()) + { + // Capture the class name first: in case of exception, don't count on + // being able to extract it later. + const char* name = typeid(*sp).name(); + try + { + // Call static method through instance function pointer. + if (! sp->mDeleteSingleton) + { + // This Should Not Happen... but carry on. + LL_WARNS() << name << "::mDeleteSingleton not initialized!" << LL_ENDL; + } + else + { + // properly initialized: call it. + // From this point on, DO NOT DEREFERENCE sp! + sp->mDeleteSingleton(); + } + } + catch (const std::exception& e) + { + LL_WARNS() << "Exception in " << name + << "::deleteSingleton(): " << e.what() << LL_ENDL; + } + catch (...) + { + LL_WARNS() << "Unknown exception in " << name + << "::deleteSingleton()" << LL_ENDL; + } + } +} + +/*------------------------ Final cleanup management ------------------------*/ +class LLSingletonBase::MasterRefcount +{ +public: + // store a POD int so it will be statically initialized to 0 + int refcount; +}; +static LLSingletonBase::MasterRefcount sMasterRefcount; + +LLSingletonBase::ref_ptr_t LLSingletonBase::get_master_refcount() +{ + // Calling this method constructs a new ref_ptr_t, which implicitly calls + // intrusive_ptr_add_ref(MasterRefcount*). + return &sMasterRefcount; +} + +void intrusive_ptr_add_ref(LLSingletonBase::MasterRefcount* mrc) +{ + // Count outstanding SingletonLifetimeManager instances. + ++mrc->refcount; +} + +void intrusive_ptr_release(LLSingletonBase::MasterRefcount* mrc) +{ + // Notice when each SingletonLifetimeManager instance is destroyed. + if (! --mrc->refcount) + { + // The last instance was destroyed. Time to kill any remaining + // LLSingletons -- but in dependency order. + LLSingletonBase::deleteAll(); + } +} + +/*---------------------------- Logging helpers -----------------------------*/ +//static +void LLSingletonBase::logerrs(const char* p1, const char* p2, const char* p3) +{ + LL_ERRS() << p1 << p2 << p3 << LL_ENDL; +} +//static +void LLSingletonBase::logwarns(const char* p1, const char* p2, const char* p3) +{ + LL_WARNS() << p1 << p2 << p3 << LL_ENDL; +} diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 5d2a26cae5..7706ed53f2 100755 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -25,10 +25,151 @@ #ifndef LLSINGLETON_H #define LLSINGLETON_H -#include "llerror.h" // *TODO: eliminate this - -#include #include +#include +#include +#include +#include +#include + +// TODO: +// Tests for all this! +class LLSingletonBase: private boost::noncopyable +{ +public: + class MasterList; + class MasterRefcount; + typedef boost::intrusive_ptr ref_ptr_t; + +private: + // All existing LLSingleton instances are tracked in this master list. + typedef std::list list_t; + static list_t& get_master(); + // This, on the other hand, is a stack whose top indicates the LLSingleton + // currently being initialized. + static list_t& get_initializing(); + // Produce a vector of master list, in dependency order. + typedef std::vector vec_t; + static vec_t dep_sort(); + + bool mCleaned; // cleanupSingleton() has been called + // we directly depend on these other LLSingletons + typedef boost::unordered_set set_t; + set_t mDepends; + +protected: + // Base-class constructor should only be invoked by the DERIVED_TYPE + // constructor. + LLSingletonBase(); + virtual ~LLSingletonBase(); + + // Every new LLSingleton should be added to/removed from the master list + void add_master(); + void remove_master(); + // with a little help from our friends. + template friend class LLSingleton_manage_master; + + // Maintain a stack of the LLSingleton subclass instance currently being + // initialized. We use this to notice direct dependencies: we want to know + // if A requires B. We deduce that if while initializing A, control + // reaches B::getInstance(). + // We want &A to be at the top of that stack during both A::A() and + // A::initSingleton(), since a call to B::getInstance() might occur during + // either. + // Unfortunately the desired timespan does not correspond neatly with a + // single C++ scope, else we'd use RAII to track it. But we do know that + // LLSingletonBase's constructor definitely runs just before + // LLSingleton's, which runs just before the specific subclass's. + void push_initializing(); + // LLSingleton is, and must remain, the only caller to initSingleton(). + // That being the case, we control exactly when it happens -- and we can + // pop the stack immediately thereafter. + void pop_initializing(); + // If a given call to B::getInstance() happens during either A::A() or + // A::initSingleton(), record that A directly depends on B. + void capture_dependency(); + + // delegate LL_ERRS() logging to llsingleton.cpp + static void logerrs(const char* p1, const char* p2="", const char* p3=""); + // delegate LL_WARNS() logging to llsingleton.cpp + static void logwarns(const char* p1, const char* p2="", const char* p3=""); + + // obtain canonical ref_ptr_t + static ref_ptr_t get_master_refcount(); + + // Default methods in case subclass doesn't declare them. + virtual void initSingleton() {} + virtual void cleanupSingleton() {} + + // deleteSingleton() isn't -- and shouldn't be -- a virtual method. It's a + // class static. However, given only Foo*, deleteAll() does need to be + // able to reach Foo::deleteSingleton(). Make LLSingleton (which declares + // deleteSingleton()) store a pointer here. Since we know it's a static + // class method, a classic-C function pointer will do. + void (*mDeleteSingleton)(); + +public: + /** + * Call this to call the cleanupSingleton() method for every LLSingleton + * constructed since the start of the last cleanupAll() call. (Any + * LLSingleton constructed DURING a cleanupAll() call won't be cleaned up + * until the next cleanupAll() call.) cleanupSingleton() neither deletes + * nor destroys its LLSingleton; therefore it's safe to include logic that + * might take significant realtime or even throw an exception. + * + * The most important property of cleanupAll() is that cleanupSingleton() + * methods are called in dependency order, leaf classes last. Thus, given + * two LLSingleton subclasses A and B, if A's dependency on B is properly + * expressed as a B::getInstance() or B::instance() call during either + * A::A() or A::initSingleton(), B will be cleaned up after A. + * + * If a cleanupSingleton() method throws an exception, the exception is + * logged, but cleanupAll() attempts to continue calling the rest of the + * cleanupSingleton() methods. + */ + static void cleanupAll(); + /** + * Call this to call the deleteSingleton() method for every LLSingleton + * constructed since the start of the last deleteAll() call. (Any + * LLSingleton constructed DURING a deleteAll() call won't be cleaned up + * until the next deleteAll() call.) deleteSingleton() deletes and + * destroys its LLSingleton. Any cleanup logic that might take significant + * realtime -- or throw an exception -- must not be placed in your + * LLSingleton's destructor, but rather in its cleanupSingleton() method. + * + * The most important property of deleteAll() is that deleteSingleton() + * methods are called in dependency order, leaf classes last. Thus, given + * two LLSingleton subclasses A and B, if A's dependency on B is properly + * expressed as a B::getInstance() or B::instance() call during either + * A::A() or A::initSingleton(), B will be cleaned up after A. + * + * If a deleteSingleton() method throws an exception, the exception is + * logged, but deleteAll() attempts to continue calling the rest of the + * deleteSingleton() methods. + */ + static void deleteAll(); +}; + +// support ref_ptr_t +void intrusive_ptr_add_ref(LLSingletonBase::MasterRefcount*); +void intrusive_ptr_release(LLSingletonBase::MasterRefcount*); + +// Most of the time, we want LLSingleton_manage_master() to forward its +// methods to LLSingletonBase::add_master() and remove_master(). +template +struct LLSingleton_manage_master +{ + void add(LLSingletonBase* sb) { sb->add_master(); } + void remove(LLSingletonBase* sb) { sb->remove_master(); } +}; + +// But for the specific case of LLSingletonBase::MasterList, don't. +template <> +struct LLSingleton_manage_master +{ + void add(LLSingletonBase*) {} + void remove(LLSingletonBase*) {} +}; /** * LLSingleton implements the getInstance() method part of the Singleton @@ -42,146 +183,225 @@ * * Foo& instance = Foo::instance(); * + * LLSingleton recognizes a couple special methods in your derived class. + * + * If you override LLSingleton::initSingleton(), your method will be called + * immediately after the instance is constructed. This is useful for breaking + * circular dependencies: if you find that your LLSingleton subclass + * constructor references other LLSingleton subclass instances in a chain + * leading back to yours, move the instance reference from your constructor to + * your initSingleton() method. + * + * If you override LLSingleton::cleanupSingleton(), your method will be + * called if someone calls LLSingletonBase::cleanupAll(). The significant part + * of this promise is that cleanupAll() will call individual + * cleanupSingleton() methods in reverse dependency order. + * + * That is, consider LLSingleton subclasses C, B and A. A depends on B, which + * in turn depends on C. These dependencies are expressed as calls to + * B::instance() or B::getInstance(), and C::instance() or C::getInstance(). + * It shouldn't matter whether these calls appear in A::A() or + * A::initSingleton(), likewise B::B() or B::initSingleton(). + * + * We promise that if you later call LLSingletonBase::cleanupAll(): + * 1. A::cleanupSingleton() will be called before + * 2. B::cleanupSingleton(), which will be called before + * 3. C::cleanupSingleton(). + * Put differently, if your LLSingleton subclass constructor or + * initSingleton() method explicitly depends on some other LLSingleton + * subclass, you may continue to rely on that other subclass in your + * cleanupSingleton() method. + * + * We introduce a special cleanupSingleton() method because cleanupSingleton() + * operations can involve nontrivial realtime, or might throw an exception. A + * destructor should do neither! + * + * If your cleanupSingleton() method throws an exception, we log that + * exception but proceed with the remaining cleanupSingleton() calls. + * + * Similarly, if at some point you call LLSingletonBase::deleteAll(), all + * remaining LLSingleton instances will be destroyed in dependency order. (Or + * call MySubclass::deleteSingleton() to specifically destroy the canonical + * MySubclass instance.) + * * As currently written, LLSingleton is not thread-safe. */ template -class LLSingleton : private boost::noncopyable +class LLSingleton : public LLSingletonBase { - private: - typedef enum e_init_state - { - UNINITIALIZED, - CONSTRUCTING, - INITIALIZING, - INITIALIZED, - DELETED - } EInitState; - + typedef enum e_init_state + { + UNINITIALIZED = 0, // must be default-initialized state + CONSTRUCTING, + INITIALIZING, + INITIALIZED, + DELETED + } EInitState; + static DERIVED_TYPE* constructSingleton() { return new DERIVED_TYPE(); } - - // stores pointer to singleton instance - struct SingletonLifetimeManager - { - SingletonLifetimeManager() - { - construct(); - } - - static void construct() - { - sData.mInitState = CONSTRUCTING; - sData.mInstance = constructSingleton(); - sData.mInitState = INITIALIZING; - } - - ~SingletonLifetimeManager() - { - if (sData.mInitState != DELETED) - { - deleteSingleton(); - } - } - }; - + + // stores pointer to singleton instance + struct SingletonLifetimeManager + { + SingletonLifetimeManager(): + mMasterRefcount(LLSingletonBase::get_master_refcount()) + { + construct(); + } + + static void construct() + { + sData.mInitState = CONSTRUCTING; + sData.mInstance = constructSingleton(); + sData.mInitState = INITIALIZING; + } + + ~SingletonLifetimeManager() + { + // The dependencies between LLSingletons, and the arbitrary order + // of static-object destruction, mean that we DO NOT WANT this + // destructor to delete this LLSingleton. This destructor will run + // without regard to any other LLSingleton whose cleanup might + // depend on its existence. What we really want is to count the + // runtime's attempts to cleanup LLSingleton static data -- and on + // the very last one, call LLSingletonBase::deleteAll(). That + // method will properly honor cross-LLSingleton dependencies. This + // is why we store an intrusive_ptr to a MasterRefcount: our + // ref_ptr_t member counts SingletonLifetimeManager instances. + // Once the runtime destroys the last of these, THEN we can delete + // every remaining LLSingleton. + } + + LLSingletonBase::ref_ptr_t mMasterRefcount; + }; + +protected: + LLSingleton() + { + // populate base-class function pointer with the static + // deleteSingleton() function for this particular specialization + mDeleteSingleton = &deleteSingleton; + + // add this new instance to the master list + LLSingleton_manage_master().add(this); + } + public: - virtual ~LLSingleton() - { - sData.mInstance = NULL; - sData.mInitState = DELETED; - } - - /** - * @brief Immediately delete the singleton. - * - * A subsequent call to LLProxy::getInstance() will construct a new - * instance of the class. - * - * LLSingletons are normally destroyed after main() has exited and the C++ - * runtime is cleaning up statically-constructed objects. Some classes - * derived from LLSingleton have objects that are part of a runtime system - * that is terminated before main() exits. Calling the destructor of those - * objects after the termination of their respective systems can cause - * crashes and other problems during termination of the project. Using this - * method to destroy the singleton early can prevent these crashes. - * - * An example where this is needed is for a LLSingleton that has an APR - * object as a member that makes APR calls on destruction. The APR system is - * shut down explicitly before main() exits. This causes a crash on exit. - * Using this method before the call to apr_terminate() and NOT calling - * getInstance() again will prevent the crash. - */ - static void deleteSingleton() - { - delete sData.mInstance; - sData.mInstance = NULL; - sData.mInitState = DELETED; - } - - - static DERIVED_TYPE* getInstance() - { - static SingletonLifetimeManager sLifeTimeMgr; - - switch (sData.mInitState) - { - case UNINITIALIZED: - // should never be uninitialized at this point - llassert(false); - return NULL; - case CONSTRUCTING: - LL_ERRS() << "Tried to access singleton " << typeid(DERIVED_TYPE).name() << " from singleton constructor!" << LL_ENDL; - return NULL; - 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: - return sData.mInstance; - case DELETED: - LL_WARNS() << "Trying to access deleted singleton " << typeid(DERIVED_TYPE).name() << " creating new instance" << LL_ENDL; - SingletonLifetimeManager::construct(); - // same as first time construction - sData.mInitState = INITIALIZED; - sData.mInstance->initSingleton(); - return sData.mInstance; - } - - return NULL; - } - - // Reference version of getInstance() - // Preferred over getInstance() as it disallows checking for NULL - static DERIVED_TYPE& instance() - { - return *getInstance(); - } - - // Has this singleton been created yet? - // Use this to avoid accessing singletons before they can safely be constructed. - static bool instanceExists() - { - return sData.mInitState == INITIALIZED; - } + virtual ~LLSingleton() + { + // remove this instance from the master list + LLSingleton_manage_master().remove(this); + sData.mInstance = NULL; + sData.mInitState = DELETED; + } -private: + /** + * @brief Immediately delete the singleton. + * + * A subsequent call to LLProxy::getInstance() will construct a new + * instance of the class. + * + * Without an explicit call to LLSingletonBase::deleteAll(), LLSingletons + * are implicitly destroyed after main() has exited and the C++ runtime is + * cleaning up statically-constructed objects. Some classes derived from + * LLSingleton have objects that are part of a runtime system that is + * terminated before main() exits. Calling the destructor of those objects + * after the termination of their respective systems can cause crashes and + * other problems during termination of the project. Using this method to + * destroy the singleton early can prevent these crashes. + * + * An example where this is needed is for a LLSingleton that has an APR + * object as a member that makes APR calls on destruction. The APR system is + * shut down explicitly before main() exits. This causes a crash on exit. + * Using this method before the call to apr_terminate() and NOT calling + * getInstance() again will prevent the crash. + */ + static void deleteSingleton() + { + delete sData.mInstance; + sData.mInstance = NULL; + sData.mInitState = DELETED; + } - virtual void initSingleton() {} + static DERIVED_TYPE* getInstance() + { + static SingletonLifetimeManager sLifeTimeMgr; + + switch (sData.mInitState) + { + case UNINITIALIZED: + // should never be uninitialized at this point + logerrs("Uninitialized singleton ", typeid(DERIVED_TYPE).name()); + return NULL; + + case CONSTRUCTING: + logerrs("Tried to access singleton ", typeid(DERIVED_TYPE).name(), + " from singleton constructor!"); + return NULL; + + 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(); + // pop this off stack of initializing singletons + sData.mInstance->pop_initializing(); + break; + + case INITIALIZED: + break; + + case DELETED: + logwarns("Trying to access deleted singleton ", typeid(DERIVED_TYPE).name(), + " -- creating new instance"); + SingletonLifetimeManager::construct(); + // same as first time construction + sData.mInitState = INITIALIZED; + sData.mInstance->initSingleton(); + // pop this off stack of initializing singletons + sData.mInstance->pop_initializing(); + break; + } + + // By this point, if DERIVED_TYPE was pushed onto the initializing + // stack, it has been popped off. So the top of that stack, if any, is + // an LLSingleton that directly depends on DERIVED_TYPE. If this call + // came from another LLSingleton, rather than from vanilla application + // code, record the dependency. + sData.mInstance->capture_dependency(); + return sData.mInstance; + } - 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; - }; - static SingletonData sData; + // Reference version of getInstance() + // Preferred over getInstance() as it disallows checking for NULL + static DERIVED_TYPE& instance() + { + return *getInstance(); + } + + // Has this singleton been created yet? + // Use this to avoid accessing singletons before they can safely be constructed. + static bool instanceExists() + { + return sData.mInitState == INITIALIZED; + } + +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; + }; + static SingletonData sData; }; template -- cgit v1.3 From 0ea1b2a164e0d985c80c8a1afea4b31670efc907 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 25 Jun 2015 16:04:01 -0400 Subject: MAINT-5232: Try to avoid circularity between LLError and LLSingleton. Part of LLError's logging infrastructure is implemented with an LLSingleton. Therefore, attempts to log from within LLSingleton machinery could potentially go south if LLError's LLSingleton is not yet initialized. Introduce LLError::is_available() in llerrorcontrol.h and llerror.cpp. Make LLSingletonBase::logwarns() and logerrs() consult LLError::is_available() before attempting to use LL_WARNS or LL_ERRS, respectively. Moreover, make all LLSingleton internal logging use logwarns() and logerrs() instead of directly engaging LL_ERRS or LL_WARNS. --- indra/llcommon/llerror.cpp | 5 ++++ indra/llcommon/llerrorcontrol.h | 5 ++++ indra/llcommon/llsingleton.cpp | 64 ++++++++++++++++++++++++++++------------- indra/llcommon/llsingleton.h | 6 ++-- 4 files changed, 58 insertions(+), 22 deletions(-) (limited to 'indra/llcommon/llsingleton.h') diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 2100989316..54524bbe8e 100755 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -402,6 +402,11 @@ namespace namespace LLError { + bool is_available() + { + return Globals::instanceExists(); + } + class SettingsConfig : public LLRefCount { friend class Settings; diff --git a/indra/llcommon/llerrorcontrol.h b/indra/llcommon/llerrorcontrol.h index 56ac52e5de..56e84f7172 100755 --- a/indra/llcommon/llerrorcontrol.h +++ b/indra/llcommon/llerrorcontrol.h @@ -189,6 +189,11 @@ namespace LLError LL_COMMON_API std::string abbreviateFile(const std::string& filePath); LL_COMMON_API int shouldLogCallCount(); + + // Check whether Globals exists. This should only be used by LLSingleton + // infrastructure to avoid trying to log when our internal LLSingleton is + // unavailable -- circularity ensues. + LL_COMMON_API bool is_available(); }; #endif // LL_LLERRORCONTROL_H diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index 204c0d24d0..a3edf925ad 100755 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -28,9 +28,11 @@ #include "llsingleton.h" #include "llerror.h" +#include "llerrorcontrol.h" // LLError::is_available() #include "lldependencies.h" #include #include +#include // std::cerr in dire emergency #include #include @@ -108,14 +110,14 @@ void LLSingletonBase::pop_initializing() list_t& list(get_initializing()); if (list.empty()) { - LL_ERRS() << "Underflow in stack of currently-initializing LLSingletons at " - << typeid(*this).name() << "::getInstance()" << LL_ENDL; + logerrs("Underflow in stack of currently-initializing LLSingletons at ", + typeid(*this).name(), "::getInstance()"); } if (list.back() != this) { - LL_ERRS() << "Push/pop mismatch in stack of currently-initializing LLSingletons: " - << typeid(*this).name() << "::getInstance() trying to pop " - << typeid(*list.back()).name() << LL_ENDL; + logerrs("Push/pop mismatch in stack of currently-initializing LLSingletons: ", + typeid(*this).name(), "::getInstance() trying to pop ", + typeid(*list.back()).name()); } // Here we're sure that list.back() == this. Whew, pop it. list.pop_back(); @@ -151,8 +153,8 @@ void LLSingletonBase::capture_dependency() // DEBUGGING: Initially, make this crump. We want to know how bad // the problem is before we add it to the long, sad list of // ominous warnings that everyone always ignores. - LL_ERRS() << "LLSingleton circularity: " << out.str() - << typeid(*this).name() << LL_ENDL; + logerrs("LLSingleton circularity: ", out.str().c_str(), + typeid(*this).name()); } else { @@ -223,13 +225,13 @@ void LLSingletonBase::cleanupAll() } catch (const std::exception& e) { - LL_WARNS() << "Exception in " << typeid(*sp).name() - << "::cleanupSingleton(): " << e.what() << LL_ENDL; + logwarns("Exception in ", typeid(*sp).name(), + "::cleanupSingleton(): ", e.what()); } catch (...) { - LL_WARNS() << "Unknown exception in " << typeid(*sp).name() - << "::cleanupSingleton()" << LL_ENDL; + logwarns("Unknown exception in ", typeid(*sp).name(), + "::cleanupSingleton()"); } } } @@ -250,7 +252,7 @@ void LLSingletonBase::deleteAll() if (! sp->mDeleteSingleton) { // This Should Not Happen... but carry on. - LL_WARNS() << name << "::mDeleteSingleton not initialized!" << LL_ENDL; + logwarns(name, "::mDeleteSingleton not initialized!"); } else { @@ -261,13 +263,11 @@ void LLSingletonBase::deleteAll() } catch (const std::exception& e) { - LL_WARNS() << "Exception in " << name - << "::deleteSingleton(): " << e.what() << LL_ENDL; + logwarns("Exception in ", name, "::deleteSingleton(): ", e.what()); } catch (...) { - LL_WARNS() << "Unknown exception in " << name - << "::deleteSingleton()" << LL_ENDL; + logwarns("Unknown exception in ", name, "::deleteSingleton()"); } } } @@ -307,13 +307,37 @@ void intrusive_ptr_release(LLSingletonBase::MasterRefcount* mrc) /*---------------------------- Logging helpers -----------------------------*/ //static -void LLSingletonBase::logerrs(const char* p1, const char* p2, const char* p3) +void LLSingletonBase::logerrs(const char* p1, const char* p2, const char* p3, const char* p4) { - LL_ERRS() << p1 << p2 << p3 << LL_ENDL; + // Check LLError::is_available() because some of LLError's infrastructure + // is itself an LLSingleton. If that LLSingleton has not yet been + // initialized, trying to log will engage LLSingleton machinery... and + // around and around we go. + if (LLError::is_available()) + { + LL_ERRS() << p1 << p2 << p3 << p4 << LL_ENDL; + } + else + { + // Caller may be a test program, or something else whose stderr is + // visible to the user. + std::cerr << p1 << p2 << p3 << p4 << std::endl; + // The other important side effect of LL_ERRS() is + // https://www.youtube.com/watch?v=OMG7paGJqhQ (emphasis on OMG) + LLError::crashAndLoop(std::string()); + } } //static -void LLSingletonBase::logwarns(const char* p1, const char* p2, const char* p3) +void LLSingletonBase::logwarns(const char* p1, const char* p2, const char* p3, const char* p4) { - LL_WARNS() << p1 << p2 << p3 << LL_ENDL; + // See logerrs() remarks about is_available(). + if (LLError::is_available()) + { + LL_WARNS() << p1 << p2 << p3 << p4 << LL_ENDL; + } + else + { + std::cerr << p1 << p2 << p3 << p4 << std::endl; + } } diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 7706ed53f2..d66ea33c0c 100755 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -90,9 +90,11 @@ protected: void capture_dependency(); // delegate LL_ERRS() logging to llsingleton.cpp - static void logerrs(const char* p1, const char* p2="", const char* p3=""); + static void logerrs(const char* p1, const char* p2="", + const char* p3="", const char* p4=""); // delegate LL_WARNS() logging to llsingleton.cpp - static void logwarns(const char* p1, const char* p2="", const char* p3=""); + static void logwarns(const char* p1, const char* p2="", + const char* p3="", const char* p4=""); // obtain canonical ref_ptr_t static ref_ptr_t get_master_refcount(); -- cgit v1.3 From 0727519d9307ed09877073ef17b754526ee3f5b1 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 25 Jun 2015 16:07:27 -0400 Subject: MAINT-5232: Correct forward declaration of LLSingleton_manage_master. The forward declaration said it was a 'friend class', whereas the actual definition is a struct. MSVC dislikes that. --- 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 d66ea33c0c..25bdba0a0d 100755 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -67,7 +67,7 @@ protected: void add_master(); void remove_master(); // with a little help from our friends. - template friend class LLSingleton_manage_master; + template friend struct LLSingleton_manage_master; // Maintain a stack of the LLSingleton subclass instance currently being // initialized. We use this to notice direct dependencies: we want to know -- cgit v1.3 From 687efd84eabc524e339e61458b0cbf53f9a38f8a Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 26 Jun 2015 13:03:59 -0400 Subject: MAINT-5232: Loosen LLSingleton circularity constraints slightly. LLSingleton explicitly supports circular dependencies: initialization performed during an LLSingleton subclass's initSingleton() method may recursively call that same subclass's getInstance() method. On the other hand, circularity from a subclass constructor cannot be permitted, else getInstance() would have to return a partially-constructed object. Our dependency tracking circularity check initially forbade both. Loosen it to permit references from within initSingleton(). --- indra/llcommon/llsingleton.cpp | 19 +++++++++++++------ indra/llcommon/llsingleton.h | 22 +++++++++++----------- 2 files changed, 24 insertions(+), 17 deletions(-) (limited to 'indra/llcommon/llsingleton.h') diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index a3edf925ad..2813814ae1 100755 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -123,7 +123,7 @@ void LLSingletonBase::pop_initializing() list.pop_back(); } -void LLSingletonBase::capture_dependency() +void LLSingletonBase::capture_dependency(EInitState initState) { // Did this getInstance() call come from another LLSingleton, or from // vanilla application code? Note that although this is a nontrivial @@ -150,11 +150,18 @@ void LLSingletonBase::capture_dependency() // is the actual LLSingletonBase instance. out << typeid(**found).name() << " -> "; } - // DEBUGGING: Initially, make this crump. We want to know how bad - // the problem is before we add it to the long, sad list of - // ominous warnings that everyone always ignores. - logerrs("LLSingleton circularity: ", out.str().c_str(), - typeid(*this).name()); + // We promise to capture dependencies from both the constructor + // and the initSingleton() method, so an LLSingleton's instance + // pointer is on the initializing list during both. Now that we've + // detected circularity, though, we must distinguish the two. If + // the recursive call is from the constructor, we CAN'T honor it: + // otherwise we'd be returning a pointer to a partially- + // constructed object! But from initSingleton() is okay: that + // method exists specifically to support circularity. + // Decide which log helper to call based on initState. They have + // identical signatures. + ((initState == CONSTRUCTING)? logerrs : logwarns) + ("LLSingleton circularity: ", out.str().c_str(), typeid(*this).name(), ""); } else { diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 25bdba0a0d..a82101c367 100755 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -58,6 +58,15 @@ private: set_t mDepends; protected: + typedef enum e_init_state + { + UNINITIALIZED = 0, // must be default-initialized state + CONSTRUCTING, + INITIALIZING, + INITIALIZED, + DELETED + } EInitState; + // Base-class constructor should only be invoked by the DERIVED_TYPE // constructor. LLSingletonBase(); @@ -87,7 +96,7 @@ protected: void pop_initializing(); // If a given call to B::getInstance() happens during either A::A() or // A::initSingleton(), record that A directly depends on B. - void capture_dependency(); + void capture_dependency(EInitState); // delegate LL_ERRS() logging to llsingleton.cpp static void logerrs(const char* p1, const char* p2="", @@ -232,15 +241,6 @@ template class LLSingleton : public LLSingletonBase { private: - typedef enum e_init_state - { - UNINITIALIZED = 0, // must be default-initialized state - CONSTRUCTING, - INITIALIZING, - INITIALIZED, - DELETED - } EInitState; - static DERIVED_TYPE* constructSingleton() { return new DERIVED_TYPE(); @@ -377,7 +377,7 @@ public: // an LLSingleton that directly depends on DERIVED_TYPE. If this call // came from another LLSingleton, rather than from vanilla application // code, record the dependency. - sData.mInstance->capture_dependency(); + sData.mInstance->capture_dependency(sData.mInitState); return sData.mInstance; } -- cgit v1.3 From 8fee1565eb310081a7f3e26237ddd776c5a9aaaa Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Fri, 26 Jun 2015 15:28:57 -0400 Subject: MAINT-5232: Use LLError::Log::demangle() to log LLSingleton classes. --- indra/llcommon/llsingleton.cpp | 28 +++++++++++++++++----------- indra/llcommon/llsingleton.h | 10 +++++++--- 2 files changed, 24 insertions(+), 14 deletions(-) (limited to 'indra/llcommon/llsingleton.h') diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index 2813814ae1..b78110296f 100755 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -111,13 +111,13 @@ void LLSingletonBase::pop_initializing() if (list.empty()) { logerrs("Underflow in stack of currently-initializing LLSingletons at ", - typeid(*this).name(), "::getInstance()"); + demangle(typeid(*this).name()).c_str(), "::getInstance()"); } if (list.back() != this) { logerrs("Push/pop mismatch in stack of currently-initializing LLSingletons: ", - typeid(*this).name(), "::getInstance() trying to pop ", - typeid(*list.back()).name()); + demangle(typeid(*this).name()).c_str(), "::getInstance() trying to pop ", + demangle(typeid(*list.back()).name()).c_str()); } // Here we're sure that list.back() == this. Whew, pop it. list.pop_back(); @@ -148,7 +148,7 @@ void LLSingletonBase::capture_dependency(EInitState initState) { // 'found' is an iterator; *found is an LLSingletonBase*; **found // is the actual LLSingletonBase instance. - out << typeid(**found).name() << " -> "; + out << demangle(typeid(**found).name()) << " -> "; } // We promise to capture dependencies from both the constructor // and the initSingleton() method, so an LLSingleton's instance @@ -161,7 +161,8 @@ void LLSingletonBase::capture_dependency(EInitState initState) // Decide which log helper to call based on initState. They have // identical signatures. ((initState == CONSTRUCTING)? logerrs : logwarns) - ("LLSingleton circularity: ", out.str().c_str(), typeid(*this).name(), ""); + ("LLSingleton circularity: ", out.str().c_str(), + demangle(typeid(*this).name()).c_str(), ""); } else { @@ -232,12 +233,12 @@ void LLSingletonBase::cleanupAll() } catch (const std::exception& e) { - logwarns("Exception in ", typeid(*sp).name(), + logwarns("Exception in ", demangle(typeid(*sp).name()).c_str(), "::cleanupSingleton(): ", e.what()); } catch (...) { - logwarns("Unknown exception in ", typeid(*sp).name(), + logwarns("Unknown exception in ", demangle(typeid(*sp).name()).c_str(), "::cleanupSingleton()"); } } @@ -252,14 +253,14 @@ void LLSingletonBase::deleteAll() { // Capture the class name first: in case of exception, don't count on // being able to extract it later. - const char* name = typeid(*sp).name(); + const std::string name = demangle(typeid(*sp).name()); try { // Call static method through instance function pointer. if (! sp->mDeleteSingleton) { // This Should Not Happen... but carry on. - logwarns(name, "::mDeleteSingleton not initialized!"); + logwarns(name.c_str(), "::mDeleteSingleton not initialized!"); } else { @@ -270,11 +271,11 @@ void LLSingletonBase::deleteAll() } catch (const std::exception& e) { - logwarns("Exception in ", name, "::deleteSingleton(): ", e.what()); + logwarns("Exception in ", name.c_str(), "::deleteSingleton(): ", e.what()); } catch (...) { - logwarns("Unknown exception in ", name, "::deleteSingleton()"); + logwarns("Unknown exception in ", name.c_str(), "::deleteSingleton()"); } } } @@ -348,3 +349,8 @@ void LLSingletonBase::logwarns(const char* p1, const char* p2, const char* p3, c std::cerr << p1 << p2 << p3 << p4 << std::endl; } } + +std::string LLSingletonBase::demangle(const char* mangled) +{ + return LLError::Log::demangle(mangled); +} diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index a82101c367..253e0b9a6b 100755 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -104,6 +104,7 @@ protected: // delegate LL_WARNS() logging to llsingleton.cpp static void logwarns(const char* p1, const char* p2="", const char* p3="", const char* p4=""); + static std::string demangle(const char* mangled); // obtain canonical ref_ptr_t static ref_ptr_t get_master_refcount(); @@ -337,11 +338,13 @@ public: { case UNINITIALIZED: // should never be uninitialized at this point - logerrs("Uninitialized singleton ", typeid(DERIVED_TYPE).name()); + logerrs("Uninitialized singleton ", + demangle(typeid(DERIVED_TYPE).name()).c_str()); return NULL; case CONSTRUCTING: - logerrs("Tried to access singleton ", typeid(DERIVED_TYPE).name(), + logerrs("Tried to access singleton ", + demangle(typeid(DERIVED_TYPE).name()).c_str(), " from singleton constructor!"); return NULL; @@ -361,7 +364,8 @@ public: break; case DELETED: - logwarns("Trying to access deleted singleton ", typeid(DERIVED_TYPE).name(), + logwarns("Trying to access deleted singleton ", + demangle(typeid(DERIVED_TYPE).name()).c_str(), " -- creating new instance"); SingletonLifetimeManager::construct(); // same as first time construction -- cgit v1.3 From d4fb82c217bccda536f7a7b2ca1809bb8c2dba40 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 30 Jun 2015 14:49:18 -0400 Subject: MAINT-5232: Add tests for new LLSingleton dependency functionality. --- indra/llcommon/llsingleton.h | 6 +- indra/llcommon/tests/llsingleton_test.cpp | 205 ++++++++++++++++++++++++------ 2 files changed, 167 insertions(+), 44 deletions(-) (limited to 'indra/llcommon/llsingleton.h') diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 253e0b9a6b..6a7f27bed4 100755 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -32,8 +32,6 @@ #include #include -// TODO: -// Tests for all this! class LLSingletonBase: private boost::noncopyable { public: @@ -80,8 +78,8 @@ protected: // Maintain a stack of the LLSingleton subclass instance currently being // initialized. We use this to notice direct dependencies: we want to know - // if A requires B. We deduce that if while initializing A, control - // reaches B::getInstance(). + // if A requires B. We deduce a dependency if while initializing A, + // control reaches B::getInstance(). // We want &A to be at the top of that stack during both A::A() and // A::initSingleton(), since a call to B::getInstance() might occur during // either. diff --git a/indra/llcommon/tests/llsingleton_test.cpp b/indra/llcommon/tests/llsingleton_test.cpp index bed436283a..a05f650f25 100755 --- a/indra/llcommon/tests/llsingleton_test.cpp +++ b/indra/llcommon/tests/llsingleton_test.cpp @@ -30,46 +30,171 @@ #include "llsingleton.h" #include "../test/lltut.h" + +// Capture execution sequence by appending to log string. +std::string sLog; + +#define DECLARE_CLASS(CLS) \ +struct CLS: public LLSingleton \ +{ \ + static enum dep_flag { \ + DEP_NONE, /* no dependency */ \ + DEP_CTOR, /* dependency in ctor */ \ + DEP_INIT /* dependency in initSingleton */ \ + } sDepFlag; \ + \ + CLS(); \ + void initSingleton(); \ + void cleanupSingleton(); \ + ~CLS(); \ +}; \ + \ +CLS::dep_flag CLS::sDepFlag = DEP_NONE + +DECLARE_CLASS(A); +DECLARE_CLASS(B); + +#define DEFINE_MEMBERS(CLS, OTHER) \ +CLS::CLS() \ +{ \ + sLog.append(#CLS); \ + if (sDepFlag == DEP_CTOR) \ + { \ + (void)OTHER::instance(); \ + } \ +} \ + \ +void CLS::initSingleton() \ +{ \ + sLog.append("i" #CLS); \ + if (sDepFlag == DEP_INIT) \ + { \ + (void)OTHER::instance(); \ + } \ +} \ + \ +void CLS::cleanupSingleton() \ +{ \ + sLog.append("x" #CLS); \ +} \ + \ +CLS::~CLS() \ +{ \ + sLog.append("~" #CLS); \ +} + +DEFINE_MEMBERS(A, B) +DEFINE_MEMBERS(B, A) + namespace tut { - struct singleton - { - // We need a class created with the LLSingleton template to test with. - class LLSingletonTest: public LLSingleton - { - - }; - }; - - typedef test_group singleton_t; - typedef singleton_t::object singleton_object_t; - tut::singleton_t tut_singleton("LLSingleton"); - - template<> template<> - void singleton_object_t::test<1>() - { - - } - template<> template<> - void singleton_object_t::test<2>() - { - LLSingletonTest* singleton_test = LLSingletonTest::getInstance(); - ensure(singleton_test); - } - template<> template<> - void singleton_object_t::test<3>() - { - //Construct the instance - LLSingletonTest::getInstance(); - ensure(LLSingletonTest::instanceExists()); - - //Delete the instance - LLSingletonTest::deleteSingleton(); - ensure(!LLSingletonTest::instanceExists()); - - //Construct it again. - LLSingletonTest* singleton_test = LLSingletonTest::getInstance(); - ensure(singleton_test); - ensure(LLSingletonTest::instanceExists()); - } + struct singleton + { + // We need a class created with the LLSingleton template to test with. + class LLSingletonTest: public LLSingleton + { + + }; + }; + + typedef test_group singleton_t; + typedef singleton_t::object singleton_object_t; + tut::singleton_t tut_singleton("LLSingleton"); + + template<> template<> + void singleton_object_t::test<1>() + { + + } + template<> template<> + void singleton_object_t::test<2>() + { + LLSingletonTest* singleton_test = LLSingletonTest::getInstance(); + ensure(singleton_test); + } + + template<> template<> + void singleton_object_t::test<3>() + { + //Construct the instance + LLSingletonTest::getInstance(); + ensure(LLSingletonTest::instanceExists()); + + //Delete the instance + LLSingletonTest::deleteSingleton(); + ensure(!LLSingletonTest::instanceExists()); + + //Construct it again. + LLSingletonTest* singleton_test = LLSingletonTest::getInstance(); + ensure(singleton_test); + ensure(LLSingletonTest::instanceExists()); + } + +#define TESTS(CLS, OTHER, N0, N1, N2, N3) \ + template<> template<> \ + void singleton_object_t::test() \ + { \ + set_test_name("just " #CLS); \ + CLS::sDepFlag = CLS::DEP_NONE; \ + OTHER::sDepFlag = OTHER::DEP_NONE; \ + sLog.clear(); \ + \ + (void)CLS::instance(); \ + ensure_equals(sLog, #CLS "i" #CLS); \ + LLSingletonBase::cleanupAll(); \ + ensure_equals(sLog, #CLS "i" #CLS "x" #CLS); \ + LLSingletonBase::deleteAll(); \ + ensure_equals(sLog, #CLS "i" #CLS "x" #CLS "~" #CLS); \ + } \ + \ + template<> template<> \ + void singleton_object_t::test() \ + { \ + set_test_name(#CLS " ctor depends " #OTHER); \ + CLS::sDepFlag = CLS::DEP_CTOR; \ + OTHER::sDepFlag = OTHER::DEP_NONE; \ + sLog.clear(); \ + \ + (void)CLS::instance(); \ + ensure_equals(sLog, #CLS #OTHER "i" #OTHER "i" #CLS); \ + LLSingletonBase::cleanupAll(); \ + ensure_equals(sLog, #CLS #OTHER "i" #OTHER "i" #CLS "x" #CLS "x" #OTHER); \ + LLSingletonBase::deleteAll(); \ + ensure_equals(sLog, #CLS #OTHER "i" #OTHER "i" #CLS "x" #CLS "x" #OTHER "~" #CLS "~" #OTHER); \ + } \ + \ + template<> template<> \ + void singleton_object_t::test() \ + { \ + set_test_name(#CLS " init depends " #OTHER); \ + CLS::sDepFlag = CLS::DEP_INIT; \ + OTHER::sDepFlag = OTHER::DEP_NONE; \ + sLog.clear(); \ + \ + (void)CLS::instance(); \ + ensure_equals(sLog, #CLS "i" #CLS #OTHER "i" #OTHER); \ + LLSingletonBase::cleanupAll(); \ + ensure_equals(sLog, #CLS "i" #CLS #OTHER "i" #OTHER "x" #CLS "x" #OTHER); \ + LLSingletonBase::deleteAll(); \ + ensure_equals(sLog, #CLS "i" #CLS #OTHER "i" #OTHER "x" #CLS "x" #OTHER "~" #CLS "~" #OTHER); \ + } \ + \ + template<> template<> \ + void singleton_object_t::test() \ + { \ + set_test_name(#CLS " circular init"); \ + CLS::sDepFlag = CLS::DEP_INIT; \ + OTHER::sDepFlag = OTHER::DEP_CTOR; \ + sLog.clear(); \ + \ + (void)CLS::instance(); \ + ensure_equals(sLog, #CLS "i" #CLS #OTHER "i" #OTHER); \ + LLSingletonBase::cleanupAll(); \ + ensure_equals(sLog, #CLS "i" #CLS #OTHER "i" #OTHER "x" #CLS "x" #OTHER); \ + LLSingletonBase::deleteAll(); \ + ensure_equals(sLog, #CLS "i" #CLS #OTHER "i" #OTHER "x" #CLS "x" #OTHER "~" #CLS "~" #OTHER); \ + } + + TESTS(A, B, 4, 5, 6, 7) + TESTS(B, A, 8, 9, 10, 11) } -- cgit v1.3