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.

2 files changed, 82 insertions, 27 deletions

diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp
index 479244400d..3f65820f2e 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 @@ private:
 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 78092fdc11..92fba4c1a8 100644
--- a/indra/llcommon/llsingleton.h
+++ b/indra/llcommon/llsingleton.h
@@ -46,6 +46,7 @@ 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();
@@ -65,11 +66,19 @@ protected:
         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 @@ private:
 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 @@ private:
     };
 
 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 @@ public:
             // 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 @@ public:
             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 @@ 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.mInitState);
+        sData.mInstance->capture_dependency(
+            LLSingleton_manage_master<DERIVED_TYPE>().get_initializing(sData.mInstance),
+            sData.mInitState);
         return sData.mInstance;
     }