Start up crash of LLErrorThread due to corrupt map in CheckLevelMap

Wrapped gSettings and the global objects returned by Settings::get() and
Globals::get() in AIThreadSafe, forcing thread-safe access.

This solves the problem of possible corruption of the various LevelMap's
in LLError::Settings due to thread unsafe accesses.

doc/contributions.txt

@@ -112,6 +112,7 @@ Aleric Inglewood
 	IMP-664
 	IMP-670
 	IMP-701
+	IMP-734
 Alissa Sabre
 	VWR-81
 	VWR-83

indra/llcommon/llapp.cpp

@@ -95,7 +95,6 @@ LLAppChildCallback LLApp::sDefaultChildCallback = NULL;
 LLApp::LLApp() : mThreadErrorp(NULL)
 {
 	commonCtor();
-	startErrorThread();
 }
 
 void LLApp::commonCtor()

indra/llcommon/llapp.h

@@ -256,9 +256,12 @@ protected:
 	  */
 	void stepFrame();
 
-private:
+	/**
+	 * @ brief This method is called once as soon as logging is initialized.
+	 */
 	void startErrorThread();
-	
+
+private:
 	void setupErrorHandling();		// Do platform-specific error-handling setup (signals, structured exceptions)
 	static void runErrorHandler(); // run shortly after we detect an error, ran in the relatively robust context of the LLErrorThread - preferred.
 	static void runSyncErrorHandler(); // run IMMEDIATELY when we get an error, ran in the context of the faulting thread.

indra/llcommon/llerror.cpp

@@ -55,6 +55,7 @@
 #include "llsdserialize.h"
 #include "llstl.h"
 #include "lltimer.h"
+#include "aithreadsafe.h"
 
 extern apr_thread_mutex_t* gCallStacksLogMutexp;
 
@@ -357,12 +358,15 @@ namespace
 		void addCallSite(LLError::CallSite&);
 		void invalidateCallSites();
 		
-		static Globals& get();
+		static AIThreadSafeSimple<Globals>& get();
 			// return the one instance of the globals
 
 	private:
 		CallSiteVector callSites;
 
+		friend class AIThreadSafeSimpleDC<Globals>;		// Calls constructor.
+		friend class AIThreadSafeSimple<Globals>;		// Calls destructor.
+
 		Globals()
 			:	messageStreamInUse(false)
 			{ }
@@ -386,7 +390,7 @@ namespace
 		callSites.clear();
 	}
 
-	Globals& Globals::get()
+	AIThreadSafeSimple<Globals>& Globals::get()
 	{
 		/* This pattern, of returning a reference to a static function
 		   variable, is to ensure that this global is constructed before
@@ -394,8 +398,8 @@ namespace
 		   is.
 		   See C++ FAQ Lite, sections 10.12 through 10.14
 		*/
-		static Globals* globals = new Globals;		
+		static AIThreadSafeSimpleDCRootPool<Globals>* ts_globals_ptr = new AIThreadSafeSimpleDCRootPool<Globals>;
-		return *globals;
+		return *ts_globals_ptr;
 	}
 }
 
@@ -424,13 +428,16 @@ namespace LLError
 		
 		int shouldLogCallCounter;
 		
-		static Settings& get();
+		static AIThreadSafeSimple<Settings>& get();
 	
 		static void reset();
-		static Settings* saveAndReset();
+		static AIThreadSafeSimple<Settings>* saveAndReset();
-		static void restore(Settings*);
+		static void restore(AIThreadSafeSimple<Settings>*);
 		
 	private:
+		friend class AIThreadSafeBits<Settings>;		// Calls destructor.
+		friend class AIThreadSafeSimpleDC<Settings>;	// Calls constructor.
+
 		Settings()
 			:	printLocation(false),
 				defaultLevel(LLError::LEVEL_DEBUG),
@@ -446,53 +453,42 @@ namespace LLError
 			for_each(recorders.begin(), recorders.end(),
 					 DeletePointer());
 		}
-		
+
-		static Settings*& getPtr();
+		static AIThreadSafeSimple<Settings>* sSettings;
 	};
+
+	// Pointer to current AIThreadSafeSimple<Settings> object if any (NULL otherwise).
+	AIThreadSafeSimple<Settings>* Settings::sSettings;
 	
-	Settings& Settings::get()
+	AIThreadSafeSimple<Settings>& Settings::get()
 	{
-		Settings* p = getPtr();
+		if (!sSettings)
-		if (!p)
 		{
 			reset();
-			p = getPtr();
 		}
-		return *p;
+		return *sSettings;
 	}
 	
 	void Settings::reset()
 	{
-		Globals::get().invalidateCallSites();
+		AIAccess<Globals>(Globals::get())->invalidateCallSites();
-		
+		delete sSettings;
-		Settings*& p = getPtr();
+		sSettings = new AIThreadSafeSimpleDC<Settings>;
-		delete p;
-		p = new Settings();
 	}
 	
-	Settings* Settings::saveAndReset()
+	AIThreadSafeSimple<Settings>* Settings::saveAndReset()
 	{
-		Globals::get().invalidateCallSites();
+		AIAccess<Globals>(Globals::get())->invalidateCallSites();
-		
+		AIThreadSafeSimple<Settings>* originalSettings = sSettings;
-		Settings*& p = getPtr();
+		sSettings = new AIThreadSafeSimpleDC<Settings>;
-		Settings* originalSettings = p;
-		p = new Settings();
 		return originalSettings;
 	}
 	
