From ed98ebb8115bffc5cf78ec00e33adf2243c55f0d Mon Sep 17 00:00:00 2001
From: Andrey Kleshchev <andreykproductengine@lindenlab.com>
Date: Fri, 12 Mar 2021 18:19:53 +0200
Subject: [PATCH] SL-14961 Coroutine crash was not reported to bugsplat
---
indra/llcommon/llapp.h | 4 +++
indra/llcommon/llcoros.cpp | 58 +++++++++++++++++++++---------
indra/llcommon/llcoros.h | 7 ++--
indra/newview/llappviewer.cpp | 2 +-
indra/newview/llappviewerwin32.cpp | 10 ++++++
indra/newview/llappviewerwin32.h | 22 ++++++------
6 files changed, 72 insertions(+), 31 deletions(-)
diff --git a/indra/llcommon/llapp.h b/indra/llcommon/llapp.h
index 245c73e3a22..5fa91b8bf57 100644
--- a/indra/llcommon/llapp.h
+++ b/indra/llcommon/llapp.h
@@ -259,6 +259,10 @@ class LL_COMMON_API LLApp
*/
LLRunner& getRunner() { return mRunner; }
+#ifdef LL_WINDOWS
+ virtual void reportCrashToBugsplat(void* pExcepInfo /*EXCEPTION_POINTERS*/) { }
+#endif
+
public:
typedef std::map<std::string, std::string> string_map;
string_map mOptionMap; // Contains all command-line options and arguments in a map
diff --git a/indra/llcommon/llcoros.cpp b/indra/llcommon/llcoros.cpp
index 262929006dd..111c50af93b 100644
--- a/indra/llcommon/llcoros.cpp
+++ b/indra/llcommon/llcoros.cpp
@@ -255,8 +255,21 @@ std::string LLCoros::launch(const std::string& prefix, const callable_t& callabl
static const U32 STATUS_MSC_EXCEPTION = 0xE06D7363; // compiler specific
-U32 exception_filter(U32 code, struct _EXCEPTION_POINTERS *exception_infop)
+U32 cpp_exception_filter(U32 code, struct _EXCEPTION_POINTERS *exception_infop, const std::string& name)
{
+ // C++ exceptions were logged in toplevelTryWrapper, but not SEH
+ // log SEH exceptions here, to make sure it gets into bugsplat's
+ // report and because __try won't allow std::string operations
+ if (code != STATUS_MSC_EXCEPTION)
+ {
+ LL_WARNS() << "SEH crash in " << name << ", code: " << code << LL_ENDL;
+ }
+ // Handle bugsplat here, since GetExceptionInformation() can only be
+ // called from within filter for __except(filter), not from __except's {}
+ // Bugsplat should get all exceptions, C++ and SEH
+ LLApp::instance()->reportCrashToBugsplat(exception_infop);
+
+ // Only convert non C++ exceptions.
if (code == STATUS_MSC_EXCEPTION)
{
// C++ exception, go on
@@ -269,29 +282,29 @@ U32 exception_filter(U32 code, struct _EXCEPTION_POINTERS *exception_infop)
}
}
-void LLCoros::winlevel(const callable_t& callable)
+void LLCoros::winlevel(const std::string& name, const callable_t& callable)
{
__try
{
- callable();
+ toplevelTryWrapper(name, callable);
}
- __except (exception_filter(GetExceptionCode(), GetExceptionInformation()))
+ __except (cpp_exception_filter(GetExceptionCode(), GetExceptionInformation(), name))
{
- // convert to C++ styled exception
+ // convert to C++ styled exception for handlers other than bugsplat
// Note: it might be better to use _se_set_translator
// if you want exception to inherit full callstack
- char integer_string[32];
- sprintf(integer_string, "SEH, code: %lu\n", GetExceptionCode());
+ //
+ // in case of bugsplat this will get to exceptionTerminateHandler and
+ // looks like fiber will terminate application after that
+ char integer_string[512];
+ sprintf(integer_string, "SEH crash in %s, code: %lu\n", name.c_str(), GetExceptionCode());
throw std::exception(integer_string);
}
}
#endif
-// Top-level wrapper around caller's coroutine callable.
-// Normally we like to pass strings and such by const reference -- but in this
-// case, we WANT to copy both the name and the callable to our local stack!
-void LLCoros::toplevel(std::string name, callable_t callable)
+void LLCoros::toplevelTryWrapper(const std::string& name, const callable_t& callable)
{
// keep the CoroData on this top-level function's stack frame
CoroData corodata(name);
@@ -301,16 +314,12 @@ void LLCoros::toplevel(std::string name, callable_t callable)
// run the code the caller actually wants in the coroutine
try
{
-#if LL_WINDOWS && LL_RELEASE_FOR_DOWNLOAD
- winlevel(callable);
-#else
callable();
-#endif
}
catch (const Stop& exc)
{
LL_INFOS("LLCoros") << "coroutine " << name << " terminating because "
- << exc.what() << LL_ENDL;
+ << exc.what() << LL_ENDL;
}
catch (const LLContinueError&)
{
@@ -323,10 +332,25 @@ void LLCoros::toplevel(std::string name, callable_t callable)
{
// Any OTHER kind of uncaught exception will cause the viewer to
// crash, hopefully informatively.
- CRASH_ON_UNHANDLED_EXCEPTION(STRINGIZE("coroutine " << name));
+ LOG_UNHANDLED_EXCEPTION(STRINGIZE("coroutine " << name));
+ // to not modify callstack
+ throw;
}
}
+// Top-level wrapper around caller's coroutine callable.
+// Normally we like to pass strings and such by const reference -- but in this
+// case, we WANT to copy both the name and the callable to our local stack!
+void LLCoros::toplevel(std::string name, callable_t callable)
+{
+#if LL_WINDOWS
+ // Can not use __try in functions that require unwinding, so use one more wrapper
+ winlevel(name, callable);
+#else
+ toplevelTryWrapper(name, callable);
+#endif
+}
+
//static
void LLCoros::checkStop()
{
diff --git a/indra/llcommon/llcoros.h b/indra/llcommon/llcoros.h
index 38c2356c99d..6c0bec3ef96 100644
--- a/indra/llcommon/llcoros.h
+++ b/indra/llcommon/llcoros.h
@@ -290,11 +290,12 @@ class LL_COMMON_API LLCoros: public LLSingleton<LLCoros>
private:
std::string generateDistinctName(const std::string& prefix) const;
- void toplevel(std::string name, callable_t callable);
- struct CoroData;
#if LL_WINDOWS
- static void winlevel(const callable_t& callable);
+ void winlevel(const std::string& name, const callable_t& callable);
#endif
+ void toplevelTryWrapper(const std::string& name, const callable_t& callable);
+ void toplevel(std::string name, callable_t callable);
+ struct CoroData;
static CoroData& get_CoroData(const std::string& caller);
S32 mStackSize;
diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp
index a1d73acfa5f..7c2f622abd1 100644
--- a/indra/newview/llappviewer.cpp
+++ b/indra/newview/llappviewer.cpp
@@ -5552,7 +5552,7 @@ void LLAppViewer::forceErrorDriverCrash()
void LLAppViewer::forceErrorCoroutineCrash()
{
LL_WARNS() << "Forcing a crash in LLCoros" << LL_ENDL;
- LLCoros::instance().launch("LLAppViewer::crashyCoro", [] {throw std::exception("A deliberate crash from LLCoros"); });
+ LLCoros::instance().launch("LLAppViewer::crashyCoro", [] {throw LLException("A deliberate crash from LLCoros"); });
}
void LLAppViewer::forceErrorThreadCrash()
diff --git a/indra/newview/llappviewerwin32.cpp b/indra/newview/llappviewerwin32.cpp
index bb6156e209c..ce36d3458ef 100644
--- a/indra/newview/llappviewerwin32.cpp
+++ b/indra/newview/llappviewerwin32.cpp
@@ -709,6 +709,16 @@ bool LLAppViewerWin32::cleanup()
return result;
}
+void LLAppViewerWin32::reportCrashToBugsplat(void* pExcepInfo)
+{
+#if defined(LL_BUGSPLAT)
+ if (sBugSplatSender)
+ {
+ sBugSplatSender->createReport((EXCEPTION_POINTERS*)pExcepInfo);
+ }
+#endif // LL_BUGSPLAT
+}
+
void LLAppViewerWin32::initLoggingAndGetLastDuration()
{
LLAppViewer::initLoggingAndGetLastDuration();
diff --git a/indra/newview/llappviewerwin32.h b/indra/newview/llappviewerwin32.h
index c5fae6a3a3b..83ae875a151 100644
--- a/indra/newview/llappviewerwin32.h
+++ b/indra/newview/llappviewerwin32.h
@@ -40,20 +40,22 @@ class LLAppViewerWin32 : public LLAppViewer
//
// Main application logic
//
- virtual bool init(); // Override to do application initialization
- virtual bool cleanup();
+ bool init() override; // Override to do application initialization
+ bool cleanup() override;
+
+ void reportCrashToBugsplat(void* pExcepInfo) override;
protected:
- virtual void initLoggingAndGetLastDuration(); // Override to clean stack_trace info.
- virtual void initConsole(); // Initialize OS level debugging console.
- virtual bool initHardwareTest(); // Win32 uses DX9 to test hardware.
- virtual bool initParseCommandLine(LLCommandLineParser& clp);
+ void initLoggingAndGetLastDuration() override; // Override to clean stack_trace info.
+ void initConsole() override; // Initialize OS level debugging console.
+ bool initHardwareTest() override; // Win32 uses DX9 to test hardware.
+ bool initParseCommandLine(LLCommandLineParser& clp) override;
- virtual bool beingDebugged();
- virtual bool restoreErrorTrap();
- virtual void initCrashReporting(bool reportFreeze);
+ bool beingDebugged() override;
+ bool restoreErrorTrap() override;
+ void initCrashReporting(bool reportFreeze) override;
- virtual bool sendURLToOtherInstance(const std::string& url);
+ bool sendURLToOtherInstance(const std::string& url) override;
std::string generateSerialNumber();
--
GitLab