From 3424a1f965721493b4e4c78ca83b16bb00ce6b32 Mon Sep 17 00:00:00 2001
From: Andrey Kleshchev <andreykproductengine@lindenlab.com>
Date: Tue, 22 Sep 2020 23:38:23 +0300
Subject: SL-13979 Crash of logging system at LLError::Settings::getInstance()

LLSingleton depends onto logging system, having logging system be based on LLSingleton causes crashes and deadlocks
---
 indra/llcommon/llerror.cpp | 84 +++++++++++++++++++++-------------------------
 1 file changed, 38 insertions(+), 46 deletions(-)

(limited to 'indra/llcommon/llerror.cpp')

diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp
index 411412c883..f876b8ee4a 100644
--- a/indra/llcommon/llerror.cpp
+++ b/indra/llcommon/llerror.cpp
@@ -332,12 +332,9 @@ namespace LLError
 		}
 		// huh, that's odd, we should see one or the other prefix -- but don't
 		// try to log unless logging is already initialized
-		if (is_available())
-		{
-			// in Python, " or ".join(vector) -- but in C++, a PITB
-			LL_DEBUGS() << "Did not see 'class' or 'struct' prefix on '"
-				<< name << "'" << LL_ENDL;
-		}
+		// in Python, " or ".join(vector) -- but in C++, a PITB
+		LL_DEBUGS() << "Did not see 'class' or 'struct' prefix on '"
+			<< name << "'" << LL_ENDL;
 		return name;
 
 #else  // neither GCC nor Visual Studio
@@ -438,9 +435,12 @@ namespace
 	typedef std::vector<LLError::RecorderPtr> Recorders;
 	typedef std::vector<LLError::CallSite*> CallSiteVector;
 
-	class Globals : public LLSingleton<Globals>
+	class Globals
 	{
-		LLSINGLETON(Globals);
+    public:
+        static Globals* getInstance();
+    protected:
+		Globals();
 	public:
 		std::ostringstream messageStream;
 		bool messageStreamInUse;
@@ -460,6 +460,16 @@ namespace
 	{
 	}
 
+    Globals* Globals::getInstance()
+    {
+        // According to C++11 Function-Local Initialization
+        // of static variables is supposed to be thread safe
+        // without risk of deadlocks.
+        static Globals inst;
+
+        return &inst;
+    }
+
 	void Globals::addCallSite(LLError::CallSite& site)
 	{
 		callSites.push_back(&site);
@@ -512,14 +522,17 @@ namespace LLError
 
 	typedef LLPointer<SettingsConfig> SettingsConfigPtr;
 
-	class Settings : public LLSingleton<Settings>
+	class Settings
 	{
-		LLSINGLETON(Settings);
+    public:
+        static Settings* getInstance();
+    protected:
+		Settings();
 	public:
 		SettingsConfigPtr getSettingsConfig();
 
 		void reset();
-		SettingsStoragePtr saveAndReset();
+		SettingsStoragePtr saveAndReset(); 
 		void restore(SettingsStoragePtr pSettingsStorage);
 		
 	private:
@@ -553,6 +566,16 @@ namespace LLError
 	{
 	}
 
+    Settings* Settings::getInstance()
+    {
+        // According to C++11 Function-Local Initialization
+        // of static variables is supposed to be thread safe
+        // without risk of deadlocks.
+        static Settings inst;
+
+        return &inst;
+    }
+
 	SettingsConfigPtr Settings::getSettingsConfig()
 	{
 		return mSettingsConfig;
@@ -577,11 +600,6 @@ namespace LLError
 		SettingsConfigPtr newSettingsConfig(dynamic_cast<SettingsConfig *>(pSettingsStorage.get()));
 		mSettingsConfig = newSettingsConfig;
 	}
-
-	bool is_available()
-	{
-		return Settings::instanceExists() && Globals::instanceExists();
-	}
 }
 
 namespace LLError
@@ -1028,7 +1046,7 @@ namespace LLError
     std::pair<boost::shared_ptr<RECORDER>, Recorders::iterator>
     findRecorderPos()
     {
-        SettingsConfigPtr s = Settings::instance().getSettingsConfig();
+        SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig();
         // Since we promise to return an iterator, use a classic iterator
         // loop.
         auto end{s->mRecorders.end()};
@@ -1071,7 +1089,7 @@ namespace LLError
         auto found = findRecorderPos<RECORDER>();
         if (found.first)
         {
-            SettingsConfigPtr s = Settings::instance().getSettingsConfig();
+            SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig();
             s->mRecorders.erase(found.second);
         }
         return bool(found.first);
@@ -1307,14 +1325,6 @@ namespace LLError
 			return false;
 		}
 
-		// If we hit a logging request very late during shutdown processing,
-		// when either of the relevant LLSingletons has already been deleted,
-		// DO NOT resurrect them.
-		if (Settings::wasDeleted() || Globals::wasDeleted())
-		{
-			return false;
-		}
-
 		SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig();
 		
 		s->mShouldLogCallCounter++;
@@ -1353,10 +1363,8 @@ namespace LLError
 	std::ostringstream* Log::out()
 	{
 		LLMutexTrylock lock(getMutex<LOG_MUTEX>(),5);
-		// If we hit a logging request very late during shutdown processing,
-		// when either of the relevant LLSingletons has already been deleted,
-		// DO NOT resurrect them.
-		if (lock.isLocked() && ! (Settings::wasDeleted() || Globals::wasDeleted()))
+
+		if (lock.isLocked())
 		{
 			Globals* g = Globals::getInstance();
 
@@ -1378,14 +1386,6 @@ namespace LLError
 			return;
 		}
 
-		// If we hit a logging request very late during shutdown processing,
-		// when either of the relevant LLSingletons has already been deleted,
-		// DO NOT resurrect them.
-		if (Settings::wasDeleted() || Globals::wasDeleted())
-		{
-			return;
-		}
-
 		if(strlen(out->str().c_str()) < 128)
 		{
 			strcpy(message, out->str().c_str());
@@ -1418,14 +1418,6 @@ namespace LLError
 			return;
 		}
 
-		// If we hit a logging request very late during shutdown processing,
-		// when either of the relevant LLSingletons has already been deleted,
-		// DO NOT resurrect them.
-		if (Settings::wasDeleted() || Globals::wasDeleted())
-		{
-			return;
-		}
-
 		Globals* g = Globals::getInstance();
 		SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig();
 
-- 
cgit v1.2.3