-	void Settings::restore(Settings* originalSettings)
+	void Settings::restore(AIThreadSafeSimple<Settings>* originalSettings)
 	{
-		Globals::get().invalidateCallSites();
+		AIAccess<Globals>(Globals::get())->invalidateCallSites();
-		
+		delete sSettings;
-		Settings*& p = getPtr();
+		sSettings = originalSettings;
-		delete p;
-		p = originalSettings;
-	}
-	
-	Settings*& Settings::getPtr()
-	{
-		static Settings* currentSettings = NULL;
-		return currentSettings;
 	}
 }
 
@@ -596,68 +592,64 @@ namespace LLError
 		commonInit(dir);
 	}
 
+	void setPrintLocation(AIAccess<Settings> const& settings_w, bool print)
+	{
+		settings_w->printLocation = print;
+	}
+
 	void setPrintLocation(bool print)
 	{
-		Settings& s = Settings::get();
+		setPrintLocation(AIAccess<Settings>(Settings::get()), print);
-		s.printLocation = print;
 	}
 
 	void setFatalFunction(const FatalFunction& f)
 	{
-		Settings& s = Settings::get();
+		AIAccess<Settings>(Settings::get())->crashFunction = f;
-		s.crashFunction = f;
 	}
 
     FatalFunction getFatalFunction()
     {
-        Settings& s = Settings::get();
+		return AIAccess<Settings>(Settings::get())->crashFunction;
-        return s.crashFunction;
     }
 
 	void setTimeFunction(TimeFunction f)
 	{
-		Settings& s = Settings::get();
+		AIAccess<Settings>(Settings::get())->timeFunction = f;
-		s.timeFunction = f;
+	}
+
+	void setDefaultLevel(AIAccess<Settings> const& settings_w, ELevel level)
+	{
+		AIAccess<Globals>(Globals::get())->invalidateCallSites();
+		settings_w->defaultLevel = level;
 	}
 
 	void setDefaultLevel(ELevel level)
 	{
-		Globals& g = Globals::get();
+		setDefaultLevel(AIAccess<Settings>(Settings::get()), level);
-		Settings& s = Settings::get();
-		g.invalidateCallSites();
-		s.defaultLevel = level;
 	}
 
 	void setFunctionLevel(const std::string& function_name, ELevel level)
 	{
-		Globals& g = Globals::get();
+		AIAccess<Globals>(Globals::get())->invalidateCallSites();
-		Settings& s = Settings::get();
+		AIAccess<Settings>(Settings::get())->functionLevelMap[function_name] = level;
-		g.invalidateCallSites();
-		s.functionLevelMap[function_name] = level;
 	}
 
 	void setClassLevel(const std::string& class_name, ELevel level)
 	{
-		Globals& g = Globals::get();
+		AIAccess<Globals>(Globals::get())->invalidateCallSites();
-		Settings& s = Settings::get();
+		AIAccess<Settings>(Settings::get())->classLevelMap[class_name] = level;
-		g.invalidateCallSites();
-		s.classLevelMap[class_name] = level;
 	}
 
 	void setFileLevel(const std::string& file_name, ELevel level)
 	{
-		Globals& g = Globals::get();
+		AIAccess<Globals>(Globals::get())->invalidateCallSites();
-		Settings& s = Settings::get();
+		AIAccess<Settings>(Settings::get())->fileLevelMap[file_name] = level;
-		g.invalidateCallSites();
-		s.fileLevelMap[file_name] = level;
 	}
 
 	void setTagLevel(const std::string& tag_name, ELevel level)
 	{
-		Globals& g = Globals::get();
+		AIAccess<Globals>(Globals::get())->invalidateCallSites();
-		Settings& s = Settings::get();
+		AIAccess<Settings>(Settings::get())->tagLevelMap[tag_name] = level;
-		g.invalidateCallSites();
-		s.tagLevelMap[tag_name] = level;
 	}
 }
 
@@ -701,18 +693,16 @@ namespace LLError
 {
 	void configure(const LLSD& config)
 	{
-		Globals& g = Globals::get();
+		AIAccess<Settings> settings_w(Settings::get());
-		Settings& s = Settings::get();
+		AIAccess<Globals>(Globals::get())->invalidateCallSites();
+		settings_w->functionLevelMap.clear();
+		settings_w->classLevelMap.clear();
+		settings_w->fileLevelMap.clear();
+		settings_w->tagLevelMap.clear();
+		settings_w->uniqueLogMessages.clear();
 		
-		g.invalidateCallSites();
+		setPrintLocation(settings_w, config["print-location"]);
-		s.functionLevelMap.clear();
+		setDefaultLevel(settings_w, decodeLevel(config["default-level"]));
-		s.classLevelMap.clear();
-		s.fileLevelMap.clear();
-		s.tagLevelMap.clear();
-		s.uniqueLogMessages.clear();
-		
-		setPrintLocation(config["print-location"]);
-		setDefaultLevel(decodeLevel(config["default-level"]));
 		
 		LLSD sets = config["settings"];
 		LLSD::array_const_iterator a, end;
@@ -722,10 +712,10 @@ namespace LLError
 			
 			ELevel level = decodeLevel(entry["level"]);
 			
-			setLevels(s.functionLevelMap,	entry["functions"],	level);
+			setLevels(settings_w->functionLevelMap, entry["functions"], level);
-			setLevels(s.classLevelMap,		entry["classes"],	level);
+			setLevels(settings_w->classLevelMap,	 entry["classes"],   level);
-			setLevels(s.fileLevelMap,		entry["files"],		level);
+			setLevels(settings_w->fileLevelMap,	 entry["files"],	 level);
-			setLevels(s.tagLevelMap,		entry["tags"],		level);
+			setLevels(settings_w->tagLevelMap,		 entry["tags"],		 level);
 		}
 	}
 }
