1
0
mirror of synced 2025-02-16 10:32:35 +01:00

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
This commit is contained in:
iTrooz 2023-07-01 12:32:28 +02:00 committed by GitHub
parent 301418c728
commit c6c3ca4d26
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 68 additions and 52 deletions

View File

@ -47,7 +47,7 @@ namespace hex {
*/ */
static void setProjectFunctions( static void setProjectFunctions(
const std::function<bool(const std::fs::path&)> &loadFun, const std::function<bool(const std::fs::path&)> &loadFun,
const std::function<bool(std::optional<std::fs::path>)> &storeFun const std::function<bool(std::optional<std::fs::path>, bool)> &storeFun
); );
/** /**
@ -63,10 +63,11 @@ namespace hex {
* @brief Store a project file * @brief Store a project file
* *
* @param filePath Path to the 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 true if the project file was stored successfully
* @return false if the project file was not stored successfully * @return false if the project file was not stored successfully
*/ */
static bool store(std::optional<std::fs::path> filePath = std::nullopt); static bool store(std::optional<std::fs::path> filePath = std::nullopt, bool updateLocation = true);
/** /**
* @brief Check if a project file is currently loaded * @brief Check if a project file is currently loaded
@ -131,7 +132,7 @@ namespace hex {
ProjectFile() = default; ProjectFile() = default;
static std::function<bool(const std::fs::path&)> s_loadProjectFunction; static std::function<bool(const std::fs::path&)> s_loadProjectFunction;
static std::function<bool(std::optional<std::fs::path>)> s_storeProjectFunction; static std::function<bool(std::optional<std::fs::path>, bool)> s_storeProjectFunction;
static std::fs::path s_currProjectPath; static std::fs::path s_currProjectPath;
static std::vector<Handler> s_handlers; static std::vector<Handler> s_handlers;

View File

@ -17,11 +17,11 @@ namespace hex {
std::fs::path ProjectFile::s_currProjectPath; std::fs::path ProjectFile::s_currProjectPath;
std::function<bool(const std::fs::path&)> ProjectFile::s_loadProjectFunction; std::function<bool(const std::fs::path&)> ProjectFile::s_loadProjectFunction;
std::function<bool(std::optional<std::fs::path>)> ProjectFile::s_storeProjectFunction; std::function<bool(std::optional<std::fs::path>, bool)> ProjectFile::s_storeProjectFunction;
void ProjectFile::setProjectFunctions( void ProjectFile::setProjectFunctions(
const std::function<bool(const std::fs::path&)> &loadFun, const std::function<bool(const std::fs::path&)> &loadFun,
const std::function<bool(std::optional<std::fs::path>)> &storeFun const std::function<bool(std::optional<std::fs::path>, bool)> &storeFun
) { ) {
ProjectFile::s_loadProjectFunction = loadFun; ProjectFile::s_loadProjectFunction = loadFun;
ProjectFile::s_storeProjectFunction = storeFun; ProjectFile::s_storeProjectFunction = storeFun;
@ -31,8 +31,8 @@ namespace hex {
return s_loadProjectFunction(filePath); return s_loadProjectFunction(filePath);
} }
bool ProjectFile::store(std::optional<std::fs::path> filePath) { bool ProjectFile::store(std::optional<std::fs::path> filePath, bool updateLocation) {
return s_storeProjectFunction(filePath); return s_storeProjectFunction(filePath, updateLocation);
} }
bool ProjectFile::hasPath() { bool ProjectFile::hasPath() {

View File

@ -27,6 +27,8 @@ namespace hex::crash {
constexpr static auto Signals = {SIGSEGV, SIGILL, SIGABRT,SIGFPE}; constexpr static auto Signals = {SIGSEGV, SIGILL, SIGABRT,SIGFPE};
static std::terminate_handler originalHandler; static std::terminate_handler originalHandler;
void resetCrashHandlers();
static void sendNativeMessage(const std::string& message) { static void sendNativeMessage(const std::string& message) {
hex::nativeErrorMessage(hex::format("ImHex crashed during its loading.\nError: {}", 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 void handleCrash(const std::string &msg, int signalNumber) {
static void signalHandler(int signalNumber, const std::string &signalName) { crashCallback(msg);
// 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));
printStackTrace(); printStackTrace();
// Trigger an event so that plugins can handle crashes // 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<EventAbnormalTermination>(signalNumber); EventManager::post<EventAbnormalTermination>(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 // Detect if the crash was due to an uncaught exception
if (std::uncaught_exceptions() > 0) { if (std::uncaught_exceptions() > 0) {
@ -113,51 +115,60 @@ namespace hex::crash {
#undef HANDLE_SIGNAL #undef HANDLE_SIGNAL
} }
originalHandler = std::set_terminate([]{ // reset uncaught C++ exception handler
try { {
std::rethrow_exception(std::current_exception()); originalHandler = std::set_terminate([]{
} catch (std::exception &ex) { // reset crash handlers so we can't have a recursion if this code crashes
std::string exceptionStr = hex::format("{}()::what() -> {}", llvm::itaniumDemangle(typeid(ex).name(), nullptr, nullptr, nullptr), ex.what()); resetCrashHandlers();
log::fatal("Program terminated with uncaught exception: {}", exceptionStr);
EventManager::post<EventAbnormalTermination>(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 // Actually handle the crash
crashCallback(hex::format("Uncaught exception: {}", exceptionStr)); 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 #if defined(DEBUG)
for(auto signal : Signals) std::signal(signal, SIG_DFL); assert(!"Debug build, triggering breakpoint");
#else
// Restore the original handler of C++ std std::exit(100);
std::set_terminate(originalHandler); #endif
}
#if defined(DEBUG) });
assert(!"Debug build, triggering breakpoint"); }
#else
std::exit(100);
#endif
}
});
// Save a backup project when the application crashes // Save a backup project when the application crashes
// We need to save the project no mater if it is dirty, // We need to save the project no mater if it is dirty,
// because this save is responsible for telling us which files // because this save is responsible for telling us which files
// were opened in case there wasn't a project // were opened in case there wasn't a project
EventManager::subscribe<EventAbnormalTermination>([](int) { // Only do it when ImHex has finished its loading
auto imguiSettingsPath = hex::getImGuiSettingsPath(); EventManager::subscribe<EventImHexStartupFinished>([] {
if (!imguiSettingsPath.empty()) EventManager::subscribe<EventAbnormalTermination>([](int) {
ImGui::SaveIniSettingsToDisk(wolv::util::toUTF8String(imguiSettingsPath).c_str()); auto imguiSettingsPath = hex::getImGuiSettingsPath();
if (!imguiSettingsPath.empty())
ImGui::SaveIniSettingsToDisk(wolv::util::toUTF8String(imguiSettingsPath).c_str());
for (const auto &path : fs::getDefaultPaths(fs::ImHexPath::Config)) { for (const auto &path : fs::getDefaultPaths(fs::ImHexPath::Config)) {
if (ProjectFile::store(path / CrashBackupFileName)) if (ProjectFile::store(path / CrashBackupFileName, false))
break; break;
} }
});
}); });
// change the crash callback when ImHex has finished startup
EventManager::subscribe<EventImHexStartupFinished>([]{ EventManager::subscribe<EventImHexStartupFinished>([]{
crashCallback = saveCrashFile; crashCallback = saveCrashFile;
}); });
} }
void resetCrashHandlers() {
std::set_terminate(originalHandler);
for(auto signal : Signals) std::signal(signal, SIG_DFL);
}
} }

View File

@ -107,7 +107,7 @@ namespace hex::plugin::builtin {
return true; return true;
} }
bool store(std::optional<std::fs::path> filePath = std::nullopt) { bool store(std::optional<std::fs::path> filePath = std::nullopt, bool updateLocation = true) {
auto originalPath = ProjectFile::getPath(); auto originalPath = ProjectFile::getPath();
if (!filePath.has_value()) if (!filePath.has_value())
@ -155,7 +155,11 @@ namespace hex::plugin::builtin {
} }
ImHexApi::Provider::resetDirty(); ImHexApi::Provider::resetDirty();
resetPath.release();
// if saveLocation is false, reset the project path (do not release the lock)
if (updateLocation) {
resetPath.release();
}
return result; return result;
} }