From faee5dbabf035acf77c26f1a98832b3d0db1e88b Mon Sep 17 00:00:00 2001 From: James Turner Date: Tue, 7 Apr 2020 09:38:54 +0100 Subject: [PATCH] Code changes for crash reporting with Sentry. --- CMakeLists.txt | 9 -- CMakeModules/SetupFGFSLibraries.cmake | 4 - src/Include/config_cmake.h.in | 2 - src/Main/CMakeLists.txt | 2 + src/Main/bootstrap.cxx | 77 +++---------- src/Main/fg_init.cxx | 15 ++- src/Main/fg_init.hxx | 2 + src/Main/main.cxx | 44 ++++---- src/Main/sentryIntegration.cxx | 151 ++++++++++++++++++++++++++ src/Main/sentryIntegration.hxx | 39 +++++++ test_suite/CMakeLists.txt | 6 +- test_suite/bootstrap.cxx | 3 - 12 files changed, 249 insertions(+), 105 deletions(-) create mode 100644 src/Main/sentryIntegration.cxx create mode 100644 src/Main/sentryIntegration.hxx diff --git a/CMakeLists.txt b/CMakeLists.txt index 456199ab3..c49a1edd8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -321,15 +321,6 @@ if (TARGET sentry::sentry) set(HAVE_SENTRY 1) endif() -if (MSVC) - find_package(CrashRpt) - if (CRASHRPT_FOUND) - set(HAVE_CRASHRPT 1) - message(STATUS "Using CrashRpt") - include_directories( ${CRASHRPT_INCLUDE_DIR}) - endif() -endif() - #if(${CMAKE_SYSTEM_NAME} MATCHES "OpenBSD") # include_directories("${CMAKE_SOURCE_DIR}/3rdparty/iaxclient/lib/libspeex/include") #endif() diff --git a/CMakeModules/SetupFGFSLibraries.cmake b/CMakeModules/SetupFGFSLibraries.cmake index d0d44ea82..09d42233a 100644 --- a/CMakeModules/SetupFGFSLibraries.cmake +++ b/CMakeModules/SetupFGFSLibraries.cmake @@ -40,10 +40,6 @@ function(setup_fgfs_libraries target) target_link_libraries(${target} ${GooglePerfTools_LIBRARIES}) endif() - if(CRASHRPT_FOUND) - target_link_libraries(${target} ${CRASHRPT_LIBRARY}) - endif() - if(X11_FOUND) target_link_libraries(${target} ${X11_LIBRARIES}) endif() diff --git a/src/Include/config_cmake.h.in b/src/Include/config_cmake.h.in index b55bc29a3..8d67682e1 100644 --- a/src/Include/config_cmake.h.in +++ b/src/Include/config_cmake.h.in @@ -47,8 +47,6 @@ #cmakedefine HAVE_DBUS -#cmakedefine HAVE_CRASHRPT - #cmakedefine ENABLE_HID_INPUT #cmakedefine ENABLE_PLIB_JOYSTICK diff --git a/src/Main/CMakeLists.txt b/src/Main/CMakeLists.txt index aad68a96e..f4958c664 100644 --- a/src/Main/CMakeLists.txt +++ b/src/Main/CMakeLists.txt @@ -29,6 +29,7 @@ set(SOURCES subsystemFactory.cxx util.cxx XLIFFParser.cxx + sentryIntegration.cxx ${MS_RESOURCE_FILE} ) @@ -49,6 +50,7 @@ set(HEADERS subsystemFactory.hxx util.hxx XLIFFParser.hxx + sentryIntegration.hxx ) flightgear_component(Main "${SOURCES}" "${HEADERS}") diff --git a/src/Main/bootstrap.cxx b/src/Main/bootstrap.cxx index b0799a967..e78a152fb 100644 --- a/src/Main/bootstrap.cxx +++ b/src/Main/bootstrap.cxx @@ -65,6 +65,7 @@ #include
#include
#include
+#include
#include #include "fg_os.hxx" @@ -73,13 +74,6 @@ #include #endif -#if defined(HAVE_CRASHRPT) - #include - -bool global_crashRptEnabled = false; - -#endif - using std::cerr; using std::endl; @@ -260,62 +254,23 @@ int main ( int argc, char **argv ) char _hostname[256]; gethostname(_hostname, 256); hostname = _hostname; - signal(SIGPIPE, SIG_IGN); -# ifndef NDEBUG - signal(SIGSEGV, segfault_handler); -# endif #endif _bootstrap_OSInit = 0; + +#if defined(HAVE_SENTRY) + if (flightgear::Options::checkForArg(argc, argv, "enable-sentry")) { + flightgear::initSentry(); + } +#endif -#if defined(HAVE_CRASHRPT) - // Define CrashRpt configuration parameters - CR_INSTALL_INFO info; - memset(&info, 0, sizeof(CR_INSTALL_INFO)); - info.cb = sizeof(CR_INSTALL_INFO); - info.pszAppName = "FlightGear"; - info.pszAppVersion = FLIGHTGEAR_VERSION; - info.pszEmailSubject = "FlightGear " FLIGHTGEAR_VERSION " crash report"; - info.pszEmailTo = "fgcrash@goneabitbursar.com"; - info.pszUrl = "http://fgfs.goneabitbursar.com/crashreporter/crashrpt.php"; - info.uPriorities[CR_HTTP] = 3; - info.uPriorities[CR_SMTP] = 2; - info.uPriorities[CR_SMAPI] = 1; - - // Install all available exception handlers - info.dwFlags |= CR_INST_ALL_POSSIBLE_HANDLERS; - - // Restart the app on crash - info.dwFlags |= CR_INST_SEND_QUEUED_REPORTS; - - // automatically install handlers for all threads - info.dwFlags |= CR_INST_AUTO_THREAD_HANDLERS; - - // Define the Privacy Policy URL - info.pszPrivacyPolicyURL = "http://flightgear.org/crash-privacypolicy.html"; - - // Install crash reporting - int nResult = crInstall(&info); - if(nResult!=0) { - // don't warn about missing CrashRpt in developer builds - if (strcmp(FG_BUILD_TYPE, "Dev") != 0) { - char buf[1024]; - crGetLastErrorMsg(buf, 1024); - flightgear::modalMessageBox("CrashRpt setup failed", - "Failed to setup crash-reporting engine, check the installation is not damaged.", - buf); - } - } else { - global_crashRptEnabled = true; - - crAddProperty("hudson-build-id", JENKINS_BUILD_ID); - char buf[16]; - ::snprintf(buf, 16, "%d", JENKINS_BUILD_NUMBER); - crAddProperty("hudson-build-number", buf); - crAddProperty("git-revision", REVISION); - crAddProperty("build-type", FG_BUILD_TYPE); - } +// if we're not using the normal crash-reported, install our +// custom segfault handler on Linux, in debug builds +#if !defined(SG_WINDOWS) && !defined(NDEBUG) + if (!flightgear::isSentryEnabled()) { + signal(SIGSEGV, segfault_handler); + } #endif initFPE(flightgear::Options::checkForArg(argc, argv, "enable-fpe")); @@ -377,11 +332,6 @@ int main ( int argc, char **argv ) #if defined(HAVE_QT) flightgear::shutdownQtApp(); #endif - -#if defined(HAVE_CRASHRPT) - crUninstall(); -#endif - return exitStatus; } @@ -407,4 +357,5 @@ void fgExitCleanup() { simgear::GroundLightManager::instance()->getGroundLightStateSet()->clear(); simgear::shutdownLogging(); + flightgear::shutdownSentry(); } diff --git a/src/Main/fg_init.cxx b/src/Main/fg_init.cxx index 90f860a3c..518579f88 100644 --- a/src/Main/fg_init.cxx +++ b/src/Main/fg_init.cxx @@ -151,6 +151,7 @@ #include "positioninit.hxx" #include "util.hxx" #include "AircraftDirVisitorBase.hxx" +#include
#if defined(SG_MAC) #include // for Mac impl of platformDefaultDataPath() @@ -449,9 +450,14 @@ static SGPath platformDefaultDataPath() static HANDLE static_fgHomeWriteMutex = nullptr; #endif +SGPath fgHomePath() +{ + return SGPath::fromEnv("FG_HOME", platformDefaultDataPath()); +} + bool fgInitHome() { - SGPath dataPath = SGPath::fromEnv("FG_HOME", platformDefaultDataPath()); + SGPath dataPath = fgHomePath(); globals->set_fg_home(dataPath); simgear::Dir fgHome(dataPath); @@ -718,6 +724,8 @@ int fgInitAircraft(bool reinit) SGPropertyNode* aircraftProp = fgGetNode("/sim/aircraft", true); string aircraftId(aircraftProp->getStringValue()); + flightgear::addSentryTag("aircraft", aircraftId); + PackageRef acftPackage; if (!haveExplicit) { acftPackage = pkgRoot->getPackageById(aircraftId); @@ -1131,6 +1139,8 @@ void fgStartReposition() SGPropertyNode *master_freeze = fgGetNode("/sim/freeze/master"); SG_LOG( SG_GENERAL, SG_INFO, "fgStartReposition()"); + flightgear::addSentryBreadcrumb("start reposition", "info"); + // ensure we are frozen bool freeze = master_freeze->getBoolValue(); if ( !freeze ) { @@ -1149,8 +1159,7 @@ void fgStartReposition() // main-loop flightgear::initPosition(); - simgear::SGTerraSync* terraSync = - static_cast(globals->get_subsystem("terrasync")); + auto terraSync = globals->get_subsystem(); if (terraSync) { terraSync->reposition(); } diff --git a/src/Main/fg_init.hxx b/src/Main/fg_init.hxx index 755222e3a..02dd84752 100644 --- a/src/Main/fg_init.hxx +++ b/src/Main/fg_init.hxx @@ -35,6 +35,8 @@ class SGPath; // Return the current base package version std::string fgBasePackageVersion(const SGPath& path); +SGPath fgHomePath(); + bool fgInitHome(); void fgShutdownHome(); diff --git a/src/Main/main.cxx b/src/Main/main.cxx index 6b1b24ccf..62e18d122 100755 --- a/src/Main/main.cxx +++ b/src/Main/main.cxx @@ -20,10 +20,7 @@ // // $Id$ - -#ifdef HAVE_CONFIG_H -# include -#endif +#include #include #include @@ -34,13 +31,6 @@ #include #include -#if defined(HAVE_CRASHRPT) - #include - -// defined in bootstrap.cxx -extern bool global_crashRptEnabled; - -#endif // Class references #include @@ -85,6 +75,7 @@ extern bool global_crashRptEnabled; #include "screensaver_control.hxx" #include "subsystemFactory.hxx" #include "options.hxx" +#include
#include #include @@ -214,6 +205,10 @@ static void initTerrasync() terra_sync->bind(); terra_sync->init(); + + if (fgGetBool("/sim/terrasync/enabled")) { + flightgear::addSentryTag("terrasync", "enabled"); + } } static void fgSetVideoOptions() @@ -255,6 +250,10 @@ static void fgSetVideoOptions() static void checkOpenGLVersion() { + flightgear::addSentryTag("gl-version", fgGetString("/sim/rendering/gl-version")); + flightgear::addSentryTag("gl-renderer", fgGetString("/sim/rendering/gl-vendor")); + flightgear::addSentryTag("gl-vendor", fgGetString("/sim/rendering/gl-renderer")); + #if defined(SG_MAC) // Mac users can't upgrade their drivers, so complaining about // versions doesn't help them much @@ -430,9 +429,12 @@ static void fgIdleFunction ( void ) { ngccn = new simgear::Notifications::NasalGarbageCollectionConfigurationNotification(nasal_gc_threaded->getBoolValue(), nasal_gc_threaded_wait->getBoolValue()); simgear::Emesary::GlobalTransmitter::instance()->NotifyAll(*ngccn); simgear::Emesary::GlobalTransmitter::instance()->NotifyAll(mln_started); + + flightgear::addSentryBreadcrumb("entering main loop", "info"); } if ( idle_state == 2000 ) { + flightgear::addSentryBreadcrumb("starting reset", "info"); fgStartNewReset(); idle_state = 2005; } @@ -517,15 +519,6 @@ static void logToHome() logPath.append("fgfs.log"); } sglog().logToFile(logPath, SG_ALL, SG_INFO); - -#if defined(HAVE_CRASHRPT) - if (global_crashRptEnabled) { - crAddFile2(logPath.c_str(), NULL, "FlightGear Log File", CR_AF_MAKE_FILE_COPY); - SG_LOG( SG_GENERAL, SG_INFO, "CrashRpt enabled"); - } else { - SG_LOG(SG_GENERAL, SG_WARN, "CrashRpt enabled at compile time but failed to install"); - } -#endif } // Main top level initialization @@ -551,6 +544,10 @@ int fgMainInit( int argc, char **argv ) logToHome(); } + if (readOnlyFGHome) { + flightgear::addSentryTag("fghome-readonly", "true"); + } + std::string version(FLIGHTGEAR_VERSION); SG_LOG( SG_GENERAL, SG_INFO, "FlightGear: Version " << version ); SG_LOG( SG_GENERAL, SG_INFO, "FlightGear: Build Type " << FG_BUILD_TYPE ); @@ -562,6 +559,9 @@ int fgMainInit( int argc, char **argv ) SG_LOG(SG_GENERAL, SG_ALERT, "Minimum supported OpenScenegraph is V3.4.1 - currently using " << osgGetVersion() << " This can cause fatal OSG 'final reference count' errors at runtime"); #endif + flightgear::addSentryTag("jenkins-build-number", JENKINS_BUILD_NUMBER); + flightgear::addSentryTag("osg-version", osgGetVersion()); + #ifdef __OpenBSD__ { /* OpenBSD defaults to a small maximum data segment, which can cause @@ -628,9 +628,12 @@ int fgMainInit( int argc, char **argv ) #if defined(HAVE_QT) if (showLauncher) { + flightgear::addSentryBreadcrumb("starting launcher", "info"); if (!flightgear::runLauncherDialog()) { return EXIT_SUCCESS; } + + flightgear::addSentryBreadcrumb("completed launcher", "info"); } #else if (showLauncher) { @@ -692,6 +695,7 @@ int fgMainInit( int argc, char **argv ) if (fgGetBool("/sim/ati-viewport-hack", true)) { SG_LOG(SG_GENERAL, SG_WARN, "Enabling ATI/AMD viewport hack"); + flightgear::addSentryTag("ati-viewport-hack", "enabled"); ATIScreenSizeHack(); } diff --git a/src/Main/sentryIntegration.cxx b/src/Main/sentryIntegration.cxx new file mode 100644 index 000000000..dd4f9cc9e --- /dev/null +++ b/src/Main/sentryIntegration.cxx @@ -0,0 +1,151 @@ +// sentryIntegration.cxx - Interface with Sentry.io crash reporting +// +// Copyright (C) 2020 James Turner james@flightgear.org +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License as +// published by the Free Software Foundation; either version 2 of the +// License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, but +// WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +#include "config.h" + +#include "sentryIntegration.hxx" + +#include + +#include +#include
+ +using namespace std; + +static bool static_sentryEnabled = false; + +// we don't want sentry enabled for the test suite +#if defined(HAVE_SENTRY) && !defined(BUILDING_TESTSUITE) + +#include + +namespace flightgear +{ + +void initSentry() +{ + sentry_options_t *options = sentry_options_new(); + sentry_options_set_dsn(options, "https://3a3f0bf24d5d482388dd060860c18ffe@sentry.io/5188535"); + + if (strcmp(FG_BUILD_TYPE, "Dev") == 0) { + sentry_options_set_release(options, "flightgear-dev@" FLIGHTGEAR_VERSION); + } else { + sentry_options_set_release(options, "flightgear@" FLIGHTGEAR_VERSION); + } + + SGPath dataPath = fgHomePath() / "sentry_db"; +#if defined(SG_WINDOWS) + const auto homePathString = dataPath.wstr(); + sentry_options_set_database_pathw(options, homePathString.c_str()); + + const auto logPath = (fgHomePath() / "fgfs.log").wstr(); + sentry_options_add_attachmentw(options, "log", logPath.c_str()); +#else + const auto homePathString = dataPath.utf8Str(); + sentry_options_set_database_path(options, homePathString.c_str()); + + const auto logPath = (fgHomePath() / "fgfs.log").utf8Str(); + sentry_options_add_attachment(options, "log", logPath.c_str()); +#endif + + sentry_init(options); + static_sentryEnabled = true; +} + +void shutdownSentry() +{ + if (static_sentryEnabled) { + sentry_shutdown(); + static_sentryEnabled = false; + } +} + +bool isSentryEnabled() +{ + return static_sentryEnabled; +} + +void addSentryBreadcrumb(const std::string& msg, const std::string& level) +{ + if (!static_sentryEnabled) + return; + + sentry_value_t crumb = sentry_value_new_breadcrumb("default", msg.c_str()); + sentry_value_set_by_key(crumb, "level", sentry_value_new_string(level.c_str())); + sentry_add_breadcrumb(crumb); +} + +void addSentryTag(const char* tag, const char* value) +{ + if (!static_sentryEnabled) + return; + + if (!tag || !value) + return; + + sentry_set_tag(tag, value); +} + +} // of namespace + +#else + +// stubs for non-sentry case + +namespace flightgear +{ + +void initSentry() +{ +} + +void shutdownSentry() +{ +} + +bool isSentryEnabled() +{ + return false; +} + +void addSentryBreadcrumb(const std::string&, const std::string&) +{ +} + +void addSentryTag(const char*, const char*) +{ +} + +} // of namespace + +#endif + +// common helpers + +namespace flightgear +{ + +void addSentryTag(const std::string& tag, const std::string& value) +{ + if (tag.empty() || value.empty()) + return; + + addSentryTag(tag.c_str(), value.c_str()); +} + +} // of namespace flightgear diff --git a/src/Main/sentryIntegration.hxx b/src/Main/sentryIntegration.hxx new file mode 100644 index 000000000..e1be9c0c4 --- /dev/null +++ b/src/Main/sentryIntegration.hxx @@ -0,0 +1,39 @@ +// sentryIntegration.hxx - Interface with Sentry.io crash reporting +// +// Copyright (C) 2020 James Turner james@flightgear.org +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License as +// published by the Free Software Foundation; either version 2 of the +// License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, but +// WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + +#pragma once + +#include + +namespace flightgear +{ +void initSentry(); + +void shutdownSentry(); + +bool isSentryEnabled(); + +void addSentryBreadcrumb(const std::string& msg, const std::string& level); + +void addSentryTag(const char* tag, const char* value); + +void addSentryTag(const std::string& tag, const std::string& value); + + +} // of namespace flightgear + diff --git a/test_suite/CMakeLists.txt b/test_suite/CMakeLists.txt index bcd803974..dc0d2d474 100644 --- a/test_suite/CMakeLists.txt +++ b/test_suite/CMakeLists.txt @@ -50,9 +50,11 @@ if (NOT SYSTEM_CPPUNIT) get_property(CPPUNIT_SOURCES GLOBAL PROPERTY CPPUNIT_SOURCES) get_property(CPPUNIT_HEADERS GLOBAL PROPERTY CPPUNIT_HEADERS) - include_directories("${PROJECT_SOURCE_DIR}/3rdparty/cppunit/include") add_library(CppUnitLib STATIC ${CPPUNIT_SOURCES} ${CPPUNIT_HEADERS}) + + target_include_directories(CppUnitLib PUBLIC "${PROJECT_SOURCE_DIR}/3rdparty/cppunit/include") + set(CPPUNIT_LIBRARIES CppUnitLib) else() message(STATUS "CppUnit: Linking to the system supplied CppUnit library") @@ -144,6 +146,8 @@ endif(ENABLE_AUTOTESTING) # Set up the target links. setup_fgfs_libraries(fgfs_test_suite) +target_compile_definitions(fgfs_test_suite PUBLIC BUILDING_TESTSUITE) + # Additional libraries just for the test suite. target_link_libraries(fgfs_test_suite ${CPPUNIT_LIBRARIES}) diff --git a/test_suite/bootstrap.cxx b/test_suite/bootstrap.cxx index 387846c7e..6e3077991 100644 --- a/test_suite/bootstrap.cxx +++ b/test_suite/bootstrap.cxx @@ -22,8 +22,5 @@ #include - -bool global_crashRptEnabled = false; - int _bootstrap_OSInit; std::string hostname;