livie
/
phoenix-firestorm
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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
master
Andrey Kleshchev 2020-09-22 23:38:23 +03:00
parent 738a651efa
commit 3424a1f965
4 changed files with 42 additions and 84 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

84
indra/llcommon/llerror.cpp
View File

@ -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();

5
indra/llcommon/llerrorcontrol.h
View File

@ -203,11 +203,6 @@ namespace LLError
LL_COMMON_API std::string abbreviateFile(const std::string& filePath);
LL_COMMON_API int shouldLogCallCount();
// Check whether Globals exists. This should only be used by LLSingleton
// infrastructure to avoid trying to log when our internal LLSingleton is
// unavailable -- circularity ensues.
LL_COMMON_API bool is_available();
};
#endif // LL_LLERRORCONTROL_H

31
indra/llcommon/llsingleton.cpp
View File

@ -28,7 +28,7 @@
#include "llsingleton.h"
#include "llerror.h"
#include "llerrorcontrol.h" // LLError::is_available()
#include "llerrorcontrol.h"
#include "lldependencies.h"
#include "llexception.h"
#include "llcoros.h"
@ -41,8 +41,6 @@
namespace {
void log(LLError::ELevel level,
const char* p1, const char* p2, const char* p3, const char* p4);
bool oktolog();
} // anonymous namespace
// Our master list of all LLSingletons is itself an LLSingleton. We used to
@ -279,8 +277,6 @@ void LLSingletonBase::reset_initializing(list_t::size_type size)
void LLSingletonBase::MasterList::LockedInitializing::log(const char* verb, const char* name)
{
if (oktolog())
{
LL_DEBUGS("LLSingleton") << verb << ' ' << demangle(name) << ';';
if (mList)
{
@ -292,7 +288,6 @@ void LLSingletonBase::MasterList::LockedInitializing::log(const char* verb, cons
}
}
LL_ENDL;
}
}
void LLSingletonBase::capture_dependency()
@ -455,33 +450,11 @@ void LLSingletonBase::deleteAll()
/*---------------------------- Logging helpers -----------------------------*/
namespace {
bool oktolog()
{
// See comments in log() below.
return LLError::is_available();
}
void log(LLError::ELevel level,
const char* p1, const char* p2, const char* p3, const char* p4)
{
// The is_available() test below ensures that we'll stop logging once
// LLError has been cleaned up. If we had a similar portable test for
// std::cerr, this would be a good place to use it.
// Check LLError::is_available() because some of LLError's infrastructure
// is itself an LLSingleton. If that LLSingleton has not yet been
// initialized, trying to log will engage LLSingleton machinery... and
// around and around we go.
if (LLError::is_available())
{
LL_VLOGS(level, "LLSingleton") << p1 << p2 << p3 << p4 << LL_ENDL;
}
else
{
// Caller may be a test program, or something else whose stderr is
// visible to the user.
std::cerr << p1 << p2 << p3 << p4 << std::endl;
}
LL_VLOGS(level, "LLSingleton") << p1 << p2 << p3 << p4 << LL_ENDL;
}
} // anonymous namespace

6
indra/newview/llappviewer.cpp
View File

@ -2131,14 +2131,12 @@ bool LLAppViewer::cleanup()
// still see above are calls that MUST happen before the generic cleanup
// kicks in.
// The logging subsystem depends on an LLSingleton. Any logging after
// LLSingletonBase::deleteAll() won't be recorded.
LL_INFOS() << "Goodbye!" << LL_ENDL;
// This calls every remaining LLSingleton's cleanupSingleton() and
// deleteSingleton() methods.
LLSingletonBase::deleteAll();
LL_INFOS() << "Goodbye!" << LL_ENDL;
removeDumpDir();
// return 0;