@@ -742,26 +732,34 @@ namespace LLError
 
 
 
-	void addRecorder(Recorder* recorder)
+	void addRecorder(AIAccess<Settings> const& settings_w, Recorder* recorder)
 	{
 		if (recorder == NULL)
 		{
 			return;
 		}
-		Settings& s = Settings::get();
+		settings_w->recorders.push_back(recorder);
-		s.recorders.push_back(recorder);
+	}
+
+	void addRecorder(Recorder* recorder)
+	{
+		addRecorder(AIAccess<Settings>(Settings::get()), recorder);
+	}
+
+	void removeRecorder(AIAccess<Settings> const& settings_w, Recorder* recorder)
+	{
+		if (recorder == NULL)
+		{
+			return;
+		}
+		settings_w->recorders.erase(
+			std::remove(settings_w->recorders.begin(), settings_w->recorders.end(), recorder),
+			settings_w->recorders.end());
 	}
 
 	void removeRecorder(Recorder* recorder)
 	{
-		if (recorder == NULL)
+		removeRecorder(AIAccess<Settings>(Settings::get()), recorder);
-		{
-			return;
-		}
-		Settings& s = Settings::get();
-		s.recorders.erase(
-			std::remove(s.recorders.begin(), s.recorders.end(), recorder),
-			s.recorders.end());
 	}
 }
 
