From c6c3ca4d26a2cc109895d22e135528e6fb6714b2 Mon Sep 17 00:00:00 2001 From: iTrooz Date: Sat, 1 Jul 2023 12:32:28 +0200 Subject: [PATCH] fix: Reset terminate handler directly when being called + some other crashes to crash handling (#1174) This PR fixes some things about crash handling: - when the terminate handler is called, immediately set it back to the original one, so can't make a recursion if the crash-handling code fails - Only save projects if the crash occured after Imhex finished startup - do not update the project location when saving the crash backup file: this will remove problems when `EventAbnormalTermination` is called before `crashCallback()` I also added a bit more documentation --- .../include/hex/api/project_file_manager.hpp | 7 +- .../source/api/project_file_manager.cpp | 8 +- main/source/crash_handlers.cpp | 97 +++++++++++-------- plugins/builtin/source/content/project.cpp | 8 +- 4 files changed, 68 insertions(+), 52 deletions(-) diff --git a/lib/libimhex/include/hex/api/project_file_manager.hpp b/lib/libimhex/include/hex/api/project_file_manager.hpp index 9723c3522..f34edf35a 100644 --- a/lib/libimhex/include/hex/api/project_file_manager.hpp +++ b/lib/libimhex/include/hex/api/project_file_manager.hpp @@ -47,7 +47,7 @@ namespace hex { */ static void setProjectFunctions( const std::function &loadFun, - const std::function)> &storeFun + const std::function, bool)> &storeFun ); /** @@ -63,10 +63,11 @@ namespace hex { * @brief Store a project file * * @param filePath Path to the project file + * @param updateLocation update the project location so subssequent saves will save there * @return true if the project file was stored successfully * @return false if the project file was not stored successfully */ - static bool store(std::optional filePath = std::nullopt); + static bool store(std::optional filePath = std::nullopt, bool updateLocation = true); /** * @brief Check if a project file is currently loaded @@ -131,7 +132,7 @@ namespace hex { ProjectFile() = default; static std::function s_loadProjectFunction; - static std::function)> s_storeProjectFunction; + static std::function, bool)> s_storeProjectFunction; static std::fs::path s_currProjectPath; static std::vector s_handlers; diff --git a/lib/libimhex/source/api/project_file_manager.cpp b/lib/libimhex/source/api/project_file_manager.cpp index a9fb11d10..d186a2070 100644 --- a/lib/libimhex/source/api/project_file_manager.cpp +++ b/lib/libimhex/source/api/project_file_manager.cpp @@ -17,11 +17,11 @@ namespace hex { std::fs::path ProjectFile::s_currProjectPath; std::function ProjectFile::s_loadProjectFunction; - std::function)> ProjectFile::s_storeProjectFunction; + std::function, bool)> ProjectFile::s_storeProjectFunction; void ProjectFile::setProjectFunctions( const std::function &loadFun, - const std::function)> &storeFun + const std::function, bool)> &storeFun ) { ProjectFile::s_loadProjectFunction = loadFun; ProjectFile::s_storeProjectFunction = storeFun; @@ -31,8 +31,8 @@ namespace hex { return s_loadProjectFunction(filePath); } - bool ProjectFile::store(std::optional filePath) { - return s_storeProjectFunction(filePath); + bool ProjectFile::store(std::optional filePath, bool updateLocation) { + return s_storeProjectFunction(filePath, updateLocation); } bool ProjectFile::hasPath() { diff --git a/main/source/crash_handlers.cpp b/main/source/crash_handlers.cpp index f564877ad..6c0004e7a 100644 --- a/main/source/crash_handlers.cpp +++ b/main/source/crash_handlers.cpp @@ -27,6 +27,8 @@ namespace hex::crash { constexpr static auto Signals = {SIGSEGV, SIGILL, SIGABRT,SIGFPE}; static std::terminate_handler originalHandler; + + void resetCrashHandlers(); static void sendNativeMessage(const std::string& message) { hex::nativeErrorMessage(hex::format("ImHex crashed during its loading.\nError: {}", message)); @@ -66,22 +68,22 @@ namespace hex::crash { } } - // Custom signal handler to print various information and a stacktrace when the application crashes - static void signalHandler(int signalNumber, const std::string &signalName) { - // Reset the signal handler to the default handler - for(auto signal : Signals) std::signal(signal, SIG_DFL); - - log::fatal("Terminating with signal '{}' ({})", signalName, signalNumber); - - // Trigger the crash callback - crashCallback(hex::format("Received signal '{}' ({})", signalName, signalNumber)); + void handleCrash(const std::string &msg, int signalNumber) { + crashCallback(msg); printStackTrace(); - + // Trigger an event so that plugins can handle crashes - // It may affect things (like the project path), - // so we do this after saving the crash file EventManager::post(signalNumber); + } + + // Custom signal handler to print various information and a stacktrace when the application crashes + static void signalHandler(int signalNumber, const std::string &signalName) { + // reset crash handlers so we can't have a recursion if this code crashes + resetCrashHandlers(); + + // Actually handle the crash + handleCrash(hex::format("Received signal '{}' ({})", signalName, signalNumber), signalNumber); // Detect if the crash was due to an uncaught exception if (std::uncaught_exceptions() > 0) { @@ -113,51 +115,60 @@ namespace hex::crash { #undef HANDLE_SIGNAL } - originalHandler = std::set_terminate([]{ - try { - std::rethrow_exception(std::current_exception()); - } catch (std::exception &ex) { - std::string exceptionStr = hex::format("{}()::what() -> {}", llvm::itaniumDemangle(typeid(ex).name(), nullptr, nullptr, nullptr), ex.what()); - log::fatal("Program terminated with uncaught exception: {}", exceptionStr); + // reset uncaught C++ exception handler + { + originalHandler = std::set_terminate([]{ + // reset crash handlers so we can't have a recursion if this code crashes + resetCrashHandlers(); - EventManager::post(0); + try { + std::rethrow_exception(std::current_exception()); + } catch (std::exception &ex) { + std::string exceptionStr = hex::format("{}()::what() -> {}", llvm::itaniumDemangle(typeid(ex).name(), nullptr, nullptr, nullptr), ex.what()); + log::fatal("Program terminated with uncaught exception: {}", exceptionStr); - // Handle crash callback - crashCallback(hex::format("Uncaught exception: {}", exceptionStr)); + // Actually handle the crash + handleCrash(hex::format("Uncaught exception: {}", exceptionStr), 0); - printStackTrace(); + // Reset signal handlers prior to calling the original handler, because it may raise a signal + for (auto signal : Signals) std::signal(signal, SIG_DFL); - // Reset signal handlers prior to calling the original handler, because it may raise a signal - for(auto signal : Signals) std::signal(signal, SIG_DFL); - - // Restore the original handler of C++ std - std::set_terminate(originalHandler); - - #if defined(DEBUG) - assert(!"Debug build, triggering breakpoint"); - #else - std::exit(100); - #endif - } - }); + #if defined(DEBUG) + assert(!"Debug build, triggering breakpoint"); + #else + std::exit(100); + #endif + } + }); + } // Save a backup project when the application crashes // We need to save the project no mater if it is dirty, // because this save is responsible for telling us which files // were opened in case there wasn't a project - EventManager::subscribe([](int) { - auto imguiSettingsPath = hex::getImGuiSettingsPath(); - if (!imguiSettingsPath.empty()) - ImGui::SaveIniSettingsToDisk(wolv::util::toUTF8String(imguiSettingsPath).c_str()); + // Only do it when ImHex has finished its loading + EventManager::subscribe([] { + EventManager::subscribe([](int) { + auto imguiSettingsPath = hex::getImGuiSettingsPath(); + if (!imguiSettingsPath.empty()) + ImGui::SaveIniSettingsToDisk(wolv::util::toUTF8String(imguiSettingsPath).c_str()); - for (const auto &path : fs::getDefaultPaths(fs::ImHexPath::Config)) { - if (ProjectFile::store(path / CrashBackupFileName)) - break; - } + for (const auto &path : fs::getDefaultPaths(fs::ImHexPath::Config)) { + if (ProjectFile::store(path / CrashBackupFileName, false)) + break; + } + }); }); + // change the crash callback when ImHex has finished startup EventManager::subscribe([]{ crashCallback = saveCrashFile; }); } + + void resetCrashHandlers() { + std::set_terminate(originalHandler); + + for(auto signal : Signals) std::signal(signal, SIG_DFL); + } } \ No newline at end of file diff --git a/plugins/builtin/source/content/project.cpp b/plugins/builtin/source/content/project.cpp index aeca9fcda..fdf8306b1 100644 --- a/plugins/builtin/source/content/project.cpp +++ b/plugins/builtin/source/content/project.cpp @@ -107,7 +107,7 @@ namespace hex::plugin::builtin { return true; } - bool store(std::optional filePath = std::nullopt) { + bool store(std::optional filePath = std::nullopt, bool updateLocation = true) { auto originalPath = ProjectFile::getPath(); if (!filePath.has_value()) @@ -155,7 +155,11 @@ namespace hex::plugin::builtin { } ImHexApi::Provider::resetDirty(); - resetPath.release(); + + // if saveLocation is false, reset the project path (do not release the lock) + if (updateLocation) { + resetPath.release(); + } return result; }