From e86c0b3d0fbc5f91090241be959ef19bfffd8636 Mon Sep 17 00:00:00 2001
From: Oz Linden <oz@lindenlab.com>
Date: Tue, 5 Mar 2019 04:44:29 -0500
Subject: [PATCH] fix error message reporting?
---
indra/llcommon/llerror.cpp | 18 +++++------
indra/llcommon/llerror.h | 6 ++--
indra/llcommon/llerrorcontrol.h | 11 +++++--
indra/llcommon/tests/llerror_test.cpp | 8 +++--
indra/llcommon/tests/wrapllerrs.h | 8 ++---
indra/newview/llappviewer.cpp | 43 +++++++++------------------
indra/newview/llappviewermacosx.cpp | 6 ++--
indra/test/test.cpp | 7 +++--
8 files changed, 51 insertions(+), 56 deletions(-)
diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp
index 77d7fe1b24c..6fa9e590cb4 100644
--- a/indra/llcommon/llerror.cpp
+++ b/indra/llcommon/llerror.cpp
@@ -719,7 +719,7 @@ namespace LLError
commonInit(user_dir, app_dir, log_to_stderr);
}
- void overrideCrashOnError(const FatalFunction& fatal_function)
+ void setFatalHandler(const FatalFunction& fatal_function)
{
SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig();
s->mCrashFunction = fatal_function;
@@ -1306,12 +1306,12 @@ namespace LLError
return ;
}
- bool Log::flush(std::ostringstream* out, const CallSite& site)
+ ErrCrashHandlerResult Log::flush(std::ostringstream* out, const CallSite& site)
{
LLMutexTrylock lock(&gLogMutex,5);
if (!lock.isLocked())
{
- return false; // because this wasn't logged, it cannot be fatal
+ return ERR_DO_NOT_CRASH; // because this wasn't logged, it cannot be fatal
}
// If we hit a logging request very late during shutdown processing,
@@ -1319,7 +1319,7 @@ namespace LLError
// DO NOT resurrect them.
if (Settings::wasDeleted() || Globals::wasDeleted())
{
- return false; // because this wasn't logged, it cannot be fatal
+ return ERR_DO_NOT_CRASH; // because this wasn't logged, it cannot be fatal
}
Globals* g = Globals::getInstance();
@@ -1353,7 +1353,7 @@ namespace LLError
}
else
{
- return false; // because this wasn't logged, it cannot be fatal
+ return ERR_DO_NOT_CRASH; // because this wasn't logged, it cannot be fatal
}
}
else
@@ -1369,20 +1369,18 @@ namespace LLError
if (site.mLevel == LEVEL_ERROR)
{
- g->mFatalMessage = message;
if (s->mCrashFunction)
{
- s->mCrashFunction(message);
- return false; // because an override is in effect
+ return s->mCrashFunction(message);
}
else
{
- return true; // calling macro should crash
+ return ERR_CRASH; // calling macro should crash
}
}
else
{
- return false; // not ERROR, so do not crash
+ return ERR_DO_NOT_CRASH; // not ERROR, so do not crash
}
}
}
diff --git a/indra/llcommon/llerror.h b/indra/llcommon/llerror.h
index 07aa5c6f8b4..2a73ccb36a7 100644
--- a/indra/llcommon/llerror.h
+++ b/indra/llcommon/llerror.h
@@ -194,6 +194,8 @@ namespace LLError
struct CallSite;
+ enum ErrCrashHandlerResult { ERR_DO_NOT_CRASH, ERR_CRASH };
+
class LL_COMMON_API Log
{
public:
@@ -203,7 +205,7 @@ namespace LLError
static void flush(std::ostringstream* out, char* message);
// returns false iff the calling macro should crash
- static bool flush(std::ostringstream*, const CallSite&);
+ static ErrCrashHandlerResult flush(std::ostringstream*, const CallSite&);
static std::string demangle(const char* mangled);
};
@@ -386,7 +388,7 @@ volatile extern int* gCauseCrash;
#define LL_ENDL \
LLError::End(); \
- if (LLError::Log::flush(_out, _site)) \
+ if (LLError::ERR_CRASH == LLError::Log::flush(_out, _site)) \
LLERROR_CRASH \
} \
} while(0)
diff --git a/indra/llcommon/llerrorcontrol.h b/indra/llcommon/llerrorcontrol.h
index af46430f74b..2fa220ba5a6 100644
--- a/indra/llcommon/llerrorcontrol.h
+++ b/indra/llcommon/llerrorcontrol.h
@@ -93,16 +93,21 @@ namespace LLError
Control functions.
*/
- typedef boost::function<void(const std::string&)> FatalFunction;
+ // A FatalFunction is called if set using setFatalHandler; its return controls
+ // whether or not the calling error logging code should crash.
+ // ERR_DO_NOT_CRASH should be used only in test code.
+ typedef boost::function<LLError::ErrCrashHandlerResult(const std::string&)> FatalFunction;
+
/// Override the default behavior of crashing on LL_ERRS; this should NEVER be used except in test code
- LL_COMMON_API void overrideCrashOnError(const FatalFunction&);
+ LL_COMMON_API void setFatalHandler(const FatalFunction&);
// The fatal function will be called when an message of LEVEL_ERROR
// is logged. Note: supressing a LEVEL_ERROR message from being logged
// (by, for example, setting a class level to LEVEL_NONE), will keep
// the that message from causing the fatal funciton to be invoked.
+ // The
- /// Undo the effect of the overrideCrashOnError above
+ /// Undo the effect of the setFatalHandler above
LL_COMMON_API void restoreCrashOnError();
LL_COMMON_API std::string getFatalMessage();
diff --git a/indra/llcommon/tests/llerror_test.cpp b/indra/llcommon/tests/llerror_test.cpp
index 2f8923d2de3..9dcb5c145fc 100644
--- a/indra/llcommon/tests/llerror_test.cpp
+++ b/indra/llcommon/tests/llerror_test.cpp
@@ -70,7 +70,11 @@ namespace
namespace
{
static bool fatalWasCalled;
- void fatalCall(const std::string&) { fatalWasCalled = true; }
+ LLError::ErrCrashHandlerResult fatalCall(const std::string&)
+ {
+ fatalWasCalled = true;
+ return LLError::ERR_DO_NOT_CRASH;
+ }
}
namespace tut
@@ -120,7 +124,7 @@ namespace tut
mPriorErrorSettings = LLError::saveAndResetSettings();
LLError::setDefaultLevel(LLError::LEVEL_DEBUG);
- LLError::overrideCrashOnError(fatalCall);
+ LLError::setFatalHandler(fatalCall);
LLError::addRecorder(mRecorder);
}
diff --git a/indra/llcommon/tests/wrapllerrs.h b/indra/llcommon/tests/wrapllerrs.h
index fedc17dbe96..a412e12a040 100644
--- a/indra/llcommon/tests/wrapllerrs.h
+++ b/indra/llcommon/tests/wrapllerrs.h
@@ -46,7 +46,7 @@
// statically reference the function in test.cpp... it's short, we could
// replicate, but better to reuse
-extern void wouldHaveCrashed(const std::string& message);
+extern LLError::ErrCrashHandlerResult wouldHaveCrashed(const std::string& message);
struct WrapLLErrs
{
@@ -57,7 +57,7 @@ struct WrapLLErrs
:mPriorErrorSettings(LLError::saveAndResetSettings())
{
// Make LL_ERRS call our own operator() method
- LLError::overrideCrashOnError(boost::bind(&WrapLLErrs::operator(), this, _1));
+ LLError::setFatalHandler(boost::bind(&WrapLLErrs::operator(), this, _1));
}
~WrapLLErrs()
@@ -71,7 +71,7 @@ struct WrapLLErrs
FatalException(const std::string& what): LLException(what) {}
};
- void operator()(const std::string& message)
+ LLError::ErrCrashHandlerResult operator()(const std::string& message)
{
// Save message for later in case consumer wants to sense the result directly
error = message;
@@ -201,7 +201,7 @@ class CaptureLog : public boost::noncopyable
mOldSettings(LLError::saveAndResetSettings()),
mRecorder(new CaptureLogRecorder())
{
- LLError::overrideCrashOnError(wouldHaveCrashed);
+ LLError::setFatalHandler(wouldHaveCrashed);
LLError::setDefaultLevel(level);
LLError::addRecorder(mRecorder);
}
diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp
index f9ee22d20a7..8b9f9085b16 100644
--- a/indra/newview/llappviewer.cpp
+++ b/indra/newview/llappviewer.cpp
@@ -751,16 +751,7 @@ class LLUITranslationBridge : public LLTranslationBridge
}
};
-namespace {
-// With Xcode 6, _exit() is too magical to use with boost::bind(), so provide
-// this little helper function.
-void fast_exit(int rc)
-{
- _exit(rc);
-}
-
-
-}
+static S32 sQAModeTermCode = 0; // set from QAModeTermCode to specify exit code for LL_ERRS
bool LLAppViewer::init()
{
@@ -800,17 +791,8 @@ bool LLAppViewer::init()
initMaxHeapSize() ;
LLCoros::instance().setStackSize(gSavedSettings.getS32("CoroutineStackSize"));
- // Although initLoggingAndGetLastDuration() is the right place to mess with
- // overrideCrashOnError(), we can't query gSavedSettings until after
- // initConfiguration().
- S32 rc(gSavedSettings.getS32("QAModeTermCode"));
- if (rc >= 0)
- {
- // QAModeTermCode set, terminate with that rc on LL_ERRS. Use
- // fast_exit() rather than exit() because normal cleanup depends too
- // much on successful startup!
- LLError::overrideCrashOnError(boost::bind(fast_exit, rc));
- }
+ // if a return code is set for error exit, save it here for use in fatalErrorHandler
+ sQAModeTermCode = gSavedSettings.getS32("QAModeTermCode");
mAlloc.setProfilingEnabled(gSavedSettings.getBOOL("MemProfiling"));
@@ -2152,8 +2134,7 @@ bool LLAppViewer::initThreads()
return true;
}
-#ifndef LL_BUGSPLAT
-void errorCallback(const std::string &error_string)
+LLError::ErrCrashHandlerResult fatalErrorHandler(const std::string &error_string)
{
#ifndef LL_RELEASE_FOR_DOWNLOAD
OSMessageBox(error_string, LLTrans::getString("MBFatalError"), OSMB_OK);
@@ -2167,8 +2148,15 @@ void errorCallback(const std::string &error_string)
// haven't actually trashed anything yet, we can afford to write the whole
// static info file.
LLAppViewer::instance()->writeDebugInfo();
+
+ if (sQAModeTermCode)
+ {
+ _exit(sQAModeTermCode);
+ return LLError::ERR_DO_NOT_CRASH; // notreached
+ }
+
+ return LLError::ERR_CRASH;
}
-#endif // ! LL_BUGSPLAT
void LLAppViewer::initLoggingAndGetLastDuration()
{
@@ -2178,6 +2166,7 @@ void LLAppViewer::initLoggingAndGetLastDuration()
LLError::initForApplication( gDirUtilp->getExpandedFilename(LL_PATH_USER_SETTINGS, "")
,gDirUtilp->getExpandedFilename(LL_PATH_APP_SETTINGS, "")
);
+ LLError::setFatalHandler(fatalErrorHandler);
// Remove the last ".old" log file.
std::string old_log_file = gDirUtilp->getExpandedFilename(LL_PATH_LOGS,
@@ -3728,12 +3717,8 @@ void LLAppViewer::processMarkerFiles()
else if (marker_is_same_version)
{
// the file existed, is ours, and matched our version, so we can report on what it says
- LL_INFOS("MarkerFile") << "Exec marker '"<< mMarkerFileName << "' found; last exec FROZE" << LL_ENDL;
-# if LL_BUGSPLAT
+ LL_INFOS("MarkerFile") << "Exec marker '"<< mMarkerFileName << "' found; last exec crashed" << LL_ENDL;
gLastExecEvent = LAST_EXEC_OTHER_CRASH;
-# else
- gLastExecEvent = LAST_EXEC_FROZE;
-# endif
}
else
{
diff --git a/indra/newview/llappviewermacosx.cpp b/indra/newview/llappviewermacosx.cpp
index 3111540a131..9c2e6eadcad 100644
--- a/indra/newview/llappviewermacosx.cpp
+++ b/indra/newview/llappviewermacosx.cpp
@@ -188,17 +188,17 @@ CrashMetadataSingleton::CrashMetadataSingleton()
LLSD info;
if (! static_file.is_open())
{
- LL_INFOS() << "Can't open '" << staticDebugPathname
+ LL_WARNS() << "Can't open '" << staticDebugPathname
<< "'; no metadata about previous run" << LL_ENDL;
}
else if (! LLSDSerialize::deserialize(info, static_file, LLSDSerialize::SIZE_UNLIMITED))
{
- LL_INFOS() << "Can't parse '" << staticDebugPathname
+ LL_WARNS() << "Can't parse '" << staticDebugPathname
<< "'; no metadata about previous run" << LL_ENDL;
}
else
{
- LL_INFOS() << "Metadata from '" << staticDebugPathname << "':" << LL_ENDL;
+ LL_INFOS() << "Previous run metadata from '" << staticDebugPathname << "':" << LL_ENDL;
logFilePathname = get_metadata(info, "SLLog");
userSettingsPathname = get_metadata(info, "SettingsFilename");
OSInfo = get_metadata(info, "OSInfo");
diff --git a/indra/test/test.cpp b/indra/test/test.cpp
index 5dabcbbc584..4f966aede86 100644
--- a/indra/test/test.cpp
+++ b/indra/test/test.cpp
@@ -75,9 +75,10 @@
#include <fstream>
-void wouldHaveCrashed(const std::string& message)
+LLError::ErrCrashHandlerResult wouldHaveCrashed(const std::string& message)
{
tut::fail("fatal error message: " + message);
+ return LLError::ERR_DO_NOT_CRASH;
}
namespace tut
@@ -149,7 +150,7 @@ class LLReplayLogReal: public LLReplayLog, public boost::noncopyable
mOldSettings(LLError::saveAndResetSettings()),
mRecorder(new RecordToTempFile(pool))
{
- LLError::overrideCrashOnError(wouldHaveCrashed);
+ LLError::setFatalHandler(wouldHaveCrashed);
LLError::setDefaultLevel(level);
LLError::addRecorder(mRecorder);
}
@@ -530,7 +531,7 @@ int main(int argc, char **argv)
LLError::initForApplication(".", ".", false /* do not log to stderr */);
LLError::setDefaultLevel(LLError::LEVEL_DEBUG);
}
- LLError::overrideCrashOnError(wouldHaveCrashed);
+ LLError::setFatalHandler(wouldHaveCrashed);
std::string test_app_name(argv[0]);
std::string test_log = test_app_name + ".log";
LLFile::remove(test_log);
--
GitLab