From 687efd84eabc524e339e61458b0cbf53f9a38f8a Mon Sep 17 00:00:00 2001
From: Nat Goodspeed <nat@lindenlab.com>
Date: Fri, 26 Jun 2015 13:03:59 -0400
Subject: [PATCH] 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(-)
diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp
index a3edf925ad4..2813814ae14 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 25bdba0a0d8..a82101c367d 100755
--- a/indra/llcommon/llsingleton.h
+++ b/indra/llcommon/llsingleton.h
@@ -58,6 +58,15 @@ class LLSingletonBase: private boost::noncopyable
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 @@ class LLSingletonBase: private boost::noncopyable
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 <typename DERIVED_TYPE>
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 @@ class LLSingleton : public LLSingletonBase
// 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;
}
--
GitLab