DRTVWR-493: Streamline LLParamSingleton, LLLockedSingleton.

Simplify LLSingleton::SingletonLifetimeManager to SingletonInitializer: that
struct has not been responsible for deletion ever since LLSingletonBase
acquired dependency-ordered deleteAll().

Move SingletonData::mInitState changes from SingletonLifetimeManager to
constructSingleton() method. Similarly, constructSingleton() now sets
SingletonData::mInstance instead of making its caller store the pointer.

Add variadic arguments to LLSingleton::constructSingleton() so we can reuse it
for LLParamSingleton.

Add finishInitializing() method to encapsulate logic reused for
getInstance()'s INITIALIZING and DELETED cases.

Make LLParamSingleton a subclass of LLSingleton, just as LLLockedSingleton is
a subclass of LLParamSingleton. Make LLParamSingleton a friend of LLSingleton,
so it can access private members of LLSingleton without also granting access
to any DERIVED_CLASS subclass. This eliminates the need for protected
getInitState().

LLParamSingleton::initParamSingleton() reuses LLSingleton::constructSingleton()
and finishInitializing(). Its getInstance() method completely replaces
LLSingleton::getInstance(): in most EInitStates, LLParamSingleton::getInstance()
is an error.

Use a std::mutex to serialize calls to LLParamSingleton::initParamSingleton()
and getInstance(). While LLSingleton::getInstance() relies on the "initialized
exactly once" guarantee for block-scope static declarations, LLParamSingleton
cannot rely on the same mechanism.

LLLockedSingleton is now a very succinct subclass of LLParamSingleton -- they
have very similar functionality.

Giving the LLSINGLETON() macro variadic arguments eliminates the need for a
separate LLPARAMSINGLETON() macro, while continuing to support existing usage.

5 files changed, 176 insertions, 233 deletions

diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h
index b127f4f529..38d5af5309 100644
--- a/indra/llcommon/llsingleton.h
+++ b/indra/llcommon/llsingleton.h
@@ -31,6 +31,18 @@
 #include <vector>
 #include <typeinfo>
 
+#if LL_WINDOWS
+#pragma warning (push)
+#pragma warning (disable:4265)
+#endif
+// warning C4265: 'std::_Pad' : class has virtual functions, but destructor is not virtual
+
+#include <mutex>
+
+#if LL_WINDOWS
+#pragma warning (pop)
+#endif
+
 class LLSingletonBase: private boost::noncopyable
 {
 public:
@@ -205,6 +217,10 @@ LLSingletonBase::LLSingletonBase(tag<DERIVED_TYPE>):
     LLSingleton_manage_master<DERIVED_TYPE>().push_initializing(this);
 }
 
