From f587710d1cdbeda70ddba22a1f13534b9f9f28f1 Mon Sep 17 00:00:00 2001 From: WerWolv Date: Tue, 26 Mar 2024 19:48:38 +0100 Subject: [PATCH] fix: Multiple memory corruption issues --- lib/libimhex/source/api/imhex_api.cpp | 4 ++-- lib/libimhex/source/api/theme_manager.cpp | 17 +++++++++++++++-- main/gui/source/init/tasks.cpp | 13 +++++++++---- main/gui/source/window/window.cpp | 1 - 4 files changed, 26 insertions(+), 9 deletions(-) diff --git a/lib/libimhex/source/api/imhex_api.cpp b/lib/libimhex/source/api/imhex_api.cpp index 51c093aaf..1120e9308 100644 --- a/lib/libimhex/source/api/imhex_api.cpp +++ b/lib/libimhex/source/api/imhex_api.cpp @@ -880,9 +880,9 @@ namespace hex { s_fontSize = size; } - static AutoReset> s_fontAtlas; + static AutoReset> s_fontAtlas; void setFontAtlas(ImFontAtlas* fontAtlas) { - s_fontAtlas = std::unique_ptr(fontAtlas); + s_fontAtlas = std::unique_ptr(fontAtlas, IM_DELETE); } static ImFont *s_boldFont = nullptr; diff --git a/lib/libimhex/source/api/theme_manager.cpp b/lib/libimhex/source/api/theme_manager.cpp index 35029fb83..0162e40da 100644 --- a/lib/libimhex/source/api/theme_manager.cpp +++ b/lib/libimhex/source/api/theme_manager.cpp @@ -17,18 +17,25 @@ namespace hex { AutoReset s_imageTheme; AutoReset s_currTheme; + std::mutex s_themeMutex; } void ThemeManager::addThemeHandler(const std::string &name, const ColorMap &colorMap, const std::function &getFunction, const std::function &setFunction) { + std::unique_lock lock(s_themeMutex); + (*s_themeHandlers)[name] = { colorMap, getFunction, setFunction }; } void ThemeManager::addStyleHandler(const std::string &name, const StyleMap &styleMap) { + std::unique_lock lock(s_themeMutex); + (*s_styleHandlers)[name] = { styleMap }; } void ThemeManager::addTheme(const std::string &content) { + std::unique_lock lock(s_themeMutex); + try { auto theme = nlohmann::json::parse(content); @@ -106,6 +113,8 @@ namespace hex { } void ThemeManager::changeTheme(std::string name) { + std::unique_lock lock(s_themeMutex); + if (!s_themes->contains(name)) { if (s_themes->empty()) { return; @@ -168,12 +177,12 @@ namespace hex { const float scale = style.needsScaling ? 1_scaled : 1.0F; if (value.is_number_float()) { - if (const auto newValue = std::get_if(&style.value); newValue != nullptr) + if (const auto newValue = std::get_if(&style.value); newValue != nullptr && *newValue != nullptr) **newValue = value.get() * scale; else log::warn("Style variable '{}' was of type ImVec2 but a float was expected.", name); } else if (value.is_array() && value.size() == 2 && value[0].is_number_float() && value[1].is_number_float()) { - if (const auto newValue = std::get_if(&style.value); newValue != nullptr) + if (const auto newValue = std::get_if(&style.value); newValue != nullptr && *newValue != nullptr) **newValue = ImVec2(value[0].get() * scale, value[1].get() * scale); else log::warn("Style variable '{}' was of type float but a ImVec2 was expected.", name); @@ -191,6 +200,8 @@ namespace hex { hex::log::error("Theme '{}' has invalid image theme!", name); s_imageTheme = "dark"; } + } else { + s_imageTheme = "dark"; } s_currTheme = name; @@ -211,6 +222,8 @@ namespace hex { } void ThemeManager::reset() { + std::unique_lock lock(s_themeMutex); + s_themes->clear(); s_styleHandlers->clear(); s_themeHandlers->clear(); diff --git a/main/gui/source/init/tasks.cpp b/main/gui/source/init/tasks.cpp index c2d0106b6..09beb6929 100644 --- a/main/gui/source/init/tasks.cpp +++ b/main/gui/source/init/tasks.cpp @@ -57,16 +57,20 @@ namespace hex::init { // Terminate all asynchronous tasks TaskManager::exit(); - // Unlock font atlas so it can be deleted in case of a crash - if (ImGui::GetCurrentContext() != nullptr) - ImGui::GetIO().Fonts->Locked = false; + // Unlock font atlas, so it can be deleted in case of a crash + if (ImGui::GetCurrentContext() != nullptr) { + if (ImGui::GetIO().Fonts != nullptr) { + ImGui::GetIO().Fonts->Locked = false; + ImGui::GetIO().Fonts = nullptr; + } + } // Print a nice message if a crash happened while cleaning up resources // To the person fixing this: // ALWAYS wrap static heap allocated objects inside libimhex such as std::vector, std::string, std::function, etc. in a AutoReset // e.g `AutoReset> m_structs;` // - // The reason this is necessary is because each plugin / dynamic library gets its own instance of `std::allocator` + // The reason this is necessary because each plugin / dynamic library gets its own instance of `std::allocator` // which will try to free the allocated memory when the object is destroyed. However since the storage is static, this // will happen only when libimhex is unloaded after main() returns. At this point all plugins have been unloaded already so // the std::allocator will try to free memory in a heap that does not exist anymore which will cause a crash. @@ -81,6 +85,7 @@ namespace hex::init { }); ImHexApi::System::impl::cleanup(); + EventImHexClosing::post(); EventManager::clear(); diff --git a/main/gui/source/window/window.cpp b/main/gui/source/window/window.cpp index 427268de9..7954bd603 100644 --- a/main/gui/source/window/window.cpp +++ b/main/gui/source/window/window.cpp @@ -985,7 +985,6 @@ namespace hex { ImGui_ImplOpenGL3_Shutdown(); ImGui_ImplGlfw_Shutdown(); - ImNodes::DestroyContext(); ImPlot::DestroyContext(); ImGui::DestroyContext(); }