@@ -769,12 +767,12 @@ namespace LLError
 {
 	void logToFile(const std::string& file_name)
 	{
-		LLError::Settings& s = LLError::Settings::get();
+		AIAccess<Settings> settings_w(Settings::get());
 
-		removeRecorder(s.fileRecorder);
+		removeRecorder(settings_w, settings_w->fileRecorder);
-		delete s.fileRecorder;
+		delete settings_w->fileRecorder;
-		s.fileRecorder = NULL;
+		settings_w->fileRecorder = NULL;
-		s.fileRecorderFileName.clear();
+		settings_w->fileRecorderFileName.clear();
 		
 		if (file_name.empty())
 		{
@@ -788,54 +786,51 @@ namespace LLError
 			return;
 		}
 
-		s.fileRecorderFileName = file_name;
+		settings_w->fileRecorderFileName = file_name;
-		s.fileRecorder = f;
+		settings_w->fileRecorder = f;
-		addRecorder(f);
+		addRecorder(settings_w, f);
 	}
 	
 	void logToFixedBuffer(LLLineBuffer* fixedBuffer)
 	{
-		LLError::Settings& s = LLError::Settings::get();
+		AIAccess<Settings> settings_w(Settings::get());
 
-		removeRecorder(s.fixedBufferRecorder);
+		removeRecorder(settings_w, settings_w->fixedBufferRecorder);
-		delete s.fixedBufferRecorder;
+		delete settings_w->fixedBufferRecorder;
-		s.fixedBufferRecorder = NULL;
+		settings_w->fixedBufferRecorder = NULL;
 		
 		if (!fixedBuffer)
 		{
 			return;
 		}
 		
-		s.fixedBufferRecorder = new RecordToFixedBuffer(fixedBuffer);
+		settings_w->fixedBufferRecorder = new RecordToFixedBuffer(fixedBuffer);
-		addRecorder(s.fixedBufferRecorder);
+		addRecorder(settings_w, settings_w->fixedBufferRecorder);
 	}
 
 	std::string logFileName()
 	{
-		LLError::Settings& s = LLError::Settings::get();
+		return AIAccess<Settings>(Settings::get())->fileRecorderFileName;
-		return s.fileRecorderFileName;
 	}
 }
 
 namespace
 {
-	void writeToRecorders(LLError::ELevel level, const std::string& message)
+	void writeToRecorders(AIAccess<LLError::Settings> const& settings_w, LLError::ELevel level, const std::string& message)
 	{
-		LLError::Settings& s = LLError::Settings::get();
-	
 		std::string messageWithTime;
-		
+
-		for (Recorders::const_iterator i = s.recorders.begin();
+		for (Recorders::const_iterator i = settings_w->recorders.begin();
-			i != s.recorders.end();
+			i != settings_w->recorders.end();
 			++i)
 		{
 			LLError::Recorder* r = *i;
 			
-			if (r->wantsTime()  &&  s.timeFunction != NULL)
+			if (r->wantsTime()  &&  settings_w->timeFunction != NULL)
 			{
 				if (messageWithTime.empty())
 				{
-					messageWithTime = s.timeFunction() + " " + message;
+					messageWithTime = settings_w->timeFunction() + " " + message;
 				}
 				
 				r->recordMessage(level, messageWithTime);
@@ -955,10 +950,9 @@ namespace LLError
 			return false;
 		}
 		
-		Globals& g = Globals::get();
+		AIAccess<Settings> settings_w(Settings::get());
-		Settings& s = Settings::get();
 		
-		s.shouldLogCallCounter += 1;
+		settings_w->shouldLogCallCounter += 1;
 		
 		std::string class_name = className(site.mClassInfo);
 		std::string function_name = functionName(site.mFunction);
@@ -967,20 +961,20 @@ namespace LLError
 			function_name = class_name + "::" + function_name;
 		}
 
-		ELevel compareLevel = s.defaultLevel;
+		ELevel compareLevel = settings_w->defaultLevel;
 
 		// The most specific match found will be used as the log level,
 		// since the computation short circuits.
 		// So, in increasing order of importance:
 		// Default < Broad Tag < File < Class < Function < Narrow Tag
-		((site.mNarrowTag != NULL) ? checkLevelMap(s.tagLevelMap, site.mNarrowTag, compareLevel) : false)
+		((site.mNarrowTag != NULL) ? checkLevelMap(settings_w->tagLevelMap, site.mNarrowTag, compareLevel) : false)
-		|| checkLevelMap(s.functionLevelMap, function_name, compareLevel)
+		|| checkLevelMap(settings_w->functionLevelMap, function_name, compareLevel)
-		|| checkLevelMap(s.classLevelMap, class_name, compareLevel)
+		|| checkLevelMap(settings_w->classLevelMap, class_name, compareLevel)
-		|| checkLevelMap(s.fileLevelMap, abbreviateFile(site.mFile), compareLevel)
+		|| checkLevelMap(settings_w->fileLevelMap, abbreviateFile(site.mFile), compareLevel)
-		|| ((site.mBroadTag != NULL) ? checkLevelMap(s.tagLevelMap, site.mBroadTag, compareLevel) : false);
+		|| ((site.mBroadTag != NULL) ? checkLevelMap(settings_w->tagLevelMap, site.mBroadTag, compareLevel) : false);
 
 		site.mCached = true;
-		g.addCallSite(site);
+		AIAccess<Globals>(Globals::get())->addCallSite(site);
 		return site.mShouldLog = site.mLevel >= compareLevel;
 	}
 
@@ -990,16 +984,16 @@ namespace LLError
 		LogLock lock;
 		if (lock.ok())
 		{
-			Globals& g = Globals::get();
+			AIAccess<Globals> globals(Globals::get());
 
-			if (!g.messageStreamInUse)
+			if (!globals->messageStreamInUse)
 			{
-				g.messageStreamInUse = true;
+				globals->messageStreamInUse = true;
-				return &g.messageStream;
+				return &globals->messageStream;			// Returns pointer to member of unlocked object, apparently "protected" by having set globals->messageStreamInUse.
 			}
 		}
 		
-		return new std::ostringstream;
+		return new std::ostringstream;				// Holy memory leak.
 	}
 	
 	void Log::flush(std::ostringstream* out, char* message)
@@ -1020,12 +1014,12 @@ namespace LLError
 		   message[127] = '\0' ;
 	   }
 	   
-	   Globals& g = Globals::get();
+	   AIAccess<Globals> globals(Globals::get());
-       if (out == &g.messageStream)
+       if (out == &globals->messageStream)
        {
-           g.messageStream.clear();
+           globals->messageStream.clear();
-           g.messageStream.str("");
+           globals->messageStream.str("");
-           g.messageStreamInUse = false;
+           globals->messageStreamInUse = false;
        }
        else
        {
@@ -1042,28 +1036,31 @@ namespace LLError
 			return;
 		}
 		
-		Globals& g = Globals::get();
-		Settings& s = Settings::get();
-
 		std::string message = out->str();
-		if (out == &g.messageStream)
+
 		{
-			g.messageStream.clear();
+			AIAccess<Globals> globals(Globals::get());
-			g.messageStream.str("");
+			if (out == &globals->messageStream)
-			g.messageStreamInUse = false;
+			{
-		}
+				globals->messageStream.clear();
-		else
+				globals->messageStream.str("");
-		{
+				globals->messageStreamInUse = false;
-			delete out;
+			}
+			else
+			{
+				delete out;
+			}
 		}
 
+		AIAccess<Settings> settings_w(Settings::get());
+
 		if (site.mLevel == LEVEL_ERROR)
 		{
 			std::ostringstream fatalMessage;
 			fatalMessage << abbreviateFile(site.mFile)
 						<< "(" << site.mLine << ") : error";
 			
-			writeToRecorders(site.mLevel, fatalMessage.str());
+			writeToRecorders(settings_w, site.mLevel, fatalMessage.str());
 		}
 		
 		
@@ -1078,7 +1075,7 @@ namespace LLError
 			default:				prefix << "XXX: ";		break;
 		};
 		
