From 6ec276c8180ccdaae214190cc5ee04dd664993ae Mon Sep 17 00:00:00 2001 From: Dale Glass Date: Sun, 25 Jun 2023 23:03:03 +0200 Subject: [PATCH 01/12] Initial reorganization --- libraries/monitoring/CMakeLists.txt | 17 +++++++++++++++++ .../src/activity-logger}/UserActivityLogger.cpp | 0 .../src/activity-logger}/UserActivityLogger.h | 0 .../src/crash-handler}/CrashHandler.h | 0 .../crash-handler}/CrashHandler_Breakpad.cpp | 0 .../crash-handler}/CrashHandler_Crashpad.cpp | 0 .../src/crash-handler}/CrashHandler_None.cpp | 0 7 files changed, 17 insertions(+) create mode 100644 libraries/monitoring/CMakeLists.txt rename libraries/{networking/src => monitoring/src/activity-logger}/UserActivityLogger.cpp (100%) rename libraries/{networking/src => monitoring/src/activity-logger}/UserActivityLogger.h (100%) rename {interface/src => libraries/monitoring/src/crash-handler}/CrashHandler.h (100%) rename {interface/src => libraries/monitoring/src/crash-handler}/CrashHandler_Breakpad.cpp (100%) rename {interface/src => libraries/monitoring/src/crash-handler}/CrashHandler_Crashpad.cpp (100%) rename {interface/src => libraries/monitoring/src/crash-handler}/CrashHandler_None.cpp (100%) diff --git a/libraries/monitoring/CMakeLists.txt b/libraries/monitoring/CMakeLists.txt new file mode 100644 index 0000000000..cb848c956a --- /dev/null +++ b/libraries/monitoring/CMakeLists.txt @@ -0,0 +1,17 @@ +set(TARGET_NAME monitoring) +setup_hifi_library() +link_hifi_libraries(shared networking) + +add_crashpad() +target_breakpad() + + +if (UNIX AND NOT APPLE) + set(THREADS_PREFER_PTHREAD_FLAG ON) + find_package(Threads REQUIRED) + target_link_libraries(Threads::Threads) +endif() + +if (WIN32) + add_compile_definitions(_USE_MATH_DEFINES) +endif() diff --git a/libraries/networking/src/UserActivityLogger.cpp b/libraries/monitoring/src/activity-logger/UserActivityLogger.cpp similarity index 100% rename from libraries/networking/src/UserActivityLogger.cpp rename to libraries/monitoring/src/activity-logger/UserActivityLogger.cpp diff --git a/libraries/networking/src/UserActivityLogger.h b/libraries/monitoring/src/activity-logger/UserActivityLogger.h similarity index 100% rename from libraries/networking/src/UserActivityLogger.h rename to libraries/monitoring/src/activity-logger/UserActivityLogger.h diff --git a/interface/src/CrashHandler.h b/libraries/monitoring/src/crash-handler/CrashHandler.h similarity index 100% rename from interface/src/CrashHandler.h rename to libraries/monitoring/src/crash-handler/CrashHandler.h diff --git a/interface/src/CrashHandler_Breakpad.cpp b/libraries/monitoring/src/crash-handler/CrashHandler_Breakpad.cpp similarity index 100% rename from interface/src/CrashHandler_Breakpad.cpp rename to libraries/monitoring/src/crash-handler/CrashHandler_Breakpad.cpp diff --git a/interface/src/CrashHandler_Crashpad.cpp b/libraries/monitoring/src/crash-handler/CrashHandler_Crashpad.cpp similarity index 100% rename from interface/src/CrashHandler_Crashpad.cpp rename to libraries/monitoring/src/crash-handler/CrashHandler_Crashpad.cpp diff --git a/interface/src/CrashHandler_None.cpp b/libraries/monitoring/src/crash-handler/CrashHandler_None.cpp similarity index 100% rename from interface/src/CrashHandler_None.cpp rename to libraries/monitoring/src/crash-handler/CrashHandler_None.cpp From 4dcc2882fd6a2f2289f2cc4d60b324956bbd0ed9 Mon Sep 17 00:00:00 2001 From: Dale Glass Date: Mon, 26 Jun 2023 00:21:14 +0200 Subject: [PATCH 02/12] Crash reporter moved to separate library UAL moved back to networking --- interface/CMakeLists.txt | 2 +- interface/src/Application.cpp | 41 ++- interface/src/CrashRecoveryHandler.cpp | 21 +- interface/src/DiscoverabilityManager.cpp | 5 +- interface/src/Menu.cpp | 8 +- interface/src/graphics/RenderEventHandler.cpp | 4 +- interface/src/main.cpp | 291 ++++++------------ libraries/monitoring/CMakeLists.txt | 7 - .../src/crash-handler/CrashHandler.cpp | 61 ++++ .../src/crash-handler/CrashHandler.h | 112 ++++++- .../src/crash-handler/CrashHandlerBackend.h | 27 ++ ...d.cpp => CrashHandlerBackend_Breakpad.cpp} | 0 ...d.cpp => CrashHandlerBackend_Crashpad.cpp} | 10 +- ..._None.cpp => CrashHandlerBackend_None.cpp} | 0 .../src}/UserActivityLogger.cpp | 9 - .../src}/UserActivityLogger.h | 49 +-- 16 files changed, 339 insertions(+), 308 deletions(-) create mode 100644 libraries/monitoring/src/crash-handler/CrashHandler.cpp create mode 100644 libraries/monitoring/src/crash-handler/CrashHandlerBackend.h rename libraries/monitoring/src/crash-handler/{CrashHandler_Breakpad.cpp => CrashHandlerBackend_Breakpad.cpp} (100%) rename libraries/monitoring/src/crash-handler/{CrashHandler_Crashpad.cpp => CrashHandlerBackend_Crashpad.cpp} (98%) rename libraries/monitoring/src/crash-handler/{CrashHandler_None.cpp => CrashHandlerBackend_None.cpp} (100%) rename libraries/{monitoring/src/activity-logger => networking/src}/UserActivityLogger.cpp (96%) rename libraries/{monitoring/src/activity-logger => networking/src}/UserActivityLogger.h (52%) diff --git a/interface/CMakeLists.txt b/interface/CMakeLists.txt index 7262ff0005..ea98b9f4df 100644 --- a/interface/CMakeLists.txt +++ b/interface/CMakeLists.txt @@ -230,7 +230,7 @@ link_hifi_libraries( ${PLATFORM_GL_BACKEND} # Plaform specific input & display plugin libraries ${PLATFORM_PLUGIN_LIBRARIES} - shaders + shaders monitoring ) include_hifi_library_headers(script-engine) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index a9af10166d..a797fdf6cf 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -179,7 +179,7 @@ #include "avatar/AvatarPackager.h" #include "avatar/MyCharacterController.h" #include "CrashRecoveryHandler.h" -#include "CrashHandler.h" +#include "crash-handler/CrashHandler.h" #include "DiscoverabilityManager.h" #include "GLCanvas.h" #include "InterfaceDynamicFactory.h" @@ -407,8 +407,10 @@ public: } void deadlockDetectionCrash() { - setCrashAnnotation("_mod_faulting_tid", std::to_string((uint64_t)_mainThreadID)); - setCrashAnnotation("deadlock", "1"); + auto &ch = CrashHandler::getInstance(); + + ch.setAnnotation("_mod_faulting_tid", std::to_string((uint64_t)_mainThreadID)); + ch.setAnnotation("deadlock", "1"); uint32_t* crashTrigger = nullptr; *crashTrigger = 0xDEAD10CC; } @@ -1054,9 +1056,11 @@ Application::Application( { // identify gpu as early as possible to help identify OpenGL initialization errors. auto gpuIdent = GPUIdent::getInstance(); - setCrashAnnotation("sentry[contexts][gpu][name]", gpuIdent->getName().toStdString()); - setCrashAnnotation("sentry[contexts][gpu][version]", gpuIdent->getDriver().toStdString()); - setCrashAnnotation("gpu_memory", std::to_string(gpuIdent->getMemory())); + auto &ch = CrashHandler::getInstance(); + + ch.setAnnotation("sentry[contexts][gpu][name]", gpuIdent->getName().toStdString()); + ch.setAnnotation("sentry[contexts][gpu][version]", gpuIdent->getDriver().toStdString()); + ch.setAnnotation("gpu_memory", std::to_string(gpuIdent->getMemory())); } // make sure the debug draw singleton is initialized on the main thread. @@ -1166,13 +1170,15 @@ Application::Application( _logger->setSessionID(accountManager->getSessionID()); #endif - setCrashAnnotation("metaverse_session_id", accountManager->getSessionID().toString().toStdString()); - setCrashAnnotation("main_thread_id", std::to_string((size_t)QThread::currentThreadId())); + auto &ch = CrashHandler::getInstance(); + + ch.setAnnotation("metaverse_session_id", accountManager->getSessionID().toString().toStdString()); + ch.setAnnotation("main_thread_id", std::to_string((size_t)QThread::currentThreadId())); if (steamClient) { qCDebug(interfaceapp) << "[VERSION] SteamVR buildID:" << steamClient->getSteamVRBuildID(); } - setCrashAnnotation("steam", property(hifi::properties::STEAM).toBool() ? "1" : "0"); + ch.setAnnotation("steam", property(hifi::properties::STEAM).toBool() ? "1" : "0"); qCDebug(interfaceapp) << "[VERSION] Build sequence:" << qPrintable(applicationVersion()); qCDebug(interfaceapp) << "[VERSION] MODIFIED_ORGANIZATION:" << BuildInfo::MODIFIED_ORGANIZATION; @@ -1236,7 +1242,8 @@ Application::Application( connect(&domainHandler, SIGNAL(domainURLChanged(QUrl)), SLOT(domainURLChanged(QUrl))); connect(&domainHandler, SIGNAL(redirectToErrorDomainURL(QUrl)), SLOT(goToErrorDomainURL(QUrl))); connect(&domainHandler, &DomainHandler::domainURLChanged, [](QUrl domainURL){ - setCrashAnnotation("domain", domainURL.toString().toStdString()); + auto &ch = CrashHandler::getInstance(); + ch.setAnnotation("domain", domainURL.toString().toStdString()); }); connect(&domainHandler, SIGNAL(resetting()), SLOT(resettingDomain())); connect(&domainHandler, SIGNAL(connectedToDomain(QUrl)), SLOT(updateWindowTitle())); @@ -1348,8 +1355,9 @@ Application::Application( setPreferredCursor(Cursor::Manager::getIconName(Cursor::Icon::SYSTEM)); } - setCrashAnnotation("display_plugin", displayPlugin->getName().toStdString()); - setCrashAnnotation("hmd", displayPlugin->isHmd() ? "1" : "0"); + auto &ch = CrashHandler::getInstance(); + ch.setAnnotation("display_plugin", displayPlugin->getName().toStdString()); + ch.setAnnotation("hmd", displayPlugin->isHmd() ? "1" : "0"); }); } connect(this, &Application::activeDisplayPluginChanged, this, &Application::updateSystemTabletMode); @@ -1388,7 +1396,8 @@ Application::Application( connect(myAvatar.get(), &MyAvatar::skeletonModelURLChanged, [](){ QUrl avatarURL = qApp->getMyAvatar()->getSkeletonModelURL(); - setCrashAnnotation("avatar", avatarURL.toString().toStdString()); + auto &ch = CrashHandler::getInstance(); + ch.setAnnotation("avatar", avatarURL.toString().toStdString()); }); // Inititalize sample before registering @@ -2698,7 +2707,8 @@ void Application::updateHeartbeat() const { } void Application::onAboutToQuit() { - setCrashAnnotation("shutdown", "1"); + auto &ch = CrashHandler::getInstance(); + ch.setAnnotation("shutdown", "1"); // quickly save AvatarEntityData before the EntityTree is dismantled getMyAvatar()->saveAvatarEntityDataToSettings(); @@ -7147,7 +7157,8 @@ void Application::updateWindowTitle() const { QString metaverseUsername = accountManager->getAccountInfo().getUsername(); QString domainUsername = domainAccountManager->getUsername(); - setCrashAnnotation("sentry[user][username]", metaverseUsername.toStdString()); + auto &ch = CrashHandler::getInstance(); + ch.setAnnotation("sentry[user][username]", metaverseUsername.toStdString()); QString currentPlaceName; if (isServerlessMode()) { diff --git a/interface/src/CrashRecoveryHandler.cpp b/interface/src/CrashRecoveryHandler.cpp index 26a0398323..1f6cbef9ba 100644 --- a/interface/src/CrashRecoveryHandler.cpp +++ b/interface/src/CrashRecoveryHandler.cpp @@ -34,6 +34,7 @@ #include #include #include +#include #include bool CrashRecoveryHandler::checkForResetSettings(bool wasLikelyCrash, bool suppressPrompt) { @@ -91,7 +92,7 @@ bool CrashRecoveryHandler::suggestCrashReporting() { QVBoxLayout* layout = new QVBoxLayout; QString explainText; - auto &ual = UserActivityLogger::getInstance(); + auto &ch = CrashHandler::getInstance(); @@ -119,7 +120,7 @@ bool CrashRecoveryHandler::suggestCrashReporting() { break; } - if (!ual.isCrashMonitorStarted()) { + if (!ch.isStarted()) { qWarning() << "Crash reporting not working, skipping suggestion to enable it."; return false; } @@ -131,8 +132,8 @@ bool CrashRecoveryHandler::suggestCrashReporting() { QCheckBox* crashReportCheckbox = new QCheckBox("Enable automatic crash reporting"); - crashReportCheckbox->setChecked(ual.isCrashReportingEnabled()); - crashReportCheckbox->setEnabled(ual.isCrashMonitorStarted()); + crashReportCheckbox->setChecked(ch.isEnabled()); + crashReportCheckbox->setEnabled(ch.isStarted()); layout->addWidget(explainLabel); layout->addSpacing(12); @@ -149,7 +150,7 @@ bool CrashRecoveryHandler::suggestCrashReporting() { crashDialog.exec(); - ual.setCrashReportingEnabled(crashReportCheckbox->isChecked()); + ch.setEnabled(crashReportCheckbox->isChecked()); return true; } @@ -157,7 +158,7 @@ bool CrashRecoveryHandler::suggestCrashReporting() { CrashRecoveryHandler::Action CrashRecoveryHandler::promptUserForAction(bool showCrashMessage) { QDialog crashDialog; QLabel* label; - auto &ual = UserActivityLogger::getInstance(); + auto &ch = CrashHandler::getInstance(); if (showCrashMessage) { crashDialog.setWindowTitle("Interface Crashed Last Run"); @@ -176,7 +177,7 @@ CrashRecoveryHandler::Action CrashRecoveryHandler::promptUserForAction(bool show QRadioButton* option3 = new QRadioButton("Continue with my current settings"); QLabel* crashReportLabel = nullptr; - if (ual.isCrashMonitorStarted()) { + if (ch.isStarted()) { crashReportLabel = new QLabel("To help us with debugging, you can enable automatic crash reports.\n" "They'll only be seen by developers trusted by the Overte e.V. organization,\n" "and will only be used for improving the code."); @@ -187,8 +188,8 @@ CrashRecoveryHandler::Action CrashRecoveryHandler::promptUserForAction(bool show QCheckBox* crashReportCheckbox = new QCheckBox("Enable automatic crash reporting"); - crashReportCheckbox->setChecked(ual.isCrashReportingEnabled()); - crashReportCheckbox->setEnabled(ual.isCrashMonitorStarted()); + crashReportCheckbox->setChecked(ch.isEnabled()); + crashReportCheckbox->setEnabled(ch.isStarted()); option3->setChecked(true); layout->addWidget(option1); @@ -218,7 +219,7 @@ CrashRecoveryHandler::Action CrashRecoveryHandler::promptUserForAction(bool show } } - ual.setCrashReportingEnabled(crashReportCheckbox->isChecked()); + ch.setEnabled(crashReportCheckbox->isChecked()); // Dialog cancelled or "do nothing" option chosen return CrashRecoveryHandler::DO_NOTHING; diff --git a/interface/src/DiscoverabilityManager.cpp b/interface/src/DiscoverabilityManager.cpp index bef429736f..624d5430a2 100644 --- a/interface/src/DiscoverabilityManager.cpp +++ b/interface/src/DiscoverabilityManager.cpp @@ -22,8 +22,8 @@ #include #include #include +#include -#include "CrashHandler.h" #include "Menu.h" const Discoverability::Mode DEFAULT_DISCOVERABILITY_MODE = Discoverability::Connections; @@ -133,7 +133,8 @@ void DiscoverabilityManager::updateLocation() { if (auto steamClient = PluginManager::getInstance()->getSteamClientPlugin()) { steamClient->updateLocation(domainHandler.getHostname(), currentAddress); } - setCrashAnnotation("address", currentAddress.toString().toStdString()); + + CrashHandler::getInstance().setAnnotation("address", currentAddress.toString().toStdString()); } void DiscoverabilityManager::handleHeartbeatResponse(QNetworkReply* requestReply) { diff --git a/interface/src/Menu.cpp b/interface/src/Menu.cpp index 9ff82b891b..d30fe66a6f 100644 --- a/interface/src/Menu.cpp +++ b/interface/src/Menu.cpp @@ -57,6 +57,7 @@ #include "LocationBookmarks.h" #include "DeferredLightingEffect.h" #include "PickManager.h" +#include "crash-handler/CrashHandler.h" #include "scripting/SettingsScriptingInterface.h" #if defined(Q_OS_MAC) || defined(Q_OS_WIN) @@ -636,12 +637,13 @@ Menu::Menu() { true, &UserActivityLogger::getInstance(), SLOT(disable(bool))); + addCheckableActionToQMenuAndActionHash(networkMenu, MenuOption::EnableCrashReporting, 0, - UserActivityLogger::getInstance().isCrashReportingEnabled(), - &UserActivityLogger::getInstance(), - SLOT(setCrashReportingEnabled(bool))); + CrashHandler::getInstance().isEnabled(), + &CrashHandler::getInstance(), + SLOT(setEnabled(bool))); addActionToQMenuAndActionHash(networkMenu, MenuOption::ShowDSConnectTable, 0, qApp, SLOT(loadDomainConnectionDialog())); diff --git a/interface/src/graphics/RenderEventHandler.cpp b/interface/src/graphics/RenderEventHandler.cpp index bdb2cae060..e5777f8f61 100644 --- a/interface/src/graphics/RenderEventHandler.cpp +++ b/interface/src/graphics/RenderEventHandler.cpp @@ -12,8 +12,8 @@ #include "Application.h" #include #include +#include -#include "CrashHandler.h" RenderEventHandler::RenderEventHandler(CheckCall checkCall, RenderCall renderCall) : _checkCall(checkCall), @@ -29,7 +29,7 @@ RenderEventHandler::RenderEventHandler(CheckCall checkCall, RenderCall renderCal void RenderEventHandler::initialize() { setObjectName("Render"); PROFILE_SET_THREAD_NAME("Render"); - setCrashAnnotation("render_thread_id", std::to_string((size_t)QThread::currentThreadId())); + CrashHandler::getInstance().setAnnotation("render_thread_id", std::to_string((size_t)QThread::currentThreadId())); } void RenderEventHandler::resumeThread() { diff --git a/interface/src/main.cpp b/interface/src/main.cpp index 4d86c2c4d7..6baa338f92 100644 --- a/interface/src/main.cpp +++ b/interface/src/main.cpp @@ -27,7 +27,7 @@ #include "AddressManager.h" #include "Application.h" -#include "CrashHandler.h" +#include "crash-handler/CrashHandler.h" #include "InterfaceLogging.h" #include "UserActivityLogger.h" #include "MainWindow.h" @@ -37,7 +37,7 @@ #ifdef Q_OS_WIN #include extern "C" { - typedef int(__stdcall * CHECKMINSPECPROC) (); +typedef int(__stdcall* CHECKMINSPECPROC)(); } #endif @@ -51,7 +51,7 @@ int main(int argc, const char* argv[]) { // This appears to resolve the issues with corrupted fonts on OSX. No // idea why. qputenv("QT_ENABLE_GLYPH_CACHE_WORKAROUND", "true"); - // https://i.kym-cdn.com/entries/icons/original/000/008/342/ihave.jpg + // https://i.kym-cdn.com/entries/icons/original/000/008/342/ihave.jpg QSurfaceFormat::setDefaultFormat(format); #endif @@ -73,179 +73,88 @@ int main(int argc, const char* argv[]) { QCommandLineOption helpOption = parser.addHelpOption(); QCommandLineOption versionOption = parser.addVersionOption(); - QCommandLineOption urlOption( - "url", - "Start at specified URL location.", - "string" - ); - QCommandLineOption protocolVersionOption( - "protocolVersion", - "Writes the protocol version base64 signature to a file?", - "path" - ); - QCommandLineOption noUpdaterOption( - "no-updater", - "Do not show auto-updater." - ); - QCommandLineOption checkMinSpecOption( - "checkMinSpec", - "Check if machine meets minimum specifications. The program will run if check passes." - ); - QCommandLineOption runServerOption( - "runServer", - "Run the server." - ); - QCommandLineOption listenPortOption( - "listenPort", - "Port to listen on.", - "port_number" - ); - QCommandLineOption serverContentPathOption( - "serverContentPath", - "Path to find server content.", // What content?? - "path" - ); - QCommandLineOption overrideAppLocalDataPathOption( - "cache", - "Set test cache.", - "dir" - ); - QCommandLineOption scriptsOption( - "scripts", - "Set path for defaultScripts. These are probably scripts that run automatically. This parameter does not seem to work.", - "dir" - ); - QCommandLineOption allowMultipleInstancesOption( - "allowMultipleInstances", - "Allow multiple instances to run." - ); - QCommandLineOption displaysOption( - "display", - "Preferred display.", - "displays" - ); - QCommandLineOption disableDisplaysOption( - "disable-displays", - "Displays to disable. Valid options include \"OpenVR (Vive)\" and \"Oculus Rift\"", - "string" - ); - QCommandLineOption disableInputsOption( - "disable-inputs", - "Inputs to disable. Valid options include \"OpenVR (Vive)\" and \"Oculus Rift\"", - "string" - ); - QCommandLineOption suppressSettingsResetOption( - "suppress-settings-reset", - "Suppress the prompt to reset interface settings." - ); - QCommandLineOption oculusStoreOption( - "oculus-store", - "Let the Oculus plugin know if interface was run from the Oculus Store." - ); - QCommandLineOption standaloneOption( - "standalone", - "Emulate a standalone device." - ); - QCommandLineOption disableWatchdogOption( - "disableWatchdog", - "Disable the watchdog thread. The interface will crash on deadlocks." - ); - QCommandLineOption systemCursorOption( - "system-cursor", - "Use the default system cursor." - ); - QCommandLineOption concurrentDownloadsOption( - "concurrent-downloads", - "Maximum concurrent resource downloads. Default is 16, except for Android where it is 4.", - "integer" - ); - QCommandLineOption avatarURLOption( - "avatarURL", - "Override the avatar U.R.L.", - "url" - ); - QCommandLineOption replaceAvatarURLOption( - "replaceAvatarURL", - "Replaces the avatar U.R.L. When used with --avatarURL, this takes precedence.", - "url" - ); - QCommandLineOption setBookmarkOption( - "setBookmark", - "Set bookmark as key=value pair. Including the '=' symbol in either string is unsupported.", - "string" - ); - QCommandLineOption forceCrashReportingOption( - "forceCrashReporting", - "Force crash reporting to initialize." - ); + QCommandLineOption urlOption("url", "Start at specified URL location.", "string"); + QCommandLineOption protocolVersionOption("protocolVersion", "Writes the protocol version base64 signature to a file?", + "path"); + QCommandLineOption noUpdaterOption("no-updater", "Do not show auto-updater."); + QCommandLineOption + checkMinSpecOption("checkMinSpec", + "Check if machine meets minimum specifications. The program will run if check passes."); + QCommandLineOption runServerOption("runServer", "Run the server."); + QCommandLineOption listenPortOption("listenPort", "Port to listen on.", "port_number"); + QCommandLineOption serverContentPathOption("serverContentPath", + "Path to find server content.", // What content?? + "path"); + QCommandLineOption overrideAppLocalDataPathOption("cache", "Set test cache.", "dir"); + QCommandLineOption scriptsOption("scripts", + "Set path for defaultScripts. These are probably scripts that run automatically. This " + "parameter does not seem to work.", + "dir"); + QCommandLineOption allowMultipleInstancesOption("allowMultipleInstances", "Allow multiple instances to run."); + QCommandLineOption displaysOption("display", "Preferred display.", "displays"); + QCommandLineOption disableDisplaysOption("disable-displays", + "Displays to disable. Valid options include \"OpenVR (Vive)\" and \"Oculus Rift\"", + "string"); + QCommandLineOption disableInputsOption("disable-inputs", + "Inputs to disable. Valid options include \"OpenVR (Vive)\" and \"Oculus Rift\"", + "string"); + QCommandLineOption suppressSettingsResetOption("suppress-settings-reset", + "Suppress the prompt to reset interface settings."); + QCommandLineOption oculusStoreOption("oculus-store", + "Let the Oculus plugin know if interface was run from the Oculus Store."); + QCommandLineOption standaloneOption("standalone", "Emulate a standalone device."); + QCommandLineOption disableWatchdogOption("disableWatchdog", + "Disable the watchdog thread. The interface will crash on deadlocks."); + QCommandLineOption systemCursorOption("system-cursor", "Use the default system cursor."); + QCommandLineOption + concurrentDownloadsOption("concurrent-downloads", + "Maximum concurrent resource downloads. Default is 16, except for Android where it is 4.", + "integer"); + QCommandLineOption avatarURLOption("avatarURL", "Override the avatar U.R.L.", "url"); + QCommandLineOption replaceAvatarURLOption("replaceAvatarURL", + "Replaces the avatar U.R.L. When used with --avatarURL, this takes precedence.", + "url"); + QCommandLineOption + setBookmarkOption("setBookmark", + "Set bookmark as key=value pair. Including the '=' symbol in either string is unsupported.", + "string"); + QCommandLineOption forceCrashReportingOption("forceCrashReporting", "Force crash reporting to initialize."); // The documented "--disable-lod" does not seem to exist. // Below are undocumented. - QCommandLineOption noLauncherOption( - "no-launcher", - "Supposedly does something for the server, unrelated to the application launcher. The feature may never have been implemented." - ); - QCommandLineOption overrideScriptsPathOption( - "overrideScriptsPath", - "Specifies path to default directory where the application will look for scripts to load.", - "string" - ); - QCommandLineOption defaultScriptsOverrideOption( - "defaultScriptsOverride", - "Override default script to run automatically on start. Default is \"defaultsScripts.js\".", - "string" - ); - QCommandLineOption responseTokensOption( - "tokens", - "Set response tokens .", - "json" - ); - QCommandLineOption displayNameOption( - "displayName", - "Set user display name .", - "string" - ); - QCommandLineOption noLoginOption( - "no-login-suggestion", - "Do not show log-in dialogue." - ); - QCommandLineOption traceFileOption( - "traceFile", - "Writes a trace to a file in the documents folder. Only works if \"--traceDuration\" is specified.", - "path" - ); - QCommandLineOption traceDurationOption( - "traceDuration", - "Automatically quit interface after duration. Only works if \"--traceFile\" is specified.", - "seconds" - ); - QCommandLineOption clockSkewOption( - "clockSkew", - "Forces client instance's clock to skew for demonstration purposes.", - "integer" - ); // This should probably be removed. - QCommandLineOption testScriptOption( - "testScript", - "Undocumented. Accepts parameter as U.R.L.", - "string" - ); - QCommandLineOption testResultsLocationOption( - "testResultsLocation", - "Undocumented", - "path" - ); + QCommandLineOption noLauncherOption("no-launcher", + "Supposedly does something for the server, unrelated to the application launcher. The " + "feature may never have been implemented."); + QCommandLineOption + overrideScriptsPathOption("overrideScriptsPath", + "Specifies path to default directory where the application will look for scripts to load.", + "string"); + QCommandLineOption defaultScriptsOverrideOption("defaultScriptsOverride", + "Override default script to run automatically on start. Default is " + "\"defaultsScripts.js\".", + "string"); + QCommandLineOption responseTokensOption("tokens", "Set response tokens .", "json"); + QCommandLineOption displayNameOption("displayName", "Set user display name .", "string"); + QCommandLineOption noLoginOption("no-login-suggestion", "Do not show log-in dialogue."); + QCommandLineOption + traceFileOption("traceFile", + "Writes a trace to a file in the documents folder. Only works if \"--traceDuration\" is specified.", + "path"); + QCommandLineOption + traceDurationOption("traceDuration", + "Automatically quit interface after duration. Only works if \"--traceFile\" is specified.", + "seconds"); + QCommandLineOption clockSkewOption("clockSkew", "Forces client instance's clock to skew for demonstration purposes.", + "integer"); // This should probably be removed. + QCommandLineOption testScriptOption("testScript", "Undocumented. Accepts parameter as U.R.L.", "string"); + QCommandLineOption testResultsLocationOption("testResultsLocation", "Undocumented", "path"); QCommandLineOption quitWhenFinishedOption( - "quitWhenFinished", - "Only works if \"--testScript\" is provided." - ); // Should probably also be made to work on testResultsLocationOption. - QCommandLineOption fastHeartbeatOption( - "fast-heartbeat", - "Change stats polling interval from 10000ms to 1000ms." - ); - QCommandLineOption logOption( - "logOptions", - "Logging options, comma separated: color,nocolor,process_id,thread_id,milliseconds,keep_repeats,journald,nojournald", - "options" - ); + "quitWhenFinished", + "Only works if \"--testScript\" is provided."); // Should probably also be made to work on testResultsLocationOption. + QCommandLineOption fastHeartbeatOption("fast-heartbeat", "Change stats polling interval from 10000ms to 1000ms."); + QCommandLineOption logOption("logOptions", + "Logging options, comma separated: " + "color,nocolor,process_id,thread_id,milliseconds,keep_repeats,journald,nojournald", + "options"); // "--qmljsdebugger", which appears in output from "--help-all". // Those below don't seem to be optional. // --ignore-gpu-blacklist @@ -296,7 +205,7 @@ int main(int argc, const char* argv[]) { { QCoreApplication tempApp(argc, const_cast(argv)); - parser.process(QCoreApplication::arguments()); // Must be run after QCoreApplication is initalised. + parser.process(QCoreApplication::arguments()); // Must be run after QCoreApplication is initalised. #ifdef Q_OS_OSX if (QFileInfo::exists(QCoreApplication::applicationDirPath() + "/../../../config.json")) { @@ -310,10 +219,10 @@ int main(int argc, const char* argv[]) { } // We want to configure the logging system as early as possible - auto &logHandler = LogHandler::getInstance(); + auto& logHandler = LogHandler::getInstance(); if (parser.isSet(logOption)) { if (!logHandler.parseOptions(parser.value(logOption).toUtf8(), logOption.names().first())) { - QCoreApplication mockApp(argc, const_cast(argv)); // required for call to showHelp() + QCoreApplication mockApp(argc, const_cast(argv)); // required for call to showHelp() parser.showHelp(); Q_UNREACHABLE(); } @@ -325,7 +234,7 @@ int main(int argc, const char* argv[]) { Q_UNREACHABLE(); } if (parser.isSet(helpOption)) { - QCoreApplication mockApp(argc, const_cast(argv)); // required for call to showHelp() + QCoreApplication mockApp(argc, const_cast(argv)); // required for call to showHelp() parser.showHelp(); Q_UNREACHABLE(); } @@ -378,7 +287,7 @@ int main(int argc, const char* argv[]) { // Early check for --traceFile argument auto tracer = DependencyManager::set(); - const char * traceFile = nullptr; + const char* traceFile = nullptr; float traceDuration = 0.0f; if (parser.isSet(traceFileOption)) { traceFile = parser.value(traceFileOption).toStdString().c_str(); @@ -412,6 +321,8 @@ int main(int argc, const char* argv[]) { // Instance UserActivityLogger now that the settings are loaded auto& ual = UserActivityLogger::getInstance(); + auto& ch = CrashHandler::getInstance(); + // once the settings have been loaded, check if we need to flip the default for UserActivityLogger if (!ual.isDisabledSettingSet()) { // the user activity logger is opt-out for Interface @@ -423,13 +334,12 @@ int main(int argc, const char* argv[]) { if (parser.isSet(forceCrashReportingOption)) { qInfo() << "Crash reporting enabled on the command-line"; - ual.setCrashReportingEnabled(true); + ch.setEnabled(true); } - auto crashHandlerStarted = startCrashHandler(argv[0]); + auto crashHandlerStarted = ch.start(argv[0]); if (crashHandlerStarted) { - ual.setCrashMonitorStarted(true); - qDebug() << "Crash handler started:" << crashHandlerStarted; + qDebug() << "Crash handler started"; } else { qWarning() << "Crash handler failed to start"; } @@ -472,9 +382,9 @@ int main(int argc, const char* argv[]) { if (socket.waitForConnected(LOCAL_SERVER_TIMEOUT_MS)) { if (parser.isSet(urlOption)) { QUrl url = QUrl(parser.value(urlOption)); - if (url.isValid() && (url.scheme() == URL_SCHEME_OVERTE || url.scheme() == URL_SCHEME_OVERTEAPP - || url.scheme() == HIFI_URL_SCHEME_HTTP || url.scheme() == HIFI_URL_SCHEME_HTTPS - || url.scheme() == HIFI_URL_SCHEME_FILE)) { + if (url.isValid() && (url.scheme() == URL_SCHEME_OVERTE || url.scheme() == URL_SCHEME_OVERTEAPP || + url.scheme() == HIFI_URL_SCHEME_HTTP || url.scheme() == HIFI_URL_SCHEME_HTTPS || + url.scheme() == HIFI_URL_SCHEME_FILE)) { qDebug() << "Writing URL to local socket"; socket.write(url.toString().toUtf8()); if (!socket.waitForBytesWritten(5000)) { @@ -495,7 +405,6 @@ int main(int argc, const char* argv[]) { #endif } - // FIXME this method of checking the OpenGL version screws up the `QOpenGLContext::globalShareContext()` value, which in turn // leads to crashes when creating the real OpenGL instance. Disabling for now until we come up with a better way of checking // the GL version on the system without resorting to creating a full Qt application @@ -522,7 +431,6 @@ int main(int argc, const char* argv[]) { } #endif - // Debug option to demonstrate that the client's local time does not // need to be in sync with any other network node. This forces clock // skew for the individual client @@ -590,7 +498,8 @@ int main(int argc, const char* argv[]) { #if defined(Q_OS_LINUX) app.setWindowIcon(QIcon(PathUtils::resourcesPath() + "images/brand-logo.svg")); #endif - startCrashHookMonitor(&app); + ch.startMonitor(&app); + QTimer exitTimer; if (traceDuration > 0.0f) { @@ -618,14 +527,14 @@ int main(int argc, const char* argv[]) { #endif // Setup local server - QLocalServer server { &app }; + QLocalServer server{ &app }; // We failed to connect to a local server, so we remove any existing servers. server.removeServer(applicationName); server.listen(applicationName); - QObject::connect(&server, &QLocalServer::newConnection, - &app, &Application::handleLocalServerConnection, Qt::DirectConnection); + QObject::connect(&server, &QLocalServer::newConnection, &app, &Application::handleLocalServerConnection, + Qt::DirectConnection); printSystemInformation(); diff --git a/libraries/monitoring/CMakeLists.txt b/libraries/monitoring/CMakeLists.txt index cb848c956a..2800a2379b 100644 --- a/libraries/monitoring/CMakeLists.txt +++ b/libraries/monitoring/CMakeLists.txt @@ -5,13 +5,6 @@ link_hifi_libraries(shared networking) add_crashpad() target_breakpad() - -if (UNIX AND NOT APPLE) - set(THREADS_PREFER_PTHREAD_FLAG ON) - find_package(Threads REQUIRED) - target_link_libraries(Threads::Threads) -endif() - if (WIN32) add_compile_definitions(_USE_MATH_DEFINES) endif() diff --git a/libraries/monitoring/src/crash-handler/CrashHandler.cpp b/libraries/monitoring/src/crash-handler/CrashHandler.cpp new file mode 100644 index 0000000000..3345bc61a7 --- /dev/null +++ b/libraries/monitoring/src/crash-handler/CrashHandler.cpp @@ -0,0 +1,61 @@ +// +// CrashHandler.cpp +// +// +// Created by Dale Glass on 25/06/2023. +// Copyright 2023 Overte e.V. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +#include "CrashHandler.h" +#include "CrashHandlerBackend.h" + + +CrashHandler& CrashHandler::getInstance() { + static CrashHandler sharedInstance; + return sharedInstance; +} + +bool CrashHandler::start(const QString &path) { + if (isStarted()) { + qCWarning(crash_handler) << "Crash handler already started"; + return false; + } + + auto started = startCrashHandler(path.toStdString()); + setStarted(started); + + if ( started ) { + qCInfo(crash_handler) << "Crash handler started"; + } else { + qCWarning(crash_handler) << "Crash handler failed to start"; + } + + return started; +} + +void CrashHandler::startMonitor(QCoreApplication *app) { + startCrashHookMonitor(app); +} + +void CrashHandler::setEnabled(bool enabled) { + if (enabled != _crashReportingEnabled.get()) { + _crashReportingEnabled.set(enabled); + + setCrashReportingEnabled(enabled); + } +} + +void CrashHandler::setAnnotation(const std::string &key, const char *value) { + setCrashAnnotation(key, std::string(value)); +} + +void CrashHandler::setAnnotation(const std::string &key, const QString &value) { + setCrashAnnotation(key, value.toStdString()); +} + +void CrashHandler::setAnnotation(const std::string &key, const std::string &value) { + setCrashAnnotation(key, value); +} \ No newline at end of file diff --git a/libraries/monitoring/src/crash-handler/CrashHandler.h b/libraries/monitoring/src/crash-handler/CrashHandler.h index a22a614fc5..9433dc80c3 100644 --- a/libraries/monitoring/src/crash-handler/CrashHandler.h +++ b/libraries/monitoring/src/crash-handler/CrashHandler.h @@ -1,27 +1,107 @@ // -// CrashHandler.h -// interface/src +// CrashHandler.cpp // -// Created by Clement Brisset on 01/19/18. -// Copyright 2018 High Fidelity, Inc. +// +// Created by Dale Glass on 25/06/2023. +// Copyright 2023 Overte e.V. // // Distributed under the Apache License, Version 2.0. // See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html // -#ifndef hifi_CrashHandler_h -#define hifi_CrashHandler_h +#pragma once -#include +#include #include -#include - -Q_DECLARE_LOGGING_CATEGORY(crash_handler) - -bool startCrashHandler(std::string appPath); -void setCrashAnnotation(std::string name, std::string value); -void startCrashHookMonitor(QCoreApplication* app); -void setCrashReportingEnabled(bool value); +#include + + + + +/** + * @brief The global object in charge of setting up and controlling crash reporting. + * + * This object initializes and talks to crash reporting backends. + * + */ +class CrashHandler : public QObject { + Q_OBJECT + +public: + static CrashHandler& getInstance(); + +public slots: + + /** + * @brief Start the crash handler + * + * @param path Database path + * @return true Started successfully + * @return false Failed to start + */ + bool start(const QString &path); + + + void startMonitor(QCoreApplication *app); + + + + /** + * @brief Whether the crash monitor has been successfully started + * + * Reasons for it failing to start include: + * + * * Not having a crash reporter for the platform + * * Crash reporter not being configured with reporting URLs (CMAKE_BACKTRACE_TOKEN and CMAKE_BACKTRACE_URL) + * * Crash reporter is present and configured, but failed to initialize for some reason + * + * @return true Crash reporter is present, configured and working. + * @return false Crash reporter has not been started for one of the above reasons. + */ + bool isStarted() const { return _crashMonitorStarted; } + + + /** + * @brief Whether the crash monitor will report crashes if they occur + * + * This setting is independent of isCrashMonitorStarted() -- crash reporting may be enabled but fail to work + * due to the crash reporting component being missing or failing to initialize. + * + * @return true Crashes will be reported to CMAKE_BACKTRACE_URL + * @return false Crashes will not be reported + */ + bool isEnabled() const { return _crashReportingEnabled.get(); } + + /** + * @brief Set whether we want to submit crash reports to the report server + * + * The report server is configured with CMAKE_BACKTRACE_URL. + * Emits crashReportingEnabledChanged signal. + * + * @param enabled Whether it's enabled. + */ + void setEnabled(bool enabled); + + + void setAnnotation(const std::string &key, const char *value); + void setAnnotation(const std::string &key, const QString &value); + void setAnnotation(const std::string &key, const std::string &value); + + +private: + /** + * @brief Marks the crash monitor as started + * + * @warning Only to be used as part of the startup process + * + * @param started + */ + void setStarted(bool started) { _crashMonitorStarted = started; } + + + + Setting::Handle _crashReportingEnabled { "CrashReportingEnabled", false }; + bool _crashMonitorStarted {false}; +}; -#endif // hifi_CrashHandler_h diff --git a/libraries/monitoring/src/crash-handler/CrashHandlerBackend.h b/libraries/monitoring/src/crash-handler/CrashHandlerBackend.h new file mode 100644 index 0000000000..f0d7e31b43 --- /dev/null +++ b/libraries/monitoring/src/crash-handler/CrashHandlerBackend.h @@ -0,0 +1,27 @@ +// +// CrashHandler.h +// interface/src +// +// Created by Clement Brisset on 01/19/18. +// Copyright 2018 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +#ifndef hifi_CrashHandlerBackend_h +#define hifi_CrashHandlerBackend_h + +#include +#include +#include + +Q_DECLARE_LOGGING_CATEGORY(crash_handler) + +bool startCrashHandler(std::string appPath); +void setCrashAnnotation(std::string name, std::string value); +void startCrashHookMonitor(QCoreApplication* app); +void setCrashReportingEnabled(bool value); + + +#endif // hifi_CrashHandlerBackend_h diff --git a/libraries/monitoring/src/crash-handler/CrashHandler_Breakpad.cpp b/libraries/monitoring/src/crash-handler/CrashHandlerBackend_Breakpad.cpp similarity index 100% rename from libraries/monitoring/src/crash-handler/CrashHandler_Breakpad.cpp rename to libraries/monitoring/src/crash-handler/CrashHandlerBackend_Breakpad.cpp diff --git a/libraries/monitoring/src/crash-handler/CrashHandler_Crashpad.cpp b/libraries/monitoring/src/crash-handler/CrashHandlerBackend_Crashpad.cpp similarity index 98% rename from libraries/monitoring/src/crash-handler/CrashHandler_Crashpad.cpp rename to libraries/monitoring/src/crash-handler/CrashHandlerBackend_Crashpad.cpp index 5b39a5f88a..be479a2be3 100644 --- a/libraries/monitoring/src/crash-handler/CrashHandler_Crashpad.cpp +++ b/libraries/monitoring/src/crash-handler/CrashHandlerBackend_Crashpad.cpp @@ -421,12 +421,12 @@ bool startCrashHandler(std::string appPath) { } // Enable automated uploads. - QObject::connect(&UserActivityLogger::getInstance(), &UserActivityLogger::crashReportingEnabledChanged, []() { - auto &ual = UserActivityLogger::getInstance(); - setCrashReportingEnabled(ual.isCrashReportingEnabled()); - }); + // QObject::connect(&UserActivityLogger::getInstance(), &UserActivityLogger::crashReportingEnabledChanged, []() { + // auto &ual = UserActivityLogger::getInstance(); + // setCrashReportingEnabled(ual.isCrashReportingEnabled()); + // }); - crashpadDatabase->GetSettings()->SetUploadsEnabled(UserActivityLogger::getInstance().isCrashReportingEnabled()); + crashpadDatabase->GetSettings()->SetUploadsEnabled(CrashHandler::getInstance().isEnabled()); if (!client->StartHandler(handler, db, db, BACKTRACE_URL, annotations, arguments, true, true)) { diff --git a/libraries/monitoring/src/crash-handler/CrashHandler_None.cpp b/libraries/monitoring/src/crash-handler/CrashHandlerBackend_None.cpp similarity index 100% rename from libraries/monitoring/src/crash-handler/CrashHandler_None.cpp rename to libraries/monitoring/src/crash-handler/CrashHandlerBackend_None.cpp diff --git a/libraries/monitoring/src/activity-logger/UserActivityLogger.cpp b/libraries/networking/src/UserActivityLogger.cpp similarity index 96% rename from libraries/monitoring/src/activity-logger/UserActivityLogger.cpp rename to libraries/networking/src/UserActivityLogger.cpp index 96a1625dda..d506e0c549 100644 --- a/libraries/monitoring/src/activity-logger/UserActivityLogger.cpp +++ b/libraries/networking/src/UserActivityLogger.cpp @@ -82,15 +82,6 @@ void UserActivityLogger::requestError(QNetworkReply* errorReply) { qCDebug(networking) << errorReply->error() << "-" << errorReply->errorString(); } -void UserActivityLogger::setCrashReportingEnabled(bool enabled) { - bool old = _crashReportingEnabled.get(); - _crashReportingEnabled.set(enabled); - - if (old != enabled) { - emit crashReportingEnabledChanged(); - } - } - void UserActivityLogger::launch(QString applicationVersion, bool previousSessionCrashed, int previousSessionRuntime) { const QString ACTION_NAME = "launch"; QJsonObject actionDetails; diff --git a/libraries/monitoring/src/activity-logger/UserActivityLogger.h b/libraries/networking/src/UserActivityLogger.h similarity index 52% rename from libraries/monitoring/src/activity-logger/UserActivityLogger.h rename to libraries/networking/src/UserActivityLogger.h index 251468b0d3..9858316806 100644 --- a/libraries/monitoring/src/activity-logger/UserActivityLogger.h +++ b/libraries/networking/src/UserActivityLogger.h @@ -36,51 +36,6 @@ public slots: bool isDisabledSettingSet() const { return _disabled.isSet(); } - /** - * @brief Whether the crash monitor has been successfully started - * - * Reasons for it failing to start include: - * - * * Not having a crash reporter for the platform - * * Crash reporter not being configured with reporting URLs (CMAKE_BACKTRACE_TOKEN and CMAKE_BACKTRACE_URL) - * * Crash reporter is present and configured, but failed to initialize for some reason - * - * @return true Crash reporter is present, configured and working. - * @return false Crash reporter has not been started for one of the above reasons. - */ - bool isCrashMonitorStarted() const { return _crashMonitorStarted; } - - - /** - * @brief Whether the crash monitor will report crashes if they occur - * - * This setting is independent of isCrashMonitorStarted() -- crash reporting may be enabled but fail to work - * due to the crash reporting component being missing or failing to initialize. - * - * @return true Crashes will be reported to CMAKE_BACKTRACE_URL - * @return false Crashes will not be reported - */ - bool isCrashReportingEnabled() { return _crashReportingEnabled.get(); } - - /** - * @brief Marks the crash monitor as started - * - * @warning Only to be used as part of the startup process - * - * @param started - */ - void setCrashMonitorStarted(bool started) { _crashMonitorStarted = started; } - - /** - * @brief Set whether we want to submit crash reports to the report server - * - * The report server is configured with CMAKE_BACKTRACE_URL. - * Emits crashReportingEnabledChanged signal. - * - * @param enabled Whether it's enabled. - */ - void setCrashReportingEnabled(bool enabled); - void disable(bool disable); void logAction(QString action, QJsonObject details = QJsonObject(), JSONCallbackParameters params = JSONCallbackParameters()); @@ -111,9 +66,9 @@ private slots: private: UserActivityLogger(); Setting::Handle _disabled { "UserActivityLoggerDisabled", true }; - Setting::Handle _crashReportingEnabled { "CrashReportingEnabled", false }; - bool _crashMonitorStarted {false}; + + QElapsedTimer _timer; }; From 2cfac3a8968c66cced8cbe0a8295a5bc2f0266a2 Mon Sep 17 00:00:00 2001 From: Dale Glass Date: Mon, 26 Jun 2023 01:02:22 +0200 Subject: [PATCH 03/12] Make crash reporting work on domain-server --- domain-server/CMakeLists.txt | 5 ++++- domain-server/src/DomainServer.cpp | 8 ++++++++ domain-server/src/DomainServer.h | 4 ++++ domain-server/src/main.cpp | 14 ++++++++++++++ interface/src/main.cpp | 2 ++ 5 files changed, 32 insertions(+), 1 deletion(-) diff --git a/domain-server/CMakeLists.txt b/domain-server/CMakeLists.txt index 49c9045cfd..2327762610 100644 --- a/domain-server/CMakeLists.txt +++ b/domain-server/CMakeLists.txt @@ -30,12 +30,15 @@ symlink_or_copy_directory_beside_target(${_SHOULD_SYMLINK_RESOURCES} "${CMAKE_CU include_hifi_library_headers(gpu) include_hifi_library_headers(graphics) include_hifi_library_headers(script-engine) -link_hifi_libraries(embedded-webserver networking shared avatars octree) +link_hifi_libraries(embedded-webserver networking shared avatars octree monitoring) target_zlib() target_quazip() target_openssl() +add_crashpad() +target_breakpad() + # libcrypto uses dlopen in libdl if (UNIX) target_link_libraries(${TARGET_NAME} ${CMAKE_DL_LIBS}) diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 62d6572ae3..4164133058 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -85,6 +85,8 @@ QUuid DomainServer::_overridingDomainID; bool DomainServer::_getTempName { false }; QString DomainServer::_userConfigFilename; int DomainServer::_parentPID { -1 }; +bool DomainServer::_forceCrashReporting{false}; + /// @brief The Domain server can proxy requests to the Directory Server, this function handles those forwarding requests. /// @param connection The HTTP connection object. @@ -417,6 +419,8 @@ void DomainServer::parseCommandLine(int argc, char* argv[]) { const QCommandLineOption logOption("logOptions", "Logging options, comma separated: color,nocolor,process_id,thread_id,milliseconds,keep_repeats,journald,nojournald", "options"); parser.addOption(logOption); + const QCommandLineOption forceCrashReportingOption("forceCrashReporting", "Force crash reporting to initialize."); + parser.addOption(forceCrashReportingOption); QStringList arguments; for (int i = 0; i < argc; ++i) { @@ -488,6 +492,10 @@ void DomainServer::parseCommandLine(int argc, char* argv[]) { qDebug() << "Parent process PID is" << _parentPID; } } + + if (parser.isSet(forceCrashReportingOption)) { + _forceCrashReporting = true; + } } DomainServer::~DomainServer() { diff --git a/domain-server/src/DomainServer.h b/domain-server/src/DomainServer.h index 38cc63b1da..a22671916b 100644 --- a/domain-server/src/DomainServer.h +++ b/domain-server/src/DomainServer.h @@ -83,6 +83,8 @@ public: void screensharePresence(QString roomname, QUuid avatarID, int expiration_seconds = 0); + static bool forceCrashReporting() { return _forceCrashReporting; } + public slots: /// Called by NodeList to inform us a node has been added void nodeAdded(SharedNodePointer node); @@ -311,6 +313,8 @@ private: static bool _getTempName; static QString _userConfigFilename; static int _parentPID; + static bool _forceCrashReporting; + bool _sendICEServerAddressToMetaverseAPIInProgress { false }; bool _sendICEServerAddressToMetaverseAPIRedo { false }; diff --git a/domain-server/src/main.cpp b/domain-server/src/main.cpp index ba12349347..640643d9b3 100644 --- a/domain-server/src/main.cpp +++ b/domain-server/src/main.cpp @@ -21,6 +21,8 @@ #include #include "DomainServer.h" +#include + int main(int argc, char* argv[]) { setupHifiApplication(BuildInfo::DOMAIN_SERVER_NAME); @@ -32,9 +34,21 @@ int main(int argc, char* argv[]) { int currentExitCode = 0; // use a do-while to handle domain-server restart + auto &ch = CrashHandler::getInstance(); + ch.start(argv[0]); + + if ( DomainServer::forceCrashReporting() ) { + ch.setEnabled(true); + } + + ch.setAnnotation("program", "domain-server"); + + do { crash::annotations::setShutdownState(false); DomainServer domainServer(argc, argv); + ch.startMonitor(&domainServer); + currentExitCode = domainServer.exec(); } while (currentExitCode == DomainServer::EXIT_CODE_REBOOT); diff --git a/interface/src/main.cpp b/interface/src/main.cpp index 6baa338f92..3931fb97cf 100644 --- a/interface/src/main.cpp +++ b/interface/src/main.cpp @@ -344,6 +344,8 @@ int main(int argc, const char* argv[]) { qWarning() << "Crash handler failed to start"; } + ch.setAnnotation("program", "interface"); + const QString& applicationName = getInterfaceSharedMemoryName(); bool instanceMightBeRunning = true; #ifdef Q_OS_WIN From e7fb5049eca67509cf8c6e8d32c8b43c2c69efc3 Mon Sep 17 00:00:00 2001 From: Dale Glass Date: Tue, 27 Jun 2023 00:08:04 +0200 Subject: [PATCH 04/12] Add crash reporting to assignment client --- assignment-client/CMakeLists.txt | 5 ++++- assignment-client/src/AssignmentClientApp.cpp | 14 ++++++++++++++ assignment-client/src/main.cpp | 12 +++++++++++- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/assignment-client/CMakeLists.txt b/assignment-client/CMakeLists.txt index 9b0d0c21c0..88a6f9dc7f 100644 --- a/assignment-client/CMakeLists.txt +++ b/assignment-client/CMakeLists.txt @@ -19,10 +19,13 @@ link_hifi_libraries( audio avatars octree gpu graphics shaders model-serializers hfm entities networking animation recording shared script-engine embedded-webserver controllers physics plugins midi image - material-networking model-networking ktx shaders + material-networking model-networking ktx shaders monitoring ) include_hifi_library_headers(procedural) +add_crashpad() +target_breakpad() + if (BUILD_TOOLS) add_dependencies(${TARGET_NAME} oven) diff --git a/assignment-client/src/AssignmentClientApp.cpp b/assignment-client/src/AssignmentClientApp.cpp index 7a983657fe..088b5304ef 100644 --- a/assignment-client/src/AssignmentClientApp.cpp +++ b/assignment-client/src/AssignmentClientApp.cpp @@ -24,6 +24,8 @@ #include #include #include +#include + #include "Assignment.h" #include "AssignmentClient.h" @@ -106,6 +108,9 @@ AssignmentClientApp::AssignmentClientApp(int argc, char* argv[]) : const QCommandLineOption logOption("logOptions", "Logging options, comma separated: color,nocolor,process_id,thread_id,milliseconds,keep_repeats,journald,nojournald", "options"); parser.addOption(logOption); + const QCommandLineOption forceCrashReportingOption("forceCrashReporting", "Force crash reporting to initialize."); + parser.addOption(forceCrashReportingOption); + if (!parser.parse(QCoreApplication::arguments())) { std::cout << parser.errorText().toStdString() << std::endl; // Avoid Qt log spam parser.showHelp(); @@ -174,6 +179,7 @@ AssignmentClientApp::AssignmentClientApp(int argc, char* argv[]) : disableDomainPortAutoDiscovery = true; } + Assignment::Type requestAssignmentType = Assignment::AllTypes; if (argumentVariantMap.contains(ASSIGNMENT_TYPE_OVERRIDE_OPTION)) { requestAssignmentType = (Assignment::Type) argumentVariantMap.value(ASSIGNMENT_TYPE_OVERRIDE_OPTION).toInt(); @@ -182,6 +188,9 @@ AssignmentClientApp::AssignmentClientApp(int argc, char* argv[]) : requestAssignmentType = (Assignment::Type) parser.value(clientTypeOption).toInt(); } + auto &ch = CrashHandler::getInstance(); + ch.setAnnotation("type", QString::number(requestAssignmentType)); + QString assignmentPool; // check for an assignment pool passed on the command line or in the config if (argumentVariantMap.contains(ASSIGNMENT_POOL_OPTION)) { @@ -247,6 +256,11 @@ AssignmentClientApp::AssignmentClientApp(int argc, char* argv[]) : } } + if (parser.isSet(forceCrashReportingOption)) { + auto &ch = CrashHandler::getInstance(); + ch.setEnabled(true); + } + QThread::currentThread()->setObjectName("main thread"); LogHandler::getInstance().moveToThread(thread()); diff --git a/assignment-client/src/main.cpp b/assignment-client/src/main.cpp index 971e9ed272..93040734ef 100644 --- a/assignment-client/src/main.cpp +++ b/assignment-client/src/main.cpp @@ -13,12 +13,22 @@ #include #include "AssignmentClientApp.h" +#include + int main(int argc, char* argv[]) { setupHifiApplication(BuildInfo::ASSIGNMENT_CLIENT_NAME); + auto &ch = CrashHandler::getInstance(); + ch.start(argv[0]); + ch.setAnnotation("program", "assignment-client"); + + + AssignmentClientApp app(argc, argv); - + ch.startMonitor(&app); + + int acReturn = app.exec(); qDebug() << "assignment-client process" << app.applicationPid() << "exiting with status code" << acReturn; From f218e54eac6b52652203625a783429b5cdd8a140 Mon Sep 17 00:00:00 2001 From: Dale Glass Date: Tue, 27 Jun 2023 00:33:18 +0200 Subject: [PATCH 05/12] Add crash settings to web UI --- domain-server/resources/describe-settings.json | 15 +++++++++++++++ domain-server/src/DomainServer.cpp | 8 ++++++++ 2 files changed, 23 insertions(+) diff --git a/domain-server/resources/describe-settings.json b/domain-server/resources/describe-settings.json index d4a4924ff5..1087b6ae31 100644 --- a/domain-server/resources/describe-settings.json +++ b/domain-server/resources/describe-settings.json @@ -73,6 +73,21 @@ } ] }, + { + "label": "Networking / Crash Reporting", + "name": "crash_reporting", + "restart": false, + "settings": [ + { + "name": "enable_crash_reporter", + "label": "Enable Crash Reporter", + "help": "Enable the automatic submission of crash reports to Overte e.V. This will help us find bugs and improve the code. Crash data will only be shared with developers trusted by Overte e.V., and will only be used to aid in development.", + "default": false, + "type": "checkbox", + "advanced": true + } + ] + }, { "name": "webrtc", "label": "Networking / WebRTC", diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index 4164133058..89c2bd0bb0 100644 --- a/domain-server/src/DomainServer.cpp +++ b/domain-server/src/DomainServer.cpp @@ -61,6 +61,8 @@ #include #include +#include + using namespace std::chrono; @@ -199,6 +201,8 @@ DomainServer::DomainServer(int argc, char* argv[]) : _httpManager(QHostAddress::AnyIPv4, DOMAIN_SERVER_HTTP_PORT, QString("%1/resources/web/").arg(QCoreApplication::applicationDirPath()), this) { + static const QString CRASH_REPORTER = "crash_reporting.enable_crash_reporter"; + if (_parentPID != -1) { watchParentProcess(_parentPID); } @@ -239,12 +243,16 @@ DomainServer::DomainServer(int argc, char* argv[]) : _settingsManager.setupConfigMap(userConfigFilename); // setup a shutdown event listener to handle SIGTERM or WM_CLOSE for us + #ifdef _WIN32 installNativeEventFilter(&ShutdownEventListener::getInstance()); #else ShutdownEventListener::getInstance(); #endif + auto &ch = CrashHandler::getInstance(); + ch.setEnabled(_settingsManager.valueOrDefaultValueForKeyPath(CRASH_REPORTER).toBool()); + qRegisterMetaType("DomainServerWebSessionData"); qRegisterMetaTypeStreamOperators("DomainServerWebSessionData"); From 25755f9c88ed5a4957d746ab88a26d3ccc8d0086 Mon Sep 17 00:00:00 2001 From: Dale Glass Date: Sun, 2 Jul 2023 21:18:09 +0200 Subject: [PATCH 06/12] Many improvements: * Get crash settings in assignment clients * Allow custom crash reporting URL and token * Fix setting system -- the client's one doesn't belong in the server * Lots more documentation Commit just before moving things around. --- assignment-client/src/assets/AssetServer.cpp | 2 + assignment-client/src/audio/AudioMixer.cpp | 4 + assignment-client/src/avatars/AvatarMixer.cpp | 2 + .../src/entities/EntityServer.cpp | 10 +- assignment-client/src/main.cpp | 7 +- .../src/messages/MessagesMixer.cpp | 2 + assignment-client/src/octree/OctreeServer.cpp | 2 + .../src/scripts/EntityScriptServer.cpp | 11 +- .../resources/describe-settings.json | 19 ++- .../src/DomainServerSettingsManager.cpp | 6 +- .../src/DomainServerSettingsManager.h | 48 ++++++- domain-server/src/main.cpp | 2 +- interface/src/main.cpp | 14 +- libraries/monitoring/CMakeLists.txt | 2 +- .../src/crash-handler/CrashHandler.cpp | 62 ++++++++- .../src/crash-handler/CrashHandler.h | 130 +++++++++++++++++- .../src/crash-handler/CrashHandlerBackend.h | 2 +- .../CrashHandlerBackend_Breakpad.cpp | 2 +- .../CrashHandlerBackend_Crashpad.cpp | 24 +++- .../CrashHandlerBackend_None.cpp | 2 +- libraries/networking/CMakeLists.txt | 2 +- libraries/networking/src/Assignment.cpp | 51 +++++-- libraries/networking/src/Assignment.h | 18 ++- 23 files changed, 361 insertions(+), 63 deletions(-) diff --git a/assignment-client/src/assets/AssetServer.cpp b/assignment-client/src/assets/AssetServer.cpp index ffb6747fd7..731687b637 100644 --- a/assignment-client/src/assets/AssetServer.cpp +++ b/assignment-client/src/assets/AssetServer.cpp @@ -371,6 +371,8 @@ void AssetServer::completeSetup() { auto& domainHandler = nodeList->getDomainHandler(); const QJsonObject& settingsObject = domainHandler.getSettingsObject(); + commonParseSettingsObject(settingsObject); + static const QString ASSET_SERVER_SETTINGS_KEY = "asset_server"; if (!settingsObject.contains(ASSET_SERVER_SETTINGS_KEY)) { diff --git a/assignment-client/src/audio/AudioMixer.cpp b/assignment-client/src/audio/AudioMixer.cpp index 4ba5802026..788dfeab93 100644 --- a/assignment-client/src/audio/AudioMixer.cpp +++ b/assignment-client/src/audio/AudioMixer.cpp @@ -38,6 +38,7 @@ #include "AudioMixerClientData.h" #include "AvatarAudioStream.h" #include "InjectedAudioStream.h" +#include "crash-handler/CrashHandler.h" using namespace std; @@ -49,6 +50,7 @@ static const QString AUDIO_ENV_GROUP_KEY = "audio_env"; static const QString AUDIO_BUFFER_GROUP_KEY = "audio_buffer"; static const QString AUDIO_THREADING_GROUP_KEY = "audio_threading"; + int AudioMixer::_numStaticJitterFrames{ DISABLE_STATIC_JITTER_FRAMES }; float AudioMixer::_noiseMutingThreshold{ DEFAULT_NOISE_MUTING_THRESHOLD }; float AudioMixer::_attenuationPerDoublingInDistance{ DEFAULT_ATTENUATION_PER_DOUBLING_IN_DISTANCE }; @@ -560,6 +562,8 @@ void AudioMixer::clearDomainSettings() { void AudioMixer::parseSettingsObject(const QJsonObject& settingsObject) { qCDebug(audio) << "AVX2 Support:" << (cpuSupportsAVX2() ? "enabled" : "disabled"); + commonParseSettingsObject(settingsObject); + if (settingsObject.contains(AUDIO_THREADING_GROUP_KEY)) { QJsonObject audioThreadingGroupObject = settingsObject[AUDIO_THREADING_GROUP_KEY].toObject(); const QString AUTO_THREADS = "auto_threads"; diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index 330bb0fd1b..7279627daf 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -988,6 +988,8 @@ void AvatarMixer::handlePacketVersionMismatch(PacketType type, const SockAddr& s } void AvatarMixer::parseDomainServerSettings(const QJsonObject& domainSettings) { + commonParseSettingsObject(domainSettings); + const QString AVATAR_MIXER_SETTINGS_KEY = "avatar_mixer"; QJsonObject avatarMixerGroupObject = domainSettings[AVATAR_MIXER_SETTINGS_KEY].toObject(); diff --git a/assignment-client/src/entities/EntityServer.cpp b/assignment-client/src/entities/EntityServer.cpp index 94847e0340..d27a69ff7c 100644 --- a/assignment-client/src/entities/EntityServer.cpp +++ b/assignment-client/src/entities/EntityServer.cpp @@ -272,7 +272,7 @@ int EntityServer::sendSpecialPackets(const SharedNodePointer& node, OctreeQueryN #ifdef EXTRA_ERASE_DEBUGGING if (packetsSent > 0) { - qDebug() << "EntityServer::sendSpecialPackets() sent " << packetsSent << "special packets of " + qDebug() << "EntityServer::sendSpecialPackets() sent " << packetsSent << "special packets of " << totalBytes << " total bytes to node:" << node->getUUID(); } #endif @@ -326,14 +326,14 @@ void EntityServer::readAdditionalConfiguration(const QJsonObject& settingsSectio } else { tree->setEntityScriptSourceWhitelist(""); } - + auto entityEditFilters = DependencyManager::get(); - + QString filterURL; if (readOptionString("entityEditFilter", settingsSectionObject, filterURL) && !filterURL.isEmpty()) { // connect the filterAdded signal, and block edits until you hear back connect(entityEditFilters.data(), &EntityEditFilters::filterAdded, this, &EntityServer::entityFilterAdded); - + entityEditFilters->addFilter(EntityItemID(), filterURL); } } @@ -367,7 +367,7 @@ void EntityServer::nodeKilled(SharedNodePointer node) { // FIXME - this stats tracking is somewhat temporary to debug the Whiteboard issues. It's not a bad // set of stats to have, but we'd probably want a different data structure if we keep it very long. -// Since this version uses a single shared QMap for all senders, there could be some lock contention +// Since this version uses a single shared QMap for all senders, there could be some lock contention // on this QWriteLocker void EntityServer::trackSend(const QUuid& dataID, quint64 dataLastEdited, const QUuid& sessionID) { QWriteLocker locker(&_viewerSendingStatsLock); diff --git a/assignment-client/src/main.cpp b/assignment-client/src/main.cpp index 93040734ef..7b4f12756d 100644 --- a/assignment-client/src/main.cpp +++ b/assignment-client/src/main.cpp @@ -19,13 +19,8 @@ int main(int argc, char* argv[]) { setupHifiApplication(BuildInfo::ASSIGNMENT_CLIENT_NAME); - auto &ch = CrashHandler::getInstance(); - ch.start(argv[0]); - ch.setAnnotation("program", "assignment-client"); - - - AssignmentClientApp app(argc, argv); + auto &ch = CrashHandler::getInstance(); ch.startMonitor(&app); diff --git a/assignment-client/src/messages/MessagesMixer.cpp b/assignment-client/src/messages/MessagesMixer.cpp index 017d3a80b7..74efadf6e1 100644 --- a/assignment-client/src/messages/MessagesMixer.cpp +++ b/assignment-client/src/messages/MessagesMixer.cpp @@ -121,6 +121,8 @@ void MessagesMixer::domainSettingsRequestComplete() { } void MessagesMixer::parseDomainServerSettings(const QJsonObject& domainSettings) { + commonParseSettingsObject(domainSettings); + const QString MESSAGES_MIXER_SETTINGS_KEY = "messages_mixer"; QJsonObject messagesMixerGroupObject = domainSettings[MESSAGES_MIXER_SETTINGS_KEY].toObject(); diff --git a/assignment-client/src/octree/OctreeServer.cpp b/assignment-client/src/octree/OctreeServer.cpp index b70ae63a99..d9a2faa2de 100644 --- a/assignment-client/src/octree/OctreeServer.cpp +++ b/assignment-client/src/octree/OctreeServer.cpp @@ -1023,6 +1023,8 @@ void OctreeServer::readConfiguration() { const QJsonObject& settingsObject = DependencyManager::get()->getDomainHandler().getSettingsObject(); + commonParseSettingsObject(settingsObject); + QString settingsKey = getMyDomainSettingsKey(); QJsonObject settingsSectionObject = settingsObject[settingsKey].toObject(); _settings = settingsSectionObject; // keep this for later diff --git a/assignment-client/src/scripts/EntityScriptServer.cpp b/assignment-client/src/scripts/EntityScriptServer.cpp index d8fb0bed06..b16e4561d6 100644 --- a/assignment-client/src/scripts/EntityScriptServer.cpp +++ b/assignment-client/src/scripts/EntityScriptServer.cpp @@ -149,11 +149,14 @@ void EntityScriptServer::handleEntityScriptGetStatusPacket(QSharedPointer(); auto& domainHandler = nodeList->getDomainHandler(); const QJsonObject& settingsObject = domainHandler.getSettingsObject(); + commonParseSettingsObject(settingsObject); + static const QString ENTITY_SCRIPT_SERVER_SETTINGS_KEY = "entity_script_server"; if (!settingsObject.contains(ENTITY_SCRIPT_SERVER_SETTINGS_KEY)) { @@ -292,7 +295,7 @@ void EntityScriptServer::run() { entityScriptingInterface->init(); _entityViewer.init(); - + // setup the JSON filter that asks for entities with a non-default serverScripts property QJsonObject queryJSONParameters; queryJSONParameters[EntityJSONQueryProperties::SERVER_SCRIPTS_PROPERTY] = EntityQueryFilterSymbol::NonDefault; @@ -303,7 +306,7 @@ void EntityScriptServer::run() { queryFlags[EntityJSONQueryProperties::INCLUDE_DESCENDANTS_PROPERTY] = true; queryJSONParameters[EntityJSONQueryProperties::FLAGS_PROPERTY] = queryFlags; - + // setup the JSON parameters so that OctreeQuery does not use a frustum and uses our JSON filter _entityViewer.getOctreeQuery().setJSONParameters(queryJSONParameters); @@ -380,7 +383,7 @@ void EntityScriptServer::nodeKilled(SharedNodePointer killedNode) { if (!hasAnotherEntityServer) { clear(); } - + break; } case NodeType::Agent: { @@ -584,7 +587,7 @@ void EntityScriptServer::sendStatsPacket() { } scriptEngineStats["number_running_scripts"] = numberRunningScripts; statsObject["script_engine_stats"] = scriptEngineStats; - + auto nodeList = DependencyManager::get(); QJsonObject nodesObject; diff --git a/domain-server/resources/describe-settings.json b/domain-server/resources/describe-settings.json index 1087b6ae31..ebfb519eac 100644 --- a/domain-server/resources/describe-settings.json +++ b/domain-server/resources/describe-settings.json @@ -76,7 +76,8 @@ { "label": "Networking / Crash Reporting", "name": "crash_reporting", - "restart": false, + "restart": true, + "assignment-types": [ 0, 1, 3, 4, 5, 6 ], "settings": [ { "name": "enable_crash_reporter", @@ -85,6 +86,22 @@ "default": false, "type": "checkbox", "advanced": true + }, + { + "name": "custom_crash_url", + "label": "Custom crash URL", + "help": "If this is set, it overrides the internal crash reporting URL. This can be used for instance to direct crash reports to Sentry.", + "default": "", + "type": "string", + "advanced": true + }, + { + "name": "custom_crash_token", + "label": "Custom crash token", + "help": "This is a token that identifies the thing sending a crash report, such as a product name and version number. If not set, the compile time default will be used.", + "default": "", + "type": "string", + "advanced": true } ] }, diff --git a/domain-server/src/DomainServerSettingsManager.cpp b/domain-server/src/DomainServerSettingsManager.cpp index 13ab81fa93..c59fc43d34 100644 --- a/domain-server/src/DomainServerSettingsManager.cpp +++ b/domain-server/src/DomainServerSettingsManager.cpp @@ -63,7 +63,9 @@ const QString SETTINGS_VIEWPOINT_KEY = "viewpoint"; DomainServerSettingsManager::DomainServerSettingsManager() { // load the description object from the settings description - QFile descriptionFile(QCoreApplication::applicationDirPath() + SETTINGS_DESCRIPTION_RELATIVE_PATH); + qDebug() << "Application dir: " << QCoreApplication::applicationDirPath(); + QString descriptionFilePath = QCoreApplication::applicationDirPath() + SETTINGS_DESCRIPTION_RELATIVE_PATH; + QFile descriptionFile(descriptionFilePath); descriptionFile.open(QIODevice::ReadOnly); QJsonParseError parseError; @@ -89,7 +91,7 @@ DomainServerSettingsManager::DomainServerSettingsManager() { static const QString MISSING_SETTINGS_DESC_MSG = QString("Did not find settings description in JSON at %1 - Unable to continue. domain-server will quit.\n%2 at %3") - .arg(SETTINGS_DESCRIPTION_RELATIVE_PATH).arg(parseError.errorString()).arg(parseError.offset); + .arg(descriptionFilePath).arg(parseError.errorString()).arg(parseError.offset); static const int MISSING_SETTINGS_DESC_ERROR_CODE = 6; QMetaObject::invokeMethod(QCoreApplication::instance(), "queuedQuit", Qt::QueuedConnection, diff --git a/domain-server/src/DomainServerSettingsManager.h b/domain-server/src/DomainServerSettingsManager.h index 4c74f2301b..6f23737059 100644 --- a/domain-server/src/DomainServerSettingsManager.h +++ b/domain-server/src/DomainServerSettingsManager.h @@ -50,12 +50,42 @@ enum SettingsType { ContentSettings }; +/** + * @brief Manages the domain-wide settings + * + * The domain server doesn't use the interface's QSettings based system, but this one. + * The domain holds all the settings and distributes it to the connected assignment clients. + * + * Assignment clients request their settings by making a DomainSettingsRequest. The request + * is specific to the client's type. + * + * The response is generated in settingsResponseObjectForType and filtered according to what + * the client is allowed to see. + * + * To add a new setting, add it in resources/describe-settings.json + * + * To make a setting be sent to assignment clients, set the assignment-types value to an array + * of the desired assignment clients. The type is defined numerically. + * + * The canonical list of assignment types is in the Assignment::Type enum. + * + * + * + */ class DomainServerSettingsManager : public QObject { Q_OBJECT public: DomainServerSettingsManager(); bool handleAuthenticatedHTTPRequest(HTTPConnection* connection, const QUrl& url); + + /** + * @brief Loads the configuration from the specified file + * + * Performs version upgrades when we're loading an older version of the config. + * + * @param userConfigFilename + */ void setupConfigMap(const QString& userConfigFilename); // each of the three methods in this group takes a read lock of _settingsLock @@ -126,7 +156,21 @@ public: enum DefaultSettingsInclusion { NoDefaultSettings, IncludeDefaultSettings }; enum SettingsBackupFlag { NotForBackup, ForBackup }; - /// thread safe method to retrieve a JSON representation of settings + /** + * @brief Generates a JSON representation of settings + * + * This is what answers an assignment client's request for domain settings. + * + * @note thread safe + * + * @param typeValue Type of assignment client + * @param authentication + * @param domainSettingsInclusion + * @param contentSettingsInclusion + * @param defaultSettingsInclusion + * @param settingsBackupFlag + * @return QJsonObject + */ QJsonObject settingsResponseObjectForType(const QString& typeValue, SettingsRequestAuthentication authentication = NotAuthenticated, DomainSettingsInclusion domainSettingsInclusion = IncludeDomainSettings, @@ -211,7 +255,7 @@ private: // keep track of answers to api queries about which users are in which groups QHash> _groupMembership; // QHash> - /// guard read/write access from multiple threads to settings + /// guard read/write access from multiple threads to settings QReadWriteLock _settingsLock { QReadWriteLock::Recursive }; friend class DomainServer; diff --git a/domain-server/src/main.cpp b/domain-server/src/main.cpp index 640643d9b3..ef8b68d1a1 100644 --- a/domain-server/src/main.cpp +++ b/domain-server/src/main.cpp @@ -35,7 +35,7 @@ int main(int argc, char* argv[]) { // use a do-while to handle domain-server restart auto &ch = CrashHandler::getInstance(); - ch.start(argv[0]); + ch.setPath(argv[0]); if ( DomainServer::forceCrashReporting() ) { ch.setEnabled(true); diff --git a/interface/src/main.cpp b/interface/src/main.cpp index 3931fb97cf..cb5111f456 100644 --- a/interface/src/main.cpp +++ b/interface/src/main.cpp @@ -323,6 +323,13 @@ int main(int argc, const char* argv[]) { auto& ual = UserActivityLogger::getInstance(); auto& ch = CrashHandler::getInstance(); + QObject::connect(&ch, &CrashHandler::enabledChanged, [](bool enabled) { + Settings s; + s.beginGroup("Crash"); + s.setValue("ReportingEnabled", enabled); + s.endGroup(); + }); + // once the settings have been loaded, check if we need to flip the default for UserActivityLogger if (!ual.isDisabledSettingSet()) { // the user activity logger is opt-out for Interface @@ -337,13 +344,6 @@ int main(int argc, const char* argv[]) { ch.setEnabled(true); } - auto crashHandlerStarted = ch.start(argv[0]); - if (crashHandlerStarted) { - qDebug() << "Crash handler started"; - } else { - qWarning() << "Crash handler failed to start"; - } - ch.setAnnotation("program", "interface"); const QString& applicationName = getInterfaceSharedMemoryName(); diff --git a/libraries/monitoring/CMakeLists.txt b/libraries/monitoring/CMakeLists.txt index 2800a2379b..ee4077ae0e 100644 --- a/libraries/monitoring/CMakeLists.txt +++ b/libraries/monitoring/CMakeLists.txt @@ -1,6 +1,6 @@ set(TARGET_NAME monitoring) setup_hifi_library() -link_hifi_libraries(shared networking) +link_hifi_libraries(shared) add_crashpad() target_breakpad() diff --git a/libraries/monitoring/src/crash-handler/CrashHandler.cpp b/libraries/monitoring/src/crash-handler/CrashHandler.cpp index 3345bc61a7..71ab38ec1a 100644 --- a/libraries/monitoring/src/crash-handler/CrashHandler.cpp +++ b/libraries/monitoring/src/crash-handler/CrashHandler.cpp @@ -11,6 +11,8 @@ #include "CrashHandler.h" #include "CrashHandlerBackend.h" +#include +#include CrashHandler& CrashHandler::getInstance() { @@ -18,13 +20,32 @@ CrashHandler& CrashHandler::getInstance() { return sharedInstance; } -bool CrashHandler::start(const QString &path) { +CrashHandler::CrashHandler(QObject *parent) : QObject(parent) { + +} + + +void CrashHandler::setPath(const QString &path) { + QFileInfo fi(path); + if (isStarted()) { - qCWarning(crash_handler) << "Crash handler already started"; + qCWarning(crash_handler) << "Crash handler already started, too late to set the path."; + } + + if (fi.isFile()) { + _path = fi.absolutePath(); + } else { + _path = path; + } +} + +bool CrashHandler::start() { + if (isStarted()) { + //qCWarning(crash_handler) << "Crash handler already started"; return false; } - auto started = startCrashHandler(path.toStdString()); + auto started = startCrashHandler(_path.toStdString(), _crashUrl.toStdString(), _crashToken.toStdString()); setStarted(started); if ( started ) { @@ -41,21 +62,50 @@ void CrashHandler::startMonitor(QCoreApplication *app) { } void CrashHandler::setEnabled(bool enabled) { - if (enabled != _crashReportingEnabled.get()) { - _crashReportingEnabled.set(enabled); + start(); + if (enabled != _crashReportingEnabled) { + _crashReportingEnabled = enabled; setCrashReportingEnabled(enabled); + + emit enabledChanged(enabled); + } +} + +void CrashHandler::setUrl(const QString &url) { + // This can be called both from the settings system in an assignment client + // and from the commandline parser. We only emit a warning if the commandline + // argument causes the domain setting to be ignored. + + if (isStarted() && url != _crashUrl) { + qCWarning(crash_handler) << "Setting crash reporting URL to " << url << "after the crash handler is already running has no effect"; + } else { + _crashUrl = url; + } +} + +void CrashHandler::setToken(const QString &token) { + if (isStarted() && token != _crashToken) { + qCWarning(crash_handler) << "Setting crash reporting token to " << token << "after the crash handler is already running has no effect"; + } else { + _crashToken = token; } } void CrashHandler::setAnnotation(const std::string &key, const char *value) { + setAnnotation(key, std::string(value)); setCrashAnnotation(key, std::string(value)); } void CrashHandler::setAnnotation(const std::string &key, const QString &value) { - setCrashAnnotation(key, value.toStdString()); + setAnnotation(key, value.toStdString()); } void CrashHandler::setAnnotation(const std::string &key, const std::string &value) { + if (!isStarted()) { + qCWarning(crash_handler) << "Can't set annotation" << QString::fromStdString(key) << "to" << QString::fromStdString(value) << "crash handler not yet started"; + return; + } + setCrashAnnotation(key, value); } \ No newline at end of file diff --git a/libraries/monitoring/src/crash-handler/CrashHandler.h b/libraries/monitoring/src/crash-handler/CrashHandler.h index 9433dc80c3..3f863326ad 100644 --- a/libraries/monitoring/src/crash-handler/CrashHandler.h +++ b/libraries/monitoring/src/crash-handler/CrashHandler.h @@ -21,7 +21,31 @@ /** * @brief The global object in charge of setting up and controlling crash reporting. * - * This object initializes and talks to crash reporting backends. + * This object initializes and talks to crash reporting backends. For those, see + * CrashHandlerBackend.h and the .cpp files that implement that interface. + * + * The crash URL and token can only be passed to the underlying system on start, so + * things should be set up in such a way that startup is only done after those are set. + * + * start() will be automatically called when setEnabled() is called with true. + * setAnnotation() can only be called after start. + * + * + * To use, follow this general pattern in an application: + * + * @code {.cpp} + * auto &ch = CrashHandler::getInstance(); + * ch.setPath(...); + * ch.setUrl("https://server.com/crash-reports"); + * ch.setToken("1.2beta"); + * ch.setEnabled(true); + * ch.setAnnotation("version", "1.3"); // Needs a started handler to work + * @endcode + * + * For an assignment client, there are two potential ways to start, through the command-line + * and through the settings system. Since the path, URL and token only apply on startup, the + * code must be written such that if command arguments are not given, setEnabled() or start() + * are not called until receiving the settings from the domain. * */ class CrashHandler : public QObject { @@ -30,18 +54,42 @@ class CrashHandler : public QObject { public: static CrashHandler& getInstance(); + public slots: + + /** + * @brief Set the directory for the crash reports + * + * This sets the path for writing crash reports. This should be done on application startup. + * + * @param path Directory where to store crash reports. It's allowed to set this to argv[0], + * if the path is a filename, then the base directory will be automatically used. + */ + void setPath(const QString &path); + /** * @brief Start the crash handler * - * @param path Database path + * This is called automatically if it wasn't started yet when setEnabled() is called. + * + * @param path Path where to store the crash database * @return true Started successfully * @return false Failed to start */ - bool start(const QString &path); + bool start(); + /** + * @brief Starts the unhandled exception monitor. + * + * On Windows, it's possible for the unhandled exception handler to be reset. This starts a timer + * to periodically set it back. + * + * On non-Windows systems this has no effect. + * + * @param app Main application + */ void startMonitor(QCoreApplication *app); @@ -70,7 +118,7 @@ public slots: * @return true Crashes will be reported to CMAKE_BACKTRACE_URL * @return false Crashes will not be reported */ - bool isEnabled() const { return _crashReportingEnabled.get(); } + bool isEnabled() const { return _crashReportingEnabled; } /** * @brief Set whether we want to submit crash reports to the report server @@ -78,17 +126,84 @@ public slots: * The report server is configured with CMAKE_BACKTRACE_URL. * Emits crashReportingEnabledChanged signal. * + * @note This automatically calls start(), so it should be called after setPath(), setUrl() and setToken() * @param enabled Whether it's enabled. */ void setEnabled(bool enabled); + /** + * @brief Set the URL where to send crash reports to + * + * If not set, a predefined URL specified at compile time via CMAKE_BACKTRACE_URL + * will be used. + * + * @param url URL + */ + void setUrl(const QString &url); + /** + * @brief Set the token for the crash reporter + * + * This is an identifier in the crash collection service, such as Sentry, and may contain + * a branch name or a version number. + * + * If not set, a predefined token specified at compile time via CMAKE_BACKTRACE_TOKEN + * will be used. + * + * @param token Token + */ + void setToken(const QString &token); + + + + /** + * @brief Set an annotation to be added to a crash + * + * Annotations add extra information, such as the application's version number, + * the current user, or any other information of interest. + * + * @param key Key + * @param value Value + */ void setAnnotation(const std::string &key, const char *value); + + /** + * @brief Set an annotation to be added to a crash + * + * Annotations add extra information, such as the application's version number, + * the current user, or any other information of interest. + * + * @param key Key + * @param value Value + */ void setAnnotation(const std::string &key, const QString &value); + + /** + * @brief Set an annotation to be added to a crash + * + * Annotations add extra information, such as the application's version number, + * the current user, or any other information of interest. + * + * @param key Key + * @param value Value + */ void setAnnotation(const std::string &key, const std::string &value); +signals: + + /** + * @brief Emitted when the enabled/disabled state of the crash handler changes + * + * This can be used to store it as a setting. + * + * @param enabled Whether the crash handler is now enabled + */ + void enabledChanged(bool enabled); private: + CrashHandler(QObject *parent = nullptr); + + /** * @brief Marks the crash monitor as started * @@ -99,9 +214,12 @@ private: void setStarted(bool started) { _crashMonitorStarted = started; } - - Setting::Handle _crashReportingEnabled { "CrashReportingEnabled", false }; bool _crashMonitorStarted {false}; + bool _crashReportingEnabled {false}; + + QString _path; + QString _crashUrl; + QString _crashToken; }; diff --git a/libraries/monitoring/src/crash-handler/CrashHandlerBackend.h b/libraries/monitoring/src/crash-handler/CrashHandlerBackend.h index f0d7e31b43..55c1ef2760 100644 --- a/libraries/monitoring/src/crash-handler/CrashHandlerBackend.h +++ b/libraries/monitoring/src/crash-handler/CrashHandlerBackend.h @@ -18,7 +18,7 @@ Q_DECLARE_LOGGING_CATEGORY(crash_handler) -bool startCrashHandler(std::string appPath); +bool startCrashHandler(std::string appPath, std::string url="", std::string token=""); void setCrashAnnotation(std::string name, std::string value); void startCrashHookMonitor(QCoreApplication* app); void setCrashReportingEnabled(bool value); diff --git a/libraries/monitoring/src/crash-handler/CrashHandlerBackend_Breakpad.cpp b/libraries/monitoring/src/crash-handler/CrashHandlerBackend_Breakpad.cpp index fa24b43107..984cabe507 100644 --- a/libraries/monitoring/src/crash-handler/CrashHandlerBackend_Breakpad.cpp +++ b/libraries/monitoring/src/crash-handler/CrashHandlerBackend_Breakpad.cpp @@ -57,7 +57,7 @@ void flushAnnotations() { settings.sync(); } -bool startCrashHandler(std::string appPath) { +bool startCrashHandler(std::string appPath, std::string crashURL, std::string crashToken) { annotations["version"] = BuildInfo::VERSION; annotations["build_number"] = BuildInfo::BUILD_NUMBER; annotations["build_type"] = BuildInfo::BUILD_TYPE_STRING; diff --git a/libraries/monitoring/src/crash-handler/CrashHandlerBackend_Crashpad.cpp b/libraries/monitoring/src/crash-handler/CrashHandlerBackend_Crashpad.cpp index be479a2be3..298465b5d2 100644 --- a/libraries/monitoring/src/crash-handler/CrashHandlerBackend_Crashpad.cpp +++ b/libraries/monitoring/src/crash-handler/CrashHandlerBackend_Crashpad.cpp @@ -49,6 +49,10 @@ Q_LOGGING_CATEGORY(crash_handler, "overte.crash_handler") static const std::string BACKTRACE_URL{ CMAKE_BACKTRACE_URL }; static const std::string BACKTRACE_TOKEN{ CMAKE_BACKTRACE_TOKEN }; +std::string custom_backtrace_url; +std::string custom_backtrace_token; + + // ------------------------------------------------------------------------------------------------ // SpinLock - a lock that can timeout attempting to lock a block of code, and is in a busy-wait cycle while trying to acquire // note that this code will malfunction if you attempt to grab a lock while already holding it @@ -351,8 +355,16 @@ static QString findBinaryDir() { return QString(); } -bool startCrashHandler(std::string appPath) { - if (BACKTRACE_URL.empty() || BACKTRACE_TOKEN.empty()) { +bool startCrashHandler(std::string appPath, std::string crashURL, std::string crashToken) { + if (crashURL.empty()) { + crashURL = BACKTRACE_URL; + } + + if (crashToken.empty()) { + crashToken = BACKTRACE_TOKEN; + } + + if (crashURL.empty() || crashToken.empty()) { qCCritical(crash_handler) << "Backtrace URL or token not set, crash handler disabled."; return false; } @@ -362,7 +374,7 @@ bool startCrashHandler(std::string appPath) { std::vector arguments; std::map annotations; - annotations["sentry[release]"] = BACKTRACE_TOKEN; + annotations["sentry[release]"] = crashToken; annotations["sentry[contexts][app][app_version]"] = BuildInfo::VERSION.toStdString(); annotations["sentry[contexts][app][app_build]"] = BuildInfo::BUILD_NUMBER.toStdString(); annotations["build_type"] = BuildInfo::BUILD_TYPE_STRING.toStdString(); @@ -389,10 +401,10 @@ bool startCrashHandler(std::string appPath) { qCDebug(crash_handler) << "Locating own directory by platform-specific method"; interfaceDir.setPath(binaryDir); } else { + // Getting the base dir from argv[0] is already handled by CrashHandler, so we use + // the path as-is here. qCDebug(crash_handler) << "Locating own directory by argv[0]"; interfaceDir.setPath(QString::fromStdString(appPath)); - // argv[0] gets us the path including the binary file - interfaceDir.cdUp(); } if (!interfaceDir.exists(CRASHPAD_HANDLER_NAME)) { @@ -429,7 +441,7 @@ bool startCrashHandler(std::string appPath) { crashpadDatabase->GetSettings()->SetUploadsEnabled(CrashHandler::getInstance().isEnabled()); - if (!client->StartHandler(handler, db, db, BACKTRACE_URL, annotations, arguments, true, true)) { + if (!client->StartHandler(handler, db, db, crashURL, annotations, arguments, true, true)) { qCCritical(crash_handler) << "Failed to start crashpad handler"; return false; } diff --git a/libraries/monitoring/src/crash-handler/CrashHandlerBackend_None.cpp b/libraries/monitoring/src/crash-handler/CrashHandlerBackend_None.cpp index 53ff5b8286..5944125575 100644 --- a/libraries/monitoring/src/crash-handler/CrashHandlerBackend_None.cpp +++ b/libraries/monitoring/src/crash-handler/CrashHandlerBackend_None.cpp @@ -20,7 +20,7 @@ Q_LOGGING_CATEGORY(crash_handler, "overte.crash_handler") -bool startCrashHandler(std::string appPath) { +bool startCrashHandler(std::string appPath, std::string crashURL, std::string crashToken) { qCWarning(crash_handler) << "No crash handler available."; return false; } diff --git a/libraries/networking/CMakeLists.txt b/libraries/networking/CMakeLists.txt index c635059d1b..009eb6a34a 100644 --- a/libraries/networking/CMakeLists.txt +++ b/libraries/networking/CMakeLists.txt @@ -1,6 +1,6 @@ set(TARGET_NAME networking) setup_hifi_library(Network WebSockets) -link_hifi_libraries(shared platform) +link_hifi_libraries(shared platform monitoring) target_openssl() target_tbb() diff --git a/libraries/networking/src/Assignment.cpp b/libraries/networking/src/Assignment.cpp index cddfdbe6fe..03a4a91295 100644 --- a/libraries/networking/src/Assignment.cpp +++ b/libraries/networking/src/Assignment.cpp @@ -16,11 +16,17 @@ #include #include #include +#include +#include + #include "udt/PacketHeaders.h" #include "SharedUtil.h" #include "UUID.h" +static const QString CRASH_REPORTING_GROUP_KEY = "crash_reporting"; + + Assignment::Type Assignment::typeForNodeType(NodeType_t nodeType) { switch (nodeType) { case NodeType::AudioMixer: @@ -51,7 +57,7 @@ Assignment::Assignment() : _payload(), _isStatic(false) { - + } Assignment::Assignment(Assignment::Command command, Assignment::Type type, const QString& pool, Assignment::Location location, QString dataDirectory) : @@ -84,9 +90,9 @@ Assignment::Assignment(ReceivedMessage& message) : } else if (message.getType() == PacketType::CreateAssignment) { _command = Assignment::CreateCommand; } - + QDataStream packetStream(message.getMessage()); - + packetStream >> *this; } @@ -113,7 +119,7 @@ Assignment& Assignment::operator=(const Assignment& rhsAssignment) { void Assignment::swap(Assignment& otherAssignment) { using std::swap; - + swap(_uuid, otherAssignment._uuid); swap(_command, otherAssignment._command); swap(_type, otherAssignment._type); @@ -148,10 +154,36 @@ const char* Assignment::typeToString(Assignment::Type type) { } } + +void Assignment::commonParseSettingsObject(const QJsonObject &settingsObject) { + + if (settingsObject.contains(CRASH_REPORTING_GROUP_KEY)) { + + auto &ch = CrashHandler::getInstance(); + QJsonObject crashGroupObject = settingsObject[CRASH_REPORTING_GROUP_KEY].toObject(); + + const QString CRASH_REPORTING_ENABLED = "enable_crash_reporter"; + const QString CRASH_REPORTING_CUSTOM_URL = "custom_crash_url"; + const QString CRASH_REPORTING_CUSTOM_TOKEN = "custom_crash_token"; + + bool enabled = crashGroupObject[CRASH_REPORTING_ENABLED].toBool(); + QString url = crashGroupObject[CRASH_REPORTING_CUSTOM_URL].toString(); + QString token = crashGroupObject[CRASH_REPORTING_CUSTOM_TOKEN].toString(); + + ch.setUrl(url); + ch.setToken(token); + ch.setEnabled(enabled); + + ch.setAnnotation("program", "assignment-client"); + ch.setAnnotation("assignment-client", "audio-mixer"); + } + +} + QDebug operator<<(QDebug debug, const Assignment &assignment) { debug.nospace() << "UUID: " << qPrintable(assignment.getUUID().toString()) << ", Type: " << assignment.getTypeName() << " (" << assignment.getType() << ")"; - + if (!assignment.getPool().isEmpty()) { debug << ", Pool: " << assignment.getPool(); } @@ -161,11 +193,11 @@ QDebug operator<<(QDebug debug, const Assignment &assignment) { QDataStream& operator<<(QDataStream &out, const Assignment& assignment) { out << (quint8) assignment._type << assignment._uuid << assignment._pool << assignment._payload; - + if (assignment._command == Assignment::RequestCommand) { out << assignment._nodeVersion; } - + return out; } @@ -173,11 +205,11 @@ QDataStream& operator>>(QDataStream &in, Assignment& assignment) { quint8 packedType; in >> packedType >> assignment._uuid >> assignment._pool >> assignment._payload; assignment._type = (Assignment::Type) packedType; - + if (assignment._command == Assignment::RequestCommand) { in >> assignment._nodeVersion; } - + return in; } @@ -187,3 +219,4 @@ uint qHash(const Assignment::Type& key, uint seed) { // strongly typed enum for PacketType return qHash((uint8_t) key, seed); } + diff --git a/libraries/networking/src/Assignment.h b/libraries/networking/src/Assignment.h index 513450d910..4bbe158466 100644 --- a/libraries/networking/src/Assignment.h +++ b/libraries/networking/src/Assignment.h @@ -14,10 +14,12 @@ #include + #include "ReceivedMessage.h" #include "NodeList.h" + const int MAX_PAYLOAD_BYTES = 1024; const QString emptyPool = QString(); @@ -80,12 +82,12 @@ public: void setPool(const QString& pool) { _pool = pool; }; const QString& getPool() const { return _pool; } - + void setIsStatic(bool isStatic) { _isStatic = isStatic; } bool isStatic() const { return _isStatic; } - + const QString& getNodeVersion() const { return _nodeVersion; } - + const char* getTypeName() const; static const char* typeToString(Assignment::Type type); @@ -94,6 +96,16 @@ public: friend QDataStream& operator>>(QDataStream &in, Assignment& assignment); protected: + /** + * @brief Parse the part of the settings object common to all assignment clients + * + * Currently this is the crash reporting settings. + * + * @param settingsObject + */ + void commonParseSettingsObject(const QJsonObject &settingsObject); + + QUuid _uuid; /// the 16 byte UUID for this assignment Assignment::Command _command; /// the command for this assignment (Create, Deploy, Request) Assignment::Type _type; /// the type of the assignment, defines what the assignee will do From 441413020e38e03c4e5b5e61a31094202021f98b Mon Sep 17 00:00:00 2001 From: Dale Glass Date: Sun, 2 Jul 2023 22:10:27 +0200 Subject: [PATCH 07/12] Move crash handler to networking library. Turns out dependencies just don't work out well otherwise. --- assignment-client/CMakeLists.txt | 2 +- domain-server/CMakeLists.txt | 2 +- interface/CMakeLists.txt | 2 +- libraries/monitoring/CMakeLists.txt | 10 ---------- libraries/networking/CMakeLists.txt | 4 +++- libraries/networking/src/Assignment.cpp | 2 +- .../src/crash-handler/CrashHandler.cpp | 0 .../src/crash-handler/CrashHandler.h | 0 .../src/crash-handler/CrashHandlerBackend.h | 0 .../src/crash-handler/CrashHandlerBackend_Breakpad.cpp | 0 .../src/crash-handler/CrashHandlerBackend_Crashpad.cpp | 5 +++-- .../src/crash-handler/CrashHandlerBackend_None.cpp | 0 12 files changed, 10 insertions(+), 17 deletions(-) delete mode 100644 libraries/monitoring/CMakeLists.txt rename libraries/{monitoring => networking}/src/crash-handler/CrashHandler.cpp (100%) rename libraries/{monitoring => networking}/src/crash-handler/CrashHandler.h (100%) rename libraries/{monitoring => networking}/src/crash-handler/CrashHandlerBackend.h (100%) rename libraries/{monitoring => networking}/src/crash-handler/CrashHandlerBackend_Breakpad.cpp (100%) rename libraries/{monitoring => networking}/src/crash-handler/CrashHandlerBackend_Crashpad.cpp (99%) rename libraries/{monitoring => networking}/src/crash-handler/CrashHandlerBackend_None.cpp (100%) diff --git a/assignment-client/CMakeLists.txt b/assignment-client/CMakeLists.txt index 88a6f9dc7f..c8e26f5f46 100644 --- a/assignment-client/CMakeLists.txt +++ b/assignment-client/CMakeLists.txt @@ -19,7 +19,7 @@ link_hifi_libraries( audio avatars octree gpu graphics shaders model-serializers hfm entities networking animation recording shared script-engine embedded-webserver controllers physics plugins midi image - material-networking model-networking ktx shaders monitoring + material-networking model-networking ktx shaders ) include_hifi_library_headers(procedural) diff --git a/domain-server/CMakeLists.txt b/domain-server/CMakeLists.txt index 2327762610..011fd115bd 100644 --- a/domain-server/CMakeLists.txt +++ b/domain-server/CMakeLists.txt @@ -30,7 +30,7 @@ symlink_or_copy_directory_beside_target(${_SHOULD_SYMLINK_RESOURCES} "${CMAKE_CU include_hifi_library_headers(gpu) include_hifi_library_headers(graphics) include_hifi_library_headers(script-engine) -link_hifi_libraries(embedded-webserver networking shared avatars octree monitoring) +link_hifi_libraries(embedded-webserver networking shared avatars octree) target_zlib() target_quazip() diff --git a/interface/CMakeLists.txt b/interface/CMakeLists.txt index ea98b9f4df..7262ff0005 100644 --- a/interface/CMakeLists.txt +++ b/interface/CMakeLists.txt @@ -230,7 +230,7 @@ link_hifi_libraries( ${PLATFORM_GL_BACKEND} # Plaform specific input & display plugin libraries ${PLATFORM_PLUGIN_LIBRARIES} - shaders monitoring + shaders ) include_hifi_library_headers(script-engine) diff --git a/libraries/monitoring/CMakeLists.txt b/libraries/monitoring/CMakeLists.txt deleted file mode 100644 index ee4077ae0e..0000000000 --- a/libraries/monitoring/CMakeLists.txt +++ /dev/null @@ -1,10 +0,0 @@ -set(TARGET_NAME monitoring) -setup_hifi_library() -link_hifi_libraries(shared) - -add_crashpad() -target_breakpad() - -if (WIN32) - add_compile_definitions(_USE_MATH_DEFINES) -endif() diff --git a/libraries/networking/CMakeLists.txt b/libraries/networking/CMakeLists.txt index 009eb6a34a..1d46236e6c 100644 --- a/libraries/networking/CMakeLists.txt +++ b/libraries/networking/CMakeLists.txt @@ -1,9 +1,11 @@ set(TARGET_NAME networking) setup_hifi_library(Network WebSockets) -link_hifi_libraries(shared platform monitoring) +link_hifi_libraries(shared platform) target_openssl() target_tbb() +add_crashpad() +target_breakpad() if (WIN32 OR (UNIX AND NOT APPLE)) target_webrtc() diff --git a/libraries/networking/src/Assignment.cpp b/libraries/networking/src/Assignment.cpp index 03a4a91295..2d1dedc003 100644 --- a/libraries/networking/src/Assignment.cpp +++ b/libraries/networking/src/Assignment.cpp @@ -17,7 +17,7 @@ #include #include #include -#include +#include "crash-handler/CrashHandler.h" #include "udt/PacketHeaders.h" diff --git a/libraries/monitoring/src/crash-handler/CrashHandler.cpp b/libraries/networking/src/crash-handler/CrashHandler.cpp similarity index 100% rename from libraries/monitoring/src/crash-handler/CrashHandler.cpp rename to libraries/networking/src/crash-handler/CrashHandler.cpp diff --git a/libraries/monitoring/src/crash-handler/CrashHandler.h b/libraries/networking/src/crash-handler/CrashHandler.h similarity index 100% rename from libraries/monitoring/src/crash-handler/CrashHandler.h rename to libraries/networking/src/crash-handler/CrashHandler.h diff --git a/libraries/monitoring/src/crash-handler/CrashHandlerBackend.h b/libraries/networking/src/crash-handler/CrashHandlerBackend.h similarity index 100% rename from libraries/monitoring/src/crash-handler/CrashHandlerBackend.h rename to libraries/networking/src/crash-handler/CrashHandlerBackend.h diff --git a/libraries/monitoring/src/crash-handler/CrashHandlerBackend_Breakpad.cpp b/libraries/networking/src/crash-handler/CrashHandlerBackend_Breakpad.cpp similarity index 100% rename from libraries/monitoring/src/crash-handler/CrashHandlerBackend_Breakpad.cpp rename to libraries/networking/src/crash-handler/CrashHandlerBackend_Breakpad.cpp diff --git a/libraries/monitoring/src/crash-handler/CrashHandlerBackend_Crashpad.cpp b/libraries/networking/src/crash-handler/CrashHandlerBackend_Crashpad.cpp similarity index 99% rename from libraries/monitoring/src/crash-handler/CrashHandlerBackend_Crashpad.cpp rename to libraries/networking/src/crash-handler/CrashHandlerBackend_Crashpad.cpp index 298465b5d2..1b93ebecb4 100644 --- a/libraries/monitoring/src/crash-handler/CrashHandlerBackend_Crashpad.cpp +++ b/libraries/networking/src/crash-handler/CrashHandlerBackend_Crashpad.cpp @@ -41,9 +41,10 @@ Q_LOGGING_CATEGORY(crash_handler, "overte.crash_handler") #endif #include -#include +#include "../FingerprintUtils.h" +#include "../UserActivityLogger.h" #include -#include + static const std::string BACKTRACE_URL{ CMAKE_BACKTRACE_URL }; diff --git a/libraries/monitoring/src/crash-handler/CrashHandlerBackend_None.cpp b/libraries/networking/src/crash-handler/CrashHandlerBackend_None.cpp similarity index 100% rename from libraries/monitoring/src/crash-handler/CrashHandlerBackend_None.cpp rename to libraries/networking/src/crash-handler/CrashHandlerBackend_None.cpp From 37d5734f28f161aa9c2513c51aa5eb5ba5e48a5b Mon Sep 17 00:00:00 2001 From: Dale Glass Date: Mon, 3 Jul 2023 00:23:40 +0200 Subject: [PATCH 08/12] Undo some weird formatting It seems I accidentally ran some sort of auto-format, undoing. --- interface/src/main.cpp | 257 ++++++++++++++++++++++++++++------------- 1 file changed, 175 insertions(+), 82 deletions(-) diff --git a/interface/src/main.cpp b/interface/src/main.cpp index cb5111f456..15bb5faa85 100644 --- a/interface/src/main.cpp +++ b/interface/src/main.cpp @@ -37,7 +37,7 @@ #ifdef Q_OS_WIN #include extern "C" { -typedef int(__stdcall* CHECKMINSPECPROC)(); + typedef int(__stdcall* CHECKMINSPECPROC)(); } #endif @@ -73,93 +73,185 @@ int main(int argc, const char* argv[]) { QCommandLineOption helpOption = parser.addHelpOption(); QCommandLineOption versionOption = parser.addVersionOption(); - QCommandLineOption urlOption("url", "Start at specified URL location.", "string"); - QCommandLineOption protocolVersionOption("protocolVersion", "Writes the protocol version base64 signature to a file?", - "path"); - QCommandLineOption noUpdaterOption("no-updater", "Do not show auto-updater."); - QCommandLineOption - checkMinSpecOption("checkMinSpec", - "Check if machine meets minimum specifications. The program will run if check passes."); - QCommandLineOption runServerOption("runServer", "Run the server."); - QCommandLineOption listenPortOption("listenPort", "Port to listen on.", "port_number"); - QCommandLineOption serverContentPathOption("serverContentPath", - "Path to find server content.", // What content?? - "path"); - QCommandLineOption overrideAppLocalDataPathOption("cache", "Set test cache.", "dir"); - QCommandLineOption scriptsOption("scripts", - "Set path for defaultScripts. These are probably scripts that run automatically. This " - "parameter does not seem to work.", - "dir"); - QCommandLineOption allowMultipleInstancesOption("allowMultipleInstances", "Allow multiple instances to run."); - QCommandLineOption displaysOption("display", "Preferred display.", "displays"); - QCommandLineOption disableDisplaysOption("disable-displays", - "Displays to disable. Valid options include \"OpenVR (Vive)\" and \"Oculus Rift\"", - "string"); - QCommandLineOption disableInputsOption("disable-inputs", - "Inputs to disable. Valid options include \"OpenVR (Vive)\" and \"Oculus Rift\"", - "string"); - QCommandLineOption suppressSettingsResetOption("suppress-settings-reset", - "Suppress the prompt to reset interface settings."); - QCommandLineOption oculusStoreOption("oculus-store", - "Let the Oculus plugin know if interface was run from the Oculus Store."); - QCommandLineOption standaloneOption("standalone", "Emulate a standalone device."); - QCommandLineOption disableWatchdogOption("disableWatchdog", - "Disable the watchdog thread. The interface will crash on deadlocks."); - QCommandLineOption systemCursorOption("system-cursor", "Use the default system cursor."); - QCommandLineOption - concurrentDownloadsOption("concurrent-downloads", - "Maximum concurrent resource downloads. Default is 16, except for Android where it is 4.", - "integer"); - QCommandLineOption avatarURLOption("avatarURL", "Override the avatar U.R.L.", "url"); - QCommandLineOption replaceAvatarURLOption("replaceAvatarURL", - "Replaces the avatar U.R.L. When used with --avatarURL, this takes precedence.", - "url"); - QCommandLineOption - setBookmarkOption("setBookmark", - "Set bookmark as key=value pair. Including the '=' symbol in either string is unsupported.", - "string"); - QCommandLineOption forceCrashReportingOption("forceCrashReporting", "Force crash reporting to initialize."); + QCommandLineOption urlOption( + "url", + "Start at specified URL location.", + "string" + ); + QCommandLineOption protocolVersionOption( + "protocolVersion", + "Writes the protocol version base64 signature to a file?", + "path" + ); + QCommandLineOption noUpdaterOption( + "no-updater", + "Do not show auto-updater." + ); + QCommandLineOption checkMinSpecOption( + "checkMinSpec", + "Check if machine meets minimum specifications. The program will run if check passes." + ); + QCommandLineOption runServerOption( + "runServer", + "Run the server." + ); + QCommandLineOption listenPortOption( + "listenPort", + "Port to listen on.", + "port_number" + ); + QCommandLineOption serverContentPathOption( + "serverContentPath", + "Path to find server content.", // What content?? + "path" + ); + QCommandLineOption overrideAppLocalDataPathOption( + "cache", + "Set test cache.", + "dir" + ); + QCommandLineOption scriptsOption( + "scripts", + "Set path for defaultScripts. These are probably scripts that run automatically. This parameter does not seem to work.", + "dir" + ); + QCommandLineOption allowMultipleInstancesOption( + "allowMultipleInstances", + "Allow multiple instances to run." + ); + QCommandLineOption displaysOption( + "display", + "Preferred display.", + "displays" + ); + QCommandLineOption disableDisplaysOption( + "disable-displays", + "Displays to disable. Valid options include \"OpenVR (Vive)\" and \"Oculus Rift\"", + "string" + ); + QCommandLineOption disableInputsOption( + "disable-inputs", + "Inputs to disable. Valid options include \"OpenVR (Vive)\" and \"Oculus Rift\"", + "string" + ); + QCommandLineOption suppressSettingsResetOption( + "suppress-settings-reset", + "Suppress the prompt to reset interface settings." + ); + QCommandLineOption oculusStoreOption( + "oculus-store", + "Let the Oculus plugin know if interface was run from the Oculus Store." + ); + QCommandLineOption standaloneOption( + "standalone", + "Emulate a standalone device." + ); + QCommandLineOption disableWatchdogOption( + "disableWatchdog", + "Disable the watchdog thread. The interface will crash on deadlocks." + ); + QCommandLineOption systemCursorOption( + "system-cursor", + "Use the default system cursor." + ); + QCommandLineOption concurrentDownloadsOption( + "concurrent-downloads", + "Maximum concurrent resource downloads. Default is 16, except for Android where it is 4.", + "integer" + ); + QCommandLineOption avatarURLOption( + "avatarURL", + "Override the avatar U.R.L.", + "url" + ); + QCommandLineOption replaceAvatarURLOption( + "replaceAvatarURL", + "Replaces the avatar U.R.L. When used with --avatarURL, this takes precedence.", + "url" + ); + QCommandLineOption setBookmarkOption( + "setBookmark", + "Set bookmark as key=value pair. Including the '=' symbol in either string is unsupported.", + "string" + ); + QCommandLineOption forceCrashReportingOption( + "forceCrashReporting", + "Force crash reporting to initialize." + ); // The documented "--disable-lod" does not seem to exist. // Below are undocumented. - QCommandLineOption noLauncherOption("no-launcher", - "Supposedly does something for the server, unrelated to the application launcher. The " - "feature may never have been implemented."); - QCommandLineOption - overrideScriptsPathOption("overrideScriptsPath", - "Specifies path to default directory where the application will look for scripts to load.", - "string"); - QCommandLineOption defaultScriptsOverrideOption("defaultScriptsOverride", - "Override default script to run automatically on start. Default is " - "\"defaultsScripts.js\".", - "string"); - QCommandLineOption responseTokensOption("tokens", "Set response tokens .", "json"); - QCommandLineOption displayNameOption("displayName", "Set user display name .", "string"); - QCommandLineOption noLoginOption("no-login-suggestion", "Do not show log-in dialogue."); - QCommandLineOption - traceFileOption("traceFile", - "Writes a trace to a file in the documents folder. Only works if \"--traceDuration\" is specified.", - "path"); - QCommandLineOption - traceDurationOption("traceDuration", - "Automatically quit interface after duration. Only works if \"--traceFile\" is specified.", - "seconds"); - QCommandLineOption clockSkewOption("clockSkew", "Forces client instance's clock to skew for demonstration purposes.", - "integer"); // This should probably be removed. - QCommandLineOption testScriptOption("testScript", "Undocumented. Accepts parameter as U.R.L.", "string"); - QCommandLineOption testResultsLocationOption("testResultsLocation", "Undocumented", "path"); + QCommandLineOption noLauncherOption( + "no-launcher", + "Supposedly does something for the server, unrelated to the application launcher. The feature may never have been implemented." + ); + QCommandLineOption overrideScriptsPathOption( + "overrideScriptsPath", + "Specifies path to default directory where the application will look for scripts to load.", + "string" + ); + QCommandLineOption defaultScriptsOverrideOption( + "defaultScriptsOverride", + "Override default script to run automatically on start. Default is \"defaultsScripts.js\".", + "string" + ); + QCommandLineOption responseTokensOption( + "tokens", + "Set response tokens .", + "json" + ); + QCommandLineOption displayNameOption( + "displayName", + "Set user display name .", + "string" + ); + QCommandLineOption noLoginOption( + "no-login-suggestion", + "Do not show log-in dialogue." + ); + QCommandLineOption traceFileOption( + "traceFile", + "Writes a trace to a file in the documents folder. Only works if \"--traceDuration\" is specified.", + "path" + ); + QCommandLineOption traceDurationOption( + "traceDuration", + "Automatically quit interface after duration. Only works if \"--traceFile\" is specified.", + "seconds" + ); + QCommandLineOption clockSkewOption( + "clockSkew", + "Forces client instance's clock to skew for demonstration purposes.", + "integer" + ); // This should probably be removed. + QCommandLineOption testScriptOption( + "testScript", + "Undocumented. Accepts parameter as U.R.L.", + "string" + ); + QCommandLineOption testResultsLocationOption( + "testResultsLocation", + "Undocumented", + "path" + ); QCommandLineOption quitWhenFinishedOption( - "quitWhenFinished", - "Only works if \"--testScript\" is provided."); // Should probably also be made to work on testResultsLocationOption. - QCommandLineOption fastHeartbeatOption("fast-heartbeat", "Change stats polling interval from 10000ms to 1000ms."); - QCommandLineOption logOption("logOptions", - "Logging options, comma separated: " - "color,nocolor,process_id,thread_id,milliseconds,keep_repeats,journald,nojournald", - "options"); + "quitWhenFinished", + "Only works if \"--testScript\" is provided." + ); // Should probably also be made to work on testResultsLocationOption. + QCommandLineOption fastHeartbeatOption( + "fast-heartbeat", + "Change stats polling interval from 10000ms to 1000ms." + ); + QCommandLineOption logOption( + "logOptions", + "Logging options, comma separated: color,nocolor,process_id,thread_id,milliseconds,keep_repeats,journald,nojournald", + "options" + ); // "--qmljsdebugger", which appears in output from "--help-all". // Those below don't seem to be optional. // --ignore-gpu-blacklist // --suppress-settings-reset + parser.addOption(urlOption); parser.addOption(protocolVersionOption); parser.addOption(noUpdaterOption); @@ -220,6 +312,7 @@ int main(int argc, const char* argv[]) { // We want to configure the logging system as early as possible auto& logHandler = LogHandler::getInstance(); + if (parser.isSet(logOption)) { if (!logHandler.parseOptions(parser.value(logOption).toUtf8(), logOption.names().first())) { QCoreApplication mockApp(argc, const_cast(argv)); // required for call to showHelp() @@ -535,8 +628,8 @@ int main(int argc, const char* argv[]) { server.removeServer(applicationName); server.listen(applicationName); - QObject::connect(&server, &QLocalServer::newConnection, &app, &Application::handleLocalServerConnection, - Qt::DirectConnection); + QObject::connect(&server, &QLocalServer::newConnection, + &app, &Application::handleLocalServerConnection, Qt::DirectConnection); printSystemInformation(); From 14e0a8220b0035545813f66b7ecd1875aad7fa78 Mon Sep 17 00:00:00 2001 From: Dale Glass Date: Mon, 3 Jul 2023 00:27:36 +0200 Subject: [PATCH 09/12] Load settings on start, missed during refactoring --- interface/src/main.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/interface/src/main.cpp b/interface/src/main.cpp index 15bb5faa85..18da2c2e00 100644 --- a/interface/src/main.cpp +++ b/interface/src/main.cpp @@ -437,6 +437,15 @@ int main(int argc, const char* argv[]) { ch.setEnabled(true); } + { + Settings crashSettings; + crashSettings.beginGroup("Crash"); + if (crashSettings.value("ReportingEnabled").toBool()) { + ch.setEnabled(true); + } + crashSettings.endGroup(); + } + ch.setAnnotation("program", "interface"); const QString& applicationName = getInterfaceSharedMemoryName(); From 2babda52631658bacf92acec3834c1f0b34878f3 Mon Sep 17 00:00:00 2001 From: Dale Glass Date: Mon, 3 Jul 2023 01:14:56 +0200 Subject: [PATCH 10/12] Review fix --- assignment-client/src/AssignmentClientApp.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/assignment-client/src/AssignmentClientApp.cpp b/assignment-client/src/AssignmentClientApp.cpp index 088b5304ef..fef2a3139d 100644 --- a/assignment-client/src/AssignmentClientApp.cpp +++ b/assignment-client/src/AssignmentClientApp.cpp @@ -257,7 +257,6 @@ AssignmentClientApp::AssignmentClientApp(int argc, char* argv[]) : } if (parser.isSet(forceCrashReportingOption)) { - auto &ch = CrashHandler::getInstance(); ch.setEnabled(true); } From 69f1ddcec0650d140ac621cec24687ff849e9356 Mon Sep 17 00:00:00 2001 From: Dale Glass Date: Tue, 4 Jul 2023 01:20:30 +0200 Subject: [PATCH 11/12] Add crash reporting to Oven --- tools/oven/CMakeLists.txt | 3 ++ tools/oven/src/OvenCLIApplication.cpp | 34 ++++++++++---- tools/oven/src/OvenCLIApplication.h | 7 ++- tools/oven/src/main.cpp | 66 +++++++++++++++++++++------ 4 files changed, 87 insertions(+), 23 deletions(-) diff --git a/tools/oven/CMakeLists.txt b/tools/oven/CMakeLists.txt index 4ffdd13d26..3cf954b213 100644 --- a/tools/oven/CMakeLists.txt +++ b/tools/oven/CMakeLists.txt @@ -11,6 +11,9 @@ include_hifi_library_headers(script-engine) setup_memory_debugger() setup_thread_debugger() +add_crashpad() +target_breakpad() + if (WIN32) package_libraries_for_deployment() diff --git a/tools/oven/src/OvenCLIApplication.cpp b/tools/oven/src/OvenCLIApplication.cpp index 6d3a8912ab..c02b42c0dc 100644 --- a/tools/oven/src/OvenCLIApplication.cpp +++ b/tools/oven/src/OvenCLIApplication.cpp @@ -18,7 +18,7 @@ #include #include - +#include #include "BakerCLI.h" static const QString CLI_INPUT_PARAMETER = "i"; @@ -38,7 +38,7 @@ OvenCLIApplication::OvenCLIApplication(int argc, char* argv[]) : Q_ARG(QString, _outputUrlParameter.toString()), Q_ARG(QString, _typeParameter)); } -void OvenCLIApplication::parseCommandLine(int argc, char* argv[]) { +OvenCLIApplication::parseResult OvenCLIApplication::parseCommandLine(int argc, char* argv[], bool *enableCrashHandler) { // parse the command line parameters QCommandLineParser parser; @@ -50,6 +50,11 @@ void OvenCLIApplication::parseCommandLine(int argc, char* argv[]) { { CLI_DISABLE_TEXTURE_COMPRESSION_PARAMETER, "Disable texture compression." } }); + + const QCommandLineOption forceCrashReportingOption("forceCrashReporting", "Force crash reporting to initialize."); + parser.addOption(forceCrashReportingOption); + + auto versionOption = parser.addVersionOption(); auto helpOption = parser.addHelpOption(); @@ -65,6 +70,10 @@ void OvenCLIApplication::parseCommandLine(int argc, char* argv[]) { Q_UNREACHABLE(); } + if (parser.isSet(forceCrashReportingOption)) { + *enableCrashHandler = true; + } + if (parser.isSet(versionOption)) { parser.showVersion(); Q_UNREACHABLE(); @@ -75,20 +84,27 @@ void OvenCLIApplication::parseCommandLine(int argc, char* argv[]) { Q_UNREACHABLE(); } - if (!parser.isSet(CLI_INPUT_PARAMETER) || !parser.isSet(CLI_OUTPUT_PARAMETER)) { + // If one argument is given, so must be the other + if ((parser.isSet(CLI_INPUT_PARAMETER) != parser.isSet(CLI_OUTPUT_PARAMETER)) || !parser.positionalArguments().empty()) { std::cout << "Error: Input and Output not set" << std::endl; // Avoid Qt log spam QCoreApplication mockApp(argc, argv); // required for call to showHelp() parser.showHelp(); Q_UNREACHABLE(); } - _inputUrlParameter = QDir::fromNativeSeparators(parser.value(CLI_INPUT_PARAMETER)); - _outputUrlParameter = QDir::fromNativeSeparators(parser.value(CLI_OUTPUT_PARAMETER)); + if (parser.isSet(CLI_INPUT_PARAMETER) && parser.isSet(CLI_OUTPUT_PARAMETER)) { + _inputUrlParameter = QDir::fromNativeSeparators(parser.value(CLI_INPUT_PARAMETER)); + _outputUrlParameter = QDir::fromNativeSeparators(parser.value(CLI_OUTPUT_PARAMETER)); - _typeParameter = parser.isSet(CLI_TYPE_PARAMETER) ? parser.value(CLI_TYPE_PARAMETER) : QString(); + _typeParameter = parser.isSet(CLI_TYPE_PARAMETER) ? parser.value(CLI_TYPE_PARAMETER) : QString(); - if (parser.isSet(CLI_DISABLE_TEXTURE_COMPRESSION_PARAMETER)) { - qDebug() << "Disabling texture compression"; - TextureBaker::setCompressionEnabled(false); + if (parser.isSet(CLI_DISABLE_TEXTURE_COMPRESSION_PARAMETER)) { + qDebug() << "Disabling texture compression"; + TextureBaker::setCompressionEnabled(false); + } + + return OvenCLIApplication::CLIMode; + } else { + return OvenCLIApplication::GUIMode; } } diff --git a/tools/oven/src/OvenCLIApplication.h b/tools/oven/src/OvenCLIApplication.h index 21fc9e9ba1..e4f75df16e 100644 --- a/tools/oven/src/OvenCLIApplication.h +++ b/tools/oven/src/OvenCLIApplication.h @@ -21,7 +21,12 @@ class OvenCLIApplication : public QCoreApplication, public Oven { public: OvenCLIApplication(int argc, char* argv[]); - static void parseCommandLine(int argc, char* argv[]); + enum parseResult { + GUIMode, + CLIMode + }; + + static parseResult parseCommandLine(int argc, char* argv[], bool *enableCrashHandler); static OvenCLIApplication* instance() { return dynamic_cast(QCoreApplication::instance()); } diff --git a/tools/oven/src/main.cpp b/tools/oven/src/main.cpp index 586dae06a5..66e79806fa 100644 --- a/tools/oven/src/main.cpp +++ b/tools/oven/src/main.cpp @@ -14,24 +14,64 @@ #include #include #include +#include +#include +#include +#include + + +// This needs to be run after a QApplication has been created +void postAppInit(QCoreApplication *app, bool enableCrashHandler) { + Setting::init(); + + auto &ch = CrashHandler::getInstance(); + + + + QObject::connect(&ch, &CrashHandler::enabledChanged, [](bool enabled) { + Settings s; + s.beginGroup("Crash"); + s.setValue("ReportingEnabled", enabled); + s.endGroup(); + }); + + + Settings crashSettings; + crashSettings.beginGroup("Crash"); + ch.setEnabled(crashSettings.value("ReportingEnabled").toBool() || enableCrashHandler); + ch.startMonitor(app); +} + int main (int argc, char** argv) { setupHifiApplication("Oven"); + + DependencyManager::set(); + + auto &ch = CrashHandler::getInstance(); + ch.setPath(argv[0]); + + + // figure out if we're launching our GUI application or just the simple command line interface - if (argc > 1) { - OvenCLIApplication::parseCommandLine(argc, argv); + bool enableCrashHandler = false; + OvenCLIApplication::parseResult res = OvenCLIApplication::parseCommandLine(argc, argv, &enableCrashHandler); - // init the settings interface so we can save and load settings - Setting::init(); - - OvenCLIApplication app { argc, argv }; - return app.exec(); - } else { - // init the settings interface so we can save and load settings - Setting::init(); - - OvenGUIApplication app { argc, argv }; - return app.exec(); + switch(res) { + case OvenCLIApplication::CLIMode: + { + OvenCLIApplication app { argc, argv }; + postAppInit(&app, enableCrashHandler); + return app.exec(); + break; + } + case OvenCLIApplication::GUIMode: + { + OvenGUIApplication app { argc, argv }; + postAppInit(&app, enableCrashHandler); + return app.exec(); + break; + } } } From 0de10029f3a56e92910a0f31ac7819c03cd178c6 Mon Sep 17 00:00:00 2001 From: Dale Glass Date: Sun, 9 Jul 2023 20:19:36 +0200 Subject: [PATCH 12/12] Review fix, change pointer to reference --- tools/oven/src/OvenCLIApplication.cpp | 4 ++-- tools/oven/src/OvenCLIApplication.h | 15 ++++++++++++++- tools/oven/src/main.cpp | 2 +- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/tools/oven/src/OvenCLIApplication.cpp b/tools/oven/src/OvenCLIApplication.cpp index c02b42c0dc..ab56fd9255 100644 --- a/tools/oven/src/OvenCLIApplication.cpp +++ b/tools/oven/src/OvenCLIApplication.cpp @@ -38,7 +38,7 @@ OvenCLIApplication::OvenCLIApplication(int argc, char* argv[]) : Q_ARG(QString, _outputUrlParameter.toString()), Q_ARG(QString, _typeParameter)); } -OvenCLIApplication::parseResult OvenCLIApplication::parseCommandLine(int argc, char* argv[], bool *enableCrashHandler) { +OvenCLIApplication::parseResult OvenCLIApplication::parseCommandLine(int argc, char* argv[], bool &enableCrashHandler) { // parse the command line parameters QCommandLineParser parser; @@ -71,7 +71,7 @@ OvenCLIApplication::parseResult OvenCLIApplication::parseCommandLine(int argc, c } if (parser.isSet(forceCrashReportingOption)) { - *enableCrashHandler = true; + enableCrashHandler = true; } if (parser.isSet(versionOption)) { diff --git a/tools/oven/src/OvenCLIApplication.h b/tools/oven/src/OvenCLIApplication.h index e4f75df16e..6dd26967f5 100644 --- a/tools/oven/src/OvenCLIApplication.h +++ b/tools/oven/src/OvenCLIApplication.h @@ -26,7 +26,20 @@ public: CLIMode }; - static parseResult parseCommandLine(int argc, char* argv[], bool *enableCrashHandler); + /** + * @brief Parses the command line arguments + * + * Oven can operate both in GUI and CLI mode. Depending on which arguments are passed, + * we pick the mode to use, and this function returns it. + * + * Both modes can have the crash handler enabled. + * + * @param argc argc + * @param argv argv + * @param enableCrashHandler Output parameter -- whether the crash handler should be enabled. + * @return parseResult Which application we should run + */ + static parseResult parseCommandLine(int argc, char* argv[], bool &enableCrashHandler); static OvenCLIApplication* instance() { return dynamic_cast(QCoreApplication::instance()); } diff --git a/tools/oven/src/main.cpp b/tools/oven/src/main.cpp index 66e79806fa..3781ae597f 100644 --- a/tools/oven/src/main.cpp +++ b/tools/oven/src/main.cpp @@ -56,7 +56,7 @@ int main (int argc, char** argv) { // figure out if we're launching our GUI application or just the simple command line interface bool enableCrashHandler = false; - OvenCLIApplication::parseResult res = OvenCLIApplication::parseCommandLine(argc, argv, &enableCrashHandler); + OvenCLIApplication::parseResult res = OvenCLIApplication::parseCommandLine(argc, argv, enableCrashHandler); switch(res) { case OvenCLIApplication::CLIMode: