From 6c533888ba3770572f2d7958460688562f03bfde Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Mon, 14 Jan 2019 16:30:49 -0500 Subject: build hack for upload failures --- build.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/build.sh b/build.sh index 1f9daa78b2..875b39f9c1 100755 --- a/build.sh +++ b/build.sh @@ -447,6 +447,9 @@ then # nat 2016-12-22: without RELEASE_CRASH_REPORTING, we have no symbol file. if [ "${RELEASE_CRASH_REPORTING:-}" != "OFF" ] then + # This next upload is a frequent failure; see if giving the last one some time helps + # JJ is making changes to Codeticket that we hope will eliminate this failure soon + sleep 30 # Upload crash reporter file python_cmd "$helpers/codeticket.py" addoutput "Symbolfile" "$VIEWER_SYMBOL_FILE" \ || fatal "Upload of symbolfile failed" -- cgit v1.2.3 From f648758c2a3da2dd03c8f57e98598c085b2030a6 Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Wed, 16 Jan 2019 11:05:55 -0500 Subject: SL-10297: Modify LL_ERRS and other deliberate crashes to avoid a common stack frame --- BuildParams | 0 indra/llcommon/llerror.cpp | 51 ++++++-------------- indra/llcommon/llerror.h | 24 ++++++++-- indra/llcommon/llerrorcontrol.h | 36 ++------------ indra/llcommon/llleap.cpp | 12 ++--- indra/llcommon/llsingleton.cpp | 10 +--- indra/llcommon/tests/llerror_test.cpp | 2 +- indra/llcommon/tests/wrapllerrs.h | 10 ++-- indra/newview/llappviewer.cpp | 89 ++++++----------------------------- indra/newview/llwatchdog.cpp | 18 ++----- indra/newview/llwatchdog.h | 6 +-- indra/test/test.cpp | 6 +-- 12 files changed, 70 insertions(+), 194 deletions(-) mode change 100755 => 100644 BuildParams diff --git a/BuildParams b/BuildParams old mode 100755 new mode 100644 diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 335a0995fe..0ddce353f1 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -683,7 +683,6 @@ namespace LLError::setDefaultLevel(LLError::LEVEL_INFO); LLError::setAlwaysFlush(true); LLError::setEnabledLogTypesMask(0xFFFFFFFF); - LLError::setFatalFunction(LLError::crashAndLoop); LLError::setTimeFunction(LLError::utcTime); // log_to_stderr is only false in the unit and integration tests to keep builds quieter @@ -719,16 +718,16 @@ namespace LLError commonInit(user_dir, app_dir, log_to_stderr); } - void setFatalFunction(const FatalFunction& f) + void overrideCrashOnError(const FatalFunction& fatal_function) { SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig(); - s->mCrashFunction = f; + s->mCrashFunction = fatal_function; } - FatalFunction getFatalFunction() + void restoreCrashOnError() { SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig(); - return s->mCrashFunction; + s->mCrashFunction = NULL; } std::string getFatalMessage() @@ -1306,12 +1305,12 @@ namespace LLError return ; } - void Log::flush(std::ostringstream* out, const CallSite& site) + bool Log::flush(std::ostringstream* out, const CallSite& site) { LLMutexTrylock lock(&gLogMutex,5); if (!lock.isLocked()) { - return; + return true; } // If we hit a logging request very late during shutdown processing, @@ -1319,7 +1318,7 @@ namespace LLError // DO NOT resurrect them. if (Settings::wasDeleted() || Globals::wasDeleted()) { - return; + return true; } Globals* g = Globals::getInstance(); @@ -1353,7 +1352,7 @@ namespace LLError } else { - return; + return true; } } else @@ -1370,11 +1369,14 @@ namespace LLError if (site.mLevel == LEVEL_ERROR) { g->mFatalMessage = message; - if (s->mCrashFunction) - { - s->mCrashFunction(message); - } + if (s->mCrashFunction) + { + s->mCrashFunction(message); + return false; + } } + + return true; } } @@ -1437,29 +1439,6 @@ namespace LLError return s->mShouldLogCallCounter; } -#if LL_WINDOWS - // VC80 was optimizing the error away. - #pragma optimize("", off) -#endif - void crashAndLoop(const std::string& message) - { - // Now, we go kaboom! - int* make_me_crash = NULL; - - *make_me_crash = 0; - - while(true) - { - // Loop forever, in case the crash didn't work? - } - - // this is an attempt to let Coverity and other semantic scanners know that this function won't be returning ever. - exit(EXIT_FAILURE); - } -#if LL_WINDOWS - #pragma optimize("", on) -#endif - std::string utcTime() { time_t now = time(NULL); diff --git a/indra/llcommon/llerror.h b/indra/llcommon/llerror.h index 0a78229555..cbade88f61 100644 --- a/indra/llcommon/llerror.h +++ b/indra/llcommon/llerror.h @@ -199,8 +199,12 @@ namespace LLError public: static bool shouldLog(CallSite&); static std::ostringstream* out(); + static void flush(std::ostringstream* out, char* message); - static void flush(std::ostringstream*, const CallSite&); + + // returns false iff there is a fatal crash override in effect + static bool flush(std::ostringstream*, const CallSite&); + static std::string demangle(const char* mangled); }; @@ -367,10 +371,20 @@ typedef LLError::NoClassInfo _LL_CLASS_TO_LOG; #define LL_NEWLINE '\n' -#define LL_ENDL \ - LLError::End(); \ - LLError::Log::flush(_out, _site); \ - } \ +// Use this only in LL_ERRS or in a place that LL_ERRS may not be used +#define LLERROR_CRASH \ +{ \ + int* make_me_crash = NULL;\ + *make_me_crash = 0; \ + exit(*make_me_crash); \ +} + +#define LL_ENDL \ + LLError::End(); \ + if (LLError::Log::flush(_out, _site) \ + && _site.mLevel == LLError::LEVEL_ERROR) \ + LLERROR_CRASH \ + } \ } while(0) // NEW Macros for debugging, allow the passing of a string tag diff --git a/indra/llcommon/llerrorcontrol.h b/indra/llcommon/llerrorcontrol.h index 276d22fc36..7ca6ddb737 100644 --- a/indra/llcommon/llerrorcontrol.h +++ b/indra/llcommon/llerrorcontrol.h @@ -92,41 +92,13 @@ namespace LLError /* Control functions. */ - typedef boost::function FatalFunction; - LL_COMMON_API void crashAndLoop(const std::string& message); - // Default fatal function: access null pointer and loops forever - - LL_COMMON_API void setFatalFunction(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. - - LL_COMMON_API FatalFunction getFatalFunction(); - // Retrieve the previously-set FatalFunction + LL_COMMON_API void overrideCrashOnError(const FatalFunction&); + LL_COMMON_API void restoreCrashOnError(); + LL_COMMON_API std::string getFatalMessage(); - // Retrieve the message last passed to FatalFunction, if any - - /// temporarily override the FatalFunction for the duration of a - /// particular scope, e.g. for unit tests - class LL_COMMON_API OverrideFatalFunction - { - public: - OverrideFatalFunction(const FatalFunction& func): - mPrev(getFatalFunction()) - { - setFatalFunction(func); - } - ~OverrideFatalFunction() - { - setFatalFunction(mPrev); - } - - private: - FatalFunction mPrev; - }; + // Retrieve the message last passed to LL_ERRS, if any typedef std::string (*TimeFunction)(); LL_COMMON_API std::string utcTime(); diff --git a/indra/llcommon/llleap.cpp b/indra/llcommon/llleap.cpp index cf8f8cc6a5..f7bfa36bb5 100644 --- a/indra/llcommon/llleap.cpp +++ b/indra/llcommon/llleap.cpp @@ -59,7 +59,6 @@ public: // pump name -- so it should NOT need tweaking for uniqueness. mReplyPump(LLUUID::generateNewID().asString()), mExpect(0), - mPrevFatalFunction(LLError::getFatalFunction()), // Instantiate a distinct LLLeapListener for this plugin. (Every // plugin will want its own collection of managed listeners, etc.) // Pass it a callback to our connect() method, so it can send events @@ -146,7 +145,7 @@ public: .listen("LLLeap", boost::bind(&LLLeapImpl::rstderr, this, _1)); // For our lifespan, intercept any LL_ERRS so we can notify plugin - LLError::setFatalFunction(boost::bind(&LLLeapImpl::fatalFunction, this, _1)); + LLError::overrideCrashOnError(boost::bind(&LLLeapImpl::fatalFunction, this, _1)); // Send child a preliminary event reporting our own reply-pump name -- // which would otherwise be pretty tricky to guess! @@ -162,8 +161,8 @@ public: virtual ~LLLeapImpl() { LL_DEBUGS("LLLeap") << "destroying LLLeap(\"" << mDesc << "\")" << LL_ENDL; - // Restore original FatalFunction - LLError::setFatalFunction(mPrevFatalFunction); + // Restore original fatal crash behavior for LL_ERRS + LLError::restoreCrashOnError(); } // Listener for failed launch attempt @@ -397,8 +396,8 @@ public: mainloop.post(nop); } - // forward the call to the previous FatalFunction - mPrevFatalFunction(error); + // go ahead and do the crash that LLError would have done + LLERROR_CRASH } private: @@ -421,7 +420,6 @@ private: mStdinConnection, mStdoutConnection, mStdoutDataConnection, mStderrConnection; boost::scoped_ptr mBlocker; LLProcess::ReadPipe::size_type mExpect; - LLError::FatalFunction mPrevFatalFunction; boost::scoped_ptr mListener; }; diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index c45c144570..6f254ef670 100644 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -461,15 +461,7 @@ void LLSingletonBase::logerrs(const char* p1, const char* p2, const char* p3, co // https://www.youtube.com/watch?v=OMG7paGJqhQ (emphasis on OMG) std::ostringstream out; out << p1 << p2 << p3 << p4; - auto crash = LLError::getFatalFunction(); - if (crash) - { - crash(out.str()); - } - else - { - LLError::crashAndLoop(out.str()); - } + LLERROR_CRASH; } std::string LLSingletonBase::demangle(const char* mangled) diff --git a/indra/llcommon/tests/llerror_test.cpp b/indra/llcommon/tests/llerror_test.cpp index 8e1f4c14ac..2f8923d2de 100644 --- a/indra/llcommon/tests/llerror_test.cpp +++ b/indra/llcommon/tests/llerror_test.cpp @@ -120,7 +120,7 @@ namespace tut mPriorErrorSettings = LLError::saveAndResetSettings(); LLError::setDefaultLevel(LLError::LEVEL_DEBUG); - LLError::setFatalFunction(fatalCall); + LLError::overrideCrashOnError(fatalCall); LLError::addRecorder(mRecorder); } diff --git a/indra/llcommon/tests/wrapllerrs.h b/indra/llcommon/tests/wrapllerrs.h index b07d5afbd8..b280271476 100644 --- a/indra/llcommon/tests/wrapllerrs.h +++ b/indra/llcommon/tests/wrapllerrs.h @@ -54,17 +54,15 @@ struct WrapLLErrs // Resetting Settings discards the default Recorder that writes to // stderr. Otherwise, expected llerrs (LL_ERRS) messages clutter the // console output of successful tests, potentially confusing things. - mPriorErrorSettings(LLError::saveAndResetSettings()), - // Save shutdown function called by LL_ERRS - mPriorFatal(LLError::getFatalFunction()) + mPriorErrorSettings(LLError::saveAndResetSettings()) { // Make LL_ERRS call our own operator() method - LLError::setFatalFunction(boost::bind(&WrapLLErrs::operator(), this, _1)); + LLError::overrideCrashOnError(boost::bind(&WrapLLErrs::operator(), this, _1)); } ~WrapLLErrs() { - LLError::setFatalFunction(mPriorFatal); + LLError::restoreCrashOnError(); LLError::restoreSettings(mPriorErrorSettings); } @@ -203,7 +201,7 @@ public: mOldSettings(LLError::saveAndResetSettings()), mRecorder(new CaptureLogRecorder()) { - LLError::setFatalFunction(wouldHaveCrashed); + LLError::overrideCrashOnError(wouldHaveCrashed); LLError::setDefaultLevel(level); LLError::addRecorder(mRecorder); } diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index a33e978c0a..e6ba3e4f7c 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -751,17 +751,6 @@ public: } }; -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); -} - - -} - bool LLAppViewer::init() { @@ -801,19 +790,6 @@ bool LLAppViewer::init() initMaxHeapSize() ; LLCoros::instance().setStackSize(gSavedSettings.getS32("CoroutineStackSize")); - - // Although initLoggingAndGetLastDuration() is the right place to mess with - // setFatalFunction(), 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::setFatalFunction(boost::bind(fast_exit, rc)); - } - mAlloc.setProfilingEnabled(gSavedSettings.getBOOL("MemProfiling")); // Initialize the non-LLCurl libcurl library. Should be called @@ -2120,28 +2096,9 @@ bool LLAppViewer::cleanup() return true; } -// A callback for LL_ERRS() to call during the watchdog error. -void watchdog_llerrs_callback(const std::string &error_string) -{ - gLLErrorActivated = true; - gDebugInfo["FatalMessage"] = error_string; LLAppViewer::instance()->writeDebugInfo(); -#ifdef LL_WINDOWS - RaiseException(0,0,0,0); -#else - raise(SIGQUIT); -#endif -} - -// A callback for the watchdog to call. -void watchdog_killer_callback() -{ - LLError::setFatalFunction(watchdog_llerrs_callback); - LL_ERRS() << "Watchdog killer event" << LL_ENDL; -} - bool LLAppViewer::initThreads() { static const bool enable_threads = true; @@ -2176,24 +2133,6 @@ bool LLAppViewer::initThreads() return true; } -void errorCallback(const std::string &error_string) -{ -#ifndef LL_RELEASE_FOR_DOWNLOAD - OSMessageBox(error_string, LLTrans::getString("MBFatalError"), OSMB_OK); -#endif - - //Set the ErrorActivated global so we know to create a marker file - gLLErrorActivated = true; - - gDebugInfo["FatalMessage"] = error_string; - // We're not already crashing -- we simply *intend* to crash. Since we - // haven't actually trashed anything yet, we can afford to write the whole - // static info file. - LLAppViewer::instance()->writeDebugInfo(); - - LLError::crashAndLoop(error_string); -} - void LLAppViewer::initLoggingAndGetLastDuration() { // @@ -2202,8 +2141,6 @@ void LLAppViewer::initLoggingAndGetLastDuration() LLError::initForApplication( gDirUtilp->getExpandedFilename(LL_PATH_USER_SETTINGS, "") ,gDirUtilp->getExpandedFilename(LL_PATH_APP_SETTINGS, "") ); - LLError::setFatalFunction(errorCallback); - //LLError::setTimeFunction(getRuntime); // Remove the last ".old" log file. std::string old_log_file = gDirUtilp->getExpandedFilename(LL_PATH_LOGS, @@ -2952,6 +2889,7 @@ bool LLAppViewer::initWindow() // Need to load feature table before cheking to start watchdog. bool use_watchdog = false; int watchdog_enabled_setting = gSavedSettings.getS32("WatchdogEnabled"); + if (watchdog_enabled_setting == -1) { use_watchdog = !LLFeatureManager::getInstance()->isFeatureAvailable("WatchdogDisabled"); @@ -2962,11 +2900,16 @@ bool LLAppViewer::initWindow() use_watchdog = bool(watchdog_enabled_setting); } + LL_INFOS("AppInit") << "watchdog" + << (use_watchdog ? " " : " NOT ") + << "enabled" + << " (setting = " << watchdog_enabled_setting << ")" + << LL_ENDL; + if (use_watchdog) { - LLWatchdog::getInstance()->init(watchdog_killer_callback); + LLWatchdog::getInstance()->init(); } - LL_INFOS("AppInit") << "watchdog setting is done." << LL_ENDL; LLNotificationsUI::LLNotificationManager::getInstance(); @@ -3409,12 +3352,10 @@ void LLAppViewer::writeSystemInfo() gDebugInfo["MainloopThreadID"] = (S32)thread_id; #endif - // "CrashNotHandled" is set here, while things are running well, - // in case of a freeze. If there is a freeze, the crash logger will be launched - // and can read this value from the debug_info.log. - // If the crash is handled by LLAppViewer::handleViewerCrash, ie not a freeze, - // then the value of "CrashNotHandled" will be set to true. - gDebugInfo["CrashNotHandled"] = (LLSD::Boolean)true; + // "CrashNotHandled" is obsolete; it used (not very successsfully) + // to try to distinguish crashes from freezes + gDebugInfo["CrashNotHandled"] = (LLSD::Boolean)false; + gDebugInfo["FatalMessage"] = LLError::getFatalMessage(); // Insert crash host url (url to post crash log to) if configured. This insures // that the crash report will go to the proper location in the case of a @@ -3567,10 +3508,6 @@ void LLAppViewer::handleViewerCrash() gDebugInfo["Dynamic"]["MainloopTimeoutState"] = LLAppViewer::instance()->mMainloopTimeout->getState(); } - // The crash is being handled here so set this value to false. - // Otherwise the crash logger will think this crash was a freeze. - gDebugInfo["Dynamic"]["CrashNotHandled"] = (LLSD::Boolean)false; - //Write out the crash status file //Use marker file style setup, as that's the simplest, especially since //we're already in a crash situation @@ -3642,6 +3579,8 @@ void LLAppViewer::handleViewerCrash() if (LLWorld::instanceExists()) LLWorld::getInstance()->getInfo(gDebugInfo["Dynamic"]); + gDebugInfo["Dynamic"]["FatalMessage"] = LLError::getFatalMessage(); + // Close the debug file pApp->writeDebugInfo(false); //false answers the isStatic question with the least overhead. } diff --git a/indra/newview/llwatchdog.cpp b/indra/newview/llwatchdog.cpp index dd6c77ca7d..2f3e5db84f 100644 --- a/indra/newview/llwatchdog.cpp +++ b/indra/newview/llwatchdog.cpp @@ -31,15 +31,6 @@ const U32 WATCHDOG_SLEEP_TIME_USEC = 1000000; -void default_killer_callback() -{ -#ifdef LL_WINDOWS - RaiseException(0,0,0,0); -#else - raise(SIGQUIT); -#endif -} - // This class runs the watchdog timing thread. class LLWatchdogTimerThread : public LLThread { @@ -157,8 +148,7 @@ void LLWatchdogTimeout::ping(const std::string& state) LLWatchdog::LLWatchdog() : mSuspectsAccessMutex(), mTimer(NULL), - mLastClockCount(0), - mKillerCallback(&default_killer_callback) + mLastClockCount(0) { } @@ -180,9 +170,8 @@ void LLWatchdog::remove(LLWatchdogEntry* e) unlockThread(); } -void LLWatchdog::init(killer_event_callback func) +void LLWatchdog::init() { - mKillerCallback = func; if(!mSuspectsAccessMutex && !mTimer) { mSuspectsAccessMutex = new LLMutex(); @@ -249,8 +238,7 @@ void LLWatchdog::run() mTimer->stop(); } - LL_INFOS() << "Watchdog detected error:" << LL_ENDL; - mKillerCallback(); + LL_ERRS() << "Watchdog timer expired; assuming viewer is hung and crashing" << LL_ENDL; } } diff --git a/indra/newview/llwatchdog.h b/indra/newview/llwatchdog.h index 9a6624258e..ce5cf748f4 100644 --- a/indra/newview/llwatchdog.h +++ b/indra/newview/llwatchdog.h @@ -83,9 +83,7 @@ public: void add(LLWatchdogEntry* e); void remove(LLWatchdogEntry* e); - typedef boost::function killer_event_callback; - - void init(killer_event_callback func = NULL); + void init(); void run(); void cleanup(); @@ -98,8 +96,6 @@ private: LLMutex* mSuspectsAccessMutex; LLWatchdogTimerThread* mTimer; U64 mLastClockCount; - - killer_event_callback mKillerCallback; }; #endif // LL_LLTHREADWATCHDOG_H diff --git a/indra/test/test.cpp b/indra/test/test.cpp index b14c2eb255..125de72b79 100644 --- a/indra/test/test.cpp +++ b/indra/test/test.cpp @@ -146,7 +146,7 @@ public: mOldSettings(LLError::saveAndResetSettings()), mRecorder(new RecordToTempFile(pool)) { - LLError::setFatalFunction(wouldHaveCrashed); + LLError::overrideCrashOnError(wouldHaveCrashed); LLError::setDefaultLevel(level); LLError::addRecorder(mRecorder); } @@ -508,7 +508,7 @@ void stream_groups(std::ostream& s, const char* app) void wouldHaveCrashed(const std::string& message) { - tut::fail("llerrs message: " + message); + tut::fail("fatal error message: " + message); } static LLTrace::ThreadRecorder* sMasterThreadRecorder = NULL; @@ -532,7 +532,7 @@ int main(int argc, char **argv) LLError::initForApplication(".", ".", false /* do not log to stderr */); LLError::setDefaultLevel(LLError::LEVEL_DEBUG); } - LLError::setFatalFunction(wouldHaveCrashed); + LLError::overrideCrashOnError(wouldHaveCrashed); std::string test_app_name(argv[0]); std::string test_log = test_app_name + ".log"; LLFile::remove(test_log); -- cgit v1.2.3 From fc90cad4f366c4bb85add832a2fa8137b1f120ff Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Wed, 16 Jan 2019 16:42:34 -0500 Subject: change the default crash type from "freeze" to "other" --- build.sh | 20 ++++++++++++++++---- indra/newview/llappviewer.cpp | 9 ++++++++- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/build.sh b/build.sh index 875b39f9c1..9f070f2d10 100755 --- a/build.sh +++ b/build.sh @@ -412,6 +412,15 @@ then fi fi +# Some of the uploads takes a long time to finish in the codeticket backend, +# causing the next codeticket upload attempt to fail. +# Inserting this after each potentially large upload may prevent those errors. +# JJ is making changes to Codeticket that we hope will eliminate this failure, then this can be removed +wait_for_codeticket() +{ + sleep $(( 60 * 6 )) +} + # check status and upload results to S3 if $succeeded then @@ -428,6 +437,7 @@ then # Upload base package. python_cmd "$helpers/codeticket.py" addoutput Installer "$package" \ || fatal "Upload of installer failed" + wait_for_codeticket # Upload additional packages. for package_id in $additional_packages @@ -437,6 +447,7 @@ then then python_cmd "$helpers/codeticket.py" addoutput "Installer $package_id" "$package" \ || fatal "Upload of installer $package_id failed" + wait_for_codeticket else record_failure "Failed to find additional package for '$package_id'." fi @@ -447,12 +458,10 @@ then # nat 2016-12-22: without RELEASE_CRASH_REPORTING, we have no symbol file. if [ "${RELEASE_CRASH_REPORTING:-}" != "OFF" ] then - # This next upload is a frequent failure; see if giving the last one some time helps - # JJ is making changes to Codeticket that we hope will eliminate this failure soon - sleep 30 # Upload crash reporter file python_cmd "$helpers/codeticket.py" addoutput "Symbolfile" "$VIEWER_SYMBOL_FILE" \ || fatal "Upload of symbolfile failed" + wait_for_codeticket fi # Upload the llphysicsextensions_tpv package, if one was produced @@ -460,6 +469,9 @@ then if [ -r "$build_dir/llphysicsextensions_package" ] then llphysicsextensions_package=$(cat $build_dir/llphysicsextensions_package) + # This next upload is a frequent failure; see if giving the last one some time helps + # JJ is making changes to Codeticket that we hope will eliminate this failure soon + sleep 300 python_cmd "$helpers/codeticket.py" addoutput "Physics Extensions Package" "$llphysicsextensions_package" --private \ || fatal "Upload of physics extensions package failed" fi @@ -470,6 +482,7 @@ then for extension in ${build_dir}/packages/upload-extensions/*.sh; do begin_section "Upload Extension $extension" . $extension + wait_for_codeticket end_section "Upload Extension $extension" done fi @@ -479,7 +492,6 @@ then record_event "skipping upload of installer" fi - else record_event "skipping upload of installer due to failed build" fi diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index e6ba3e4f7c..1fda07edd6 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -3508,6 +3508,10 @@ void LLAppViewer::handleViewerCrash() gDebugInfo["Dynamic"]["MainloopTimeoutState"] = LLAppViewer::instance()->mMainloopTimeout->getState(); } + // The crash is being handled here so set this value to false. + // Otherwise the crash logger will think this crash was a freeze. + gDebugInfo["Dynamic"]["CrashNotHandled"] = (LLSD::Boolean)false; + //Write out the crash status file //Use marker file style setup, as that's the simplest, especially since //we're already in a crash situation @@ -3680,8 +3684,11 @@ void LLAppViewer::processMarkerFiles() { // 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 + gLastExecEvent = LAST_EXEC_OTHER_CRASH; +# else gLastExecEvent = LAST_EXEC_FROZE; - +# endif } else { -- cgit v1.2.3 From e409c0492f1b1ce63606c0b693c92cdb36dcc28b Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Sat, 2 Mar 2019 11:58:11 -0500 Subject: convert to an explicit USE_BUGSPLAT switch in cmake, revise LL_ERRS approach --- build.sh | 5 ---- indra/CMakeLists.txt | 10 +++++++ indra/cmake/Copy3rdPartyLibs.cmake | 4 +-- indra/cmake/LLAddBuildTest.cmake | 13 ++++++++- indra/cmake/Variables.cmake | 1 - indra/cmake/bugsplat.cmake | 53 +++++++++++++++++++--------------- indra/llcommon/CMakeLists.txt | 9 +++--- indra/llcommon/llerror.cpp | 21 +++++++++----- indra/llcommon/llerror.h | 20 +++++++------ indra/llcommon/llerrorcontrol.h | 14 +++++++-- indra/llcommon/llleap.cpp | 30 +------------------ indra/llcommon/tests/wrapllerrs.h | 5 ++-- indra/llcorehttp/CMakeLists.txt | 1 + indra/llimage/CMakeLists.txt | 1 + indra/llmath/CMakeLists.txt | 1 + indra/newview/CMakeLists.txt | 36 +++++++++++------------ indra/newview/llappviewer.cpp | 59 +++++++++++++++++++++++++++++++++----- indra/newview/llwatchdog.cpp | 8 +++--- indra/test/CMakeLists.txt | 6 +++- indra/test/test.cpp | 10 +++---- 20 files changed, 185 insertions(+), 122 deletions(-) diff --git a/build.sh b/build.sh index 9f070f2d10..aa6ba475a3 100755 --- a/build.sh +++ b/build.sh @@ -129,11 +129,6 @@ pre_build() then # show that we're doing this, just not the contents echo source "$bugsplat_sh" source "$bugsplat_sh" - # important: we test this and use its value in [grand-]child processes - if [ -n "${BUGSPLAT_DB:-}" ] - then echo export BUGSPLAT_DB - export BUGSPLAT_DB - fi fi set -x diff --git a/indra/CMakeLists.txt b/indra/CMakeLists.txt index 62a8f3f003..cf6029c4ae 100644 --- a/indra/CMakeLists.txt +++ b/indra/CMakeLists.txt @@ -13,6 +13,7 @@ project(${ROOT_PROJECT_NAME}) set(CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/cmake") include(Variables) +include(bugsplat) include(BuildVersion) if (NOT CMAKE_BUILD_TYPE) @@ -89,6 +90,15 @@ set_property( PROPERTY VS_STARTUP_PROJECT secondlife-bin ) +if (USE_BUGSPLAT) + if (BUGSPLAT_DB) + message(STATUS "Building with BugSplat; database '${BUGSPLAT_DB}'") + else (BUGSPLAT_DB) + message(WARNING "Building with BugSplat, but no database name set (BUGSPLAT_DB)") + endif (BUGSPLAT_DB) +else (USE_BUGSPLAT) + message(STATUS "Not building with BugSplat") +endif (USE_BUGSPLAT) if (LL_TESTS) # Define after the custom targets are created so # individual apps can add themselves as dependencies diff --git a/indra/cmake/Copy3rdPartyLibs.cmake b/indra/cmake/Copy3rdPartyLibs.cmake index c73a1fdb47..ebb6379aed 100644 --- a/indra/cmake/Copy3rdPartyLibs.cmake +++ b/indra/cmake/Copy3rdPartyLibs.cmake @@ -50,7 +50,7 @@ if(WINDOWS) # Filenames are different for 32/64 bit BugSplat file and we don't # have any control over them so need to branch. - if (BUGSPLAT_DB) + if (USE_BUGSPLAT) if(ADDRESS_SIZE EQUAL 32) set(release_files ${release_files} BugSplat.dll) set(release_files ${release_files} BugSplatRc.dll) @@ -60,7 +60,7 @@ if(WINDOWS) set(release_files ${release_files} BugSplatRc64.dll) set(release_files ${release_files} BsSndRpt64.exe) endif(ADDRESS_SIZE EQUAL 32) - endif (BUGSPLAT_DB) + endif (USE_BUGSPLAT) if (FMODEX) diff --git a/indra/cmake/LLAddBuildTest.cmake b/indra/cmake/LLAddBuildTest.cmake index b3f42c1a5e..fa10a9d443 100644 --- a/indra/cmake/LLAddBuildTest.cmake +++ b/indra/cmake/LLAddBuildTest.cmake @@ -2,6 +2,7 @@ include(00-Common) include(LLTestCommand) include(GoogleMock) +include(bugsplat) include(Tut) #***************************************************************************** @@ -22,7 +23,6 @@ MACRO(LL_ADD_PROJECT_UNIT_TESTS project sources) # there is another branch that will conflict heavily with any changes here. INCLUDE(GoogleMock) - IF(LL_TEST_VERBOSE) MESSAGE("LL_ADD_PROJECT_UNIT_TESTS UNITTEST_PROJECT_${project} sources: ${sources}") ENDIF(LL_TEST_VERBOSE) @@ -87,6 +87,12 @@ INCLUDE(GoogleMock) IF(LL_TEST_VERBOSE) MESSAGE("LL_ADD_PROJECT_UNIT_TESTS ${name}_test_SOURCE_FILES ${${name}_test_SOURCE_FILES}") ENDIF(LL_TEST_VERBOSE) + + if (USE_BUGSPLAT) + SET_PROPERTY(SOURCE ${${name}_test_SOURCE_FILES} + APPEND PROPERTY COMPILE_DEFINITIONS "${BUGSPLAT_DEFINE}") + endif (USE_BUGSPLAT) + # Headers GET_OPT_SOURCE_FILE_PROPERTY(${name}_test_additional_HEADER_FILES ${source} LL_TEST_ADDITIONAL_HEADER_FILES) SET(${name}_test_HEADER_FILES ${name}.h ${${name}_test_additional_HEADER_FILES}) @@ -223,6 +229,11 @@ FUNCTION(LL_ADD_INTEGRATION_TEST SET_TARGET_PROPERTIES(INTEGRATION_TEST_${testname} PROPERTIES COMPILE_FLAGS -I"${TUT_INCLUDE_DIR}") endif(USESYSTEMLIBS) + if (USE_BUGSPLAT) + SET_PROPERTY(SOURCE ${source_files} + APPEND PROPERTY COMPILE_DEFINITIONS "${BUGSPLAT_DEFINE}") + endif (USE_BUGSPLAT) + # The following was copied to llcorehttp/CMakeLists.txt's texture_load target. # Any changes made here should be replicated there. if (WINDOWS) diff --git a/indra/cmake/Variables.cmake b/indra/cmake/Variables.cmake index a5770c5528..c81b22e572 100644 --- a/indra/cmake/Variables.cmake +++ b/indra/cmake/Variables.cmake @@ -34,7 +34,6 @@ set(LL_TESTS ON CACHE BOOL "Build and run unit and integration tests (disable fo set(INCREMENTAL_LINK OFF CACHE BOOL "Use incremental linking on win32 builds (enable for faster links on some machines)") set(ENABLE_MEDIA_PLUGINS ON CACHE BOOL "Turn off building media plugins if they are imported by third-party library mechanism") set(VIEWER_SYMBOL_FILE "" CACHE STRING "Name of tarball into which to place symbol files") -set(BUGSPLAT_DB "" CACHE STRING "BugSplat database name, if BugSplat crash reporting is desired") if(LIBS_CLOSED_DIR) file(TO_CMAKE_PATH "${LIBS_CLOSED_DIR}" LIBS_CLOSED_DIR) diff --git a/indra/cmake/bugsplat.cmake b/indra/cmake/bugsplat.cmake index 59644b73ce..749ea05403 100644 --- a/indra/cmake/bugsplat.cmake +++ b/indra/cmake/bugsplat.cmake @@ -1,25 +1,32 @@ -# BugSplat is engaged by setting BUGSPLAT_DB to the target BugSplat database -# name. -if (BUGSPLAT_DB) - if (USESYSTEMLIBS) - message(STATUS "Looking for system BugSplat") - set(BUGSPLAT_FIND_QUIETLY ON) - set(BUGSPLAT_FIND_REQUIRED ON) - include(FindBUGSPLAT) - else (USESYSTEMLIBS) - message(STATUS "Engaging autobuild BugSplat") - include(Prebuilt) - use_prebuilt_binary(bugsplat) - if (WINDOWS) - set(BUGSPLAT_LIBRARIES - ${ARCH_PREBUILT_DIRS_RELEASE}/bugsplat.lib - ) - elseif (DARWIN) - find_library(BUGSPLAT_LIBRARIES BugsplatMac - PATHS "${ARCH_PREBUILT_DIRS_RELEASE}") - else (WINDOWS) +if (INSTALL_PROPRIETARY) + set(USE_BUGSPLAT ON CACHE BOOL "Use the BugSplat crash reporting system") +else (INSTALL_PROPRIETARY) + set(USE_BUGSPLAT OFF CACHE BOOL "Use the BugSplat crash reporting system") +endif (INSTALL_PROPRIETARY) + +if (USE_BUGSPLAT) + if (NOT USESYSTEMLIBS) + include(Prebuilt) + use_prebuilt_binary(bugsplat) + if (WINDOWS) + set(BUGSPLAT_LIBRARIES + ${ARCH_PREBUILT_DIRS_RELEASE}/bugsplat.lib + ) + elseif (DARWIN) + find_library(BUGSPLAT_LIBRARIES BugsplatMac + PATHS "${ARCH_PREBUILT_DIRS_RELEASE}") + else (WINDOWS) + message(FATAL_ERROR "BugSplat is not supported; add -DUSE_BUGSPLAT=OFF") + endif (WINDOWS) + else (NOT USESYSTEMLIBS) + set(BUGSPLAT_FIND_QUIETLY ON) + set(BUGSPLAT_FIND_REQUIRED ON) + include(FindBUGSPLAT) + endif (NOT USESYSTEMLIBS) + + set(BUGSPLAT_DB "" CACHE STRING "BugSplat crash database name") - endif (WINDOWS) set(BUGSPLAT_INCLUDE_DIR ${LIBS_PREBUILT_DIR}/include/bugsplat) - endif (USESYSTEMLIBS) -endif (BUGSPLAT_DB) + set(BUGSPLAT_DEFINE "LL_BUGSPLAT") +endif (USE_BUGSPLAT) + diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt index af41b9e460..e3afa5eca4 100644 --- a/indra/llcommon/CMakeLists.txt +++ b/indra/llcommon/CMakeLists.txt @@ -5,6 +5,7 @@ project(llcommon) include(00-Common) include(LLCommon) +include(bugsplat) include(Linking) include(Boost) include(LLSharedLibs) @@ -257,10 +258,10 @@ set(llcommon_HEADER_FILES set_source_files_properties(${llcommon_HEADER_FILES} PROPERTIES HEADER_FILE_ONLY TRUE) -if (BUGSPLAT_DB) - set_source_files_properties(llapp.cpp - PROPERTIES COMPILE_DEFINITIONS "LL_BUGSPLAT") -endif (BUGSPLAT_DB) +if (USE_BUGSPLAT) + set_source_files_properties(${llcommon_SOURCE_FILES} + PROPERTIES COMPILE_DEFINITIONS "${BUGSPLAT_DEFINE}") +endif (USE_BUGSPLAT) list(APPEND llcommon_SOURCE_FILES ${llcommon_HEADER_FILES}) diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 0ddce353f1..77d7fe1b24 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -25,6 +25,7 @@ * $/LicenseInfo$ */ +#define _LLERROR_CPP_ #include "linden_common.h" #include "llerror.h" @@ -724,7 +725,7 @@ namespace LLError s->mCrashFunction = fatal_function; } - void restoreCrashOnError() + void restoreCrashOnError() { SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig(); s->mCrashFunction = NULL; @@ -1310,7 +1311,7 @@ namespace LLError LLMutexTrylock lock(&gLogMutex,5); if (!lock.isLocked()) { - return true; + return false; // because this wasn't logged, it cannot be fatal } // If we hit a logging request very late during shutdown processing, @@ -1318,7 +1319,7 @@ namespace LLError // DO NOT resurrect them. if (Settings::wasDeleted() || Globals::wasDeleted()) { - return true; + return false; // because this wasn't logged, it cannot be fatal } Globals* g = Globals::getInstance(); @@ -1352,7 +1353,7 @@ namespace LLError } else { - return true; + return false; // because this wasn't logged, it cannot be fatal } } else @@ -1372,11 +1373,17 @@ namespace LLError if (s->mCrashFunction) { s->mCrashFunction(message); - return false; + return false; // because an override is in effect + } + else + { + return true; // calling macro should crash } } - - return true; + else + { + return false; // not ERROR, so do not crash + } } } diff --git a/indra/llcommon/llerror.h b/indra/llcommon/llerror.h index cbade88f61..07aa5c6f8b 100644 --- a/indra/llcommon/llerror.h +++ b/indra/llcommon/llerror.h @@ -202,7 +202,7 @@ namespace LLError static void flush(std::ostringstream* out, char* message); - // returns false iff there is a fatal crash override in effect + // returns false iff the calling macro should crash static bool flush(std::ostringstream*, const CallSite&); static std::string demangle(const char* mangled); @@ -371,18 +371,22 @@ typedef LLError::NoClassInfo _LL_CLASS_TO_LOG; #define LL_NEWLINE '\n' +#ifdef _LLERROR_CPP_ +volatile int* gCauseCrash = NULL; +#else +volatile extern int* gCauseCrash; +#endif // _LLERROR_CPP_ + // Use this only in LL_ERRS or in a place that LL_ERRS may not be used -#define LLERROR_CRASH \ -{ \ - int* make_me_crash = NULL;\ - *make_me_crash = 0; \ - exit(*make_me_crash); \ +#define LLERROR_CRASH \ +{ \ + *gCauseCrash = 0; \ + exit(*gCauseCrash); \ } #define LL_ENDL \ LLError::End(); \ - if (LLError::Log::flush(_out, _site) \ - && _site.mLevel == LLError::LEVEL_ERROR) \ + if (LLError::Log::flush(_out, _site)) \ LLERROR_CRASH \ } \ } while(0) diff --git a/indra/llcommon/llerrorcontrol.h b/indra/llcommon/llerrorcontrol.h index 7ca6ddb737..af46430f74 100644 --- a/indra/llcommon/llerrorcontrol.h +++ b/indra/llcommon/llerrorcontrol.h @@ -92,11 +92,19 @@ namespace LLError /* Control functions. */ + typedef boost::function FatalFunction; - LL_COMMON_API void overrideCrashOnError(const FatalFunction&); - LL_COMMON_API void restoreCrashOnError(); - + /// 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&); + // 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. + + /// Undo the effect of the overrideCrashOnError above + LL_COMMON_API void restoreCrashOnError(); + LL_COMMON_API std::string getFatalMessage(); // Retrieve the message last passed to LL_ERRS, if any diff --git a/indra/llcommon/llleap.cpp b/indra/llcommon/llleap.cpp index f7bfa36bb5..bf20c87c89 100644 --- a/indra/llcommon/llleap.cpp +++ b/indra/llcommon/llleap.cpp @@ -59,6 +59,7 @@ public: // pump name -- so it should NOT need tweaking for uniqueness. mReplyPump(LLUUID::generateNewID().asString()), mExpect(0), + // Instantiate a distinct LLLeapListener for this plugin. (Every // plugin will want its own collection of managed listeners, etc.) // Pass it a callback to our connect() method, so it can send events @@ -144,9 +145,6 @@ public: mStderrConnection = childerr.getPump() .listen("LLLeap", boost::bind(&LLLeapImpl::rstderr, this, _1)); - // For our lifespan, intercept any LL_ERRS so we can notify plugin - LLError::overrideCrashOnError(boost::bind(&LLLeapImpl::fatalFunction, this, _1)); - // Send child a preliminary event reporting our own reply-pump name -- // which would otherwise be pretty tricky to guess! wstdin(mReplyPump.getName(), @@ -161,8 +159,6 @@ public: virtual ~LLLeapImpl() { LL_DEBUGS("LLLeap") << "destroying LLLeap(\"" << mDesc << "\")" << LL_ENDL; - // Restore original fatal crash behavior for LL_ERRS - LLError::restoreCrashOnError(); } // Listener for failed launch attempt @@ -376,30 +372,6 @@ public: return false; } - void fatalFunction(const std::string& error) - { - // Notify plugin - LLSD event; - event["type"] = "error"; - event["error"] = error; - mReplyPump.post(event); - - // All the above really accomplished was to buffer the serialized - // event in our WritePipe. Have to pump mainloop a couple times to - // really write it out there... but time out in case we can't write. - LLProcess::WritePipe& childin(mChild->getWritePipe(LLProcess::STDIN)); - LLEventPump& mainloop(LLEventPumps::instance().obtain("mainloop")); - LLSD nop; - F64 until = (LLTimer::getElapsedSeconds() + 2).value(); - while (childin.size() && LLTimer::getElapsedSeconds() < until) - { - mainloop.post(nop); - } - - // go ahead and do the crash that LLError would have done - LLERROR_CRASH - } - private: /// We always want to listen on mReplyPump with wstdin(); under some /// circumstances we'll also echo other LLEventPumps to the plugin. diff --git a/indra/llcommon/tests/wrapllerrs.h b/indra/llcommon/tests/wrapllerrs.h index b280271476..fedc17dbe9 100644 --- a/indra/llcommon/tests/wrapllerrs.h +++ b/indra/llcommon/tests/wrapllerrs.h @@ -50,11 +50,11 @@ extern void wouldHaveCrashed(const std::string& message); struct WrapLLErrs { - WrapLLErrs(): + WrapLLErrs() // Resetting Settings discards the default Recorder that writes to // stderr. Otherwise, expected llerrs (LL_ERRS) messages clutter the // console output of successful tests, potentially confusing things. - mPriorErrorSettings(LLError::saveAndResetSettings()) + :mPriorErrorSettings(LLError::saveAndResetSettings()) { // Make LL_ERRS call our own operator() method LLError::overrideCrashOnError(boost::bind(&WrapLLErrs::operator(), this, _1)); @@ -210,6 +210,7 @@ public: { LLError::removeRecorder(mRecorder); LLError::restoreSettings(mOldSettings); + LLError::restoreCrashOnError(); } /// Don't assume the message we want is necessarily the LAST log message diff --git a/indra/llcorehttp/CMakeLists.txt b/indra/llcorehttp/CMakeLists.txt index 9dbc6f447e..286c275b90 100644 --- a/indra/llcorehttp/CMakeLists.txt +++ b/indra/llcorehttp/CMakeLists.txt @@ -13,6 +13,7 @@ include(LLAddBuildTest) include(LLMessage) include(LLCommon) include(Tut) +include(bugsplat) include_directories (${CMAKE_CURRENT_SOURCE_DIR}) diff --git a/indra/llimage/CMakeLists.txt b/indra/llimage/CMakeLists.txt index 293ada7548..28b8e8c06d 100644 --- a/indra/llimage/CMakeLists.txt +++ b/indra/llimage/CMakeLists.txt @@ -11,6 +11,7 @@ include(LLKDU) include(LLImageJ2COJ) include(ZLIB) include(LLAddBuildTest) +include(bugsplat) include(Tut) include_directories( diff --git a/indra/llmath/CMakeLists.txt b/indra/llmath/CMakeLists.txt index 379c3ee9ea..625459823c 100644 --- a/indra/llmath/CMakeLists.txt +++ b/indra/llmath/CMakeLists.txt @@ -4,6 +4,7 @@ project(llmath) include(00-Common) include(LLCommon) +include(bugsplat) include(Boost) include_directories( diff --git a/indra/newview/CMakeLists.txt b/indra/newview/CMakeLists.txt index 04f6c9b7f0..bbfa838827 100644 --- a/indra/newview/CMakeLists.txt +++ b/indra/newview/CMakeLists.txt @@ -8,9 +8,7 @@ include(00-Common) include(Linking) include(Boost) -if (BUGSPLAT_DB) - include(bugsplat) -endif (BUGSPLAT_DB) +include(bugsplat) include(BuildPackagesInfo) include(BuildVersion) include(CMakeCopyIfDifferent) @@ -97,18 +95,18 @@ include_directories( ${CMAKE_CURRENT_SOURCE_DIR} ) -if (BUGSPLAT_DB) - include_directories( - ${BUGSPLAT_INCLUDE_DIR} - ) -endif (BUGSPLAT_DB) - include_directories(SYSTEM ${LLCOMMON_SYSTEM_INCLUDE_DIRS} ${LLXML_SYSTEM_INCLUDE_DIRS} ${LLPHYSICSEXTENSIONS_INCLUDE_DIRS} ) +if (USE_BUGSPLAT) + include_directories(AFTER + ${BUGSPLAT_INCLUDE_DIR} + ) +endif (USE_BUGSPLAT) + set(viewer_SOURCE_FILES groupchatlistener.cpp llaccountingcostmanager.cpp @@ -1390,11 +1388,11 @@ if (DARWIN) ${COREAUDIO_LIBRARY} ) - if (BUGSPLAT_DB) + if (USE_BUGSPLAT) list(APPEND viewer_LIBRARIES ${BUGSPLAT_LIBRARIES} ) - endif (BUGSPLAT_DB) + endif (USE_BUGSPLAT) # Add resource files to the project. set(viewer_RESOURCE_FILES @@ -1729,10 +1727,10 @@ if (SDL_FOUND) ) endif (SDL_FOUND) -if (BUGSPLAT_DB) +if (USE_BUGSPLAT) set_property(TARGET ${VIEWER_BINARY_NAME} PROPERTY COMPILE_DEFINITIONS "LL_BUGSPLAT") -endif (BUGSPLAT_DB) +endif (USE_BUGSPLAT) # add package files file(GLOB EVENT_HOST_SCRIPT_GLOB_LIST @@ -2018,11 +2016,11 @@ target_link_libraries(${VIEWER_BINARY_NAME} ${LLAPPEARANCE_LIBRARIES} ) -if (BUGSPLAT_DB) +if (USE_BUGSPLAT) target_link_libraries(${VIEWER_BINARY_NAME} ${BUGSPLAT_LIBRARIES} ) -endif (BUGSPLAT_DB) +endif (USE_BUGSPLAT) set(ARTWORK_DIR ${CMAKE_CURRENT_SOURCE_DIR} CACHE PATH "Path to artwork files.") @@ -2206,7 +2204,7 @@ endif (INSTALL) # Note that the conventional VIEWER_SYMBOL_FILE is set by ../../build.sh if (PACKAGE AND (RELEASE_CRASH_REPORTING OR NON_RELEASE_CRASH_REPORTING) AND VIEWER_SYMBOL_FILE) - if (NOT BUGSPLAT_DB) + if (NOT USE_BUGSPLAT) # Breakpad symbol-file generation set(SYMBOL_SEARCH_DIRS "") if (WINDOWS) @@ -2261,7 +2259,7 @@ if (PACKAGE AND (RELEASE_CRASH_REPORTING OR NON_RELEASE_CRASH_REPORTING) AND VIE add_dependencies(generate_symbols "${VIEWER_COPY_MANIFEST}") endif (WINDOWS OR LINUX) - else (NOT BUGSPLAT_DB) + else (NOT USE_BUGSPLAT) # BugSplat symbol-file generation if (WINDOWS) # Just pack up a tarball containing only the .pdb file for the @@ -2345,9 +2343,9 @@ if (PACKAGE AND (RELEASE_CRASH_REPORTING OR NON_RELEASE_CRASH_REPORTING) AND VIE if (LINUX) # TBD endif (LINUX) - endif (NOT BUGSPLAT_DB) + endif (NOT USE_BUGSPLAT) - # for both BUGSPLAT_DB and Breakpad + # for both Bugsplat and Breakpad add_dependencies(llpackage generate_symbols) endif () diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index 1fda07edd6..f9ee22d20a 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -719,14 +719,14 @@ LLAppViewer::LLAppViewer() // from the previous viewer run between this constructor call and the // init() call, which will overwrite the static_debug_info.log file for // THIS run. So setDebugFileNames() early. -#if LL_BUGSPLAT +# ifdef LL_BUGSPLAT // MAINT-8917: don't create a dump directory just for the // static_debug_info.log file std::string logdir = gDirUtilp->getExpandedFilename(LL_PATH_LOGS, ""); -#else // ! LL_BUGSPLAT +# else // ! LL_BUGSPLAT // write Google Breakpad minidump files to a per-run dump directory to avoid multiple viewer issues. std::string logdir = gDirUtilp->getExpandedFilename(LL_PATH_DUMP, ""); -#endif // ! LL_BUGSPLAT +# endif // ! LL_BUGSPLAT mDumpPath = logdir; setMiniDumpDir(logdir); setDebugFileNames(logdir); @@ -751,6 +751,16 @@ public: } }; +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); +} + + +} bool LLAppViewer::init() { @@ -790,6 +800,18 @@ 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)); + } + mAlloc.setProfilingEnabled(gSavedSettings.getBOOL("MemProfiling")); // Initialize the non-LLCurl libcurl library. Should be called @@ -2096,9 +2118,6 @@ bool LLAppViewer::cleanup() return true; } - gDebugInfo["FatalMessage"] = error_string; - LLAppViewer::instance()->writeDebugInfo(); - bool LLAppViewer::initThreads() { static const bool enable_threads = true; @@ -2133,6 +2152,24 @@ bool LLAppViewer::initThreads() return true; } +#ifndef LL_BUGSPLAT +void errorCallback(const std::string &error_string) +{ +#ifndef LL_RELEASE_FOR_DOWNLOAD + OSMessageBox(error_string, LLTrans::getString("MBFatalError"), OSMB_OK); +#endif + + //Set the ErrorActivated global so we know to create a marker file + gLLErrorActivated = true; + + gDebugInfo["FatalMessage"] = error_string; + // We're not already crashing -- we simply *intend* to crash. Since we + // haven't actually trashed anything yet, we can afford to write the whole + // static info file. + LLAppViewer::instance()->writeDebugInfo(); +} +#endif // ! LL_BUGSPLAT + void LLAppViewer::initLoggingAndGetLastDuration() { // @@ -2889,7 +2926,6 @@ bool LLAppViewer::initWindow() // Need to load feature table before cheking to start watchdog. bool use_watchdog = false; int watchdog_enabled_setting = gSavedSettings.getS32("WatchdogEnabled"); - if (watchdog_enabled_setting == -1) { use_watchdog = !LLFeatureManager::getInstance()->isFeatureAvailable("WatchdogDisabled"); @@ -3352,10 +3388,19 @@ void LLAppViewer::writeSystemInfo() gDebugInfo["MainloopThreadID"] = (S32)thread_id; #endif +#ifndef LL_BUGSPLAT + // "CrashNotHandled" is set here, while things are running well, + // in case of a freeze. If there is a freeze, the crash logger will be launched + // and can read this value from the debug_info.log. + // If the crash is handled by LLAppViewer::handleViewerCrash, ie not a freeze, + // then the value of "CrashNotHandled" will be set to true. + gDebugInfo["CrashNotHandled"] = (LLSD::Boolean)true; +#else // LL_BUGSPLAT // "CrashNotHandled" is obsolete; it used (not very successsfully) // to try to distinguish crashes from freezes gDebugInfo["CrashNotHandled"] = (LLSD::Boolean)false; gDebugInfo["FatalMessage"] = LLError::getFatalMessage(); +#endif // ! LL_BUGSPLAT // Insert crash host url (url to post crash log to) if configured. This insures // that the crash report will go to the proper location in the case of a diff --git a/indra/newview/llwatchdog.cpp b/indra/newview/llwatchdog.cpp index 2f3e5db84f..0a01113224 100644 --- a/indra/newview/llwatchdog.cpp +++ b/indra/newview/llwatchdog.cpp @@ -145,10 +145,10 @@ void LLWatchdogTimeout::ping(const std::string& state) } // LLWatchdog -LLWatchdog::LLWatchdog() : - mSuspectsAccessMutex(), - mTimer(NULL), - mLastClockCount(0) +LLWatchdog::LLWatchdog() + :mSuspectsAccessMutex() + ,mTimer(NULL) + ,mLastClockCount(0) { } diff --git a/indra/test/CMakeLists.txt b/indra/test/CMakeLists.txt index 8344cead57..7ac1f1db23 100644 --- a/indra/test/CMakeLists.txt +++ b/indra/test/CMakeLists.txt @@ -13,7 +13,7 @@ include(LLXML) include(Linking) include(Tut) include(LLAddBuildTest) - +include(bugsplat) include(GoogleMock) include_directories( @@ -82,6 +82,10 @@ list(APPEND test_SOURCE_FILES ${test_HEADER_FILES}) add_executable(lltest ${test_SOURCE_FILES}) +if (USE_BUGSPLAT) + set_target_properties(lltest PROPERTIES COMPILE_DEFINITIONS "${BUGSPLAT_DEFINE}") +endif (USE_BUGSPLAT) + target_link_libraries(lltest ${LLDATABASE_LIBRARIES} ${LLINVENTORY_LIBRARIES} diff --git a/indra/test/test.cpp b/indra/test/test.cpp index 125de72b79..5dabcbbc58 100644 --- a/indra/test/test.cpp +++ b/indra/test/test.cpp @@ -75,7 +75,10 @@ #include -void wouldHaveCrashed(const std::string& message); +void wouldHaveCrashed(const std::string& message) +{ + tut::fail("fatal error message: " + message); +} namespace tut { @@ -506,11 +509,6 @@ void stream_groups(std::ostream& s, const char* app) } } -void wouldHaveCrashed(const std::string& message) -{ - tut::fail("fatal error message: " + message); -} - static LLTrace::ThreadRecorder* sMasterThreadRecorder = NULL; int main(int argc, char **argv) -- cgit v1.2.3 From e86c0b3d0fbc5f91090241be959ef19bfffd8636 Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Tue, 5 Mar 2019 04:44:29 -0500 Subject: 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 77d7fe1b24..6fa9e590cb 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 07aa5c6f8b..2a73ccb36a 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 af46430f74..2fa220ba5a 100644 --- a/indra/llcommon/llerrorcontrol.h +++ b/indra/llcommon/llerrorcontrol.h @@ -93,16 +93,21 @@ namespace LLError Control functions. */ - typedef boost::function 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 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 2f8923d2de..9dcb5c145f 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 fedc17dbe9..a412e12a04 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 @@ public: 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 f9ee22d20a..8b9f9085b1 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -751,16 +751,7 @@ public: } }; -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 3111540a13..9c2e6eadca 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 5dabcbbc58..4f966aede8 100644 --- a/indra/test/test.cpp +++ b/indra/test/test.cpp @@ -75,9 +75,10 @@ #include -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 @@ public: 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); -- cgit v1.2.3 From 07870a9ebb17e7a31df276f4ac2d8f52fec2a3b0 Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Tue, 5 Mar 2019 13:57:02 -0500 Subject: rename crash handler to hook, and make them chainable by putting back the get and documenting it --- indra/llcommon/llerror.cpp | 18 +++++++++--------- indra/llcommon/llerror.h | 6 +++--- indra/llcommon/llerrorcontrol.h | 26 ++++++++++++++------------ indra/llcommon/llleap.cpp | 32 +++++++++++++++++++++++++++++++- indra/llcommon/tests/llerror_test.cpp | 4 ++-- indra/llcommon/tests/wrapllerrs.h | 23 +++++++++++++---------- indra/newview/llappviewer.cpp | 4 ++-- indra/test/test.cpp | 6 +++--- 8 files changed, 77 insertions(+), 42 deletions(-) diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 6fa9e590cb..5f3edf7f84 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -483,7 +483,7 @@ namespace LLError LevelMap mTagLevelMap; std::map mUniqueLogMessages; - LLError::FatalFunction mCrashFunction; + LLError::FatalHook mFatalHook; LLError::TimeFunction mTimeFunction; Recorders mRecorders; @@ -523,7 +523,7 @@ namespace LLError mFileLevelMap(), mTagLevelMap(), mUniqueLogMessages(), - mCrashFunction(NULL), + mFatalHook(NULL), mTimeFunction(NULL), mRecorders(), mFileRecorder(), @@ -719,16 +719,16 @@ namespace LLError commonInit(user_dir, app_dir, log_to_stderr); } - void setFatalHandler(const FatalFunction& fatal_function) + void setFatalHook(const FatalHook& fatal_hook) { SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig(); - s->mCrashFunction = fatal_function; + s->mFatalHook = fatal_hook; } - void restoreCrashOnError() + FatalHook getFatalHook() { SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig(); - s->mCrashFunction = NULL; + return s->mFatalHook; } std::string getFatalMessage() @@ -1306,7 +1306,7 @@ namespace LLError return ; } - ErrCrashHandlerResult Log::flush(std::ostringstream* out, const CallSite& site) + ErrFatalHookResult Log::flush(std::ostringstream* out, const CallSite& site) { LLMutexTrylock lock(&gLogMutex,5); if (!lock.isLocked()) @@ -1369,9 +1369,9 @@ namespace LLError if (site.mLevel == LEVEL_ERROR) { - if (s->mCrashFunction) + if (s->mFatalHook) { - return s->mCrashFunction(message); + return s->mFatalHook(message); } else { diff --git a/indra/llcommon/llerror.h b/indra/llcommon/llerror.h index 2a73ccb36a..6bdb2e852f 100644 --- a/indra/llcommon/llerror.h +++ b/indra/llcommon/llerror.h @@ -194,7 +194,7 @@ namespace LLError struct CallSite; - enum ErrCrashHandlerResult { ERR_DO_NOT_CRASH, ERR_CRASH }; + enum ErrFatalHookResult { ERR_DO_NOT_CRASH, ERR_CRASH }; class LL_COMMON_API Log { @@ -205,7 +205,7 @@ namespace LLError static void flush(std::ostringstream* out, char* message); // returns false iff the calling macro should crash - static ErrCrashHandlerResult flush(std::ostringstream*, const CallSite&); + static ErrFatalHookResult flush(std::ostringstream*, const CallSite&); static std::string demangle(const char* mangled); }; @@ -272,7 +272,7 @@ namespace LLError //when LLAppViewer::handleViewerCrash() is triggered. // //Note: to be simple, efficient and necessary to keep track of correct call stacks, - //LLCallStacks is designed not to be thread-safe. + //LLCallStacks is designed not to be thread-safe. //so try not to use it in multiple parallel threads at same time. //Used in a single thread at a time is fine. class LL_COMMON_API LLCallStacks diff --git a/indra/llcommon/llerrorcontrol.h b/indra/llcommon/llerrorcontrol.h index 2fa220ba5a..e7c5241cc9 100644 --- a/indra/llcommon/llerrorcontrol.h +++ b/indra/llcommon/llerrorcontrol.h @@ -93,22 +93,24 @@ namespace LLError Control functions. */ - // A FatalFunction is called if set using setFatalHandler; its return controls + // A FatalHook is called if set using setFatalHook; 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 FatalFunction; - - - /// Override the default behavior of crashing on LL_ERRS; this should NEVER be used except in test code - LL_COMMON_API void setFatalHandler(const FatalFunction&); - // The fatal function will be called when an message of LEVEL_ERROR + typedef boost::function FatalHook; + + /// Supplement and control the default behavior of crashing on LL_ERRS + /// This may be used to suppress crashes only in test code; + /// otherwise, the FatalHook should always either return + /// the result from the previous hook (see getFatalHook below), + /// or return LLError::ERR_CRASH + LL_COMMON_API void setFatalHook(const FatalHook& fatal_hook); + // The fatal_hook 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 + // (by, for example, setting a class level to LEVEL_NONE), will also + // prevent the fatal_hook being called and the resulting deliberate crash - /// Undo the effect of the setFatalHandler above - LL_COMMON_API void restoreCrashOnError(); + /// Retrieve the previously-set FatalHook + LL_COMMON_API FatalHook getFatalHook(); LL_COMMON_API std::string getFatalMessage(); // Retrieve the message last passed to LL_ERRS, if any diff --git a/indra/llcommon/llleap.cpp b/indra/llcommon/llleap.cpp index bf20c87c89..161d25bc34 100644 --- a/indra/llcommon/llleap.cpp +++ b/indra/llcommon/llleap.cpp @@ -59,7 +59,7 @@ public: // pump name -- so it should NOT need tweaking for uniqueness. mReplyPump(LLUUID::generateNewID().asString()), mExpect(0), - + mPrevFatalHook(LLError::getFatalHook()), // Instantiate a distinct LLLeapListener for this plugin. (Every // plugin will want its own collection of managed listeners, etc.) // Pass it a callback to our connect() method, so it can send events @@ -145,6 +145,9 @@ public: mStderrConnection = childerr.getPump() .listen("LLLeap", boost::bind(&LLLeapImpl::rstderr, this, _1)); + // For our lifespan, intercept any LL_ERRS so we can notify plugin + LLError::setFatalHook(boost::bind(&LLLeapImpl::fatalHook, this, _1)); + // Send child a preliminary event reporting our own reply-pump name -- // which would otherwise be pretty tricky to guess! wstdin(mReplyPump.getName(), @@ -159,6 +162,8 @@ public: virtual ~LLLeapImpl() { LL_DEBUGS("LLLeap") << "destroying LLLeap(\"" << mDesc << "\")" << LL_ENDL; + // Restore original FatalHook + LLError::setFatalHook(mPrevFatalHook); } // Listener for failed launch attempt @@ -372,6 +377,30 @@ public: return false; } + LLError::ErrFatalHookResult fatalHook(const std::string& error) + { + // Notify plugin + LLSD event; + event["type"] = "error"; + event["error"] = error; + mReplyPump.post(event); + + // All the above really accomplished was to buffer the serialized + // event in our WritePipe. Have to pump mainloop a couple times to + // really write it out there... but time out in case we can't write. + LLProcess::WritePipe& childin(mChild->getWritePipe(LLProcess::STDIN)); + LLEventPump& mainloop(LLEventPumps::instance().obtain("mainloop")); + LLSD nop; + F64 until = (LLTimer::getElapsedSeconds() + 2).value(); + while (childin.size() && LLTimer::getElapsedSeconds() < until) + { + mainloop.post(nop); + } + + // forward the call to the previous FatalHook, default to crashing if there isn't one + return mPrevFatalHook ? mPrevFatalHook(error) : LLError::ERR_CRASH; + } + private: /// We always want to listen on mReplyPump with wstdin(); under some /// circumstances we'll also echo other LLEventPumps to the plugin. @@ -392,6 +421,7 @@ private: mStdinConnection, mStdoutConnection, mStdoutDataConnection, mStderrConnection; boost::scoped_ptr mBlocker; LLProcess::ReadPipe::size_type mExpect; + LLError::FatalHook mPrevFatalHook; boost::scoped_ptr mListener; }; diff --git a/indra/llcommon/tests/llerror_test.cpp b/indra/llcommon/tests/llerror_test.cpp index 9dcb5c145f..9895abc1f3 100644 --- a/indra/llcommon/tests/llerror_test.cpp +++ b/indra/llcommon/tests/llerror_test.cpp @@ -70,7 +70,7 @@ namespace namespace { static bool fatalWasCalled; - LLError::ErrCrashHandlerResult fatalCall(const std::string&) + LLError::ErrFatalHookResult fatalHook(const std::string&) { fatalWasCalled = true; return LLError::ERR_DO_NOT_CRASH; @@ -124,7 +124,7 @@ namespace tut mPriorErrorSettings = LLError::saveAndResetSettings(); LLError::setDefaultLevel(LLError::LEVEL_DEBUG); - LLError::setFatalHandler(fatalCall); + LLError::setFatalHook(fatalHook); LLError::addRecorder(mRecorder); } diff --git a/indra/llcommon/tests/wrapllerrs.h b/indra/llcommon/tests/wrapllerrs.h index a412e12a04..a6c44d5fdd 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 LLError::ErrCrashHandlerResult wouldHaveCrashed(const std::string& message); +extern LLError::ErrFatalHookResult wouldHaveCrashed(const std::string& message); struct WrapLLErrs { @@ -55,14 +55,15 @@ struct WrapLLErrs // stderr. Otherwise, expected llerrs (LL_ERRS) messages clutter the // console output of successful tests, potentially confusing things. :mPriorErrorSettings(LLError::saveAndResetSettings()) + ,mPriorFatalHook(LLError::getFatalHook()) { // Make LL_ERRS call our own operator() method - LLError::setFatalHandler(boost::bind(&WrapLLErrs::operator(), this, _1)); + LLError::setFatalHook(boost::bind(&WrapLLErrs::operator(), this, _1)); } ~WrapLLErrs() { - LLError::restoreCrashOnError(); + LLError::setFatalHook(mPriorFatalHook); LLError::restoreSettings(mPriorErrorSettings); } @@ -71,7 +72,7 @@ struct WrapLLErrs FatalException(const std::string& what): LLException(what) {} }; - LLError::ErrCrashHandlerResult operator()(const std::string& message) + LLError::ErrFatalHookResult operator()(const std::string& message) { // Save message for later in case consumer wants to sense the result directly error = message; @@ -107,7 +108,7 @@ struct WrapLLErrs std::string error; LLError::SettingsStoragePtr mPriorErrorSettings; - LLError::FatalFunction mPriorFatal; + LLError::FatalHook mPriorFatalHook; }; /** @@ -197,11 +198,12 @@ public: // with that output. If it turns out that saveAndResetSettings() has // some bad effect, give up and just let the DEBUG level log messages // display. - : boost::noncopyable(), - mOldSettings(LLError::saveAndResetSettings()), - mRecorder(new CaptureLogRecorder()) + : boost::noncopyable() + , mOldSettings(LLError::saveAndResetSettings()) + , mPriorFatalHook(LLError::getFatalHook()) + , mRecorder(new CaptureLogRecorder()) { - LLError::setFatalHandler(wouldHaveCrashed); + LLError::setFatalHook(wouldHaveCrashed); LLError::setDefaultLevel(level); LLError::addRecorder(mRecorder); } @@ -210,7 +212,7 @@ public: { LLError::removeRecorder(mRecorder); LLError::restoreSettings(mOldSettings); - LLError::restoreCrashOnError(); + LLError::setFatalHook(mPriorFatalHook); } /// Don't assume the message we want is necessarily the LAST log message @@ -228,6 +230,7 @@ public: private: LLError::SettingsStoragePtr mOldSettings; + LLError::FatalHook mPriorFatalHook; LLError::RecorderPtr mRecorder; }; diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index 8b9f9085b1..8616fe7c76 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -2134,7 +2134,7 @@ bool LLAppViewer::initThreads() return true; } -LLError::ErrCrashHandlerResult fatalErrorHandler(const std::string &error_string) +LLError::ErrFatalHookResult fatalErrorHook(const std::string &error_string) { #ifndef LL_RELEASE_FOR_DOWNLOAD OSMessageBox(error_string, LLTrans::getString("MBFatalError"), OSMB_OK); @@ -2166,7 +2166,7 @@ void LLAppViewer::initLoggingAndGetLastDuration() LLError::initForApplication( gDirUtilp->getExpandedFilename(LL_PATH_USER_SETTINGS, "") ,gDirUtilp->getExpandedFilename(LL_PATH_APP_SETTINGS, "") ); - LLError::setFatalHandler(fatalErrorHandler); + LLError::setFatalHook(fatalErrorHook); // Remove the last ".old" log file. std::string old_log_file = gDirUtilp->getExpandedFilename(LL_PATH_LOGS, diff --git a/indra/test/test.cpp b/indra/test/test.cpp index 4f966aede8..52ac855d9f 100644 --- a/indra/test/test.cpp +++ b/indra/test/test.cpp @@ -75,7 +75,7 @@ #include -LLError::ErrCrashHandlerResult wouldHaveCrashed(const std::string& message) +LLError::ErrFatalHookResult wouldHaveCrashed(const std::string& message) { tut::fail("fatal error message: " + message); return LLError::ERR_DO_NOT_CRASH; @@ -150,7 +150,7 @@ public: mOldSettings(LLError::saveAndResetSettings()), mRecorder(new RecordToTempFile(pool)) { - LLError::setFatalHandler(wouldHaveCrashed); + LLError::setFatalHook(wouldHaveCrashed); LLError::setDefaultLevel(level); LLError::addRecorder(mRecorder); } @@ -531,7 +531,7 @@ int main(int argc, char **argv) LLError::initForApplication(".", ".", false /* do not log to stderr */); LLError::setDefaultLevel(LLError::LEVEL_DEBUG); } - LLError::setFatalHandler(wouldHaveCrashed); + LLError::setFatalHook(wouldHaveCrashed); std::string test_app_name(argv[0]); std::string test_log = test_app_name + ".log"; LLFile::remove(test_log); -- cgit v1.2.3 From 46fb225134ef871a686d0571cc55256923eee110 Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Tue, 21 May 2019 12:09:26 -0400 Subject: suppress unreachable code warning because tut::fail triggers it --- indra/test/test.cpp | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/indra/test/test.cpp b/indra/test/test.cpp index 52ac855d9f..9d327ff3be 100644 --- a/indra/test/test.cpp +++ b/indra/test/test.cpp @@ -57,16 +57,11 @@ #include #endif -#if LL_MSVC -#pragma warning (push) +#if LL_WINDOWS #pragma warning (disable : 4702) // warning C4702: unreachable code #endif #include #include -#if LL_MSVC -#pragma warning (pop) -#endif - #include #include #include -- cgit v1.2.3 From 1e98a607d08019f66ae878e4cb247e850a7443cf Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Tue, 4 Jun 2019 10:06:43 -0400 Subject: try putting fatal message into dynamic crash data --- indra/newview/llappviewer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index 8616fe7c76..0e97ed5a47 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -3388,7 +3388,7 @@ void LLAppViewer::writeSystemInfo() // "CrashNotHandled" is obsolete; it used (not very successsfully) // to try to distinguish crashes from freezes gDebugInfo["CrashNotHandled"] = (LLSD::Boolean)false; - gDebugInfo["FatalMessage"] = LLError::getFatalMessage(); + gDebugInfo["Dynamic"]["FatalMessage"] = LLError::getFatalMessage(); #endif // ! LL_BUGSPLAT // Insert crash host url (url to post crash log to) if configured. This insures -- cgit v1.2.3 From e711376cc75a51973291d1730467c0b6e3a74226 Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Wed, 5 Jun 2019 14:27:48 -0400 Subject: assorted cleanup --- indra/llcommon/llapp.cpp | 5 +++++ indra/newview/CMakeLists.txt | 2 +- indra/newview/llappdelegate-objc.mm | 2 ++ indra/newview/llappviewer.cpp | 10 ++-------- indra/newview/llappviewerwin32.cpp | 3 +-- indra/newview/llviewerwindow.cpp | 3 ++- 6 files changed, 13 insertions(+), 12 deletions(-) diff --git a/indra/llcommon/llapp.cpp b/indra/llcommon/llapp.cpp index 421af3006e..34c969437b 100644 --- a/indra/llcommon/llapp.cpp +++ b/indra/llcommon/llapp.cpp @@ -526,7 +526,12 @@ void LLApp::setupErrorHandling(bool second_instance) #endif // LL_LINUX #endif // ! LL_WINDOWS + +#ifdef LL_BUGSPLAT + // do not start our own error thread +#else // ! LL_BUGSPLAT startErrorThread(); +#endif } void LLApp::startErrorThread() diff --git a/indra/newview/CMakeLists.txt b/indra/newview/CMakeLists.txt index bbfa838827..fd520d2c6f 100644 --- a/indra/newview/CMakeLists.txt +++ b/indra/newview/CMakeLists.txt @@ -1729,7 +1729,7 @@ endif (SDL_FOUND) if (USE_BUGSPLAT) set_property(TARGET ${VIEWER_BINARY_NAME} - PROPERTY COMPILE_DEFINITIONS "LL_BUGSPLAT") + PROPERTY COMPILE_DEFINITIONS "${BUGSPLAT_DEFINE}") endif (USE_BUGSPLAT) # add package files diff --git a/indra/newview/llappdelegate-objc.mm b/indra/newview/llappdelegate-objc.mm index 47fde299c7..30bfe1f439 100644 --- a/indra/newview/llappdelegate-objc.mm +++ b/indra/newview/llappdelegate-objc.mm @@ -66,6 +66,7 @@ constructViewer(); #if defined(LL_BUGSPLAT) + infos("bugsplat setup"); // Engage BugsplatStartupManager *before* calling initViewer() to handle // any crashes during initialization. // https://www.bugsplat.com/docs/platforms/os-x#initialization @@ -74,6 +75,7 @@ [BugsplatStartupManager sharedManager].delegate = self; [[BugsplatStartupManager sharedManager] start]; #endif + infos("post-bugsplat setup"); frameTimer = nil; diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index 0e97ed5a47..35f9d7f4fd 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -3386,9 +3386,8 @@ void LLAppViewer::writeSystemInfo() gDebugInfo["CrashNotHandled"] = (LLSD::Boolean)true; #else // LL_BUGSPLAT // "CrashNotHandled" is obsolete; it used (not very successsfully) - // to try to distinguish crashes from freezes + // to try to distinguish crashes from freezes - the intent here to to avoid calling it a freeze gDebugInfo["CrashNotHandled"] = (LLSD::Boolean)false; - gDebugInfo["Dynamic"]["FatalMessage"] = LLError::getFatalMessage(); #endif // ! LL_BUGSPLAT // Insert crash host url (url to post crash log to) if configured. This insures @@ -3617,7 +3616,7 @@ void LLAppViewer::handleViewerCrash() if (LLWorld::instanceExists()) LLWorld::getInstance()->getInfo(gDebugInfo["Dynamic"]); - gDebugInfo["Dynamic"]["FatalMessage"] = LLError::getFatalMessage(); + gDebugInfo["FatalMessage"] = LLError::getFatalMessage(); // Close the debug file pApp->writeDebugInfo(false); //false answers the isStatic question with the least overhead. @@ -5476,11 +5475,6 @@ void LLAppViewer::pauseMainloopTimeout() void LLAppViewer::pingMainloopTimeout(const std::string& state, F32 secs) { -// if(!restoreErrorTrap()) -// { -// LL_WARNS() << "!!!!!!!!!!!!! Its an error trap!!!!" << state << LL_ENDL; -// } - if(mMainloopTimeout) { if(secs < 0.0f) diff --git a/indra/newview/llappviewerwin32.cpp b/indra/newview/llappviewerwin32.cpp index d208e135bb..6a6cc225d4 100644 --- a/indra/newview/llappviewerwin32.cpp +++ b/indra/newview/llappviewerwin32.cpp @@ -802,8 +802,7 @@ bool LLAppViewerWin32::beingDebugged() bool LLAppViewerWin32::restoreErrorTrap() { - return true; - //return LLWinDebug::checkExceptionHandler(); + return false; } void LLAppViewerWin32::initCrashReporting(bool reportFreeze) diff --git a/indra/newview/llviewerwindow.cpp b/indra/newview/llviewerwindow.cpp index c214984e1d..b0f5b550e6 100644 --- a/indra/newview/llviewerwindow.cpp +++ b/indra/newview/llviewerwindow.cpp @@ -1791,7 +1791,8 @@ LLViewerWindow::LLViewerWindow(const Params& p) if (!LLAppViewer::instance()->restoreErrorTrap()) { - LL_WARNS("Window") << " Someone took over my signal/exception handler (post createWindow)!" << LL_ENDL; + // this always happens, so downgrading it to INFO + LL_INFOS("Window") << " Someone took over my signal/exception handler (post createWindow; normal)" << LL_ENDL; } const bool do_not_enforce = false; -- cgit v1.2.3 From b6f1529b5abaf36b15efb6d9215bcc89e6a3f167 Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Thu, 6 Jun 2019 09:25:15 -0400 Subject: remove the old crash logger modules when using BugSplat --- indra/CMakeLists.txt | 40 +++++++++++++++++++++++----------------- indra/newview/CMakeLists.txt | 28 +++++++++++++++++++--------- 2 files changed, 42 insertions(+), 26 deletions(-) diff --git a/indra/CMakeLists.txt b/indra/CMakeLists.txt index cf6029c4ae..4dd2dcc2e9 100644 --- a/indra/CMakeLists.txt +++ b/indra/CMakeLists.txt @@ -60,29 +60,18 @@ add_subdirectory(${LIBS_OPEN_PREFIX}media_plugins) endif (ENABLE_MEDIA_PLUGINS) if (LINUX) - add_subdirectory(${VIEWER_PREFIX}linux_crash_logger) if (INSTALL_PROPRIETARY) include(LLAppearanceUtility) add_subdirectory(${LLAPPEARANCEUTILITY_SRC_DIR} ${LLAPPEARANCEUTILITY_BIN_DIR}) endif (INSTALL_PROPRIETARY) - add_dependencies(viewer linux-crash-logger-strip-target) -elseif (DARWIN) - add_subdirectory(${VIEWER_PREFIX}mac_crash_logger) - add_dependencies(viewer mac-crash-logger) -elseif (WINDOWS) - add_subdirectory(${VIEWER_PREFIX}win_crash_logger) - # cmake EXISTS requires an absolute path, see indra/cmake/Variables.cmake - if (EXISTS ${VIEWER_DIR}win_setup) - add_subdirectory(${VIEWER_DIR}win_setup) - endif (EXISTS ${VIEWER_DIR}win_setup) - # add_dependencies(viewer windows-setup windows-crash-logger) - add_dependencies(viewer windows-crash-logger) endif (LINUX) -add_subdirectory(${VIEWER_PREFIX}newview) -add_dependencies(viewer secondlife-bin) - -add_subdirectory(${VIEWER_PREFIX}doxygen EXCLUDE_FROM_ALL) +if (WINDOWS) + # cmake EXISTS requires an absolute path, see indra/cmake/Variables.cmake + if (EXISTS ${VIEWER_DIR}win_setup) + add_subdirectory(${VIEWER_DIR}win_setup) + endif (EXISTS ${VIEWER_DIR}win_setup) +endif (WINDOWS) # sets the 'startup project' for debugging from visual studio. set_property( @@ -98,7 +87,24 @@ if (USE_BUGSPLAT) endif (BUGSPLAT_DB) else (USE_BUGSPLAT) message(STATUS "Not building with BugSplat") + if (LINUX) + add_subdirectory(${VIEWER_PREFIX}linux_crash_logger) + add_dependencies(viewer linux-crash-logger-strip-target) + elseif (DARWIN) + add_subdirectory(${VIEWER_PREFIX}mac_crash_logger) + add_dependencies(viewer mac-crash-logger) + elseif (WINDOWS) + add_subdirectory(${VIEWER_PREFIX}win_crash_logger) + # add_dependencies(viewer windows-setup windows-crash-logger) + add_dependencies(viewer windows-crash-logger) + endif (LINUX) endif (USE_BUGSPLAT) + +add_subdirectory(${VIEWER_PREFIX}newview) +add_dependencies(viewer secondlife-bin) + +add_subdirectory(${VIEWER_PREFIX}doxygen EXCLUDE_FROM_ALL) + if (LL_TESTS) # Define after the custom targets are created so # individual apps can add themselves as dependencies diff --git a/indra/newview/CMakeLists.txt b/indra/newview/CMakeLists.txt index fd520d2c6f..40ff1263f2 100644 --- a/indra/newview/CMakeLists.txt +++ b/indra/newview/CMakeLists.txt @@ -1808,9 +1808,12 @@ if (WINDOWS) media_plugin_libvlc media_plugin_example winmm_shim - windows-crash-logger ) + if (NOT USE_BUGSPLAT) + LIST(APPEND COPY_INPUT_DEPENDENCIES windows-crash-logger) + endif (NOT USE_BUGSPLAT) + if (ADDRESS_SIZE EQUAL 64) list(APPEND COPY_INPUT_DEPENDENCIES ${SHARED_LIB_STAGING_DIR}/${CMAKE_CFG_INTDIR}/vivoxsdk_x64.dll @@ -1864,10 +1867,11 @@ if (WINDOWS) add_dependencies(${VIEWER_BINARY_NAME} copy_win_scripts) endif (EXISTS ${CMAKE_SOURCE_DIR}/copy_win_scripts) - add_dependencies(${VIEWER_BINARY_NAME} - SLPlugin - windows-crash-logger - ) + add_dependencies(${VIEWER_BINARY_NAME} SLPlugin) + + if (NOT USE_BUGSPLAT) + add_dependencies(${VIEWER_BINARY_NAME} windows-crash-logger) + endif (NOT USE_BUGSPLAT) # sets the 'working directory' for debugging from visual studio. # Condition for version can be moved to requirements once build agents will be updated (see TOOL-3865) @@ -2031,13 +2035,16 @@ if (LINUX) # These are the generated targets that are copied to package/ set(COPY_INPUT_DEPENDENCIES ${VIEWER_BINARY_NAME} - linux-crash-logger SLPlugin media_plugin_gstreamer010 media_plugin_libvlc llcommon ) + if (NOT USE_BUGSPLAT) + LIST(APPEND COPY_INPUT_DEPENDENCIES linux-crash-logger) + endif (NOT USE_BUGSPLAT) + add_custom_command( OUTPUT ${product}.tar.bz2 COMMAND ${PYTHON_EXECUTABLE} @@ -2162,8 +2169,11 @@ if (DARWIN) ${CMAKE_CURRENT_SOURCE_DIR}/viewer_manifest.py ) - add_dependencies(${VIEWER_BINARY_NAME} SLPlugin media_plugin_libvlc media_plugin_cef mac-crash-logger) - add_dependencies(${VIEWER_BINARY_NAME} mac-crash-logger) + add_dependencies(${VIEWER_BINARY_NAME} SLPlugin media_plugin_libvlc media_plugin_cef) + + if (NOT USE_BUGSPLAT) + add_dependencies(${VIEWER_BINARY_NAME} mac-crash-logger) + endif (USE_BUGSPLAT) if (ENABLE_SIGNING) set(SIGNING_SETTING "--signature=${SIGNING_IDENTITY}") @@ -2221,7 +2231,7 @@ if (PACKAGE AND (RELEASE_CRASH_REPORTING OR NON_RELEASE_CRASH_REPORTING) AND VIE list(APPEND SYMBOL_SEARCH_DIRS "${CMAKE_BINARY_DIR}/llplugin/slplugin/${CMAKE_CFG_INTDIR}") list(APPEND SYMBOL_SEARCH_DIRS "${CMAKE_BINARY_DIR}/mac_crash_logger/${CMAKE_CFG_INTDIR}") list(APPEND SYMBOL_SEARCH_DIRS "${CMAKE_BINARY_DIR}/media_plugins/gstreamer010/${CMAKE_CFG_INTDIR}") - set(VIEWER_EXE_GLOBS "'${product}' SLPlugin mac-crash-logger") + set(VIEWER_EXE_GLOBS "'${product}' SLPlugin") set(VIEWER_EXE_GLOBS "'${product}' mac-crash-logger") set(VIEWER_LIB_GLOB "*.dylib") endif (DARWIN) -- cgit v1.2.3 From 4c525832cc7a32abce94823ac9c8ec2c4d510498 Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Wed, 12 Jun 2019 08:49:59 -0400 Subject: remove old crash logger more --- indra/newview/llappviewermacosx.cpp | 9 +++++++-- indra/newview/llappviewerwin32.cpp | 6 +----- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/indra/newview/llappviewermacosx.cpp b/indra/newview/llappviewermacosx.cpp index 9c2e6eadca..4a519b944b 100644 --- a/indra/newview/llappviewermacosx.cpp +++ b/indra/newview/llappviewermacosx.cpp @@ -241,7 +241,7 @@ bool LLAppViewerMacOSX::init() { bool success = LLAppViewer::init(); -#if LL_SEND_CRASH_REPORTS +#if !defined LL_BUGSPLAT && LL_SEND_CRASH_REPORTS if (success) { LLAppViewer* pApp = LLAppViewer::instance(); @@ -368,6 +368,7 @@ bool LLAppViewerMacOSX::restoreErrorTrap() void LLAppViewerMacOSX::initCrashReporting(bool reportFreeze) { +#ifndef LL_BUGSPLAT std::string command_str = "mac-crash-logger.app"; std::stringstream pid_str; @@ -379,6 +380,9 @@ void LLAppViewerMacOSX::initCrashReporting(bool reportFreeze) LL_WARNS() << "about to launch mac-crash-logger" << pid_str.str() << " " << logdir << " " << appname << LL_ENDL; launchApplication(&command_str, &args); +#else + LL_DEBUGS("InitOSX") << "using BugSplat instead of legacy crash logger" << LL_ENDL; +#endif // ! LL_BUGSPLAT } std::string LLAppViewerMacOSX::generateSerialNumber() @@ -390,7 +394,8 @@ std::string LLAppViewerMacOSX::generateSerialNumber() CFStringRef serialNumber = NULL; io_service_t platformExpert = IOServiceGetMatchingService(kIOMasterPortDefault, IOServiceMatching("IOPlatformExpertDevice")); - if (platformExpert) { + if (platformExpert) + { serialNumber = (CFStringRef) IORegistryEntryCreateCFProperty(platformExpert, CFSTR(kIOPlatformSerialNumberKey), kCFAllocatorDefault, 0); diff --git a/indra/newview/llappviewerwin32.cpp b/indra/newview/llappviewerwin32.cpp index 6a6cc225d4..235f7cc8ed 100644 --- a/indra/newview/llappviewerwin32.cpp +++ b/indra/newview/llappviewerwin32.cpp @@ -174,7 +174,7 @@ static void exceptionTerminateHandler() long *null_ptr; null_ptr = 0; *null_ptr = 0xDEADBEEF; //Force an exception that will trigger breakpad. - //LLAppViewer::handleViewerCrash(); + // we've probably been killed-off before now, but... gOldTerminateHandler(); // call old terminate() handler } @@ -362,10 +362,6 @@ int APIENTRY WINMAIN(HINSTANCE hInstance, viewer_app_ptr->setErrorHandler(LLAppViewer::handleViewerCrash); -#if LL_SEND_CRASH_REPORTS - // ::SetUnhandledExceptionFilter(catchallCrashHandler); -#endif - // Set a debug info flag to indicate if multiple instances are running. bool found_other_instance = !create_app_mutex(); gDebugInfo["FoundOtherInstanceAtStartup"] = LLSD::Boolean(found_other_instance); -- cgit v1.2.3 From 9627eddb102580703d6229bc6e827696dcb9e9d4 Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Wed, 12 Jun 2019 08:51:01 -0400 Subject: document that old crash logger is not used in production builds --- indra/linux_crash_logger/README.txt | 3 +++ indra/llcrashlogger/README.txt | 3 +++ indra/mac_crash_logger/README.txt | 3 +++ indra/win_crash_logger/README.txt | 3 +++ 4 files changed, 12 insertions(+) create mode 100644 indra/linux_crash_logger/README.txt create mode 100644 indra/llcrashlogger/README.txt create mode 100644 indra/mac_crash_logger/README.txt create mode 100644 indra/win_crash_logger/README.txt diff --git a/indra/linux_crash_logger/README.txt b/indra/linux_crash_logger/README.txt new file mode 100644 index 0000000000..6932a8d9c3 --- /dev/null +++ b/indra/linux_crash_logger/README.txt @@ -0,0 +1,3 @@ +This component is no longer used in Linden Lab builds. +Change requests to support continued use by open source +builds are welcome. diff --git a/indra/llcrashlogger/README.txt b/indra/llcrashlogger/README.txt new file mode 100644 index 0000000000..6932a8d9c3 --- /dev/null +++ b/indra/llcrashlogger/README.txt @@ -0,0 +1,3 @@ +This component is no longer used in Linden Lab builds. +Change requests to support continued use by open source +builds are welcome. diff --git a/indra/mac_crash_logger/README.txt b/indra/mac_crash_logger/README.txt new file mode 100644 index 0000000000..6932a8d9c3 --- /dev/null +++ b/indra/mac_crash_logger/README.txt @@ -0,0 +1,3 @@ +This component is no longer used in Linden Lab builds. +Change requests to support continued use by open source +builds are welcome. diff --git a/indra/win_crash_logger/README.txt b/indra/win_crash_logger/README.txt new file mode 100644 index 0000000000..6932a8d9c3 --- /dev/null +++ b/indra/win_crash_logger/README.txt @@ -0,0 +1,3 @@ +This component is no longer used in Linden Lab builds. +Change requests to support continued use by open source +builds are welcome. -- cgit v1.2.3 From 83ec9ebfbd602771dbc45c08fe203044b5ce3527 Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Tue, 24 Sep 2019 10:15:20 -0400 Subject: log calls that modify the fatal error hook --- indra/llcommon/llerror.cpp | 2 ++ indra/llcommon/tests/llerror_test.cpp | 17 ++++++++++------- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 5f3edf7f84..da68117ff5 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -723,11 +723,13 @@ namespace LLError { SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig(); s->mFatalHook = fatal_hook; + LL_DEBUGS("FatalHook") << "set fatal hook to " << fatal_hook << LL_ENDL; } FatalHook getFatalHook() { SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig(); + LL_DEBUGS("FatalHook") << "read fatal hook " << s->mFatalHook << LL_ENDL; return s->mFatalHook; } diff --git a/indra/llcommon/tests/llerror_test.cpp b/indra/llcommon/tests/llerror_test.cpp index 9895abc1f3..cdc2bf8c87 100644 --- a/indra/llcommon/tests/llerror_test.cpp +++ b/indra/llcommon/tests/llerror_test.cpp @@ -781,30 +781,33 @@ namespace tut // proper cached, efficient lookup of filtering void ErrorTestObject::test<15>() { - LLError::setDefaultLevel(LLError::LEVEL_NONE); + LLError::setDefaultLevel(LLError::LEVEL_NONE); + + // Note that the setFatalHook in the ErrorTestData constructor + // increments the shouldLogCallCount TestAlpha::doInfo(); ensure_message_count(0); - ensure_equals("first check", LLError::shouldLogCallCount(), 1); + ensure_equals("first check", LLError::shouldLogCallCount(), 2); TestAlpha::doInfo(); ensure_message_count(0); - ensure_equals("second check", LLError::shouldLogCallCount(), 1); + ensure_equals("second check", LLError::shouldLogCallCount(), 2); LLError::setClassLevel("TestAlpha", LLError::LEVEL_DEBUG); TestAlpha::doInfo(); ensure_message_count(1); - ensure_equals("third check", LLError::shouldLogCallCount(), 2); + ensure_equals("third check", LLError::shouldLogCallCount(), 3); TestAlpha::doInfo(); ensure_message_count(2); - ensure_equals("fourth check", LLError::shouldLogCallCount(), 2); + ensure_equals("fourth check", LLError::shouldLogCallCount(), 3); LLError::setClassLevel("TestAlpha", LLError::LEVEL_WARN); TestAlpha::doInfo(); ensure_message_count(2); - ensure_equals("fifth check", LLError::shouldLogCallCount(), 3); + ensure_equals("fifth check", LLError::shouldLogCallCount(), 4); TestAlpha::doInfo(); ensure_message_count(2); - ensure_equals("sixth check", LLError::shouldLogCallCount(), 3); + ensure_equals("sixth check", LLError::shouldLogCallCount(), 4); } template<> template<> -- cgit v1.2.3 From d0619f8e476c1c5118ae926dfe1746f1dda986d8 Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Tue, 24 Sep 2019 14:27:21 -0400 Subject: improve fatal hook logging --- indra/llcommon/llerror.cpp | 4 ++-- indra/llcommon/llleap.cpp | 2 ++ indra/newview/llappviewer.cpp | 7 ++++++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index da68117ff5..df8443bd7a 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -723,13 +723,13 @@ namespace LLError { SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig(); s->mFatalHook = fatal_hook; - LL_DEBUGS("FatalHook") << "set fatal hook to " << fatal_hook << LL_ENDL; + LL_DEBUGS("FatalHook") << "set fatal hook to " << reinterpret_cast(fatal_hook) << LL_ENDL; } FatalHook getFatalHook() { SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig(); - LL_DEBUGS("FatalHook") << "read fatal hook " << s->mFatalHook << LL_ENDL; + LL_DEBUGS("FatalHook") << "read fatal hook " << reinterpret_cast(s->mFatalHook) << LL_ENDL; return s->mFatalHook; } diff --git a/indra/llcommon/llleap.cpp b/indra/llcommon/llleap.cpp index 161d25bc34..bb32c73d04 100644 --- a/indra/llcommon/llleap.cpp +++ b/indra/llcommon/llleap.cpp @@ -67,6 +67,7 @@ public: // this class or method name. mListener(new LLLeapListener(boost::bind(&LLLeapImpl::connect, this, _1, _2))) { + LL_DEBUGS("FatalHook") << "previous fatal hook was " <(mPrevFatalHook) << LL_ENDL; // Rule out unpopulated Params block if (! cparams.executable.isProvided()) { @@ -398,6 +399,7 @@ public: } // forward the call to the previous FatalHook, default to crashing if there isn't one + LL_DEBUGS("FatalHook") << "end" << LL_ENDL; return mPrevFatalHook ? mPrevFatalHook(error) : LLError::ERR_CRASH; } diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index 35f9d7f4fd..9258d1a219 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -2166,7 +2166,8 @@ void LLAppViewer::initLoggingAndGetLastDuration() LLError::initForApplication( gDirUtilp->getExpandedFilename(LL_PATH_USER_SETTINGS, "") ,gDirUtilp->getExpandedFilename(LL_PATH_APP_SETTINGS, "") ); - LLError::setFatalHook(fatalErrorHook); + + // TBD fatal hook belongs here // Remove the last ".old" log file. std::string old_log_file = gDirUtilp->getExpandedFilename(LL_PATH_LOGS, @@ -2228,6 +2229,10 @@ void LLAppViewer::initLoggingAndGetLastDuration() { LL_WARNS("MarkerFile") << duration_log_msg << LL_ENDL; } + + // TBD - temporary location for fatal hook (should be above, but for now it logs...) + LLError::setFatalHook(fatalErrorHook); + } bool LLAppViewer::loadSettingsFromDirectory(const std::string& location_key, -- cgit v1.2.3 From 581b6fe5234110efa9c27751ed2c9e062ae8f248 Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Tue, 24 Sep 2019 15:08:45 -0400 Subject: simplify function pointer logging --- indra/llcommon/llerror.cpp | 10 ++++++---- indra/llcommon/llleap.cpp | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index df8443bd7a..1f947a0a74 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -722,14 +722,16 @@ namespace LLError void setFatalHook(const FatalHook& fatal_hook) { SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig(); + LL_DEBUGS("FatalHook") << "set fatal hook to "(fatal_hook ? "non-null" : "null") + << " was " << (s->mFatalHook ? "non-null" : "null") + << LL_ENDL; s->mFatalHook = fatal_hook; - LL_DEBUGS("FatalHook") << "set fatal hook to " << reinterpret_cast(fatal_hook) << LL_ENDL; - } - + } + FatalHook getFatalHook() { SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig(); - LL_DEBUGS("FatalHook") << "read fatal hook " << reinterpret_cast(s->mFatalHook) << LL_ENDL; + LL_DEBUGS("FatalHook") << "read fatal hook was " << (s->mFatalHook ? "non-null" : "null") << LL_ENDL; return s->mFatalHook; } diff --git a/indra/llcommon/llleap.cpp b/indra/llcommon/llleap.cpp index bb32c73d04..1c0678e453 100644 --- a/indra/llcommon/llleap.cpp +++ b/indra/llcommon/llleap.cpp @@ -67,7 +67,7 @@ public: // this class or method name. mListener(new LLLeapListener(boost::bind(&LLLeapImpl::connect, this, _1, _2))) { - LL_DEBUGS("FatalHook") << "previous fatal hook was " <(mPrevFatalHook) << LL_ENDL; + LL_DEBUGS("FatalHook") << "previous fatal hook was " << (mPrevFatalHook ? "non-null" : "null") << LL_ENDL; // Rule out unpopulated Params block if (! cparams.executable.isProvided()) { -- cgit v1.2.3 From 092bf4220eeaea0ae7765ea3cb49f122fb3e3dff Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Tue, 24 Sep 2019 15:33:42 -0400 Subject: further simplify function pointer logging --- indra/llcommon/llerror.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 1f947a0a74..327c3ad537 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -722,7 +722,7 @@ namespace LLError void setFatalHook(const FatalHook& fatal_hook) { SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig(); - LL_DEBUGS("FatalHook") << "set fatal hook to "(fatal_hook ? "non-null" : "null") + LL_DEBUGS("FatalHook") << "set fatal hook to " << (fatal_hook ? "non-null" : "null") << " was " << (s->mFatalHook ? "non-null" : "null") << LL_ENDL; s->mFatalHook = fatal_hook; -- cgit v1.2.3 From bc8b55e1f2d772a657e4f7affc8a6c7d20f2c2e4 Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Wed, 25 Sep 2019 12:56:05 -0400 Subject: remove fatal error hook usage from LLLeap (potential races for multiple processes) --- indra/llcommon/llleap.cpp | 33 --------------------------------- 1 file changed, 33 deletions(-) diff --git a/indra/llcommon/llleap.cpp b/indra/llcommon/llleap.cpp index 1c0678e453..8293c35516 100644 --- a/indra/llcommon/llleap.cpp +++ b/indra/llcommon/llleap.cpp @@ -59,7 +59,6 @@ public: // pump name -- so it should NOT need tweaking for uniqueness. mReplyPump(LLUUID::generateNewID().asString()), mExpect(0), - mPrevFatalHook(LLError::getFatalHook()), // Instantiate a distinct LLLeapListener for this plugin. (Every // plugin will want its own collection of managed listeners, etc.) // Pass it a callback to our connect() method, so it can send events @@ -67,7 +66,6 @@ public: // this class or method name. mListener(new LLLeapListener(boost::bind(&LLLeapImpl::connect, this, _1, _2))) { - LL_DEBUGS("FatalHook") << "previous fatal hook was " << (mPrevFatalHook ? "non-null" : "null") << LL_ENDL; // Rule out unpopulated Params block if (! cparams.executable.isProvided()) { @@ -146,9 +144,6 @@ public: mStderrConnection = childerr.getPump() .listen("LLLeap", boost::bind(&LLLeapImpl::rstderr, this, _1)); - // For our lifespan, intercept any LL_ERRS so we can notify plugin - LLError::setFatalHook(boost::bind(&LLLeapImpl::fatalHook, this, _1)); - // Send child a preliminary event reporting our own reply-pump name -- // which would otherwise be pretty tricky to guess! wstdin(mReplyPump.getName(), @@ -163,8 +158,6 @@ public: virtual ~LLLeapImpl() { LL_DEBUGS("LLLeap") << "destroying LLLeap(\"" << mDesc << "\")" << LL_ENDL; - // Restore original FatalHook - LLError::setFatalHook(mPrevFatalHook); } // Listener for failed launch attempt @@ -378,31 +371,6 @@ public: return false; } - LLError::ErrFatalHookResult fatalHook(const std::string& error) - { - // Notify plugin - LLSD event; - event["type"] = "error"; - event["error"] = error; - mReplyPump.post(event); - - // All the above really accomplished was to buffer the serialized - // event in our WritePipe. Have to pump mainloop a couple times to - // really write it out there... but time out in case we can't write. - LLProcess::WritePipe& childin(mChild->getWritePipe(LLProcess::STDIN)); - LLEventPump& mainloop(LLEventPumps::instance().obtain("mainloop")); - LLSD nop; - F64 until = (LLTimer::getElapsedSeconds() + 2).value(); - while (childin.size() && LLTimer::getElapsedSeconds() < until) - { - mainloop.post(nop); - } - - // forward the call to the previous FatalHook, default to crashing if there isn't one - LL_DEBUGS("FatalHook") << "end" << LL_ENDL; - return mPrevFatalHook ? mPrevFatalHook(error) : LLError::ERR_CRASH; - } - private: /// We always want to listen on mReplyPump with wstdin(); under some /// circumstances we'll also echo other LLEventPumps to the plugin. @@ -423,7 +391,6 @@ private: mStdinConnection, mStdoutConnection, mStdoutDataConnection, mStderrConnection; boost::scoped_ptr mBlocker; LLProcess::ReadPipe::size_type mExpect; - LLError::FatalHook mPrevFatalHook; boost::scoped_ptr mListener; }; -- cgit v1.2.3 From 79801c717de5de6b86b45eadf61b352c9951f61e Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Wed, 25 Sep 2019 12:57:51 -0400 Subject: fix warning about mismatched condition --- indra/newview/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/indra/newview/CMakeLists.txt b/indra/newview/CMakeLists.txt index 40ff1263f2..ab6b8c9c6e 100644 --- a/indra/newview/CMakeLists.txt +++ b/indra/newview/CMakeLists.txt @@ -2173,7 +2173,7 @@ if (DARWIN) if (NOT USE_BUGSPLAT) add_dependencies(${VIEWER_BINARY_NAME} mac-crash-logger) - endif (USE_BUGSPLAT) + endif (NOT USE_BUGSPLAT) if (ENABLE_SIGNING) set(SIGNING_SETTING "--signature=${SIGNING_IDENTITY}") -- cgit v1.2.3 From 66970f2a8c048647242887eb3f0c7fd974a303ac Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Wed, 25 Sep 2019 12:58:43 -0400 Subject: fix spurious per-frame warning about signal handlers in Windows --- indra/newview/llappviewerwin32.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/indra/newview/llappviewerwin32.cpp b/indra/newview/llappviewerwin32.cpp index 235f7cc8ed..63de9af566 100644 --- a/indra/newview/llappviewerwin32.cpp +++ b/indra/newview/llappviewerwin32.cpp @@ -798,7 +798,7 @@ bool LLAppViewerWin32::beingDebugged() bool LLAppViewerWin32::restoreErrorTrap() { - return false; + return true; // we don't check for handler collisions on windows, so just say they're ok } void LLAppViewerWin32::initCrashReporting(bool reportFreeze) -- cgit v1.2.3 From a45ca18ff8160bc3edc584d6516ca019dd6c6275 Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Wed, 25 Sep 2019 12:59:20 -0400 Subject: when using bugsplat, do not catch SIGABRT; also, fix signal setting in Mac (broken macro) --- indra/llcommon/llapp.cpp | 5 +++++ indra/newview/llappviewer.cpp | 2 +- indra/newview/llappviewermacosx.cpp | 9 +++++---- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/indra/llcommon/llapp.cpp b/indra/llcommon/llapp.cpp index 34c969437b..27960371f1 100644 --- a/indra/llcommon/llapp.cpp +++ b/indra/llcommon/llapp.cpp @@ -779,7 +779,9 @@ void setup_signals() act.sa_flags = SA_SIGINFO; // Synchronous signals +# ifndef LL_BUGSPLAT sigaction(SIGABRT, &act, NULL); +# endif sigaction(SIGALRM, &act, NULL); sigaction(SIGBUS, &act, NULL); sigaction(SIGFPE, &act, NULL); @@ -816,7 +818,9 @@ void clear_signals() act.sa_flags = SA_SIGINFO; // Synchronous signals +# ifndef LL_BUGSPLAT sigaction(SIGABRT, &act, NULL); +# endif sigaction(SIGALRM, &act, NULL); sigaction(SIGBUS, &act, NULL); sigaction(SIGFPE, &act, NULL); @@ -869,6 +873,7 @@ void default_unix_signal_handler(int signum, siginfo_t *info, void *) return; case SIGABRT: + // Note that this handler is not set for SIGABRT when using Bugsplat // Abort just results in termination of the app, no funky error handling. if (LLApp::sLogInSignal) { diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index 9258d1a219..8009a9c117 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -2231,8 +2231,8 @@ void LLAppViewer::initLoggingAndGetLastDuration() } // TBD - temporary location for fatal hook (should be above, but for now it logs...) + LL_DEBUGS("FatalHook") << "initial setting of default fatalhook" << LL_ENDL; LLError::setFatalHook(fatalErrorHook); - } bool LLAppViewer::loadSettingsFromDirectory(const std::string& location_key, diff --git a/indra/newview/llappviewermacosx.cpp b/indra/newview/llappviewermacosx.cpp index 4a519b944b..efa1da054c 100644 --- a/indra/newview/llappviewermacosx.cpp +++ b/indra/newview/llappviewermacosx.cpp @@ -333,11 +333,12 @@ bool LLAppViewerMacOSX::restoreErrorTrap() unsigned int reset_count = 0; -#define SET_SIG(S) sigaction(SIGABRT, &act, &old_act); \ - if(act.sa_sigaction != old_act.sa_sigaction) \ - ++reset_count; +#define SET_SIG(SIGNAL) sigaction(SIGNAL, &act, &old_act); \ + if(act.sa_sigaction != old_act.sa_sigaction) ++reset_count; // Synchronous signals - SET_SIG(SIGABRT) +# ifndef LL_BUGSPLAT + SET_SIG(SIGABRT) // let bugsplat catch this +# endif SET_SIG(SIGALRM) SET_SIG(SIGBUS) SET_SIG(SIGFPE) -- cgit v1.2.3 From de5c07ec50d192e5a34cf710e2d1ed0c1127932f Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Mon, 7 Oct 2019 10:50:32 -0400 Subject: do not build the old crash logger when using bugsplat --- indra/CMakeLists.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/indra/CMakeLists.txt b/indra/CMakeLists.txt index 4dd2dcc2e9..bd3bcd1173 100644 --- a/indra/CMakeLists.txt +++ b/indra/CMakeLists.txt @@ -46,7 +46,10 @@ endif (WINDOWS AND EXISTS ${LIBS_CLOSED_DIR}copy_win_scripts) add_custom_target(viewer) +if (NOT USE_BUGSPLAT) add_subdirectory(${LIBS_OPEN_PREFIX}llcrashlogger) +endif (NOT USE_BUGSPLAT) + add_subdirectory(${LIBS_OPEN_PREFIX}llplugin) add_subdirectory(${LIBS_OPEN_PREFIX}llui) add_subdirectory(${LIBS_OPEN_PREFIX}viewer_components) -- cgit v1.2.3 From 2961fea6933621fc7c8ebc692d4c517d4aa93096 Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Tue, 8 Oct 2019 10:15:19 -0400 Subject: Rename uploaded log file to get Bugsplat to server as text/plain --- indra/newview/llappdelegate-objc.mm | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/indra/newview/llappdelegate-objc.mm b/indra/newview/llappdelegate-objc.mm index 30bfe1f439..ccab9173a9 100644 --- a/indra/newview/llappdelegate-objc.mm +++ b/indra/newview/llappdelegate-objc.mm @@ -301,9 +301,13 @@ struct AttachmentInfo // We "happen to know" that info[0].basename is "SecondLife.old" -- due to // the fact that BugsplatMac only notices a crash during the viewer run - // following the crash. Replace .old with .log to reduce confusion. + // following the crash. + // The Bugsplat service doesn't respect the MIME type above when returning + // the log data to a browser, so take this opportunity to rename the file + // from .old to _log.txt info[0].basename = - boost::filesystem::path(info[0].pathname).stem().string() + ".log"; + boost::filesystem::path(info[0].pathname).stem().string() + "_log.txt"; + infos("attachmentsForBugsplatStartupManager attaching log " + info[0].basename); NSMutableArray *attachments = [[NSMutableArray alloc] init]; -- cgit v1.2.3 From 1082a886ce8ffe4a23943b8513356a509373f03c Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Tue, 8 Oct 2019 10:15:47 -0400 Subject: tag initialization and bugsplat related logging --- indra/newview/llappviewermacosx.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/indra/newview/llappviewermacosx.cpp b/indra/newview/llappviewermacosx.cpp index efa1da054c..999a475f8b 100644 --- a/indra/newview/llappviewermacosx.cpp +++ b/indra/newview/llappviewermacosx.cpp @@ -91,7 +91,7 @@ void constructViewer() // Set the working dir to /Contents/Resources if (chdir(gDirUtilp->getAppRODataDir().c_str()) == -1) { - LL_WARNS() << "Could not change directory to " + LL_WARNS("InitOSX") << "Could not change directory to " << gDirUtilp->getAppRODataDir() << ": " << strerror(errno) << LL_ENDL; } @@ -109,7 +109,7 @@ bool initViewer() bool ok = gViewerAppPtr->init(); if(!ok) { - LL_WARNS() << "Application init failed." << LL_ENDL; + LL_WARNS("InitOSX") << "Application init failed." << LL_ENDL; } else if (!gHandleSLURL.empty()) { @@ -172,7 +172,7 @@ class CrashMetadataSingleton: public CrashMetadata, public LLSingleton Date: Fri, 13 Dec 2019 12:20:50 -0500 Subject: clarify crash logger at startup --- indra/newview/llappviewermacosx.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/indra/newview/llappviewermacosx.cpp b/indra/newview/llappviewermacosx.cpp index 999a475f8b..5a0fe6ba0c 100644 --- a/indra/newview/llappviewermacosx.cpp +++ b/indra/newview/llappviewermacosx.cpp @@ -81,7 +81,8 @@ static void exceptionTerminateHandler() long *null_ptr; null_ptr = 0; *null_ptr = 0xDEADBEEF; //Force an exception that will trigger breakpad. - //LLAppViewer::handleViewerCrash(); + LLAppViewer::handleViewerCrash(); + // we've probably been killed-off before now, but... gOldTerminateHandler(); // call old terminate() handler } @@ -99,7 +100,7 @@ void constructViewer() gViewerAppPtr = new LLAppViewerMacOSX(); // install unexpected exception handler - gOldTerminateHandler = std::set_terminate(exceptionTerminateHandler); + //gOldTerminateHandler = std::set_terminate(exceptionTerminateHandler); gViewerAppPtr->setErrorHandler(LLAppViewer::handleViewerCrash); } @@ -241,14 +242,11 @@ bool LLAppViewerMacOSX::init() { bool success = LLAppViewer::init(); -#if !defined LL_BUGSPLAT && LL_SEND_CRASH_REPORTS if (success) { LLAppViewer* pApp = LLAppViewer::instance(); pApp->initCrashReporting(); } -#endif - return success; } @@ -369,7 +367,10 @@ bool LLAppViewerMacOSX::restoreErrorTrap() void LLAppViewerMacOSX::initCrashReporting(bool reportFreeze) { -#ifndef LL_BUGSPLAT +#if defined LL_BUGSPLAT + LL_DEBUGS("InitOSX", "Bugsplat") << "using BugSplat crash logger" << LL_ENDL; +#elif LL_SEND_CRASH_REPORTS + LL_DEBUGS("InitOSX") << "Initializing legacy crash logger" << LL_ENDL; std::string command_str = "mac-crash-logger.app"; std::stringstream pid_str; @@ -382,7 +383,7 @@ void LLAppViewerMacOSX::initCrashReporting(bool reportFreeze) << " " << logdir << " " << appname << LL_ENDL; launchApplication(&command_str, &args); #else - LL_DEBUGS("InitOSX") << "using BugSplat instead of legacy crash logger" << LL_ENDL; + LL_DEBUGS("InitOSX") << "No crash logger enabled" << LL_ENDL; #endif // ! LL_BUGSPLAT } -- cgit v1.2.3 From 5056ba6daafce402a5db6d2b75dceb20ed452e1d Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Mon, 16 Dec 2019 17:09:42 -0500 Subject: remove unused exception handler --- indra/newview/llappviewermacosx.cpp | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/indra/newview/llappviewermacosx.cpp b/indra/newview/llappviewermacosx.cpp index 5a0fe6ba0c..784a104573 100644 --- a/indra/newview/llappviewermacosx.cpp +++ b/indra/newview/llappviewermacosx.cpp @@ -68,25 +68,9 @@ namespace int gArgC; char** gArgV; LLAppViewerMacOSX* gViewerAppPtr = NULL; - - void (*gOldTerminateHandler)() = NULL; std::string gHandleSLURL; } -static void exceptionTerminateHandler() -{ - // reinstall default terminate() handler in case we re-terminate. - if (gOldTerminateHandler) std::set_terminate(gOldTerminateHandler); - // treat this like a regular viewer crash, with nice stacktrace etc. - long *null_ptr; - null_ptr = 0; - *null_ptr = 0xDEADBEEF; //Force an exception that will trigger breakpad. - LLAppViewer::handleViewerCrash(); - - // we've probably been killed-off before now, but... - gOldTerminateHandler(); // call old terminate() handler -} - void constructViewer() { // Set the working dir to /Contents/Resources @@ -99,9 +83,6 @@ void constructViewer() gViewerAppPtr = new LLAppViewerMacOSX(); - // install unexpected exception handler - //gOldTerminateHandler = std::set_terminate(exceptionTerminateHandler); - gViewerAppPtr->setErrorHandler(LLAppViewer::handleViewerCrash); } -- cgit v1.2.3 From 17598f936bb6b099f21fc32aa80b68258cdfd0b9 Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Mon, 16 Dec 2019 17:09:58 -0500 Subject: remove unused legacy crash logger --- indra/newview/viewer_manifest.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index a403760670..861c2120d8 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -1060,10 +1060,10 @@ class DarwinManifest(ViewerManifest): # our apps executable_path = {} - for app_bld_dir, app in (("mac_crash_logger", "mac-crash-logger.app"), - # plugin launcher - (os.path.join("llplugin", "slplugin"), "SLPlugin.app"), - ): + embedded_apps = [ (os.path.join("llplugin", "slplugin"), "SLPlugin.app") ] + if bugsplat_db: + embedded_apps.append(("mac_crash_logger", "mac-crash-logger.app")) + for app_bld_dir, app in embedded_apps: self.path2basename(os.path.join(os.pardir, app_bld_dir, self.args['configuration']), app) -- cgit v1.2.3 From 68105d9f06aea69980cf0402f00da0e2be450dd2 Mon Sep 17 00:00:00 2001 From: Brad Kittenbrink Date: Thu, 2 Apr 2020 10:55:31 -0700 Subject: Fixed -Wstring-plus-int related errors for compatibility with Xcode-11.4 --- indra/llcommon/llsdutil.cpp | 2 +- indra/newview/llcommandhandler.cpp | 2 +- indra/newview/llxmlrpclistener.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/indra/llcommon/llsdutil.cpp b/indra/llcommon/llsdutil.cpp index 9d00395c0a..859d2eb567 100644 --- a/indra/llcommon/llsdutil.cpp +++ b/indra/llcommon/llsdutil.cpp @@ -332,7 +332,7 @@ struct Data const char* name; } typedata[] = { -#define def(type) { LLSD::type, #type + 4 } +#define def(type) { LLSD::type, &#type[4] } def(TypeUndefined), def(TypeBoolean), def(TypeInteger), diff --git a/indra/newview/llcommandhandler.cpp b/indra/newview/llcommandhandler.cpp index 76d965b1f1..23e2271eae 100644 --- a/indra/newview/llcommandhandler.cpp +++ b/indra/newview/llcommandhandler.cpp @@ -222,7 +222,7 @@ struct symbol_info #define ent(SYMBOL) \ { \ - #SYMBOL + 28, /* skip "LLCommandHandler::UNTRUSTED_" prefix */ \ + &#SYMBOL[28], /* skip "LLCommandHandler::UNTRUSTED_" prefix */ \ SYMBOL \ } diff --git a/indra/newview/llxmlrpclistener.cpp b/indra/newview/llxmlrpclistener.cpp index 7bc8af4a0b..7db2b88951 100644 --- a/indra/newview/llxmlrpclistener.cpp +++ b/indra/newview/llxmlrpclistener.cpp @@ -100,7 +100,7 @@ public: { // from curl.h // skip the "CURLE_" prefix for each of these strings -#define def(sym) (mMap[sym] = #sym + 6) +#define def(sym) (mMap[sym] = &#sym[6]) def(CURLE_OK); def(CURLE_UNSUPPORTED_PROTOCOL); /* 1 */ def(CURLE_FAILED_INIT); /* 2 */ -- cgit v1.2.3 From 9834955be072aeac3f886de2eaf1e075000108c7 Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Tue, 16 Jun 2020 14:41:28 -0400 Subject: SL-10297: remove mac-crash-logger from viewer-manifest, require the bugsplat library on Mac --- indra/cmake/bugsplat.cmake | 4 ++-- indra/newview/viewer_manifest.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/indra/cmake/bugsplat.cmake b/indra/cmake/bugsplat.cmake index 749ea05403..5f5cc51f63 100644 --- a/indra/cmake/bugsplat.cmake +++ b/indra/cmake/bugsplat.cmake @@ -13,8 +13,8 @@ if (USE_BUGSPLAT) ${ARCH_PREBUILT_DIRS_RELEASE}/bugsplat.lib ) elseif (DARWIN) - find_library(BUGSPLAT_LIBRARIES BugsplatMac - PATHS "${ARCH_PREBUILT_DIRS_RELEASE}") + find_library(BUGSPLAT_LIBRARIES BugsplatMac REQUIRED + NO_DEFAULT_PATH PATHS "${ARCH_PREBUILT_DIRS_RELEASE}") else (WINDOWS) message(FATAL_ERROR "BugSplat is not supported; add -DUSE_BUGSPLAT=OFF") endif (WINDOWS) diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index ff0781991e..7cecfec1eb 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -1063,7 +1063,7 @@ class DarwinManifest(ViewerManifest): # our apps executable_path = {} embedded_apps = [ (os.path.join("llplugin", "slplugin"), "SLPlugin.app") ] - if bugsplat_db: + if not bugsplat_db: embedded_apps.append(("mac_crash_logger", "mac-crash-logger.app")) for app_bld_dir, app in embedded_apps: self.path2basename(os.path.join(os.pardir, -- cgit v1.2.3 From 6a9c89fd62ee458dff4f2ade76d44947b99e472a Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Tue, 25 Aug 2020 11:59:27 -0400 Subject: SL-10297 do not package win_crash_logger executable if using BugSplat --- indra/newview/viewer_manifest.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index 2105b419e7..c636f1d910 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -673,10 +673,11 @@ class WindowsManifest(ViewerManifest): self.path("libvlccore.dll") self.path("plugins/") - # pull in the crash logger from other projects - # tag:"crash-logger" here as a cue to the exporter - self.path(src='../win_crash_logger/%s/windows-crash-logger.exe' % self.args['configuration'], - dst="win_crash_logger.exe") + if not bugsplat_db: # don't include the win_crash_logger if we are using BugSplat + # pull in the crash logger from other projects + # tag:"crash-logger" here as a cue to the exporter + self.path(src='../win_crash_logger/%s/windows-crash-logger.exe' % self.args['configuration'], + dst="win_crash_logger.exe") if not self.is_packaging_viewer(): self.package_file = "copied_deps" -- cgit v1.2.3 From 50509a56b6714a1a075463a02e5a853d97a3c461 Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Tue, 25 Aug 2020 12:35:04 -0400 Subject: SL-10297 fix check for bugsplat --- indra/newview/viewer_manifest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/indra/newview/viewer_manifest.py b/indra/newview/viewer_manifest.py index c636f1d910..6161a8b413 100755 --- a/indra/newview/viewer_manifest.py +++ b/indra/newview/viewer_manifest.py @@ -673,7 +673,7 @@ class WindowsManifest(ViewerManifest): self.path("libvlccore.dll") self.path("plugins/") - if not bugsplat_db: # don't include the win_crash_logger if we are using BugSplat + if not self.args.get('bugsplat'): # don't include the win_crash_logger if we are using BugSplat # pull in the crash logger from other projects # tag:"crash-logger" here as a cue to the exporter self.path(src='../win_crash_logger/%s/windows-crash-logger.exe' % self.args['configuration'], -- cgit v1.2.3 From 4e4564dcf31766469ccf41e0eef82bc234fda72c Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Wed, 26 Aug 2020 09:53:38 -0400 Subject: SL-10297 fix MSVC warning controls (merge error) --- indra/test/test.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/indra/test/test.cpp b/indra/test/test.cpp index b16fcb84d6..748d042631 100644 --- a/indra/test/test.cpp +++ b/indra/test/test.cpp @@ -58,7 +58,8 @@ #include #endif -#if LL_WINDOWS +#if LL_MSVC +#pragma warning (push) #pragma warning (disable : 4702) // warning C4702: unreachable code #endif #include -- cgit v1.2.3 From 1aa140d3affa28d85a41d55f2566a19fb9d2cff5 Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Wed, 26 Aug 2020 15:33:20 -0400 Subject: SL-10297 fix MSVC unreachable code warning --- indra/test/test.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/indra/test/test.cpp b/indra/test/test.cpp index 748d042631..55dad94bc1 100644 --- a/indra/test/test.cpp +++ b/indra/test/test.cpp @@ -77,8 +77,14 @@ LLError::ErrFatalHookResult wouldHaveCrashed(const std::string& message) { +#if LL_MSVC +#pragma warning (push) +#pragma warning (disable : 4702) // warning C4702: unreachable code tut::fail("fatal error message: " + message); return LLError::ERR_DO_NOT_CRASH; +#if LL_MSVC +#pragma warning (pop) +#endif } namespace tut -- cgit v1.2.3 From 6007475c87d3edbb023cc64bb097d33e6c04dfd4 Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Wed, 26 Aug 2020 16:37:23 -0400 Subject: SL-10297 fix MSVC unreachable code warning (for real) --- indra/test/test.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/indra/test/test.cpp b/indra/test/test.cpp index 55dad94bc1..571f502861 100644 --- a/indra/test/test.cpp +++ b/indra/test/test.cpp @@ -80,6 +80,7 @@ LLError::ErrFatalHookResult wouldHaveCrashed(const std::string& message) #if LL_MSVC #pragma warning (push) #pragma warning (disable : 4702) // warning C4702: unreachable code +#endif tut::fail("fatal error message: " + message); return LLError::ERR_DO_NOT_CRASH; #if LL_MSVC -- cgit v1.2.3 From 95d8085fa42ca73773a06f2bd5622f69aef0e598 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Mon, 10 May 2021 15:21:51 -0400 Subject: SL-10297: Make LLSingletonBase::llerrs() et al. runtime variadic. Instead of accepting a fixed list of (const char* p1="", etc.), accept (std::initializer_list). Accepting a std::initializer_list in your parameter list allows coding (e.g.) func({T0, T1, T2, ... }); -- in other words, you can pass the initializer_list a brace-enclosed list of an arbitrary number of instances of T. Using std::string_view instead of const char* means we can pass *either* const char* or std::string. string_view is cheaply constructed from either, allowing uniform treatment within the function. Constructing string_view from std::string simply extracts the pointer and length from the std::string. Constructing string_view from const char* (e.g. a "string literal") requires scanning the string for its terminating nul byte -- but that would be necessary even if the scan were deferred until the function body. Since string_view stores the length, the scan still happens only once. --- indra/llcommon/llsingleton.cpp | 66 ++++++++++++++++++---------------- indra/llcommon/llsingleton.h | 81 ++++++++++++++++++++---------------------- 2 files changed, 75 insertions(+), 72 deletions(-) diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index ad933154c2..d0dcd463ff 100644 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -39,8 +39,7 @@ #include namespace { -void log(LLError::ELevel level, - const char* p1, const char* p2, const char* p3, const char* p4); +void log(LLError::ELevel level, std::initializer_list); } // anonymous namespace // Our master list of all LLSingletons is itself an LLSingleton. We used to @@ -218,8 +217,8 @@ void LLSingletonBase::pop_initializing() if (list.empty()) { - logerrs("Underflow in stack of currently-initializing LLSingletons at ", - classname(this).c_str(), "::getInstance()"); + logerrs({"Underflow in stack of currently-initializing LLSingletons at ", + classname(this), "::getInstance()"}); } // Now we know list.back() exists: capture it @@ -240,9 +239,9 @@ void LLSingletonBase::pop_initializing() // Now validate the newly-popped LLSingleton. if (back != this) { - logerrs("Push/pop mismatch in stack of currently-initializing LLSingletons: ", - classname(this).c_str(), "::getInstance() trying to pop ", - classname(back).c_str()); + logerrs({"Push/pop mismatch in stack of currently-initializing LLSingletons: ", + classname(this), "::getInstance() trying to pop ", + classname(back)}); } // log AFTER popping so logging singletons don't cry circularity @@ -331,15 +330,15 @@ void LLSingletonBase::capture_dependency() // // Example: LLNotifications singleton initializes default channels. // Channels register themselves with singleton once done. - logdebugs("LLSingleton circularity: ", out.str().c_str(), - classname(this).c_str(), ""); + logdebugs({"LLSingleton circularity: ", out.str(), + classname(this)}); } else { // Actual circularity with other singleton (or single singleton is used extensively). // Dependency can be unclear. - logwarns("LLSingleton circularity: ", out.str().c_str(), - classname(this).c_str(), ""); + logwarns({"LLSingleton circularity: ", out.str(), + classname(this)}); } } else @@ -352,8 +351,8 @@ void LLSingletonBase::capture_dependency() if (current->mDepends.insert(this).second) { // only log the FIRST time we hit this dependency! - logdebugs(classname(current).c_str(), - " depends on ", classname(this).c_str()); + logdebugs({classname(current), + " depends on ", classname(this)}); } } } @@ -401,7 +400,7 @@ LLSingletonBase::vec_t LLSingletonBase::dep_sort() void LLSingletonBase::cleanup_() { - logdebugs("calling ", classname(this).c_str(), "::cleanupSingleton()"); + logdebugs({"calling ", classname(this), "::cleanupSingleton()"}); try { cleanupSingleton(); @@ -427,23 +426,23 @@ void LLSingletonBase::deleteAll() if (! sp->mDeleteSingleton) { // This Should Not Happen... but carry on. - logwarns(name.c_str(), "::mDeleteSingleton not initialized!"); + logwarns({name, "::mDeleteSingleton not initialized!"}); } else { // properly initialized: call it. - logdebugs("calling ", name.c_str(), "::deleteSingleton()"); + logdebugs({"calling ", name, "::deleteSingleton()"}); // From this point on, DO NOT DEREFERENCE sp! sp->mDeleteSingleton(); } } catch (const std::exception& e) { - logwarns("Exception in ", name.c_str(), "::deleteSingleton(): ", e.what()); + logwarns({"Exception in ", name, "::deleteSingleton(): ", e.what()}); } catch (...) { - logwarns("Unknown exception in ", name.c_str(), "::deleteSingleton()"); + logwarns({"Unknown exception in ", name, "::deleteSingleton()"}); } } } @@ -451,40 +450,47 @@ void LLSingletonBase::deleteAll() /*---------------------------- Logging helpers -----------------------------*/ namespace { -void log(LLError::ELevel level, - const char* p1, const char* p2, const char* p3, const char* p4) +void log(LLError::ELevel level, std::initializer_list args) { - LL_VLOGS(level, "LLSingleton") << p1 << p2 << p3 << p4 << LL_ENDL; + LL_VLOGS(level, "LLSingleton"); + for (auto arg : args) + { + LL_CONT << arg; + } + LL_ENDL; } } // anonymous namespace //static -void LLSingletonBase::logwarns(const char* p1, const char* p2, const char* p3, const char* p4) +void LLSingletonBase::logwarns(std::initializer_list args) { - log(LLError::LEVEL_WARN, p1, p2, p3, p4); + log(LLError::LEVEL_WARN, args); } //static -void LLSingletonBase::loginfos(const char* p1, const char* p2, const char* p3, const char* p4) +void LLSingletonBase::loginfos(std::initializer_list args) { - log(LLError::LEVEL_INFO, p1, p2, p3, p4); + log(LLError::LEVEL_INFO, args); } //static -void LLSingletonBase::logdebugs(const char* p1, const char* p2, const char* p3, const char* p4) +void LLSingletonBase::logdebugs(std::initializer_list args) { - log(LLError::LEVEL_DEBUG, p1, p2, p3, p4); + log(LLError::LEVEL_DEBUG, args); } //static -void LLSingletonBase::logerrs(const char* p1, const char* p2, const char* p3, const char* p4) +void LLSingletonBase::logerrs(std::initializer_list args) { - log(LLError::LEVEL_ERROR, p1, p2, p3, p4); + log(LLError::LEVEL_ERROR, args); // The other important side effect of LL_ERRS() is // https://www.youtube.com/watch?v=OMG7paGJqhQ (emphasis on OMG) std::ostringstream out; - out << p1 << p2 << p3 << p4; + for (auto arg : args) + { + out << arg; + } auto crash = LLError::getFatalFunction(); if (crash) { diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 30a5b21cf8..b9570d42db 100644 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -34,6 +34,7 @@ #include "lockstatic.h" #include "llthread.h" // on_main_thread() #include "llmainthreadtask.h" +#include class LLSingletonBase: private boost::noncopyable { @@ -111,14 +112,10 @@ protected: void capture_dependency(); // delegate logging calls to llsingleton.cpp - static void logerrs(const char* p1, const char* p2="", - const char* p3="", const char* p4=""); - static void logwarns(const char* p1, const char* p2="", - const char* p3="", const char* p4=""); - static void loginfos(const char* p1, const char* p2="", - const char* p3="", const char* p4=""); - static void logdebugs(const char* p1, const char* p2="", - const char* p3="", const char* p4=""); + static void logerrs (std::initializer_list); + static void logwarns (std::initializer_list); + static void loginfos (std::initializer_list); + static void logdebugs(std::initializer_list); static std::string demangle(const char* mangled); // these classname() declarations restate template functions declared in // llerror.h because we avoid #including that here @@ -327,8 +324,8 @@ private: // init stack to its previous size BEFORE logging so log-machinery // LLSingletons don't record a dependency on DERIVED_TYPE! LLSingleton_manage_master().reset_initializing(prev_size); - logwarns("Error constructing ", classname().c_str(), - ": ", err.what()); + logwarns({"Error constructing ", classname(), + ": ", err.what()}); // There isn't a separate EInitState value meaning "we attempted // to construct this LLSingleton subclass but could not," so use // DELETED. That seems slightly more appropriate than UNINITIALIZED. @@ -356,8 +353,8 @@ private: // BEFORE logging, so log-machinery LLSingletons don't record a // dependency on DERIVED_TYPE! pop_initializing(lk->mInstance); - logwarns("Error in ", classname().c_str(), - "::initSingleton(): ", err.what()); + logwarns({"Error in ", classname(), + "::initSingleton(): ", err.what()}); // Get rid of the instance entirely. This call depends on our // recursive_mutex. We could have a deleteSingleton(LockStatic&) // overload and pass lk, but we don't strictly need it. @@ -506,9 +503,9 @@ public: case CONSTRUCTING: // here if DERIVED_TYPE's constructor (directly or indirectly) // calls DERIVED_TYPE::getInstance() - logerrs("Tried to access singleton ", - classname().c_str(), - " from singleton constructor!"); + logerrs({"Tried to access singleton ", + classname(), + " from singleton constructor!"}); return nullptr; case INITIALIZING: @@ -523,9 +520,9 @@ public: case DELETED: // called after deleteSingleton() - logwarns("Trying to access deleted singleton ", - classname().c_str(), - " -- creating new instance"); + logwarns({"Trying to access deleted singleton ", + classname(), + " -- creating new instance"}); // fall through case UNINITIALIZED: case QUEUED: @@ -552,8 +549,8 @@ public: } // unlock 'lk' // Per the comment block above, dispatch to the main thread. - loginfos(classname().c_str(), - "::getInstance() dispatching to main thread"); + loginfos({classname(), + "::getInstance() dispatching to main thread"}); auto instance = LLMainThreadTask::dispatch( [](){ // VERY IMPORTANT to call getInstance() on the main thread, @@ -563,16 +560,16 @@ public: // the main thread processes them, only the FIRST such request // actually constructs the instance -- every subsequent one // simply returns the existing instance. - loginfos(classname().c_str(), - "::getInstance() on main thread"); + loginfos({classname(), + "::getInstance() on main thread"}); return getInstance(); }); // record the dependency chain tracked on THIS thread, not the main // thread (consider a getInstance() overload with a tag param that // suppresses dep tracking when dispatched to the main thread) capture_dependency(instance); - loginfos(classname().c_str(), - "::getInstance() returning on requesting thread"); + loginfos({classname(), + "::getInstance() returning on requesting thread"}); return instance; } @@ -641,16 +638,16 @@ private: // For organizational purposes this function shouldn't be called twice if (lk->mInitState != super::UNINITIALIZED) { - super::logerrs("Tried to initialize singleton ", - super::template classname().c_str(), - " twice!"); + super::logerrs({"Tried to initialize singleton ", + super::template classname(), + " twice!"}); return nullptr; } else if (on_main_thread()) { // on the main thread, simply construct instance while holding lock - super::logdebugs(super::template classname().c_str(), - "::initParamSingleton()"); + super::logdebugs({super::template classname(), + "::initParamSingleton()"}); super::constructSingleton(lk, std::forward(args)...); return lk->mInstance; } @@ -662,8 +659,8 @@ private: lk->mInitState = super::QUEUED; // very important to unlock here so main thread can actually process lk.unlock(); - super::loginfos(super::template classname().c_str(), - "::initParamSingleton() dispatching to main thread"); + super::loginfos({super::template classname(), + "::initParamSingleton() dispatching to main thread"}); // Normally it would be the height of folly to reference-bind // 'args' into a lambda to be executed on some other thread! By // the time that thread executed the lambda, the references would @@ -674,12 +671,12 @@ private: // references. auto instance = LLMainThreadTask::dispatch( [&](){ - super::loginfos(super::template classname().c_str(), - "::initParamSingleton() on main thread"); + super::loginfos({super::template classname(), + "::initParamSingleton() on main thread"}); return initParamSingleton_(std::forward(args)...); }); - super::loginfos(super::template classname().c_str(), - "::initParamSingleton() returning on requesting thread"); + super::loginfos({super::template classname(), + "::initParamSingleton() returning on requesting thread"}); return instance; } } @@ -707,14 +704,14 @@ public: { case super::UNINITIALIZED: case super::QUEUED: - super::logerrs("Uninitialized param singleton ", - super::template classname().c_str()); + super::logerrs({"Uninitialized param singleton ", + super::template classname()}); break; case super::CONSTRUCTING: - super::logerrs("Tried to access param singleton ", - super::template classname().c_str(), - " from singleton constructor!"); + super::logerrs({"Tried to access param singleton ", + super::template classname(), + " from singleton constructor!"}); break; case super::INITIALIZING: @@ -726,8 +723,8 @@ public: return lk->mInstance; case super::DELETED: - super::logerrs("Trying to access deleted param singleton ", - super::template classname().c_str()); + super::logerrs({"Trying to access deleted param singleton ", + super::template classname()}); break; } -- cgit v1.2.3 From c9fc4349b7d4ab1f5a7bfc0125014a96a07e51a3 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 11 May 2021 21:42:14 -0400 Subject: SL-10297: Move LL_ERRS crash location into the LL_ERRS macro itself. Introduce Oz's LLERROR_CRASH macro analogous to the old LLError::crashAndLoop() function. Change LL_ENDL macro so that, after calling flush(), if the CallSite is for LEVEL_ERROR, we invoke LLERROR_CRASH right there. Change the meaning of LLError::FatalFunction. It used to be responsible for the actual crash (hence crashAndLoop()). Now, instead, its role is to disrupt control flow in some other way if you DON'T want to crash: throw an exception, or call exit() or some such. Any FatalFunction that returns normally will fall into the new crash in LL_ENDL. Accordingly, the new default FatalFunction is a no-op lambda. This eliminates the need to test for empty (not set) FatalFunction in Log::flush(). Remove LLError::crashAndLoop() because the official LL_ERRS crash is now in LL_ENDL. One of the two common use cases for setFatalFunction() used to be to intercept control in the last moments before crashing -- not to crash or to avoid crashing, but to capture the LL_ERRS message in some way. Especially when that's temporary, though (e.g. LLLeap), saving and restoring the previous FatalFunction only works when the lifespans of the relevant objects are strictly LIFO. Either way, that's a misuse of FatalFunction. Fortunately the Recorder mechanism exactly addresses that case. Introduce a GenericRecorder template subclass, with LLError::addGenericRecorder(callable) that accepts a callable with suitable (level, message) signature, instantiates a GenericRecorder, adds it to the logging machinery and returns the RecorderPtr for possible later use with removeRecorder(). Change llappviewer.cpp's errorCallback() to an addGenericRecorder() callable. Its role was simply to update gDebugInfo["FatalMessage"] with the LL_ERRS message, then call writeDebugInfo(), before calling crashAndLoop() to finish crashing. Remove the crashAndLoop() call, retaining the gDebugInfo logic. Pass errorCallback() to LLError::addGenericRecorder() instead of setFatalFunction(). Oddly, errorCallback()'s crashAndLoop() call was conditional on a compile-time SHADER_CRASH_NONFATAL symbol. The new mechanism provides no way to support SHADER_CRASH_NONFATAL -- it is a Bad Idea to return normally from any LL_ERRS invocation! Rename LLLeapImpl::fatalFunction() to onError(). Instead of passing it to LLError::setFatalFunction(), pass it to addGenericRecorder(). Capture the returned RecorderPtr in mRecorder, replacing mPrevFatalFunction. Then ~LLLeapImpl() calls removeRecorder(mRecorder) instead of restoring mPrevFatalFunction (which, as noted above, was order-sensitive). Of course, every enabled Recorder is called with every log message. onError() and errorCallback() must specifically test for calls with LEVEL_ERROR. LLSingletonBase::logerrs() used to call LLError::getFatalFunction(), check the return and call it if non-empty, else call LLError::crashAndLoop(). Replace all that with LLERROR_CRASH. Remove from llappviewer.cpp the watchdog_llerrs_callback() and watchdog_killer_callback() functions. watchdog_killer_callback(), passed to Watchdog::init(), used to setFatalFunction(watchdog_llerrs_callback) and then invoke LL_ERRS() -- which seems a bit roundabout. watchdog_llerrs_callback(), in turn, replicated much of the logic in the primary errorCallback() function before replicating the crash from llwatchdog.cpp's default_killer_callback(). Instead, pass LLWatchdog::init() a lambda that invokes the LL_ERRS() message formerly found in watchdog_killer_callback(). It no longer needs to override FatalFunction with watchdog_llerrs_callback() because errorCallback() will still be called as a Recorder, obviating watchdog_llerrs_callback()'s first half; and LL_ENDL will handle the crash, obviating the second half. Remove from llappviewer.cpp the static fast_exit() function, which was simply an alias for _exit() acceptable to boost::bind(). Use a lambda directly calling _exit() instead of using boost::bind() at all. In the CaptureLog class in llcommon/tests/wrapllerrs.h, instead of statically referencing the wouldHaveCrashed() function from test.cpp, simply save and restore the current FatalFunction across the LLError::saveAndResetSettings() call. llerror_test.cpp calls setFatalFunction(fatalCall), where fatalCall() was a function that simply set a fatalWasCalled bool rather than actually crashing in any way. Of course, that implementation would now lead to crashing the test program. Make fatalCall() throw a new FatalWasCalled exception. Introduce a CATCH(LL_ERRS("tag"), "message") macro that expands to: LL_ERRS("tag") << "message" << LL_ENDL; within a try/catch block that catches FatalWasCalled and sets the same bool. Change all existing LL_ERRS() in llerror_test.cpp to corresponding CATCH() calls. In fact there's also an LL_DEBUGS(bad tag) invocation that exercises an LL_ERRS internal to llerror.cpp; wrap that too. --- indra/llcommon/llerror.cpp | 37 +++---------------- indra/llcommon/llerror.h | 22 +++++++++--- indra/llcommon/llerrorcontrol.h | 57 +++++++++++++++++++++++------ indra/llcommon/llleap.cpp | 48 ++++++++++++------------- indra/llcommon/llsingleton.cpp | 15 +------- indra/llcommon/tests/llerror_test.cpp | 58 +++++++++++++++++++----------- indra/llcommon/tests/wrapllerrs.h | 22 ++++++------ indra/newview/llappviewer.cpp | 67 +++++++++-------------------------- 8 files changed, 159 insertions(+), 167 deletions(-) diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index f876b8ee4a..9d775dcef3 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -549,7 +549,7 @@ namespace LLError mFileLevelMap(), mTagLevelMap(), mUniqueLogMessages(), - mCrashFunction(NULL), + mCrashFunction([](const std::string&){}), mTimeFunction(NULL), mRecorders(), mShouldLogCallCounter(0) @@ -728,7 +728,6 @@ namespace LLError::setDefaultLevel(LLError::LEVEL_INFO); LLError::setAlwaysFlush(true); LLError::setEnabledLogTypesMask(0xFFFFFFFF); - LLError::setFatalFunction(LLError::crashAndLoop); LLError::setTimeFunction(LLError::utcTime); // log_to_stderr is only false in the unit and integration tests to keep builds quieter @@ -1436,7 +1435,7 @@ namespace LLError if (site.mPrintOnce) { - std::ostringstream message_stream; + std::ostringstream message_stream; std::map::iterator messageIter = s->mUniqueLogMessages.find(message); if (messageIter != s->mUniqueLogMessages.end()) @@ -1457,8 +1456,8 @@ namespace LLError message_stream << "ONCE: "; s->mUniqueLogMessages[message] = 1; } - message_stream << message; - message = message_stream.str(); + message_stream << message; + message = message_stream.str(); } writeToRecorders(site, message); @@ -1466,10 +1465,7 @@ namespace LLError if (site.mLevel == LEVEL_ERROR) { g->mFatalMessage = message; - if (s->mCrashFunction) - { - s->mCrashFunction(message); - } + s->mCrashFunction(message); } } } @@ -1533,29 +1529,6 @@ namespace LLError return s->mShouldLogCallCounter; } -#if LL_WINDOWS - // VC80 was optimizing the error away. - #pragma optimize("", off) -#endif - void crashAndLoop(const std::string& message) - { - // Now, we go kaboom! - int* make_me_crash = NULL; - - *make_me_crash = 0; - - while(true) - { - // Loop forever, in case the crash didn't work? - } - - // this is an attempt to let Coverity and other semantic scanners know that this function won't be returning ever. - exit(EXIT_FAILURE); - } -#if LL_WINDOWS - #pragma optimize("", on) -#endif - std::string utcTime() { time_t now = time(NULL); diff --git a/indra/llcommon/llerror.h b/indra/llcommon/llerror.h index ffaa464d77..f8c0d03aea 100644 --- a/indra/llcommon/llerror.h +++ b/indra/llcommon/llerror.h @@ -382,11 +382,23 @@ typedef LLError::NoClassInfo _LL_CLASS_TO_LOG; #define LL_NEWLINE '\n' -#define LL_ENDL \ - LLError::End(); \ - LLError::Log::flush(_out, _site); \ - } \ - } while(0) +// Use this only in LL_ERRS or in a place that LL_ERRS may not be used +#define LLERROR_CRASH \ +{ \ + int* make_me_crash = NULL;\ + *make_me_crash = 0; \ + exit(*make_me_crash); \ +} + +#define LL_ENDL \ + LLError::End(); \ + LLError::Log::flush(_out, _site); \ + if (_site.mLevel == LLError::LEVEL_ERROR) \ + { \ + LLERROR_CRASH \ + } \ + } \ + } while(0) // NEW Macros for debugging, allow the passing of a string tag diff --git a/indra/llcommon/llerrorcontrol.h b/indra/llcommon/llerrorcontrol.h index 25786d5457..e87bb7bf35 100644 --- a/indra/llcommon/llerrorcontrol.h +++ b/indra/llcommon/llerrorcontrol.h @@ -94,14 +94,16 @@ namespace LLError */ typedef boost::function FatalFunction; - LL_COMMON_API void crashAndLoop(const std::string& message); - // Default fatal function: access null pointer and loops forever LL_COMMON_API void setFatalFunction(const FatalFunction&); - // The fatal function will be called when an message of LEVEL_ERROR + // The fatal function will be called after 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. + // that message from causing the fatal function to be invoked. + // The passed FatalFunction will be the LAST log function called + // before LL_ERRS crashes its caller. A FatalFunction can throw an + // exception, or call exit(), to bypass the crash. It MUST disrupt the + // flow of control because no caller expects LL_ERRS to return. LL_COMMON_API FatalFunction getFatalFunction(); // Retrieve the previously-set FatalFunction @@ -147,14 +149,14 @@ namespace LLError virtual void recordMessage(LLError::ELevel, const std::string& message) = 0; // use the level for better display, not for filtering - virtual bool enabled() { return true; } + virtual bool enabled() { return true; } bool wantsTime(); bool wantsTags(); bool wantsLevel(); bool wantsLocation(); bool wantsFunctionName(); - bool wantsMultiline(); + bool wantsMultiline(); void showTime(bool show); void showTags(bool show); @@ -165,15 +167,35 @@ namespace LLError protected: bool mWantsTime; - bool mWantsTags; - bool mWantsLevel; - bool mWantsLocation; - bool mWantsFunctionName; - bool mWantsMultiline; + bool mWantsTags; + bool mWantsLevel; + bool mWantsLocation; + bool mWantsFunctionName; + bool mWantsMultiline; }; typedef boost::shared_ptr RecorderPtr; + /** + * Instantiate GenericRecorder with a callable(level, message) to get + * control on every log message without having to code an explicit + * Recorder subclass. + */ + template + class GenericRecorder: public Recorder + { + public: + GenericRecorder(const CALLABLE& callable): + mCallable(callable) + {} + void recordMessage(LLError::ELevel level, const std::string& message) override + { + mCallable(level, message); + } + private: + CALLABLE mCallable; + }; + /** * @NOTE: addRecorder() and removeRecorder() uses the boost::shared_ptr to allow for shared ownership * while still ensuring that the allocated memory is eventually freed @@ -181,6 +203,19 @@ namespace LLError LL_COMMON_API void addRecorder(RecorderPtr); LL_COMMON_API void removeRecorder(RecorderPtr); // each error message is passed to each recorder via recordMessage() + /** + * Call addGenericRecorder() with a callable(level, message) to get + * control on every log message without having to code an explicit + * Recorder subclass. Save the returned RecorderPtr if you later want to + * call removeRecorder(). + */ + template + RecorderPtr addGenericRecorder(const CALLABLE& callable) + { + RecorderPtr ptr{ new GenericRecorder(callable) }; + addRecorder(ptr); + return ptr; + } LL_COMMON_API void logToFile(const std::string& filename); LL_COMMON_API void logToStderr(); diff --git a/indra/llcommon/llleap.cpp b/indra/llcommon/llleap.cpp index cf8f8cc6a5..e8ea0ab398 100644 --- a/indra/llcommon/llleap.cpp +++ b/indra/llcommon/llleap.cpp @@ -59,7 +59,6 @@ public: // pump name -- so it should NOT need tweaking for uniqueness. mReplyPump(LLUUID::generateNewID().asString()), mExpect(0), - mPrevFatalFunction(LLError::getFatalFunction()), // Instantiate a distinct LLLeapListener for this plugin. (Every // plugin will want its own collection of managed listeners, etc.) // Pass it a callback to our connect() method, so it can send events @@ -146,7 +145,9 @@ public: .listen("LLLeap", boost::bind(&LLLeapImpl::rstderr, this, _1)); // For our lifespan, intercept any LL_ERRS so we can notify plugin - LLError::setFatalFunction(boost::bind(&LLLeapImpl::fatalFunction, this, _1)); + mRecorder = LLError::addGenericRecorder( + [this](LLError::ELevel level, const std::string& message) + { onError(level, message); }); // Send child a preliminary event reporting our own reply-pump name -- // which would otherwise be pretty tricky to guess! @@ -162,8 +163,7 @@ public: virtual ~LLLeapImpl() { LL_DEBUGS("LLLeap") << "destroying LLLeap(\"" << mDesc << "\")" << LL_ENDL; - // Restore original FatalFunction - LLError::setFatalFunction(mPrevFatalFunction); + LLError::removeRecorder(mRecorder); } // Listener for failed launch attempt @@ -377,28 +377,28 @@ public: return false; } - void fatalFunction(const std::string& error) + void onError(LLError::ELevel level, const std::string& error) { - // Notify plugin - LLSD event; - event["type"] = "error"; - event["error"] = error; - mReplyPump.post(event); - - // All the above really accomplished was to buffer the serialized - // event in our WritePipe. Have to pump mainloop a couple times to - // really write it out there... but time out in case we can't write. - LLProcess::WritePipe& childin(mChild->getWritePipe(LLProcess::STDIN)); - LLEventPump& mainloop(LLEventPumps::instance().obtain("mainloop")); - LLSD nop; - F64 until = (LLTimer::getElapsedSeconds() + 2).value(); - while (childin.size() && LLTimer::getElapsedSeconds() < until) + if (level == LLError::LEVEL_ERROR) { - mainloop.post(nop); + // Notify plugin + LLSD event; + event["type"] = "error"; + event["error"] = error; + mReplyPump.post(event); + + // All the above really accomplished was to buffer the serialized + // event in our WritePipe. Have to pump mainloop a couple times to + // really write it out there... but time out in case we can't write. + LLProcess::WritePipe& childin(mChild->getWritePipe(LLProcess::STDIN)); + LLEventPump& mainloop(LLEventPumps::instance().obtain("mainloop")); + LLSD nop; + F64 until = (LLTimer::getElapsedSeconds() + 2).value(); + while (childin.size() && LLTimer::getElapsedSeconds() < until) + { + mainloop.post(nop); + } } - - // forward the call to the previous FatalFunction - mPrevFatalFunction(error); } private: @@ -421,7 +421,7 @@ private: mStdinConnection, mStdoutConnection, mStdoutDataConnection, mStderrConnection; boost::scoped_ptr mBlocker; LLProcess::ReadPipe::size_type mExpect; - LLError::FatalFunction mPrevFatalFunction; + LLError::RecorderPtr mRecorder; boost::scoped_ptr mListener; }; diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index d0dcd463ff..4b1666563e 100644 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -486,20 +486,7 @@ void LLSingletonBase::logerrs(std::initializer_list args) log(LLError::LEVEL_ERROR, args); // The other important side effect of LL_ERRS() is // https://www.youtube.com/watch?v=OMG7paGJqhQ (emphasis on OMG) - std::ostringstream out; - for (auto arg : args) - { - out << arg; - } - auto crash = LLError::getFatalFunction(); - if (crash) - { - crash(out.str()); - } - else - { - LLError::crashAndLoop(out.str()); - } + LLERROR_CRASH; } std::string LLSingletonBase::demangle(const char* mangled) diff --git a/indra/llcommon/tests/llerror_test.cpp b/indra/llcommon/tests/llerror_test.cpp index 8e1f4c14ac..148c18aabe 100644 --- a/indra/llcommon/tests/llerror_test.cpp +++ b/indra/llcommon/tests/llerror_test.cpp @@ -26,6 +26,7 @@ */ #include +#include #include "linden_common.h" @@ -69,21 +70,41 @@ namespace namespace { - static bool fatalWasCalled; - void fatalCall(const std::string&) { fatalWasCalled = true; } + static bool fatalWasCalled = false; + struct FatalWasCalled: public std::runtime_error + { + FatalWasCalled(const std::string& what): std::runtime_error(what) {} + }; + void fatalCall(const std::string& msg) { throw FatalWasCalled(msg); } } +// Because we use LLError::setFatalFunction(fatalCall), any LL_ERRS call we +// issue will throw FatalWasCalled. But we want the test program to continue. +// So instead of writing: +// LL_ERRS("tag") << "some message" << LL_ENDL; +// write: +// CATCH(LL_ERRS("tag"), "some message"); +#define CATCH(logcall, expr) \ + try \ + { \ + logcall << expr << LL_ENDL; \ + } \ + catch (const FatalWasCalled&) \ + { \ + fatalWasCalled = true; \ + } + namespace tut { class TestRecorder : public LLError::Recorder { public: TestRecorder() - { - showTime(false); - } + { + showTime(false); + } virtual ~TestRecorder() - {} + {} virtual void recordMessage(LLError::ELevel level, const std::string& message) @@ -252,7 +273,7 @@ namespace LL_DEBUGS("WriteTag","AnotherTag") << "one" << LL_ENDL; LL_INFOS("WriteTag") << "two" << LL_ENDL; LL_WARNS("WriteTag") << "three" << LL_ENDL; - LL_ERRS("WriteTag") << "four" << LL_ENDL; + CATCH(LL_ERRS("WriteTag"), "four"); } }; @@ -380,7 +401,7 @@ namespace std::string errorReturningLocation() { - LL_ERRS() << "die" << LL_ENDL; int this_line = __LINE__; + int this_line = __LINE__; CATCH(LL_ERRS(), "die"); return locationString(this_line); } } @@ -701,7 +722,7 @@ public: static void doDebug() { LL_DEBUGS() << "add dice" << LL_ENDL; } static void doInfo() { LL_INFOS() << "any idea" << LL_ENDL; } static void doWarn() { LL_WARNS() << "aim west" << LL_ENDL; } - static void doError() { LL_ERRS() << "ate eels" << LL_ENDL; } + static void doError() { CATCH(LL_ERRS(), "ate eels"); } static void doAll() { doDebug(); doInfo(); doWarn(); doError(); } }; @@ -712,7 +733,7 @@ public: static void doDebug() { LL_DEBUGS() << "bed down" << LL_ENDL; } static void doInfo() { LL_INFOS() << "buy iron" << LL_ENDL; } static void doWarn() { LL_WARNS() << "bad word" << LL_ENDL; } - static void doError() { LL_ERRS() << "big easy" << LL_ENDL; } + static void doError() { CATCH(LL_ERRS(), "big easy"); } static void doAll() { doDebug(); doInfo(); doWarn(); doError(); } }; @@ -874,13 +895,10 @@ namespace tut namespace { std::string writeTagWithSpaceReturningLocation() - { - LL_DEBUGS("Write Tag") << "not allowed" << LL_ENDL; int this_line = __LINE__; - - std::ostringstream location; - location << LLError::abbreviateFile(__FILE__).c_str() << "(" << this_line << ")"; - return location.str(); - } + { + int this_line = __LINE__; CATCH(LL_DEBUGS("Write Tag"), "not allowed"); + return locationString(this_line); + } }; namespace tut @@ -894,9 +912,9 @@ namespace tut std::string location = writeTagWithSpaceReturningLocation(); std::string expected = "Space is not allowed in a log tag at " + location; - ensure_message_field_equals(0, LEVEL_FIELD, "ERROR"); - ensure_message_field_equals(0, MSG_FIELD, expected); - ensure("fatal callback called", fatalWasCalled); + ensure_message_field_equals(0, LEVEL_FIELD, "ERROR"); + ensure_message_field_equals(0, MSG_FIELD, expected); + ensure("fatal callback called", fatalWasCalled); } } diff --git a/indra/llcommon/tests/wrapllerrs.h b/indra/llcommon/tests/wrapllerrs.h index b07d5afbd8..3779fb41bc 100644 --- a/indra/llcommon/tests/wrapllerrs.h +++ b/indra/llcommon/tests/wrapllerrs.h @@ -44,10 +44,6 @@ #include #include -// statically reference the function in test.cpp... it's short, we could -// replicate, but better to reuse -extern void wouldHaveCrashed(const std::string& message); - struct WrapLLErrs { WrapLLErrs(): @@ -59,7 +55,8 @@ struct WrapLLErrs mPriorFatal(LLError::getFatalFunction()) { // Make LL_ERRS call our own operator() method - LLError::setFatalFunction(boost::bind(&WrapLLErrs::operator(), this, _1)); + LLError::setFatalFunction( + [this](const std::string& message){ (*this)(message); }); } ~WrapLLErrs() @@ -199,11 +196,13 @@ public: // with that output. If it turns out that saveAndResetSettings() has // some bad effect, give up and just let the DEBUG level log messages // display. - : boost::noncopyable(), + : boost::noncopyable(), + mFatalFunction(LLError::getFatalFunction()), mOldSettings(LLError::saveAndResetSettings()), - mRecorder(new CaptureLogRecorder()) + mRecorder(new CaptureLogRecorder()) { - LLError::setFatalFunction(wouldHaveCrashed); + // reinstate the FatalFunction we just reset + LLError::setFatalFunction(mFatalFunction); LLError::setDefaultLevel(level); LLError::addRecorder(mRecorder); } @@ -219,17 +218,18 @@ public: /// for the sought string. std::string messageWith(const std::string& search, bool required=true) { - return boost::dynamic_pointer_cast(mRecorder)->messageWith(search, required); + return boost::dynamic_pointer_cast(mRecorder)->messageWith(search, required); } std::ostream& streamto(std::ostream& out) const { - return boost::dynamic_pointer_cast(mRecorder)->streamto(out); + return boost::dynamic_pointer_cast(mRecorder)->streamto(out); } private: + LLError::FatalFunction mFatalFunction; LLError::SettingsStoragePtr mOldSettings; - LLError::RecorderPtr mRecorder; + LLError::RecorderPtr mRecorder; }; #endif /* ! defined(LL_WRAPLLERRS_H) */ diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index 0b2cdff36c..b03e821d32 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -760,17 +760,6 @@ public: } }; -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); -} - - -} - bool LLAppViewer::init() { @@ -822,9 +811,9 @@ bool LLAppViewer::init() if (rc >= 0) { // QAModeTermCode set, terminate with that rc on LL_ERRS. Use - // fast_exit() rather than exit() because normal cleanup depends too + // _exit() rather than exit() because normal cleanup depends too // much on successful startup! - LLError::setFatalFunction(boost::bind(fast_exit, rc)); + LLError::setFatalFunction([rc](const std::string&){ _exit(rc); }); } mAlloc.setProfilingEnabled(gSavedSettings.getBOOL("MemProfiling")); @@ -2185,28 +2174,6 @@ bool LLAppViewer::cleanup() return true; } -// A callback for LL_ERRS() to call during the watchdog error. -void watchdog_llerrs_callback(const std::string &error_string) -{ - gLLErrorActivated = true; - - gDebugInfo["FatalMessage"] = error_string; - LLAppViewer::instance()->writeDebugInfo(); - -#ifdef LL_WINDOWS - RaiseException(0,0,0,0); -#else - raise(SIGQUIT); -#endif -} - -// A callback for the watchdog to call. -void watchdog_killer_callback() -{ - LLError::setFatalFunction(watchdog_llerrs_callback); - LL_ERRS() << "Watchdog killer event" << LL_ENDL; -} - bool LLAppViewer::initThreads() { static const bool enable_threads = true; @@ -2241,24 +2208,23 @@ bool LLAppViewer::initThreads() return true; } -void errorCallback(const std::string &error_string) +void errorCallback(LLError::ELevel level, const std::string &error_string) { + if (level == LLError::LEVEL_ERROR) + { #ifndef LL_RELEASE_FOR_DOWNLOAD - OSMessageBox(error_string, LLTrans::getString("MBFatalError"), OSMB_OK); + OSMessageBox(error_string, LLTrans::getString("MBFatalError"), OSMB_OK); #endif - //Set the ErrorActivated global so we know to create a marker file - gLLErrorActivated = true; - - gDebugInfo["FatalMessage"] = error_string; - // We're not already crashing -- we simply *intend* to crash. Since we - // haven't actually trashed anything yet, we can afford to write the whole - // static info file. - LLAppViewer::instance()->writeDebugInfo(); + //Set the ErrorActivated global so we know to create a marker file + gLLErrorActivated = true; -#ifndef SHADER_CRASH_NONFATAL - LLError::crashAndLoop(error_string); -#endif + gDebugInfo["FatalMessage"] = error_string; + // We're not already crashing -- we simply *intend* to crash. Since we + // haven't actually trashed anything yet, we can afford to write the whole + // static info file. + LLAppViewer::instance()->writeDebugInfo(); + } } void LLAppViewer::initLoggingAndGetLastDuration() @@ -2269,7 +2235,7 @@ void LLAppViewer::initLoggingAndGetLastDuration() LLError::initForApplication( gDirUtilp->getExpandedFilename(LL_PATH_USER_SETTINGS, "") ,gDirUtilp->getExpandedFilename(LL_PATH_APP_SETTINGS, "") ); - LLError::setFatalFunction(errorCallback); + LLError::addGenericRecorder(&errorCallback); //LLError::setTimeFunction(getRuntime); // Remove the last ".old" log file. @@ -3030,7 +2996,8 @@ bool LLAppViewer::initWindow() if (use_watchdog) { - LLWatchdog::getInstance()->init(watchdog_killer_callback); + LLWatchdog::getInstance()->init( + [](){ LL_ERRS() << "Watchdog killer event" << LL_ENDL; }); } LL_INFOS("AppInit") << "watchdog setting is done." << LL_ENDL; -- cgit v1.2.3 From 147c66d67ce83778fc3f102dd132ec095e6032fc Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 12 May 2021 09:46:49 -0400 Subject: SL-10297: Simplify implementation of LLSingletonBase::logwarns() etc. Introduce 'string_params' typedef for std::initialization_list, and make logwarns(), loginfos(), logdebugs() and logerrs() accept const string_params&. Eliminate the central log() function in llsingleton.cpp that used LL_VLOGS(). To cache the result of a (moderately expensive) Log::shouldLog() call, LL_VLOGS() wants its CallSite object to be static -- but of course the shouldLog() result will differ for different ELevel values, so LL_VLOGS() instantiates a static array of CallSite instances. It seems silly to funnel distinct logwarns(), etc., functions through a common log() function only to have LL_VLOGS() tease apart ELevel values again. Instead, make logwarns() directly invoke LL_WARNS(), and similarly for the rest. To reduce boilerplate in these distinct functions, teach std::ostream how to stream a string_params instance by looping over its elements. Then each logwarns(), etc., function can simply stream its string_params argument to LL_WARNS() or whichever. In particular, eliminate the LLERROR_CRASH macro in logerrs(). The fact that it invokes LL_ERRS() ensures that its LL_ENDL macro will crash the viewer. --- indra/llcommon/llsingleton.cpp | 37 +++++++++++++++---------------------- indra/llcommon/llsingleton.h | 11 +++++++---- 2 files changed, 22 insertions(+), 26 deletions(-) diff --git a/indra/llcommon/llsingleton.cpp b/indra/llcommon/llsingleton.cpp index 4b1666563e..6b1986d0e9 100644 --- a/indra/llcommon/llsingleton.cpp +++ b/indra/llcommon/llsingleton.cpp @@ -38,10 +38,6 @@ #include #include -namespace { -void log(LLError::ELevel level, std::initializer_list); -} // anonymous namespace - // Our master list of all LLSingletons is itself an LLSingleton. We used to // store it in a function-local static, but that could get destroyed before // the last of the LLSingletons -- and ~LLSingletonBase() definitely wants to @@ -450,43 +446,40 @@ void LLSingletonBase::deleteAll() /*---------------------------- Logging helpers -----------------------------*/ namespace { -void log(LLError::ELevel level, std::initializer_list args) +std::ostream& operator<<(std::ostream& out, const LLSingletonBase::string_params& args) { - LL_VLOGS(level, "LLSingleton"); - for (auto arg : args) - { - LL_CONT << arg; - } - LL_ENDL; + // However many args there are in args, stream each of them to 'out'. + for (auto arg : args) + { + out << arg; + } + return out; } } // anonymous namespace //static -void LLSingletonBase::logwarns(std::initializer_list args) +void LLSingletonBase::logwarns(const string_params& args) { - log(LLError::LEVEL_WARN, args); + LL_WARNS("LLSingleton") << args << LL_ENDL; } //static -void LLSingletonBase::loginfos(std::initializer_list args) +void LLSingletonBase::loginfos(const string_params& args) { - log(LLError::LEVEL_INFO, args); + LL_INFOS("LLSingleton") << args << LL_ENDL; } //static -void LLSingletonBase::logdebugs(std::initializer_list args) +void LLSingletonBase::logdebugs(const string_params& args) { - log(LLError::LEVEL_DEBUG, args); + LL_DEBUGS("LLSingleton") << args << LL_ENDL; } //static -void LLSingletonBase::logerrs(std::initializer_list args) +void LLSingletonBase::logerrs(const string_params& args) { - log(LLError::LEVEL_ERROR, args); - // The other important side effect of LL_ERRS() is - // https://www.youtube.com/watch?v=OMG7paGJqhQ (emphasis on OMG) - LLERROR_CRASH; + LL_ERRS("LLSingleton") << args << LL_ENDL; } std::string LLSingletonBase::demangle(const char* mangled) diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index b9570d42db..2eb39c6c8c 100644 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -112,10 +112,13 @@ protected: void capture_dependency(); // delegate logging calls to llsingleton.cpp - static void logerrs (std::initializer_list); - static void logwarns (std::initializer_list); - static void loginfos (std::initializer_list); - static void logdebugs(std::initializer_list); +public: + typedef std::initializer_list string_params; +protected: + static void logerrs (const string_params&); + static void logwarns (const string_params&); + static void loginfos (const string_params&); + static void logdebugs(const string_params&); static std::string demangle(const char* mangled); // these classname() declarations restate template functions declared in // llerror.h because we avoid #including that here -- cgit v1.2.3 From 5b96ee0e10923a00ddb3836d4dc3c5f912ca4330 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 12 May 2021 12:02:57 -0400 Subject: SL-10297: Eliminate llerror.cpp's Globals::messageStream and bool. Instead of a single std::ostringstream instance shared by all callers, even those on different threads, make each of the relevant lllog_test_() and llcallstacks macros instantiate independent (stack) std::ostringstream objects. lllog_test_() is called by LL_DEBUGS(), LLINFOS(), LL_WARNS(), LL_ERRS(), LL_VLOGS() et al. Eliminate LLError::Log::out(), whose sole function was to arbitrate use of that shared std::ostringstream. Amusingly, if the lock couldn't be locked or if messageStreamInUse was set, out() would allocate a new (heap!) std::ostringstream anyway, which would then have to be freed by flush(). Make both LLError::Log::flush() overloads accept const std::ostringstream&. Make LL_ENDL pass the local _out instance. This eliminates the need to check whether the passed std::ostringstream* references the shared instance and (if so) reset it or (if not) delete it. Make LLError::LLCallStacks::insert() accept the local _out instance as non- const std::ostream&, rather than acquiring and returning std::ostringstream*. Make end() accept the local instance as const std::ostringstream&. --- indra/llcommon/llerror.cpp | 73 +++++++--------------------------------------- indra/llcommon/llerror.h | 30 +++++++++---------- 2 files changed, 26 insertions(+), 77 deletions(-) diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index 9d775dcef3..f7594ed815 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -442,8 +442,6 @@ namespace protected: Globals(); public: - std::ostringstream messageStream; - bool messageStreamInUse; std::string mFatalMessage; void addCallSite(LLError::CallSite&); @@ -453,12 +451,7 @@ namespace CallSiteVector callSites; }; - Globals::Globals() - : messageStream(), - messageStreamInUse(false), - callSites() - { - } + Globals::Globals() {} Globals* Globals::getInstance() { @@ -1359,25 +1352,7 @@ namespace LLError } - std::ostringstream* Log::out() - { - LLMutexTrylock lock(getMutex(),5); - - if (lock.isLocked()) - { - Globals* g = Globals::getInstance(); - - if (!g->messageStreamInUse) - { - g->messageStreamInUse = true; - return &g->messageStream; - } - } - - return new std::ostringstream; - } - - void Log::flush(std::ostringstream* out, char* message) + void Log::flush(const std::ostringstream& out, char* message) { LLMutexTrylock lock(getMutex(),5); if (!lock.isLocked()) @@ -1385,31 +1360,18 @@ namespace LLError return; } - if(strlen(out->str().c_str()) < 128) + if(strlen(out.str().c_str()) < 128) { - strcpy(message, out->str().c_str()); + strcpy(message, out.str().c_str()); } else { - strncpy(message, out->str().c_str(), 127); + strncpy(message, out.str().c_str(), 127); message[127] = '\0' ; } - - Globals* g = Globals::getInstance(); - if (out == &g->messageStream) - { - g->messageStream.clear(); - g->messageStream.str(""); - g->messageStreamInUse = false; - } - else - { - delete out; - } - return ; } - void Log::flush(std::ostringstream* out, const CallSite& site) + void Log::flush(const std::ostringstream& out, const CallSite& site) { LLMutexTrylock lock(getMutex(),5); if (!lock.isLocked()) @@ -1420,18 +1382,7 @@ namespace LLError Globals* g = Globals::getInstance(); SettingsConfigPtr s = Settings::getInstance()->getSettingsConfig(); - std::string message = out->str(); - if (out == &g->messageStream) - { - g->messageStream.clear(); - g->messageStream.str(""); - g->messageStreamInUse = false; - } - else - { - delete out; - } - + std::string message = out.str(); if (site.mPrintOnce) { @@ -1600,15 +1551,13 @@ namespace LLError } //static - std::ostringstream* LLCallStacks::insert(const char* function, const int line) + void LLCallStacks::insert(std::ostream& out, const char* function, const int line) { - std::ostringstream* _out = LLError::Log::out(); - *_out << function << " line " << line << " " ; - return _out ; + out << function << " line " << line << " " ; } //static - void LLCallStacks::end(std::ostringstream* _out) + void LLCallStacks::end(const std::ostringstream& out) { LLMutexTrylock lock(getMutex(), 5); if (!lock.isLocked()) @@ -1626,7 +1575,7 @@ namespace LLError clear() ; } - LLError::Log::flush(_out, sBuffer[sIndex++]) ; + LLError::Log::flush(out, sBuffer[sIndex++]) ; } //static diff --git a/indra/llcommon/llerror.h b/indra/llcommon/llerror.h index f8c0d03aea..51423350e6 100644 --- a/indra/llcommon/llerror.h +++ b/indra/llcommon/llerror.h @@ -198,9 +198,8 @@ namespace LLError { public: static bool shouldLog(CallSite&); - static std::ostringstream* out(); - static void flush(std::ostringstream* out, char* message); - static void flush(std::ostringstream*, const CallSite&); + static void flush(const std::ostringstream& out, char* message); + static void flush(const std::ostringstream&, const CallSite&); static std::string demangle(const char* mangled); /// classname() template @@ -289,10 +288,10 @@ namespace LLError public: static void push(const char* function, const int line) ; - static std::ostringstream* insert(const char* function, const int line) ; + static void insert(std::ostream& out, const char* function, const int line) ; static void print() ; static void clear() ; - static void end(std::ostringstream* _out) ; + static void end(const std::ostringstream& out) ; static void cleanup(); }; @@ -306,10 +305,11 @@ namespace LLError //this is cheaper than llcallstacks if no need to output other variables to call stacks. #define LL_PUSH_CALLSTACKS() LLError::LLCallStacks::push(__FUNCTION__, __LINE__) -#define llcallstacks \ - { \ - std::ostringstream* _out = LLError::LLCallStacks::insert(__FUNCTION__, __LINE__) ; \ - (*_out) +#define llcallstacks \ + { \ + std::ostringstream _out; \ + LLError::LLCallStacks::insert(_out, __FUNCTION__, __LINE__) ; \ + _out #define llcallstacksendl \ LLError::End(); \ @@ -355,11 +355,11 @@ typedef LLError::NoClassInfo _LL_CLASS_TO_LOG; static LLError::CallSite _site(lllog_site_args_(level, once, tags)); \ lllog_test_() -#define lllog_test_() \ - if (LL_UNLIKELY(_site.shouldLog())) \ - { \ - std::ostringstream* _out = LLError::Log::out(); \ - (*_out) +#define lllog_test_() \ + if (LL_UNLIKELY(_site.shouldLog())) \ + { \ + std::ostringstream _out; \ + _out #define lllog_site_args_(level, once, tags) \ level, __FILE__, __LINE__, typeid(_LL_CLASS_TO_LOG), \ @@ -378,7 +378,7 @@ typedef LLError::NoClassInfo _LL_CLASS_TO_LOG; // LL_CONT << " for " << t << " seconds" << LL_ENDL; // //Such computation is done iff the message will be logged. -#define LL_CONT (*_out) +#define LL_CONT _out #define LL_NEWLINE '\n' -- cgit v1.2.3 From 91c20363eee4e1e02435e0ee74867cdb3f6c7136 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 12 May 2021 13:37:24 -0400 Subject: SL-10297: Get rid of LLError::LLCallStacks::allocateStackBuffer(). Also freeStackBuffer() and all the funky classic-C string management of a big flat buffer divided into exactly 512 128-byte strings. Define StringVector as a std::vector, and use that instead. Retain the behavior of clearing the vector if it exceeds 512 entries. This eliminates the LLError::Log::flush(const std::ostringstream&, char*) overload as well, with its baffling mix of std::string and classic-C (e.g. strlen(out.str().c_str()). If we absolutely MUST use a big memory pool for performance reasons, let's use StringVector with allocators. --- indra/llcommon/llerror.cpp | 88 ++++++++-------------------------------------- indra/llcommon/llerror.h | 10 +++--- 2 files changed, 18 insertions(+), 80 deletions(-) diff --git a/indra/llcommon/llerror.cpp b/indra/llcommon/llerror.cpp index f7594ed815..8355df9045 100644 --- a/indra/llcommon/llerror.cpp +++ b/indra/llcommon/llerror.cpp @@ -1352,25 +1352,6 @@ namespace LLError } - void Log::flush(const std::ostringstream& out, char* message) - { - LLMutexTrylock lock(getMutex(),5); - if (!lock.isLocked()) - { - return; - } - - if(strlen(out.str().c_str()) < 128) - { - strcpy(message, out.str().c_str()); - } - else - { - strncpy(message, out.str().c_str(), 127); - message[127] = '\0' ; - } - } - void Log::flush(const std::ostringstream& out, const CallSite& site) { LLMutexTrylock lock(getMutex(),5); @@ -1496,33 +1477,7 @@ namespace LLError namespace LLError { - char** LLCallStacks::sBuffer = NULL ; - S32 LLCallStacks::sIndex = 0 ; - - //static - void LLCallStacks::allocateStackBuffer() - { - if(sBuffer == NULL) - { - sBuffer = new char*[512] ; - sBuffer[0] = new char[512 * 128] ; - for(S32 i = 1 ; i < 512 ; i++) - { - sBuffer[i] = sBuffer[i-1] + 128 ; - } - sIndex = 0 ; - } - } - - void LLCallStacks::freeStackBuffer() - { - if(sBuffer != NULL) - { - delete [] sBuffer[0] ; - delete [] sBuffer ; - sBuffer = NULL ; - } - } + LLCallStacks::StringVector LLCallStacks::sBuffer ; //static void LLCallStacks::push(const char* function, const int line) @@ -1533,21 +1488,14 @@ namespace LLError return; } - if(sBuffer == NULL) - { - allocateStackBuffer(); - } - - if(sIndex > 511) + if(sBuffer.size() > 511) { clear() ; } - strcpy(sBuffer[sIndex], function) ; - sprintf(sBuffer[sIndex] + strlen(function), " line: %d ", line) ; - sIndex++ ; - - return ; + std::ostringstream out; + insert(out, function, line); + sBuffer.push_back(out.str()); } //static @@ -1565,17 +1513,12 @@ namespace LLError return; } - if(sBuffer == NULL) - { - allocateStackBuffer(); - } - - if(sIndex > 511) + if(sBuffer.size() > 511) { clear() ; } - LLError::Log::flush(out, sBuffer[sIndex++]) ; + sBuffer.push_back(out.str()); } //static @@ -1587,33 +1530,30 @@ namespace LLError return; } - if(sIndex > 0) + if(! sBuffer.empty()) { LL_INFOS() << " ************* PRINT OUT LL CALL STACKS ************* " << LL_ENDL; - while(sIndex > 0) + for (StringVector::const_reverse_iterator ri(sBuffer.rbegin()), re(sBuffer.rend()); + ri != re; ++ri) { - sIndex-- ; - LL_INFOS() << sBuffer[sIndex] << LL_ENDL; + LL_INFOS() << (*ri) << LL_ENDL; } LL_INFOS() << " *************** END OF LL CALL STACKS *************** " << LL_ENDL; } - if(sBuffer != NULL) - { - freeStackBuffer(); - } + cleanup(); } //static void LLCallStacks::clear() { - sIndex = 0 ; + sBuffer.clear(); } //static void LLCallStacks::cleanup() { - freeStackBuffer(); + clear(); } std::ostream& operator<<(std::ostream& out, const LLStacktrace&) diff --git a/indra/llcommon/llerror.h b/indra/llcommon/llerror.h index 51423350e6..d439136ca8 100644 --- a/indra/llcommon/llerror.h +++ b/indra/llcommon/llerror.h @@ -29,7 +29,9 @@ #define LL_LLERROR_H #include +#include #include +#include #include "stdtypes.h" @@ -198,7 +200,6 @@ namespace LLError { public: static bool shouldLog(CallSite&); - static void flush(const std::ostringstream& out, char* message); static void flush(const std::ostringstream&, const CallSite&); static std::string demangle(const char* mangled); /// classname() @@ -280,11 +281,8 @@ namespace LLError class LL_COMMON_API LLCallStacks { private: - static char** sBuffer ; - static S32 sIndex ; - - static void allocateStackBuffer(); - static void freeStackBuffer(); + typedef std::vector StringVector; + static StringVector sBuffer ; public: static void push(const char* function, const int line) ; -- cgit v1.2.3 From 28862ab53b9e532bf6ff789743d80b33af10f395 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 12 May 2021 14:21:27 -0400 Subject: SL-10297: Clean up a few merge glitches. --- indra/newview/llappviewer.cpp | 29 ++++++++++++----------------- indra/test/test.cpp | 22 ++++++++-------------- 2 files changed, 20 insertions(+), 31 deletions(-) diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp index c44690ac25..7d7d41fe22 100644 --- a/indra/newview/llappviewer.cpp +++ b/indra/newview/llappviewer.cpp @@ -2298,10 +2298,6 @@ void LLAppViewer::initLoggingAndGetLastDuration() { LL_WARNS("MarkerFile") << duration_log_msg << LL_ENDL; } - - // TBD - temporary location for fatal hook (should be above, but for now it logs...) - LL_DEBUGS("FatalHook") << "initial setting of default fatalhook" << LL_ENDL; - LLError::setFatalHook(fatalErrorHook); } bool LLAppViewer::loadSettingsFromDirectory(const std::string& location_key, @@ -2998,16 +2994,15 @@ bool LLAppViewer::initWindow() use_watchdog = bool(watchdog_enabled_setting); } - LL_INFOS("AppInit") << "watchdog" - << (use_watchdog ? " " : " NOT ") - << "enabled" - << " (setting = " << watchdog_enabled_setting << ")" - << LL_ENDL; + LL_INFOS("AppInit") << "watchdog" + << (use_watchdog ? " " : " NOT ") + << "enabled" + << " (setting = " << watchdog_enabled_setting << ")" + << LL_ENDL; if (use_watchdog) { - LLWatchdog::getInstance()->init( - [](){ LL_ERRS() << "Watchdog killer event" << LL_ENDL; }); + LLWatchdog::getInstance()->init(); } LLNotificationsUI::LLNotificationManager::getInstance(); @@ -3441,8 +3436,8 @@ void LLAppViewer::writeSystemInfo() gDebugInfo["CPUInfo"]["CPUSSE"] = gSysCPU.hasSSE(); gDebugInfo["CPUInfo"]["CPUSSE2"] = gSysCPU.hasSSE2(); - gDebugInfo["RAMInfo"]["Physical"] = (LLSD::Integer)(gSysMemory.getPhysicalMemoryKB().value()); - gDebugInfo["RAMInfo"]["Allocated"] = (LLSD::Integer)(gMemoryAllocated.valueInUnits()); + gDebugInfo["RAMInfo"]["Physical"] = LLSD::Integer(gSysMemory.getPhysicalMemoryKB().value()); + gDebugInfo["RAMInfo"]["Allocated"] = LLSD::Integer(gMemoryAllocated.valueInUnits()); gDebugInfo["OSInfo"] = LLOSInfo::instance().getOSStringSimple(); // The user is not logged on yet, but record the current grid choice login url @@ -3461,11 +3456,11 @@ void LLAppViewer::writeSystemInfo() // and can read this value from the debug_info.log. // If the crash is handled by LLAppViewer::handleViewerCrash, ie not a freeze, // then the value of "CrashNotHandled" will be set to true. - gDebugInfo["CrashNotHandled"] = (LLSD::Boolean)true; + gDebugInfo["CrashNotHandled"] = LLSD::Boolean(true); #else // LL_BUGSPLAT // "CrashNotHandled" is obsolete; it used (not very successsfully) // to try to distinguish crashes from freezes - the intent here to to avoid calling it a freeze - gDebugInfo["CrashNotHandled"] = (LLSD::Boolean)false; + gDebugInfo["CrashNotHandled"] = LLSD::Boolean(false); #endif // ! LL_BUGSPLAT // Insert crash host url (url to post crash log to) if configured. This insures @@ -3497,7 +3492,7 @@ void LLAppViewer::writeSystemInfo() gDebugInfo["SettingsFilename"] = gSavedSettings.getString("ClientSettingsFile"); gDebugInfo["ViewerExePath"] = gDirUtilp->getExecutablePathAndName(); gDebugInfo["CurrentPath"] = gDirUtilp->getCurPath(); - gDebugInfo["FirstLogin"] = (LLSD::Boolean) gAgent.isFirstLogin(); + gDebugInfo["FirstLogin"] = LLSD::Boolean(gAgent.isFirstLogin()); gDebugInfo["FirstRunThisInstall"] = gSavedSettings.getBOOL("FirstRunThisInstall"); gDebugInfo["StartupState"] = LLStartUp::getStartupStateString(); @@ -3627,7 +3622,7 @@ void LLAppViewer::handleViewerCrash() // The crash is being handled here so set this value to false. // Otherwise the crash logger will think this crash was a freeze. - gDebugInfo["Dynamic"]["CrashNotHandled"] = (LLSD::Boolean)false; + gDebugInfo["Dynamic"]["CrashNotHandled"] = LLSD::Boolean(false); //Write out the crash status file //Use marker file style setup, as that's the simplest, especially since diff --git a/indra/test/test.cpp b/indra/test/test.cpp index 571f502861..87c4a8d8a3 100644 --- a/indra/test/test.cpp +++ b/indra/test/test.cpp @@ -75,18 +75,7 @@ #include -LLError::ErrFatalHookResult wouldHaveCrashed(const std::string& message) -{ -#if LL_MSVC -#pragma warning (push) -#pragma warning (disable : 4702) // warning C4702: unreachable code -#endif - tut::fail("fatal error message: " + message); - return LLError::ERR_DO_NOT_CRASH; -#if LL_MSVC -#pragma warning (pop) -#endif -} +void wouldHaveCrashed(const std::string& message); namespace tut { @@ -157,7 +146,7 @@ public: mOldSettings(LLError::saveAndResetSettings()), mRecorder(new RecordToTempFile(pool)) { - LLError::setFatalHook(wouldHaveCrashed); + LLError::setFatalFunction(wouldHaveCrashed); LLError::setDefaultLevel(level); LLError::addRecorder(mRecorder); } @@ -523,6 +512,11 @@ void stream_groups(std::ostream& s, const char* app) } } +void wouldHaveCrashed(const std::string& message) +{ + tut::fail("llerrs message: " + message); +} + static LLTrace::ThreadRecorder* sMasterThreadRecorder = NULL; int main(int argc, char **argv) @@ -632,7 +626,7 @@ int main(int argc, char **argv) replayer.reset(new LLReplayLogReal(level, gAPRPoolp)); } } - LLError::setFatalHook(wouldHaveCrashed); + LLError::setFatalFunction(wouldHaveCrashed); std::string test_app_name(argv[0]); std::string test_log = test_app_name + ".log"; LLFile::remove(test_log); -- cgit v1.2.3 From d10badf0d23f48665239117838c5daf0fd667e01 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 12 May 2021 16:26:53 -0400 Subject: SL-10297: #include in llsingleton.h --- indra/llcommon/llsingleton.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 2eb39c6c8c..163c08099f 100644 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -27,14 +27,15 @@ #include #include +#include #include -#include +#include #include +#include #include "mutex.h" #include "lockstatic.h" #include "llthread.h" // on_main_thread() #include "llmainthreadtask.h" -#include class LLSingletonBase: private boost::noncopyable { -- cgit v1.2.3 From 28deadab5b37880314a58eff095ae2844a9ebbac Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 12 May 2021 17:08:15 -0400 Subject: SL-10297: Need VS switch /std:c++17 to use std::string_view. --- indra/cmake/00-Common.cmake | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/indra/cmake/00-Common.cmake b/indra/cmake/00-Common.cmake index 8aea50e02b..83041d0663 100644 --- a/indra/cmake/00-Common.cmake +++ b/indra/cmake/00-Common.cmake @@ -66,7 +66,8 @@ if (WINDOWS) # CP changed to only append the flag for 32bit builds - on 64bit builds, # locally at least, the build output is spammed with 1000s of 'D9002' # warnings about this switch being ignored. - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /MP") + # We need std::string_view, but that's not available without /std:c++17. + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /MP /std:c++17") if( ADDRESS_SIZE EQUAL 32 ) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /p:PreferredToolArchitecture=x64") endif() -- cgit v1.2.3 From a58eea7419ac9aec6ca08bcfca17ee14bf62e72c Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 12 May 2021 22:30:34 -0400 Subject: SL-10297: Revert "Need VS switch /std:c++17 to use std::string_view." This reverts commit 28deadab5b37880314a58eff095ae2844a9ebbac. Going there implies other changes, will take up on another branch. --- indra/cmake/00-Common.cmake | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/indra/cmake/00-Common.cmake b/indra/cmake/00-Common.cmake index 83041d0663..8aea50e02b 100644 --- a/indra/cmake/00-Common.cmake +++ b/indra/cmake/00-Common.cmake @@ -66,8 +66,7 @@ if (WINDOWS) # CP changed to only append the flag for 32bit builds - on 64bit builds, # locally at least, the build output is spammed with 1000s of 'D9002' # warnings about this switch being ignored. - # We need std::string_view, but that's not available without /std:c++17. - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /MP /std:c++17") + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /MP") if( ADDRESS_SIZE EQUAL 32 ) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /p:PreferredToolArchitecture=x64") endif() -- cgit v1.2.3 From 24501dfa0ee3fd6f5755deb1bc5261cd297a2bc7 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 12 May 2021 22:35:09 -0400 Subject: SL-10297: Use initializer_list vs. . This is somewhat more expensive for string literals, but switching to std::string_view implies more extensive changes, to be considered separately. --- indra/llcommon/llsingleton.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/indra/llcommon/llsingleton.h b/indra/llcommon/llsingleton.h index 163c08099f..7c81d65a8b 100644 --- a/indra/llcommon/llsingleton.h +++ b/indra/llcommon/llsingleton.h @@ -29,7 +29,6 @@ #include #include #include -#include #include #include #include "mutex.h" @@ -114,7 +113,7 @@ protected: // delegate logging calls to llsingleton.cpp public: - typedef std::initializer_list string_params; + typedef std::initializer_list string_params; protected: static void logerrs (const string_params&); static void logwarns (const string_params&); -- cgit v1.2.3