From 90f424980a3aba06ac85cc23373795bc53a3fd87 Mon Sep 17 00:00:00 2001
From: Nat Goodspeed <nat@lindenlab.com>
Date: Tue, 6 Sep 2016 21:07:38 -0400
Subject: [PATCH] MAINT-5232: Make LLSingleton's 'initializing' stack
coro-specific.
The stack we maintain of which LLSingletons are currently initializing only
makes sense when associated with a particular C++ call stack. But each
coroutine introduces another C++ call stack!
Move the initializing stack from function-static storage to
LLSingletonBase::MasterList. Make it a map keyed by llcoro::id. Each coro then
has a stack of its own.
This introduces more dependencies on the MasterList singleton, requiring
additional LLSingleton_manage_master workarounds.
---
indra/llcommon/llsingleton.cpp | 54 +++++++++++++++++++++++----------
indra/llcommon/llsingleton.h | 55 ++++++++++++++++++++++++++--------
2 files changed, 82 insertions(+), 27 deletions(-)
diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp
index 479244400d1..3f65820f2ed 100644
--- a/indra/llcommon/llsingleton.cpp
+++ b/indra/llcommon/llsingleton.cpp
@@ -30,7 +30,9 @@
#include "llerror.h"
#include "llerrorcontrol.h" // LLError::is_available()
#include "lldependencies.h"
+#include "llcoro_get_id.h"
#include <boost/foreach.hpp>
+#include <boost/unordered_map.hpp>
#include <algorithm>
#include <iostream> // std::cerr in dire emergency
#include <sstream>
@@ -61,13 +63,43 @@ class LLSingletonBase::MasterList:
public:
// No need to make this private with accessors; nobody outside this source
// file can see it.
- LLSingletonBase::list_t mList;
+
+ // This is the master list of all instantiated LLSingletons (save the
+ // MasterList itself) in arbitrary order. You MUST call dep_sort() before
+ // traversing this list.
+ LLSingletonBase::list_t mMaster;
+
+ // We need to maintain a stack of LLSingletons currently being
+ // initialized, either in the constructor or in initSingleton(). However,
+ // managing that as a stack depends on having a DISTINCT 'initializing'
+ // stack for every C++ stack in the process! And we have a distinct C++
+ // stack for every running coroutine. It would be interesting and cool to
+ // implement a generic coroutine-local-storage mechanism and use that
+ // here. The trouble is that LLCoros is itself an LLSingleton, so
+ // depending on LLCoros functionality could dig us into infinite
+ // recursion. (Moreover, when we reimplement LLCoros on top of
+ // Boost.Fiber, that library already provides fiber_specific_ptr -- so
+ // it's not worth a great deal of time and energy implementing a generic
+ // equivalent on top of boost::dcoroutine, which is on its way out.)
+ // Instead, use a map of llcoro::id to select the appropriate
+ // coro-specific 'initializing' stack. llcoro::get_id() is carefully
+ // implemented to avoid requiring LLCoros.
+ typedef boost::unordered_map<llcoro::id, LLSingletonBase::list_t> InitializingMap;
+ InitializingMap mInitializing;
+
+ // non-static method, cf. LLSingletonBase::get_initializing()
+ list_t& get_initializing_()
+ {
+ // map::operator[] has find-or-create semantics, exactly what we need
+ // here. It returns a reference to the selected mapped_type instance.
+ return mInitializing[llcoro::get_id()];
+ }
};
//static
LLSingletonBase::list_t& LLSingletonBase::get_master()
{
- return LLSingletonBase::MasterList::instance().mList;
+ return LLSingletonBase::MasterList::instance().mMaster;
}
void LLSingletonBase::add_master()
@@ -88,23 +120,16 @@ void LLSingletonBase::remove_master()
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;
+ return LLSingletonBase::MasterList::instance().get_initializing_();
}
-LLSingletonBase::LLSingletonBase(const char* name):
- mCleaned(false),
- mDeleteSingleton(NULL)
+//static
+LLSingletonBase::list_t& LLSingletonBase::get_initializing_from(MasterList* master)
{
- // Make this the currently-initializing LLSingleton.
- push_initializing(name);
+ return master->get_initializing_();;
}
LLSingletonBase::~LLSingletonBase() {}
@@ -159,13 +184,12 @@ void LLSingletonBase::log_initializing(const char* verb, const char* name)
}
}
-void LLSingletonBase::capture_dependency(EInitState initState)
+void LLSingletonBase::capture_dependency(list_t& initializing, EInitState initState)
{
// 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
diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h
index 78092fdc119..92fba4c1a8e 100644
--- a/indra/llcommon/llsingleton.h
+++ b/indra/llcommon/llsingleton.h
@@ -46,6 +46,7 @@ class LLSingletonBase: private boost::noncopyable
// 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();
@@ -65,11 +66,19 @@ class LLSingletonBase: private boost::noncopyable
DELETED
} EInitState;
+ // Define tag<T> to pass to our template constructor. You can't explicitly
+ // invoke a template constructor with ordinary template syntax:
+ // http://stackoverflow.com/a/3960925/5533635
+ template <typename T>
+ struct tag
+ {
+ typedef T type;
+ };
+
// Base-class constructor should only be invoked by the DERIVED_TYPE
- // constructor, which passes the DERIVED_TYPE class name for logging
- // purposes. Within LLSingletonBase::LLSingletonBase, of course the
- // formula typeid(*this).name() produces "LLSingletonBase".
- LLSingletonBase(const char* name);
+ // constructor, which passes tag<DERIVED_TYPE> for various purposes.
+ template <typename DERIVED_TYPE>
+ LLSingletonBase(tag<DERIVED_TYPE>);
virtual ~LLSingletonBase();
// Every new LLSingleton should be added to/removed from the master list
@@ -100,7 +109,7 @@ class LLSingletonBase: private boost::noncopyable
protected:
// 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(EInitState);
+ void capture_dependency(list_t& initializing, EInitState);
// delegate LL_ERRS() logging to llsingleton.cpp
static void logerrs(const char* p1, const char* p2="",
@@ -171,12 +180,15 @@ 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().
+// methods to real LLSingletonBase methods.
template <class T>
struct LLSingleton_manage_master
{
void add(LLSingletonBase* sb) { sb->add_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(); }
};
// But for the specific case of LLSingletonBase::MasterList, don't.
@@ -185,8 +197,24 @@ struct LLSingleton_manage_master<LLSingletonBase::MasterList>
{
void add(LLSingletonBase*) {}
void remove(LLSingletonBase*) {}
+ void push_initializing(LLSingletonBase*) {}
+ void pop_initializing (LLSingletonBase*) {}
+ LLSingletonBase::list_t& get_initializing(LLSingletonBase::MasterList* instance)
+ {
+ return LLSingletonBase::get_initializing_from(instance);
+ }
};
+// Now we can implement LLSingletonBase's template constructor.
+template <typename DERIVED_TYPE>
+LLSingletonBase::LLSingletonBase(tag<DERIVED_TYPE>):
+ mCleaned(false),
+ mDeleteSingleton(NULL)
+{
+ // Make this the currently-initializing LLSingleton.
+ LLSingleton_manage_master<DERIVED_TYPE>().push_initializing(this);
+}
+
/**
* LLSingleton implements the getInstance() method part of the Singleton
* pattern. It can't make the derived class constructors protected, though, so
@@ -287,9 +315,10 @@ class LLSingleton : public LLSingletonBase
};
protected:
- // Use typeid(DERIVED_TYPE) rather than typeid(*this) because, until our
- // constructor completes, *this isn't yet a full-fledged DERIVED_TYPE.
- LLSingleton(): LLSingletonBase(typeid(DERIVED_TYPE).name())
+ // Pass DERIVED_TYPE explicitly to LLSingletonBase's constructor because,
+ // until our subclass constructor completes, *this isn't yet a
+ // full-fledged DERIVED_TYPE.
+ LLSingleton(): LLSingletonBase(LLSingletonBase::tag<DERIVED_TYPE>())
{
// populate base-class function pointer with the static
// deleteSingleton() function for this particular specialization
@@ -363,7 +392,7 @@ class LLSingleton : public LLSingletonBase
// breaking cyclic dependencies
sData.mInstance->initSingleton();
// pop this off stack of initializing singletons
- sData.mInstance->pop_initializing();
+ LLSingleton_manage_master<DERIVED_TYPE>().pop_initializing(sData.mInstance);
break;
case INITIALIZED:
@@ -378,7 +407,7 @@ class LLSingleton : public LLSingletonBase
sData.mInitState = INITIALIZED;
sData.mInstance->initSingleton();
// pop this off stack of initializing singletons
- sData.mInstance->pop_initializing();
+ LLSingleton_manage_master<DERIVED_TYPE>().pop_initializing(sData.mInstance);
break;
}
@@ -387,7 +416,9 @@ 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.mInitState);
+ sData.mInstance->capture_dependency(
+ LLSingleton_manage_master<DERIVED_TYPE>().get_initializing(sData.mInstance),
+ sData.mInitState);
return sData.mInstance;
}
--
GitLab