summaryrefslogtreecommitdiff
path: root/indra
diff options
context:
space:
mode:
Diffstat (limited to 'indra')
-rw-r--r--indra/llcommon/llsingleton.cpp71
-rw-r--r--indra/llcommon/llsingleton.h213
-rw-r--r--indra/newview/llimagefiltersmanager.cpp2
-rw-r--r--indra/test/test.cpp4
4 files changed, 220 insertions, 70 deletions
diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp
index 9fbd78a000..c45c144570 100644
--- a/indra/llcommon/llsingleton.cpp
+++ b/indra/llcommon/llsingleton.cpp
@@ -134,12 +134,6 @@ LLSingletonBase::list_t& LLSingletonBase::get_initializing()
return LLSingletonBase::MasterList::instance().get_initializing_();
}
-//static
-LLSingletonBase::list_t& LLSingletonBase::get_initializing_from(MasterList* master)
-{
- return master->get_initializing_();
-}
-
LLSingletonBase::~LLSingletonBase() {}
void LLSingletonBase::push_initializing(const char* name)
@@ -156,7 +150,7 @@ void LLSingletonBase::pop_initializing()
if (list.empty())
{
logerrs("Underflow in stack of currently-initializing LLSingletons at ",
- demangle(typeid(*this).name()).c_str(), "::getInstance()");
+ classname(this).c_str(), "::getInstance()");
}
// Now we know list.back() exists: capture it
@@ -178,14 +172,39 @@ void LLSingletonBase::pop_initializing()
if (back != this)
{
logerrs("Push/pop mismatch in stack of currently-initializing LLSingletons: ",
- demangle(typeid(*this).name()).c_str(), "::getInstance() trying to pop ",
- demangle(typeid(*back).name()).c_str());
+ classname(this).c_str(), "::getInstance() trying to pop ",
+ classname(back).c_str());
}
// log AFTER popping so logging singletons don't cry circularity
log_initializing("Popping", typeid(*back).name());
}
+void LLSingletonBase::reset_initializing(list_t::size_type size)
+{
+ // called for cleanup in case the LLSingleton subclass constructor throws
+ // an exception
+
+ // The tricky thing about this, the reason we have a separate method
+ // instead of just calling pop_initializing(), is (hopefully remote)
+ // possibility that the exception happened *before* the
+ // push_initializing() call in LLSingletonBase's constructor. So only
+ // remove the stack top if in fact we've pushed something more than the
+ // previous size.
+ list_t& list(get_initializing());
+
+ while (list.size() > size)
+ {
+ list.pop_back();
+ }
+
+ // as in pop_initializing()
+ if (list.empty())
+ {
+ MasterList::instance().cleanup_initializing_();
+ }
+}
+
//static
void LLSingletonBase::log_initializing(const char* verb, const char* name)
{
@@ -197,7 +216,7 @@ void LLSingletonBase::log_initializing(const char* verb, const char* name)
ri != rend; ++ri)
{
LLSingletonBase* sb(*ri);
- LL_CONT << ' ' << demangle(typeid(*sb).name());
+ LL_CONT << ' ' << classname(sb);
}
LL_ENDL;
}
@@ -231,7 +250,7 @@ void LLSingletonBase::capture_dependency(list_t& initializing, EInitState initSt
// 'found' is an iterator; *found is an LLSingletonBase*; **found
// is the actual LLSingletonBase instance.
LLSingletonBase* foundp(*found);
- out << demangle(typeid(*foundp).name()) << " -> ";
+ out << classname(foundp) << " -> ";
}
// We promise to capture dependencies from both the constructor
// and the initSingleton() method, so an LLSingleton's instance
@@ -245,7 +264,7 @@ void LLSingletonBase::capture_dependency(list_t& initializing, EInitState initSt
if (initState == CONSTRUCTING)
{
logerrs("LLSingleton circularity in Constructor: ", out.str().c_str(),
- demangle(typeid(*this).name()).c_str(), "");
+ classname(this).c_str(), "");
}
else if (it_next == initializing.end())
{
@@ -256,14 +275,14 @@ void LLSingletonBase::capture_dependency(list_t& initializing, EInitState initSt
// Example: LLNotifications singleton initializes default channels.
// Channels register themselves with singleton once done.
logdebugs("LLSingleton circularity: ", out.str().c_str(),
- demangle(typeid(*this).name()).c_str(), "");
+ classname(this).c_str(), "");
}
else
{
// Actual circularity with other singleton (or single singleton is used extensively).
// Dependency can be unclear.
logwarns("LLSingleton circularity: ", out.str().c_str(),
- demangle(typeid(*this).name()).c_str(), "");
+ classname(this).c_str(), "");
}
}
else
@@ -276,8 +295,8 @@ void LLSingletonBase::capture_dependency(list_t& initializing, EInitState initSt
if (current->mDepends.insert(this).second)
{
// only log the FIRST time we hit this dependency!
- logdebugs(demangle(typeid(*current).name()).c_str(),
- " depends on ", demangle(typeid(*this).name()).c_str());
+ logdebugs(classname(current).c_str(),
+ " depends on ", classname(this).c_str());
}
}
}
@@ -336,19 +355,19 @@ void LLSingletonBase::cleanupAll()
sp->mCleaned = true;
logdebugs("calling ",
- demangle(typeid(*sp).name()).c_str(), "::cleanupSingleton()");
+ classname(sp).c_str(), "::cleanupSingleton()");
try
{
sp->cleanupSingleton();
}
catch (const std::exception& e)
{
- logwarns("Exception in ", demangle(typeid(*sp).name()).c_str(),
+ logwarns("Exception in ", classname(sp).c_str(),
"::cleanupSingleton(): ", e.what());
}
catch (...)
{
- logwarns("Unknown exception in ", demangle(typeid(*sp).name()).c_str(),
+ logwarns("Unknown exception in ", classname(sp).c_str(),
"::cleanupSingleton()");
}
}
@@ -363,7 +382,7 @@ void LLSingletonBase::deleteAll()
{
// Capture the class name first: in case of exception, don't count on
// being able to extract it later.
- const std::string name = demangle(typeid(*sp).name());
+ const std::string name = classname(sp);
try
{
// Call static method through instance function pointer.
@@ -440,7 +459,17 @@ void LLSingletonBase::logerrs(const char* p1, const char* p2, const char* p3, co
log(LLError::LEVEL_ERROR, p1, p2, p3, p4);
// The other important side effect of LL_ERRS() is
// https://www.youtube.com/watch?v=OMG7paGJqhQ (emphasis on OMG)
- LLError::crashAndLoop(std::string());
+ std::ostringstream out;
+ out << p1 << p2 << p3 << p4;
+ auto crash = LLError::getFatalFunction();
+ if (crash)
+ {
+ crash(out.str());
+ }
+ else
+ {
+ LLError::crashAndLoop(out.str());
+ }
}
std::string LLSingletonBase::demangle(const char* mangled)
diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h
index 03b7d5a349..0da6d548ab 100644
--- a/indra/llcommon/llsingleton.h
+++ b/indra/llcommon/llsingleton.h
@@ -55,7 +55,6 @@ private:
// This, on the other hand, is a stack whose top indicates the LLSingleton
// currently being initialized.
static list_t& get_initializing();
- static list_t& get_initializing_from(MasterList*);
// Produce a vector<LLSingletonBase*> of master list, in dependency order.
typedef std::vector<LLSingletonBase*> vec_t;
static vec_t dep_sort();
@@ -69,10 +68,11 @@ protected:
typedef enum e_init_state
{
UNINITIALIZED = 0, // must be default-initialized state
- CONSTRUCTING,
- INITIALIZING,
- INITIALIZED,
- DELETED
+ CONSTRUCTING, // within DERIVED_TYPE constructor
+ CONSTRUCTED, // finished DERIVED_TYPE constructor
+ INITIALIZING, // within DERIVED_TYPE::initSingleton()
+ INITIALIZED, // normal case
+ DELETED // deleteSingleton() or deleteAll() called
} EInitState;
// Define tag<T> to pass to our template constructor. You can't explicitly
@@ -112,6 +112,9 @@ protected:
// That being the case, we control exactly when it happens -- and we can
// pop the stack immediately thereafter.
void pop_initializing();
+ // Remove 'this' from the init stack in case of exception in the
+ // LLSingleton subclass constructor.
+ static void reset_initializing(list_t::size_type size);
private:
// logging
static void log_initializing(const char* verb, const char* name);
@@ -127,6 +130,10 @@ protected:
static void logwarns(const char* p1, const char* p2="",
const char* p3="", const char* p4="");
static std::string demangle(const char* mangled);
+ template <typename T>
+ static std::string classname() { return demangle(typeid(T).name()); }
+ template <typename T>
+ static std::string classname(T* ptr) { return demangle(typeid(*ptr).name()); }
// Default methods in case subclass doesn't declare them.
virtual void initSingleton() {}
@@ -190,7 +197,15 @@ struct LLSingleton_manage_master
void remove(LLSingletonBase* sb) { sb->remove_master(); }
void push_initializing(LLSingletonBase* sb) { sb->push_initializing(typeid(T).name()); }
void pop_initializing (LLSingletonBase* sb) { sb->pop_initializing(); }
- LLSingletonBase::list_t& get_initializing(T*) { return LLSingletonBase::get_initializing(); }
+ // used for init stack cleanup in case an LLSingleton subclass constructor
+ // throws an exception
+ void reset_initializing(LLSingletonBase::list_t::size_type size)
+ {
+ LLSingletonBase::reset_initializing(size);
+ }
+ // For any LLSingleton subclass except the MasterList, obtain the init
+ // stack from the MasterList singleton instance.
+ LLSingletonBase::list_t& get_initializing() { return LLSingletonBase::get_initializing(); }
};
// But for the specific case of LLSingletonBase::MasterList, don't.
@@ -201,9 +216,14 @@ struct LLSingleton_manage_master<LLSingletonBase::MasterList>
void remove(LLSingletonBase*) {}
void push_initializing(LLSingletonBase*) {}
void pop_initializing (LLSingletonBase*) {}
- LLSingletonBase::list_t& get_initializing(LLSingletonBase::MasterList* instance)
+ // since we never pushed, no need to clean up
+ void reset_initializing(LLSingletonBase::list_t::size_type size) {}
+ LLSingletonBase::list_t& get_initializing()
{
- return LLSingletonBase::get_initializing_from(instance);
+ // The MasterList shouldn't depend on any other LLSingletons. We'd
+ // get into trouble if we tried to recursively engage that machinery.
+ static LLSingletonBase::list_t sDummyList;
+ return sDummyList;
}
};
@@ -213,7 +233,12 @@ LLSingletonBase::LLSingletonBase(tag<DERIVED_TYPE>):
mCleaned(false),
mDeleteSingleton(NULL)
{
- // Make this the currently-initializing LLSingleton.
+ // This is the earliest possible point at which we can push this new
+ // instance onto the init stack. LLSingleton::constructSingleton() can't
+ // do it before calling the constructor, because it doesn't have an
+ // instance pointer until the constructor returns. Fortunately this
+ // constructor is guaranteed to be called before any subclass constructor.
+ // Make this new instance the currently-initializing LLSingleton.
LLSingleton_manage_master<DERIVED_TYPE>().push_initializing(this);
}
@@ -297,29 +322,82 @@ private:
template <typename... Args>
static void constructSingleton(Args&&... args)
{
+ auto prev_size = LLSingleton_manage_master<DERIVED_TYPE>().get_initializing().size();
+ // getInstance() calls are from within constructor
sData.mInitState = CONSTRUCTING;
- sData.mInstance = new DERIVED_TYPE(std::forward<Args>(args)...);
- sData.mInitState = INITIALIZING;
+ try
+ {
+ sData.mInstance = new DERIVED_TYPE(std::forward<Args>(args)...);
+ // we have called constructor, have not yet called initSingleton()
+ sData.mInitState = CONSTRUCTED;
+ }
+ catch (const std::exception& err)
+ {
+ // LLSingletonBase might -- or might not -- have pushed the new
+ // instance onto the init stack before the exception. Reset the
+ // init stack to its previous size BEFORE logging so log-machinery
+ // LLSingletons don't record a dependency on DERIVED_TYPE!
+ LLSingleton_manage_master<DERIVED_TYPE>().reset_initializing(prev_size);
+ logwarns("Error constructing ", classname<DERIVED_TYPE>().c_str(),
+ ": ", err.what());
+ // There isn't a separate EInitState value meaning "we attempted
+ // to construct this LLSingleton subclass but could not," so use
+ // DELETED. That seems slightly more appropriate than UNINITIALIZED.
+ sData.mInitState = DELETED;
+ // propagate the exception
+ throw;
+ }
}
static void finishInitializing()
{
- // 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
+ // getInstance() calls are from within initSingleton()
+ sData.mInitState = INITIALIZING;
+ try
+ {
+ // 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();
+ sData.mInitState = INITIALIZED;
+
+ // pop this off stack of initializing singletons
+ pop_initializing();
+ }
+ catch (const std::exception& err)
+ {
+ // pop this off stack of initializing singletons here, too --
+ // BEFORE logging, so log-machinery LLSingletons don't record a
+ // dependency on DERIVED_TYPE!
+ pop_initializing();
+ logwarns("Error in ", classname<DERIVED_TYPE>().c_str(),
+ "::initSingleton(): ", err.what());
+ // and get rid of the instance entirely
+ deleteSingleton();
+ // propagate the exception
+ throw;
+ }
+ }
+
+ static void pop_initializing()
+ {
+ // route through LLSingleton_manage_master so we Do The Right Thing
+ // (namely, nothing) for MasterList
LLSingleton_manage_master<DERIVED_TYPE>().pop_initializing(sData.mInstance);
+ }
- // The remaining top of that stack, if any, is an LLSingleton that
- // directly depends on DERIVED_TYPE. If getInstance() was called by
- // another LLSingleton, rather than from vanilla application code,
- // record the dependency.
+ // Without this 'using' declaration, the static method we're declaring
+ // here would hide the base-class method we want it to call.
+ using LLSingletonBase::capture_dependency;
+ static void capture_dependency()
+ {
+ // 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
+ // getInstance() was called by another LLSingleton, rather than from
+ // vanilla application code, record the dependency.
sData.mInstance->capture_dependency(
- LLSingleton_manage_master<DERIVED_TYPE>().get_initializing(sData.mInstance),
+ LLSingleton_manage_master<DERIVED_TYPE>().get_initializing(),
sData.mInitState);
}
@@ -394,8 +472,7 @@ public:
static void deleteSingleton()
{
delete sData.mInstance;
- sData.mInstance = NULL;
- sData.mInitState = DELETED;
+ // SingletonData state handled by destructor, above
}
static DERIVED_TYPE* getInstance()
@@ -408,23 +485,27 @@ public:
case UNINITIALIZED:
// should never be uninitialized at this point
logerrs("Uninitialized singleton ",
- demangle(typeid(DERIVED_TYPE).name()).c_str());
+ classname<DERIVED_TYPE>().c_str());
return NULL;
case CONSTRUCTING:
// here if DERIVED_TYPE's constructor (directly or indirectly)
// calls DERIVED_TYPE::getInstance()
logerrs("Tried to access singleton ",
- demangle(typeid(DERIVED_TYPE).name()).c_str(),
+ classname<DERIVED_TYPE>().c_str(),
" from singleton constructor!");
return NULL;
- case INITIALIZING:
- // first time through: set to INITIALIZING by
- // constructSingleton(), called by sInitializer's constructor
+ case CONSTRUCTED:
+ // first time through: set to CONSTRUCTED by
+ // constructSingleton(), called by sInitializer's constructor;
+ // still have to call initSingleton()
finishInitializing();
break;
+ case INITIALIZING:
+ // here if DERIVED_TYPE::initSingleton() (directly or indirectly)
+ // calls DERIVED_TYPE::getInstance(): go ahead and allow it
case INITIALIZED:
// normal subsequent calls
break;
@@ -432,13 +513,18 @@ public:
case DELETED:
// called after deleteSingleton()
logwarns("Trying to access deleted singleton ",
- demangle(typeid(DERIVED_TYPE).name()).c_str(),
+ classname<DERIVED_TYPE>().c_str(),
" -- creating new instance");
+ // This recovery sequence is NOT thread-safe! We would need a
+ // recursive_mutex a la LLParamSingleton.
constructSingleton();
finishInitializing();
break;
}
+ // record the dependency, if any: check if we got here from another
+ // LLSingleton's constructor or initSingleton() method
+ capture_dependency();
return sData.mInstance;
}
@@ -502,6 +588,10 @@ class LLParamSingleton : public LLSingleton<DERIVED_TYPE>
{
private:
typedef LLSingleton<DERIVED_TYPE> super;
+ // Use a recursive_mutex in case of constructor circularity. With a
+ // non-recursive mutex, that would result in deadlock rather than the
+ // logerrs() call in getInstance().
+ typedef std::recursive_mutex mutex_t;
public:
using super::deleteSingleton;
@@ -515,12 +605,12 @@ public:
// In case racing threads both call initParamSingleton() at the same
// time, serialize them. One should initialize; the other should see
// mInitState already set.
- std::unique_lock<decltype(mMutex)> lk(mMutex);
+ std::unique_lock<mutex_t> lk(getMutex());
// For organizational purposes this function shouldn't be called twice
if (super::sData.mInitState != super::UNINITIALIZED)
{
super::logerrs("Tried to initialize singleton ",
- super::demangle(typeid(DERIVED_TYPE).name()).c_str(),
+ super::template classname<DERIVED_TYPE>().c_str(),
" twice!");
}
else
@@ -534,28 +624,44 @@ public:
{
// In case racing threads call getInstance() at the same moment as
// initParamSingleton(), serialize the calls.
- std::unique_lock<decltype(mMutex)> lk(mMutex);
+ std::unique_lock<mutex_t> lk(getMutex());
switch (super::sData.mInitState)
{
case super::UNINITIALIZED:
super::logerrs("Uninitialized param singleton ",
- super::demangle(typeid(DERIVED_TYPE).name()).c_str());
+ super::template classname<DERIVED_TYPE>().c_str());
break;
case super::CONSTRUCTING:
super::logerrs("Tried to access param singleton ",
- super::demangle(typeid(DERIVED_TYPE).name()).c_str(),
+ super::template classname<DERIVED_TYPE>().c_str(),
" from singleton constructor!");
break;
+ case super::CONSTRUCTED:
+ // Should never happen!? The CONSTRUCTED state is specifically to
+ // navigate through LLSingleton::SingletonInitializer getting
+ // constructed (once) before LLSingleton::getInstance()'s switch
+ // on mInitState. But our initParamSingleton() method calls
+ // constructSingleton() and then calls finishInitializing(), which
+ // immediately sets INITIALIZING. Why are we here?
+ super::logerrs("Param singleton ",
+ super::template classname<DERIVED_TYPE>().c_str(),
+ "::initSingleton() not yet called");
+ break;
+
case super::INITIALIZING:
+ // As with LLSingleton, explicitly permit circular calls from
+ // within initSingleton()
case super::INITIALIZED:
+ // for any valid call, capture dependencies
+ super::capture_dependency();
return super::sData.mInstance;
case super::DELETED:
super::logerrs("Trying to access deleted param singleton ",
- super::demangle(typeid(DERIVED_TYPE).name()).c_str());
+ super::template classname<DERIVED_TYPE>().c_str());
break;
}
@@ -573,15 +679,30 @@ public:
}
private:
- // Use a recursive_mutex in case of constructor circularity. With a
- // non-recursive mutex, that would result in deadlock rather than the
- // logerrs() call coded above.
- static std::recursive_mutex mMutex;
+ // sMutex must be a function-local static rather than a static member. One
+ // of the essential features of LLSingleton and friends is that they must
+ // support getInstance() even when the containing module's static
+ // variables have not yet been runtime-initialized. A mutex requires
+ // construction. A static class member might not yet have been
+ // constructed.
+ //
+ // We could store a dumb mutex_t*, notice when it's NULL and allocate a
+ // heap mutex -- but that's vulnerable to race conditions. And we can't
+ // defend the dumb pointer with another mutex.
+ //
+ // We could store a std::atomic<mutex_t*> -- but a default-constructed
+ // std::atomic<T> does not contain a valid T, even a default-constructed
+ // T! Which means std::atomic, too, requires runtime initialization.
+ //
+ // But a function-local static is guaranteed to be initialized exactly
+ // once, the first time control reaches that declaration.
+ static mutex_t& getMutex()
+ {
+ static mutex_t sMutex;
+ return sMutex;
+ }
};
-template<typename T>
-typename std::recursive_mutex LLParamSingleton<T>::mMutex;
-
/**
* Initialization locked singleton, only derived class can decide when to initialize.
* Starts locked.
@@ -634,7 +755,7 @@ public:
* file, use 'inline' (unless it's a template class) to avoid duplicate-symbol
* errors at link time.
*/
-#define LLSINGLETON(DERIVED_CLASS, ...) \
+#define LLSINGLETON(DERIVED_CLASS, ...) \
private: \
/* implement LLSingleton pure virtual method whose sole purpose */ \
/* is to remind people to use this macro */ \
diff --git a/indra/newview/llimagefiltersmanager.cpp b/indra/newview/llimagefiltersmanager.cpp
index ee6b39efac..c23cdc8103 100644
--- a/indra/newview/llimagefiltersmanager.cpp
+++ b/indra/newview/llimagefiltersmanager.cpp
@@ -48,7 +48,7 @@ LLImageFiltersManager::~LLImageFiltersManager()
{
}
-// virtual static
+// virtual
void LLImageFiltersManager::initSingleton()
{
loadAllFilters();
diff --git a/indra/test/test.cpp b/indra/test/test.cpp
index d4cd4b951e..b14c2eb255 100644
--- a/indra/test/test.cpp
+++ b/indra/test/test.cpp
@@ -253,7 +253,7 @@ public:
break;
case tut::test_result::ex:
++mFailedTests;
- out << "exception";
+ out << "exception: " << tr.exception_typeid;
break;
case tut::test_result::warn:
++mFailedTests;
@@ -264,7 +264,7 @@ public:
out << "abnormal termination";
break;
case tut::test_result::skip:
- ++mSkippedTests;
+ ++mSkippedTests;
out << "skipped known failure";
break;
default:
generated by cgit v1.2.3 (git 2.25.1) at 2024-11-28 04:00:31 +0000