-		if (s.printLocation)
+		if (settings_w->printLocation)
 		{
 			prefix << abbreviateFile(site.mFile)
 					<< "(" << site.mLine << ") : ";
@@ -1096,8 +1093,8 @@ namespace LLError
 
 		if (site.mPrintOnce)
 		{
-			std::map<std::string, unsigned int>::iterator messageIter = s.uniqueLogMessages.find(message);
+			std::map<std::string, unsigned int>::iterator messageIter = settings_w->uniqueLogMessages.find(message);
-			if (messageIter != s.uniqueLogMessages.end())
+			if (messageIter != settings_w->uniqueLogMessages.end())
 			{
 				messageIter->second++;
 				unsigned int num_messages = messageIter->second;
@@ -1113,14 +1110,14 @@ namespace LLError
 			else 
 			{
 				prefix << "ONCE: ";
-				s.uniqueLogMessages[message] = 1;
+				settings_w->uniqueLogMessages[message] = 1;
 			}
 		}
 
 		if (site.mPrintOnce)
 		{
-			std::map<std::string, unsigned int>::iterator messageIter = s.uniqueLogMessages.find(message);
+			std::map<std::string, unsigned int>::iterator messageIter = settings_w->uniqueLogMessages.find(message);
-			if (messageIter != s.uniqueLogMessages.end())
+			if (messageIter != settings_w->uniqueLogMessages.end())
 			{
 				messageIter->second++;
 				unsigned int num_messages = messageIter->second;
@@ -1136,18 +1133,18 @@ namespace LLError
 			else 
 			{
 				prefix << "ONCE: ";
-				s.uniqueLogMessages[message] = 1;
+				settings_w->uniqueLogMessages[message] = 1;
 			}
 		}
 		
 		prefix << message;
 		message = prefix.str();
 		
-		writeToRecorders(site.mLevel, message);
+		writeToRecorders(settings_w, site.mLevel, message);
 		
-		if (site.mLevel == LEVEL_ERROR  &&  s.crashFunction)
+		if (site.mLevel == LEVEL_ERROR  &&  settings_w->crashFunction)
 		{
-			s.crashFunction(message);
+			settings_w->crashFunction(message);
 		}
 	}
 }
@@ -1157,14 +1154,16 @@ namespace LLError
 
 namespace LLError
 {
-	Settings* saveAndResetSettings()
+	class ThreadSafeSettings { };
+
+	ThreadSafeSettings* saveAndResetSettings()
 	{
-		return Settings::saveAndReset();
+		return reinterpret_cast<ThreadSafeSettings*>(Settings::saveAndReset());
 	}
 	
-	void restoreSettings(Settings* s)
+	void restoreSettings(ThreadSafeSettings* s)
 	{
-		return Settings::restore(s);
+		Settings::restore(reinterpret_cast<AIThreadSafeSimple<Settings>*>(s));
 	}
 
 	std::string removePrefix(std::string& s, const std::string& p)
@@ -1210,8 +1209,7 @@ namespace LLError
 
 	int shouldLogCallCount()
 	{
-		Settings& s = Settings::get();
+		return AIAccess<Settings>(Settings::get())->shouldLogCallCounter;
-		return s.shouldLogCallCounter;
 	}
 
 #if LL_WINDOWS

indra/llcommon/llerrorcontrol.h

@@ -167,9 +167,9 @@ namespace LLError
 		Utilities for use by the unit tests of LLError itself.
 	*/
 
-	class Settings;
+	class ThreadSafeSettings;
-	LL_COMMON_API Settings* saveAndResetSettings();
+	LL_COMMON_API ThreadSafeSettings* saveAndResetSettings();
-	LL_COMMON_API void restoreSettings(Settings *);
+	LL_COMMON_API void restoreSettings(ThreadSafeSettings *);
 		
 	LL_COMMON_API std::string abbreviateFile(const std::string& filePath);
 	LL_COMMON_API int shouldLogCallCount();

indra/llxml/llcontrol.cpp

@@ -66,7 +66,20 @@ const S32 CURRENT_VERSION = 101;
 //So, a global it is!
 bool gCOAEnabled = false; 
 
-LLControlVariable *LLControlVariable::getCOAActive()
+LLControlVariable* LLControlVariable::getCOAActive()
+{
+	//if no coa connection, return 'this'
+	//if per account is ON and this IS a parent, return child var
+	//if per account is ON and this IS NOT a parent, return 'this'
+	//if per account is OFF and this IS NOT a parent, return parent var
+	//if per account is OFF and this IS a parent, return 'this'
+	if(getCOAConnection() && gCOAEnabled == isCOAParent())
+		return getCOAConnection();
+	else
+		return this;
+}
+
+LLControlVariable const* LLControlVariable::getCOAActive() const
 {
 	//if no coa connection, return 'this'
 	//if per account is ON and this IS a parent, return child var
@@ -282,11 +295,9 @@ LLSD LLControlVariable::getSaveValue() const
 
 #if PROF_CTRL_CALLS
 std::vector<std::pair<std::string, U32>> gSettingsCallMap;
-#endif //PROF_CTRL_CALLS
+
-LLPointer<LLControlVariable> LLControlGroup::getControl(const std::string& name)
+static update_gSettingsCallMap(ctrl_name_table_t::const_iterator const& iter)
 {
-	ctrl_name_table_t::iterator iter = mNameTable.find(name);
-#if PROF_CTRL_CALLS
 	if(iter != mNameTable.end())
 	{
 		std::vector<std::pair<std::string, U32>>::iterator iter2 = gSettingsCallMap.begin();
@@ -302,13 +313,32 @@ LLPointer<LLControlVariable> LLControlGroup::getControl(const std::string& name)
 		if(iter2 == gSettingsCallMap.end())
 			gSettingsCallMap.push_back(std::pair<std::string, U32>(name.c_str(),1));
 	}
+}
+#endif //PROF_CTRL_CALLS
+
+LLControlVariable* LLControlGroup::getControl(std::string const& name)
+{
+	ctrl_name_table_t::iterator iter = mNameTable.find(name);
+#if PROF_CTRL_CALLS
+	update_gSettingsCallMap(iter);
 #endif //PROF_CTRL_CALLS
 	if(iter != mNameTable.end())
 		return iter->second->getCOAActive();
 	else
-		return LLPointer<LLControlVariable>();
+		return NULL;
 }
 
+LLControlVariable const* LLControlGroup::getControl(std::string const& name) const
+{
+	ctrl_name_table_t::const_iterator iter = mNameTable.find(name);
+#if PROF_CTRL_CALLS
+	update_gSettingsCallMap(iter);
+#endif //PROF_CTRL_CALLS
+	if(iter != mNameTable.end())
+		return iter->second->getCOAActive();
+	else
+		return NULL;
+}
 
 ////////////////////////////////////////////////////////////////////////////
 
@@ -502,9 +532,9 @@ std::string LLControlGroup::findString(const std::string& name)
 	return LLStringUtil::null;
 }
 
-std::string LLControlGroup::getString(const std::string& name)
+std::string LLControlGroup::getString(const std::string& name) const
 {
-	LLControlVariable* control = getControl(name);
+	LLControlVariable const* control = getControl(name);
 	
 	if (control && control->isType(TYPE_STRING))
 		return control->get().asString();
@@ -649,9 +679,9 @@ LLSD LLControlGroup::getLLSD(const std::string& name)
 	return LLSD();
 }
 
-BOOL LLControlGroup::controlExists(const std::string& name)
+BOOL LLControlGroup::controlExists(const std::string& name) const
 {
-	ctrl_name_table_t::iterator iter = mNameTable.find(name);
+	ctrl_name_table_t::const_iterator iter = mNameTable.find(name);
 	return iter != mNameTable.end();
 }
 

indra/llxml/llcontrol.h

@@ -122,7 +122,7 @@ public:
 	const std::string& getComment() const { return mComment; }
 
 	eControlType type()		{ return mType; }
-	bool isType(eControlType tp) { return tp == mType; }
+	bool isType(eControlType tp) const { return tp == mType; }
 
 	void resetToDefault(bool fire_signal = false);
 
@@ -153,7 +153,8 @@ public:
 	bool isCOA()		const	{ return mIsCOA; }
 	bool isCOAParent()	const	{ return mIsCOAParent; }
 	LLControlVariable *getCOAConnection() const	{ return mCOAConnectedVar; }
-	LLControlVariable *getCOAActive();
+	LLControlVariable* getCOAActive();
+	LLControlVariable const* getCOAActive() const;
 	void setIsCOA(bool IsCOA)  { mIsCOA=IsCOA; }
 	void setCOAConnect(LLControlVariable *pConnect, bool IsParent) 
 	{
@@ -185,7 +186,8 @@ public:
 	~LLControlGroup();
 	void cleanup();
 	
-	LLPointer<LLControlVariable> getControl(const std::string& name);
+	LLControlVariable* getControl(std::string const& name);
+	LLControlVariable const* getControl(std::string const& name) const;
 
 	struct ApplyFunctor
 	{
@@ -210,7 +212,7 @@ public:
 	
 	std::string 	findString(const std::string& name);
 
-	std::string 	getString(const std::string& name);
+	std::string getString(const std::string& name) const;
 	LLWString	getWString(const std::string& name);
 	std::string	getText(const std::string& name);
 	LLVector3	getVector3(const std::string& name);
@@ -245,7 +247,7 @@ public:
 	void	setValue(const std::string& name, const LLSD& val);
 	
 	
-	BOOL    controlExists(const std::string& name);
+	BOOL    controlExists(const std::string& name) const;
 
 	// Returns number of controls loaded, 0 if failed
 	// If require_declaration is false, will auto-declare controls it finds

indra/llxml/llcontrolgroupreader.h

@@ -46,7 +46,7 @@ public:
 	LLControlGroupReader() {}
 	virtual ~LLControlGroupReader() {}
 
-	virtual std::string 	getString(const std::string& name) = 0;
+	virtual std::string 	getString(const std::string& name) const = 0;
 	//virtual LLWString	getWString(const std::string& name) = 0;
 	virtual std::string	getText(const std::string& name) = 0;
 	//virtual LLVector3	getVector3(const std::string& name) = 0;

indra/newview/llappviewer.cpp

@@ -568,7 +568,10 @@ bool LLAppViewer::init()
 	gDirUtilp->setSkinFolder("default");
 
 	initLogging();
-	
+
+	// Logging is initialized. Now it's safe to start the error thread.
+	startErrorThread();
+
 	// <edit>
 	gDeleteScheduler = new LLDeleteScheduler();
 	gBuildNewViewsScheduler = new LLBuildNewViewsScheduler();
@@ -1578,8 +1581,9 @@ bool LLAppViewer::initLogging()
 	return true;
 }
 
-bool LLAppViewer::loadSettingsFromDirectory(const std::string& location_key,
+bool LLAppViewer::loadSettingsFromDirectory(AIReadAccess<settings_map_type> const& settings_r,
-					    bool set_defaults)
+                                            std::string const& location_key,
+                                            bool set_defaults)
 {	
 	// Find and vet the location key.
 	if(!mSettingsLocationList.has(location_key))
@@ -1606,11 +1610,13 @@ bool LLAppViewer::loadSettingsFromDirectory(const std::string& location_key,
 	LLSD files = location.get("Files");
 	for(LLSD::map_iterator itr = files.beginMap(); itr != files.endMap(); ++itr)
 	{
-		std::string settings_group = (*itr).first;
+		std::string const settings_group = (*itr).first;
+		settings_map_type::const_iterator const settings_group_iter = settings_r->find(settings_group);
+
 		llinfos << "Attempting to load settings for the group " << settings_group 
 			    << " - from location " << location_key << llendl;
 
-		if(gSettings.find(settings_group) == gSettings.end())
+		if(settings_group_iter == settings_r->end())
 		{
 			llwarns << "No matching settings group for name " << settings_group << llendl;
 			continue;
@@ -1624,11 +1630,10 @@ bool LLAppViewer::loadSettingsFromDirectory(const std::string& location_key,
 			std::string custom_name_setting = file.get("NameFromSetting");
 			// *NOTE: Regardless of the group currently being lodaed,
 			// this setting is always read from the Global settings.
-			if(gSettings[sGlobalSettingsName]->controlExists(custom_name_setting))
+			LLControlGroup const* control_group = settings_r->find(sGlobalSettingsName)->second;
+			if(control_group->controlExists(custom_name_setting))
 			{
-				std::string file_name = 
+				full_settings_path = control_group->getString(custom_name_setting);
-					gSettings[sGlobalSettingsName]->getString(custom_name_setting);
-				full_settings_path = file_name;
 			}
 		}
 
@@ -1644,7 +1649,7 @@ bool LLAppViewer::loadSettingsFromDirectory(const std::string& location_key,
 			requirement = file.get("Requirement").asInteger();
 		}
 		
-		if(!gSettings[settings_group]->loadFromFile(full_settings_path, set_defaults))
+		if(!settings_group_iter->second->loadFromFile(full_settings_path, set_defaults))
 		{
 			if(requirement == 1)
 			{
@@ -1686,10 +1691,14 @@ std::string LLAppViewer::getSettingsFilename(const std::string& location_key,
 
 bool LLAppViewer::initConfiguration()
 {	
+	// Grab and hold write locks for the entire duration of this function.
+	AIWriteAccess<settings_map_type> settings_w(gSettings);
+	settings_map_type& settings(*settings_w);
+
 	//Set up internal pointers	
-	gSettings[sGlobalSettingsName] = &gSavedSettings;
+	settings[sGlobalSettingsName] = &gSavedSettings;
-	gSettings[sPerAccountSettingsName] = &gSavedPerAccountSettings;
+	settings[sPerAccountSettingsName] = &gSavedPerAccountSettings;
-	gSettings[sCrashSettingsName] = &gCrashSettings;
+	settings[sCrashSettingsName] = &gCrashSettings;
 
 	//Load settings files list
 	std::string settings_file_list = gDirUtilp->getExpandedFilename(LL_PATH_APP_SETTINGS, "settings_files.xml");
@@ -1714,7 +1723,7 @@ bool LLAppViewer::initConfiguration()
 	
 	// - load defaults
 	bool set_defaults = true;
-	if(!loadSettingsFromDirectory("Default", set_defaults))
+	if(!loadSettingsFromDirectory(settings_w, "Default", set_defaults))
 	{
 		std::ostringstream msg;
 		msg << "Second Life could not load its default settings file. \n" 
@@ -1830,7 +1839,7 @@ bool LLAppViewer::initConfiguration()
 	}
 
 	// - load overrides from user_settings 
-	loadSettingsFromDirectory("User");
+	loadSettingsFromDirectory(settings_w, "User");
 
 	// - apply command line settings 
 	clp.notify(); 
@@ -1910,7 +1919,7 @@ bool LLAppViewer::initConfiguration()
             {
                 const std::string& name = *itr;
                 const std::string& value = *(++itr);
-                LLControlVariable* c = gSettings[sGlobalSettingsName]->getControl(name);
+                LLControlVariable* c = settings[sGlobalSettingsName]->getControl(name);
                 if(c)
                 {
                     c->setValue(value, false);

indra/newview/llappviewer.h

@@ -33,7 +33,8 @@
 #ifndef LL_LLAPPVIEWER_H
 #define LL_LLAPPVIEWER_H
 
-#include "llsys.h"	// LLOSInfo
+#include "llsys.h"				// LLOSInfo
+#include "llviewercontrol.h"	// settings_map_type
 
 class LLTextureCache;
 class LLImageDecodeThread;
@@ -140,11 +141,12 @@ public:
 	// Load settings from the location specified by loction_key.
 	// Key availale and rules for loading, are specified in 
 	// 'app_settings/settings_files.xml'
-	bool loadSettingsFromDirectory(const std::string& location_key, 
+	bool loadSettingsFromDirectory(AIReadAccess<settings_map_type> const& settings_r,
-				       bool set_defaults = false);
+	                               std::string const& location_key,
+	                               bool set_defaults = false);
 
-	std::string getSettingsFilename(const std::string& location_key,
+	std::string getSettingsFilename(std::string const& location_key,
-					const std::string& file);
+	                                std::string const& file);
 
 	// For thread debugging. 
 	// llstartup needs to control init.

indra/newview/llfloatersettingsdebug.cpp

@@ -104,7 +104,7 @@ BOOL LLFloaterSettingsDebug::postBuild()
 void LLFloaterSettingsDebug::draw()
 {
 	LLComboBox* settings_combo = getChild<LLComboBox>("settings_combo");
-	LLControlVariable* controlp = (LLControlVariable*)settings_combo->getCurrentUserdata();
+	LLControlVariable* controlp = static_cast<LLControlVariable*>(settings_combo->getCurrentUserdata());
 	updateControl(controlp ? controlp->getCOAActive() : NULL);
 
 	LLFloater::draw();
@@ -127,8 +127,8 @@ void LLFloaterSettingsDebug::show(void*)
 void LLFloaterSettingsDebug::onSettingSelect(LLUICtrl* ctrl, void* user_data)
 {
 	LLFloaterSettingsDebug* floaterp = (LLFloaterSettingsDebug*)user_data;
-	LLComboBox* combo_box = (LLComboBox*)ctrl;
+	LLComboBox* combo_box = static_cast<LLComboBox*>(ctrl);
-	LLControlVariable* controlp = (LLControlVariable*)combo_box->getCurrentUserdata();
+	LLControlVariable* controlp = static_cast<LLControlVariable*>(combo_box->getCurrentUserdata());
 
 	floaterp->updateControl(controlp ? controlp->getCOAActive() : NULL);
 }
@@ -139,7 +139,7 @@ void LLFloaterSettingsDebug::onCommitSettings(LLUICtrl* ctrl, void* user_data)
 	LLFloaterSettingsDebug* floaterp = (LLFloaterSettingsDebug*)user_data;
 
 	LLComboBox* settings_combo = floaterp->getChild<LLComboBox>("settings_combo");
-	LLControlVariable* controlp = (LLControlVariable*)settings_combo->getCurrentUserdata();
+	LLControlVariable* controlp = static_cast<LLControlVariable*>(settings_combo->getCurrentUserdata());
 	controlp = controlp ? controlp->getCOAActive() : NULL;
 	if(!controlp)//Uh oh!
 		return;

indra/newview/llstartup.cpp

@@ -985,7 +985,7 @@ bool idle_startup()
 			);
 
 		// Overwrite default user settings with user settings								 
-		LLAppViewer::instance()->loadSettingsFromDirectory("Account");
+		LLAppViewer::instance()->loadSettingsFromDirectory(AIReadAccess<settings_map_type>(gSettings), "Account");
 
 		// Need to set the LastLogoff time here if we don't have one.  LastLogoff is used for "Recent Items" calculation
 		// and startup time is close enough if we don't have a real value.

indra/newview/llviewercontrol.cpp

@@ -75,6 +75,7 @@
 #include "llnetmap.h"
 #include "llrender.h"
 #include "llfloaterchat.h"
+#include "aithreadsafe.h"
 #include "llviewerobjectlist.h"
 #include "lldrawpoolbump.h"
 #include "emeraldboobutils.h"
@@ -83,8 +84,7 @@
 BOOL 				gHackGodmode = FALSE;
 #endif
 
-
+AITHREADSAFE(settings_map_type, gSettings,);
-std::map<std::string, LLControlGroup*> gSettings;
 LLControlGroup gSavedSettings;	// saved at end of session
 LLControlGroup gSavedPerAccountSettings; // saved at end of session
 LLControlGroup gColors;			// read-only

indra/newview/llviewercontrol.h

@@ -36,6 +36,7 @@
 #include <map>
 #include "llcontrol.h"
 #include "lluictrl.h"
+#include "aithreadsafe.h"
 
 // Enabled this definition to compile a 'hacked' viewer that
 // allows a hacked godmode to be toggled on and off.
@@ -48,7 +49,8 @@ extern BOOL gHackGodmode;
 //setting variables are declared in this function
 void settings_setup_listeners();
 
-extern std::map<std::string, LLControlGroup*> gSettings;
+typedef std::map<std::string, LLControlGroup*> settings_map_type;
+extern AIThreadSafe<settings_map_type> gSettings;
 
 // for the graphics settings
 void create_graphics_group(LLControlGroup& group);

indra/newview/tests/llagentaccess_test.cpp

@@ -43,7 +43,7 @@ class LLControlGroupReader_Test : public LLControlGroupReader
 public:
 	LLControlGroupReader_Test() : test_preferred_maturity(SIM_ACCESS_PG) {}
 	
-	virtual std::string getString(const std::string& name)
+	virtual std::string getString(const std::string& name) const
 	{
 		return "";
 	}