+// forward declare for friend directive within LLSingleton
+template <typename DERIVED_TYPE>
+class LLParamSingleton;
+
 /**
  * LLSingleton implements the getInstance() method part of the Singleton
  * pattern. It can't make the derived class constructors protected, though, so
@@ -270,9 +286,41 @@ template <typename DERIVED_TYPE>
 class LLSingleton : public LLSingletonBase
 {
 private:
-    static DERIVED_TYPE* constructSingleton()
+    // Allow LLParamSingleton subclass -- but NOT DERIVED_TYPE itself -- to
+    // access our private members.
+    friend class LLParamSingleton<DERIVED_TYPE>;
+
+    // LLSingleton only supports a nullary constructor. However, the specific
+    // purpose for its subclass LLParamSingleton is to support Singletons
+    // requiring constructor arguments. constructSingleton() supports both use
+    // cases.
+    template <typename... Args>
+    static void constructSingleton(Args&&... args)
     {
-        return new DERIVED_TYPE();
+        sData.mInitState = CONSTRUCTING;
+        sData.mInstance = new DERIVED_TYPE(std::forward<Args>(args)...);
+        sData.mInitState = INITIALIZING;
+    }
+
+    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
+        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.
+        sData.mInstance->capture_dependency(
+            LLSingleton_manage_master<DERIVED_TYPE>().get_initializing(sData.mInstance),
+            sData.mInitState);
     }
 
     // We know of no way to instruct the compiler that every subclass
@@ -285,34 +333,17 @@ private:
     // subclass body.
     virtual void you_must_use_LLSINGLETON_macro() = 0;
 
-    // stores pointer to singleton instance
-    struct SingletonLifetimeManager
+    // The purpose of this struct is to engage the C++11 guarantee that static
+    // variables declared in function scope are initialized exactly once, even
+    // if multiple threads concurrently reach the same declaration.
+    // https://en.cppreference.com/w/cpp/language/storage_duration#Static_local_variables
+    // Since getInstance() declares a static instance of SingletonInitializer,
+    // only the first call to getInstance() calls constructSingleton().
+    struct SingletonInitializer
     {
-        SingletonLifetimeManager()
-        {
-            construct();
-        }
-
-        static void construct()
+        SingletonInitializer()
         {
-            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. If you want to clean up LLSingletons,
-            // call LLSingletonBase::deleteAll() sometime before static-object
-            // destruction begins. That method will properly honor cross-
-            // LLSingleton dependencies. Otherwise we simply leak LLSingleton
-            // instances at shutdown. Since the whole process is terminating
-            // anyway, that's not necessarily a bad thing; it depends on what
-            // resources your LLSingleton instances are managing.
+            constructSingleton();
         }
     };
 
@@ -369,7 +400,8 @@ public:
 
     static DERIVED_TYPE* getInstance()
     {
-        static SingletonLifetimeManager sLifeTimeMgr;
+        // call constructSingleton() only the first time we get here
+        static SingletonInitializer sInitializer;
 
         switch (sData.mInitState)
         {
@@ -380,47 +412,33 @@ public:
             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(),
                     " 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
-            LLSingleton_manage_master<DERIVED_TYPE>().pop_initializing(sData.mInstance);
+            // first time through: set to INITIALIZING by
+            // constructSingleton(), called by sInitializer's constructor
+            finishInitializing();
             break;
 
         case INITIALIZED:
+            // normal subsequent calls
             break;
 
         case DELETED:
+            // called after deleteSingleton()
             logwarns("Trying to access deleted singleton ",
                      demangle(typeid(DERIVED_TYPE).name()).c_str(),
                      " -- creating new instance");
-            SingletonLifetimeManager::construct();
-            // same as first time construction
-            sData.mInitState = INITIALIZED; 
-            sData.mInstance->initSingleton(); 
-            // pop this off stack of initializing singletons
-            LLSingleton_manage_master<DERIVED_TYPE>().pop_initializing(sData.mInstance);
+            constructSingleton();
+            finishInitializing();
             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(
-            LLSingleton_manage_master<DERIVED_TYPE>().get_initializing(sData.mInstance),
-            sData.mInitState);
         return sData.mInstance;
     }
 
@@ -446,12 +464,6 @@ public:
         return sData.mInitState == DELETED;
     }
 
-protected:
-    static EInitState getInitState()
-    {
-        return sData.mInitState;
-    }
-
 private:
     struct SingletonData
     {
@@ -463,176 +475,138 @@ private:
     static SingletonData sData;
 };
 
+template<typename T>
+typename LLSingleton<T>::SingletonData LLSingleton<T>::sData;
 
+
+/**
+ * LLParamSingleton<T> is like LLSingleton<T>, except in the following ways:
+ *
+ * * It is NOT instantiated on demand (instance() or getInstance()). You must
+ *   first call initParamSingleton(constructor args...).
+ * * Before initParamSingleton(), calling instance() or getInstance() dies with
+ *   LL_ERRS.
+ * * initParamSingleton() may be called only once. A second call dies with
+ *   LL_ERRS.
+ * * However, distinct initParamSingleton() calls can be used to engage
+ *   different constructors, as long as only one such call is executed at
+ *   runtime.
+ * * Circularity is not permitted. No LLSingleton referenced by an
+ *   LLParamSingleton's constructor or initSingleton() method may call this
+ *   LLParamSingleton's instance() or getInstance() methods.
+ * * Unlike LLSingleton, an LLParamSingleton cannot be "revived" by an
+ *   instance() or getInstance() call after deleteSingleton().
+ *
+ * Importantly, though, each LLParamSingleton subclass does participate in the
+ * dependency-ordered LLSingletonBase::deleteAll() processing.
+ */
 template <typename DERIVED_TYPE>
