From e674f11757ab55c5ca7aab4cb1c8e059fa98f466 Mon Sep 17 00:00:00 2001
From: Nat Goodspeed <nat@lindenlab.com>
Date: Thu, 23 Aug 2018 12:31:54 -0400
Subject: [PATCH] DRTVWR-447: Add (some) metadata to Mac crash reports.
This required reordering certain operations during Mac viewer startup. Split
llappviewermacosx.cpp's initViewer() function into constructViewer() (which
instantiates LLAppViewerMacOSX) and initViewer() (which calls
LLAppViewerMacOSX::init()).
llappdelegate-objc.mm's applicationDidFinishLaunching override now calls
[BugsplatStartupManager start] between constructViewer() and initViewer(): we
want constructViewer() to have set up the logging subsystem so we can log the
actions of BugsplatStartupManagerDelegate override methods, but otherwise we
want BugsplatStartupManager in place as early as possible to catch any early
crashes. Besides, initViewer() ends up overwriting the static_debug_info.log
on which we depend for the *previous* run's crash metadata.
Move the code that initializes the pathname of the static_debug_info.log file
from LLAppViewerMacOSX::init() to the LLAppViewerMacOSX() constructor, since
BugsplatStartupManagerDelegate override methods need to read (the previous
run's) file.
Add code to applicationLogForBugsplatStartupManager override to set new
BugsplatMac 1.0.6 properties userName and userEmail.
Don't log empty fields from static_debug_info.log if we couldn't read it.
---
indra/newview/llappdelegate-objc.mm | 55 +++++++++++++++++-----
indra/newview/llappviewer.cpp | 29 +++++++-----
indra/newview/llappviewermacosx-for-objc.h | 1 +
indra/newview/llappviewermacosx.cpp | 17 ++++---
4 files changed, 69 insertions(+), 33 deletions(-)
diff --git a/indra/newview/llappdelegate-objc.mm b/indra/newview/llappdelegate-objc.mm
index 66bcf589616..f55304f30bb 100644
--- a/indra/newview/llappdelegate-objc.mm
+++ b/indra/newview/llappdelegate-objc.mm
@@ -54,6 +54,25 @@
- (void) applicationDidFinishLaunching:(NSNotification *)notification
{
+ // Call constructViewer() first so our logging subsystem is in place. This
+ // risks missing crashes in the LLAppViewerMacOSX constructor, but for
+ // present purposes it's more important to get the startup sequence
+ // properly logged.
+ // Someday I would like to modify the logging system so that calls before
+ // it's initialized are cached in a std::ostringstream and then, once it's
+ // initialized, "played back" into whatever handlers have been set up.
+ constructViewer();
+
+#if defined(LL_BUGSPLAT)
+ // Engage BugsplatStartupManager *before* calling initViewer() to handle
+ // any crashes during initialization.
+ // https://www.bugsplat.com/docs/platforms/os-x#initialization
+ [BugsplatStartupManager sharedManager].autoSubmitCrashReport = YES;
+ [BugsplatStartupManager sharedManager].askUserDetails = NO;
+ [BugsplatStartupManager sharedManager].delegate = self;
+ [[BugsplatStartupManager sharedManager] start];
+#endif
+
frameTimer = nil;
[self languageUpdated];
@@ -71,14 +90,6 @@
[[NSNotificationCenter defaultCenter] addObserver:self selector:@selector(languageUpdated) name:@"NSTextInputContextKeyboardSelectionDidChangeNotification" object:nil];
// [[NSAppleEventManager sharedAppleEventManager] setEventHandler:self andSelector:@selector(handleGetURLEvent:withReplyEvent:) forEventClass:kInternetEventClass andEventID:kAEGetURL];
-
-#if defined(LL_BUGSPLAT)
- // https://www.bugsplat.com/docs/platforms/os-x#initialization
- [BugsplatStartupManager sharedManager].autoSubmitCrashReport = YES;
- [BugsplatStartupManager sharedManager].askUserDetails = NO;
- [BugsplatStartupManager sharedManager].delegate = self;
- [[BugsplatStartupManager sharedManager] start];
-#endif
}
- (void) handleGetURLEvent:(NSAppleEventDescriptor *)event withReplyEvent:(NSAppleEventDescriptor *)replyEvent {
@@ -198,11 +209,29 @@
- (NSString *)applicationLogForBugsplatStartupManager:(BugsplatStartupManager *)bugsplatStartupManager
{
- std::string fatalMessage(CrashMetadata_instance().fatalMessage);
- infos("applicationLogForBugsplatStartupManager -> '" + fatalMessage + "'");
- // This strangely-named override method contributes the User Description
- // metadata field.
- return [NSString stringWithCString:fatalMessage.c_str()
+ CrashMetadata& meta(CrashMetadata_instance());
+ // As of BugsplatMac 1.0.6, userName and userEmail properties are now
+ // exposed by the BugsplatStartupManager. Set them here, since the
+ // defaultUserNameForBugsplatStartupManager and
+ // defaultUserEmailForBugsplatStartupManager methods are called later, for
+ // the *current* run, rather than for the previous crashed run whose crash
+ // report we are about to send.
+ infos("applicationLogForBugsplatStartupManager setting userName = '" +
+ meta.agentFullname + '"');
+ bugsplatStartupManager.userName =
+ [NSString stringWithCString:meta.agentFullname.c_str()
+ encoding:NSUTF8StringEncoding];
+ // Use the email field for OS version, just as we do on Windows, until
+ // BugSplat provides more metadata fields.
+ infos("applicationLogForBugsplatStartupManager setting userEmail = '" +
+ meta.OSInfo + '"');
+ bugsplatStartupManager.userEmail =
+ [NSString stringWithCString:meta.OSInfo.c_str()
+ encoding:NSUTF8StringEncoding];
+ // This strangely-named override method's return value contributes the
+ // User Description metadata field.
+ infos("applicationLogForBugsplatStartupManager -> '" + meta.fatalMessage + "'");
+ return [NSString stringWithCString:meta.fatalMessage.c_str()
encoding:NSUTF8StringEncoding];
}
diff --git a/indra/newview/llappviewer.cpp b/indra/newview/llappviewer.cpp
index d324a82bf85..846b937a4e5 100644
--- a/indra/newview/llappviewer.cpp
+++ b/indra/newview/llappviewer.cpp
@@ -707,6 +707,22 @@ LLAppViewer::LLAppViewer()
//
LLLoginInstance::instance().setPlatformInfo(gPlatform, LLOSInfo::instance().getOSVersionString(), LLOSInfo::instance().getOSStringSimple());
+
+ // Under some circumstances we want to read the static_debug_info.log file
+ // 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
+ // 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
+ // 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
+ mDumpPath = logdir;
+ setMiniDumpDir(logdir);
+ setDebugFileNames(logdir);
}
LLAppViewer::~LLAppViewer()
@@ -781,19 +797,6 @@ bool LLAppViewer::init()
initMaxHeapSize() ;
LLCoros::instance().setStackSize(gSavedSettings.getS32("CoroutineStackSize"));
-#if 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
- // 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
- mDumpPath = logdir;
- setMiniDumpDir(logdir);
- logdir += gDirUtilp->getDirDelimiter();
- setDebugFileNames(logdir);
-
// Although initLoggingAndGetLastDuration() is the right place to mess with
// setFatalFunction(), we can't query gSavedSettings until after
diff --git a/indra/newview/llappviewermacosx-for-objc.h b/indra/newview/llappviewermacosx-for-objc.h
index 79da453cbee..37e8a3917a9 100644
--- a/indra/newview/llappviewermacosx-for-objc.h
+++ b/indra/newview/llappviewermacosx-for-objc.h
@@ -24,6 +24,7 @@
#include <string>
+void constructViewer();
bool initViewer();
void handleUrl(const char* url_utf8);
bool pumpMainLoop();
diff --git a/indra/newview/llappviewermacosx.cpp b/indra/newview/llappviewermacosx.cpp
index 77a16f73073..81f04744f87 100644
--- a/indra/newview/llappviewermacosx.cpp
+++ b/indra/newview/llappviewermacosx.cpp
@@ -86,7 +86,7 @@ static void exceptionTerminateHandler()
gOldTerminateHandler(); // call old terminate() handler
}
-bool initViewer()
+void constructViewer()
{
// Set the working dir to <bundle>/Contents/Resources
if (chdir(gDirUtilp->getAppRODataDir().c_str()) == -1)
@@ -102,18 +102,20 @@ bool initViewer()
gOldTerminateHandler = std::set_terminate(exceptionTerminateHandler);
gViewerAppPtr->setErrorHandler(LLAppViewer::handleViewerCrash);
+}
-
+bool initViewer()
+{
bool ok = gViewerAppPtr->init();
if(!ok)
{
LL_WARNS() << "Application init failed." << LL_ENDL;
}
- else if (!gHandleSLURL.empty())
- {
- dispatchUrl(gHandleSLURL);
- gHandleSLURL = "";
- }
+ else if (!gHandleSLURL.empty())
+ {
+ dispatchUrl(gHandleSLURL);
+ gHandleSLURL = "";
+ }
return ok;
}
@@ -194,6 +196,7 @@ CrashMetadataSingleton::CrashMetadataSingleton()
LL_INFOS() << "Can't parse '" << staticDebugPathname
<< "'; no metadata about previous run" << LL_ENDL;
}
+ else
{
LL_INFOS() << "Metadata from '" << staticDebugPathname << "':" << LL_ENDL;
logFilePathname = get_metadata(info, "SLLog");
--
GitLab