-class LLParamSingleton : public LLSingletonBase
+class LLParamSingleton : public LLSingleton<DERIVED_TYPE>
 {
 private:
-
-    template <typename... Args>
-    static DERIVED_TYPE* constructSingleton(Args&&... args)
-    {
-        return new DERIVED_TYPE(std::forward<Args>(args)...);
-    }
-
-    // We know of no way to instruct the compiler that every subclass
-    // constructor MUST be private.
-    // However, we can make the LLPARAMSINGLETON() macro both declare
-    // a private constructor and provide the required friend declaration.
-    // How can we ensure that every subclass uses LLPARAMSINGLETON()?
-    // By making that macro provide a definition for this pure virtual
-    // method. If you get "can't instantiate class due to missing pure
-    // virtual method" for this method, then add LLPARAMSINGLETON(yourclass)
-    // in the subclass body.
-    virtual void you_must_use_LLSINGLETON_macro() = 0;
-
-protected:
-    // Pass DERIVED_TYPE explicitly to LLSingletonBase's constructor because,
-    // until our subclass constructor completes, *this isn't yet a
-    // full-fledged DERIVED_TYPE.
-    LLParamSingleton() : LLSingletonBase(LLSingletonBase::tag<DERIVED_TYPE>())
-    {
-        // 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<DERIVED_TYPE>().add(this);
-    }
+    typedef LLSingleton<DERIVED_TYPE> super;
 
 public:
+    using super::deleteSingleton;
+    using super::instance;
+    using super::instanceExists;
+    using super::wasDeleted;
 
-    virtual ~LLParamSingleton()
-    {
-        // remove this instance from the master list
-        LLSingleton_manage_master<DERIVED_TYPE>().remove(this);
-        sData.mInstance = NULL;
-        sData.mInitState = DELETED;
-    }
-
-    // Passes arguments to DERIVED_TYPE's constructor and sets apropriate states
+    // Passes arguments to DERIVED_TYPE's constructor and sets appropriate states
     template <typename... Args>
     static void initParamSingleton(Args&&... args)
     {
-        sData.mInitState = CONSTRUCTING;
-        sData.mInstance = constructSingleton(std::forward<Args>(args)...);
-        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
-        LLSingleton_manage_master<DERIVED_TYPE>().pop_initializing(sData.mInstance);
-    }
-
-    /**
-    * @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;
+        // 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<std::mutex> lk(mMutex);
+        // 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(),
+                           " twice!");
+        }
+        else
+        {
+            super::constructSingleton(std::forward<Args>(args)...);
+            super::finishInitializing();
+        }
     }
 
     static DERIVED_TYPE* getInstance()
     {
-        switch (sData.mInitState)
+        // In case racing threads call getInstance() at the same moment as
+        // initParamSingleton(), serialize the calls.
+        std::unique_lock<std::mutex> lk(mMutex);
+
+        switch (super::sData.mInitState)
         {
-        case UNINITIALIZED:
-            logerrs("Uninitialized param singleton ",
-                demangle(typeid(DERIVED_TYPE).name()).c_str());
-            return NULL;
+        case super::UNINITIALIZED:
+            super::logerrs("Uninitialized param singleton ",
+                           super::demangle(typeid(DERIVED_TYPE).name()).c_str());
+            break;
 
-        case CONSTRUCTING:
-            logerrs("Tried to access singleton ",
-                demangle(typeid(DERIVED_TYPE).name()).c_str(),
+        case super::CONSTRUCTING:
+            super::logerrs("Tried to access param singleton ",
+                           super::demangle(typeid(DERIVED_TYPE).name()).c_str(),
                 " from singleton constructor!");
-            return NULL;
-
-        case INITIALIZING:
-            logerrs("State not supported by ",
-                demangle(typeid(DERIVED_TYPE).name()).c_str(),
-                " since it is a parametric singleton!");
             break;
 
-        case INITIALIZED:
+        case super::INITIALIZING:
+            super::logerrs("Tried to access param singleton ",
+                           super::demangle(typeid(DERIVED_TYPE).name()).c_str(),
+                           " from initSingleton() method!");
             break;
 
-        case DELETED:
-            logerrs("Trying to access deleted param singleton ",
-                demangle(typeid(DERIVED_TYPE).name()).c_str());
+        case super::INITIALIZED:
+            return super::sData.mInstance;
 
+        case super::DELETED:
+            super::logerrs("Trying to access deleted param singleton ",
+                           super::demangle(typeid(DERIVED_TYPE).name()).c_str());
             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(
-            LLSingleton_manage_master<DERIVED_TYPE>().get_initializing(sData.mInstance),
-            sData.mInitState);
-        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 yet?
-    // Use this to avoid accessing singletons before they can safely be constructed.
-    static bool instanceExists()
-    {
-        return sData.mInitState == INITIALIZED;
-    }
-
-    // Has this singleton been deleted? This can be useful during shutdown
-    // processing to avoid "resurrecting" a singleton we thought we'd already
-    // cleaned up.
-    static bool wasDeleted()
-    {
-        return sData.mInitState == DELETED;
+        // should never actually get here; this is to pacify the compiler,
+        // which assumes control might return from logerrs()
+        return nullptr;
     }
 
 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;
+    static std::mutex mMutex;
 };
 
 template<typename T>
-typename LLSingleton<T>::SingletonData LLSingleton<T>::sData;
+typename std::mutex LLParamSingleton<T>::mMutex;
 
-template<typename T>
-typename LLParamSingleton<T>::SingletonData LLParamSingleton<T>::sData;
+/**
+ * Initialization locked singleton, only derived class can decide when to initialize.
+ * Starts locked.
+ * For cases when singleton has a dependency onto something or.
+ *
+ * LLLockedSingleton is like an LLParamSingleton with a nullary constructor.
+ * It cannot be instantiated on demand (instance() or getInstance() call) --
+ * it must be instantiated by calling construct(). However, it does
+ * participate in dependency-ordered LLSingletonBase::deleteAll() processing.
+ */
+template <typename DT>
+class LLLockedSingleton : public LLParamSingleton<DT>
+{
+    typedef LLParamSingleton<DT> super;
+
+public:
+    using super::deleteSingleton;
+    using super::getInstance;
+    using super::instance;
+    using super::instanceExists;
+    using super::wasDeleted;
+
+    static void construct()
+    {
+        super::initParamSingleton();
+    }
+};
 
 /**
  * Use LLSINGLETON(Foo); at the start of an LLSingleton<Foo> subclass body
@@ -658,13 +632,13 @@ typename LLParamSingleton<T>::SingletonData LLParamSingleton<T>::sData;
  * 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 */                         \
     virtual void you_must_use_LLSINGLETON_macro() {}                    \
     friend class LLSingleton<DERIVED_CLASS>;                            \
-    DERIVED_CLASS()
+    DERIVED_CLASS(__VA_ARGS__)
 
 /**
  * Use LLSINGLETON_EMPTY_CTOR(Foo); at the start of an LLSingleton<Foo>
@@ -684,35 +658,4 @@ private:                                                                \
     /* LLSINGLETON() is carefully implemented to permit exactly this */ \
     LLSINGLETON(DERIVED_CLASS) {}
 
-/**
-* Use LLPARAMSINGLETON(Foo); at the start of an LLParamSingleton<Foo> subclass body
-* when you want to declare an out-of-line constructor:
-*
-* @code
-*   class Foo: public LLParamSingleton<Foo>
-*   {
-*       // use this macro at start of every LLSingleton subclass
-*       LLPARAMSINGLETON(Foo);
-*   public:
-*       // ...
-*   };
-*   // ...
-*   [inline]
-*   Foo::Foo() { ... }
-* @endcode
-*
-* Unfortunately, this mechanism does not permit you to define even a simple
-* (but nontrivial) constructor within the class body. Use LLPARAMSINGLETON()
-* and define the constructor outside the class body. If you must define it
-* in a header file, use 'inline' (unless it's a template class) to avoid
-* duplicate-symbol errors at link time.
-*/
-#define LLPARAMSINGLETON(DERIVED_CLASS, ...)                                      \
-private:                                                                \
-    /* implement LLSingleton pure virtual method whose sole purpose */  \
-    /* is to remind people to use this macro */                         \
-    virtual void you_must_use_LLSINGLETON_macro() {}                    \
-    friend class LLParamSingleton<DERIVED_CLASS>;                            \
-    DERIVED_CLASS(__VA_ARGS__)
-
 #endif
diff --git a/indra/llimage/llimage.h b/indra/llimage/llimage.h
index e5526ba9c0..9f8d061293 100644
--- a/indra/llimage/llimage.h
+++ b/indra/llimage/llimage.h
@@ -91,7 +91,7 @@ typedef enum e_image_codec
 
 class LLImage : public LLParamSingleton<LLImage>
 {
-	LLPARAMSINGLETON(LLImage, bool use_new_byte_range = false, S32 minimal_reverse_byte_range_percent = 75);
+	LLSINGLETON(LLImage, bool use_new_byte_range = false, S32 minimal_reverse_byte_range_percent = 75);
 	~LLImage();
 public:
 
diff --git a/indra/llrender/llrender2dutils.h b/indra/llrender/llrender2dutils.h
index f2640ff998..cf408336e6 100644
--- a/indra/llrender/llrender2dutils.h
+++ b/indra/llrender/llrender2dutils.h
@@ -123,7 +123,7 @@ class LLImageProviderInterface;
 
 class LLRender2D : public LLParamSingleton<LLRender2D>
 {
-	LLPARAMSINGLETON(LLRender2D, LLImageProviderInterface* image_provider);
+	LLSINGLETON(LLRender2D, LLImageProviderInterface* image_provider);
 	LOG_CLASS(LLRender2D);
 	~LLRender2D();
 public:
diff --git a/indra/llui/llui.h b/indra/llui/llui.h
index 9decaf92cb..e1c51deba9 100644
--- a/indra/llui/llui.h
+++ b/indra/llui/llui.h
@@ -115,7 +115,7 @@ public:
 	typedef std::map<std::string, LLControlGroup*> settings_map_t;
 
 private:
-	LLPARAMSINGLETON(LLUI , const settings_map_t &settings,
+	LLSINGLETON(LLUI , const settings_map_t &settings,
 						   LLImageProviderInterface* image_provider,
 						   LLUIAudioCallback audio_callback,
 						   LLUIAudioCallback deferred_audio_callback);
diff --git a/indra/newview/llconversationlog.h b/indra/newview/llconversationlog.h
index 38247f8eff..f241276abb 100644
--- a/indra/newview/llconversationlog.h
+++ b/indra/newview/llconversationlog.h
@@ -109,7 +109,7 @@ private:
 
 class LLConversationLog : public LLParamSingleton<LLConversationLog>, LLIMSessionObserver
 {
-	LLPARAMSINGLETON(LLConversationLog);
+	LLSINGLETON(LLConversationLog);
 public:
 	void removeConversation(const LLConversation& conversation);