From ad69ac84b136441b3bf8d262bd05fac9f33e6be2 Mon Sep 17 00:00:00 2001 From: Lennard Fonteijn Date: Sat, 16 Sep 2023 13:09:59 +0200 Subject: [PATCH] feat: Added hex::group attribute and various fixes (#1302) As discussed (many times) on Discord, does the same as the new favorite tag, but instead allows you to add multiple groups. Initially, this would cause some insane issues with draw/reset (apparantly) fighting eachother in the pattern drawer. After a lot of trial and error, I decided to rewrite the flow that is responsible for calling reset. Now evaluating patterns is the one to decide when the reset happens, not the core "game"-loop. To make sure that draw and reset can never happen at the same time, the mutex originally used for the favorites has been repurposed. Due to the restructuring, the mutex in the favorite-task is no longer needed, as that will only ever kick-off after reset is called and if there are actually patterns, which can never line up to be accessed on different threads at the same time. Last but not least, I noticed that hard crashes could result in your config file getting overridden. I added a check to prevent that. Last I issue I can see is that if you use an excessive amount of favorites/groups, a crash can still happen, but it only happens when you close the program (occasionally, but unpredictable). Before, this would happen if you ran the evaluation a second time. I boiled the cause of the crash down to these lines of code in evaluator.cpp > patternDestroyed: ```cpp if (pattern->isPatternLocal()) { if (auto it = this->m_patternLocalStorage.find(pattern->getHeapAddress()); it != this->m_patternLocalStorage.end()) { auto &[key, data] = *it; data.referenceCount--; if (data.referenceCount == 0) this->m_patternLocalStorage.erase(it); } else if (!this->m_evaluated) { err::E0001.throwError(fmt::format("Double free of variable named '{}'.", pattern->getVariableName())); } } ``` Specifically, trying to access the `*it` is the reason for the crash (this was also the cause of the crashes before my fixes, but then during evaluation). I'm suspecting the root cause is somewhere in the `.clone` methods of the patterns. I'd say that for now a crash when closing the program is more acceptable than during evaluation (which can even happen if you use favorites). --- lib/libimhex/include/hex/api/event.hpp | 1 + lib/libimhex/source/api/content_registry.cpp | 5 ++ .../content/views/view_pattern_data.hpp | 2 - plugins/builtin/include/ui/pattern_drawer.hpp | 1 + .../content/views/view_pattern_data.cpp | 20 +++--- .../content/views/view_pattern_editor.cpp | 2 + plugins/builtin/source/ui/pattern_drawer.cpp | 61 +++++++++++++++++-- 7 files changed, 75 insertions(+), 17 deletions(-) diff --git a/lib/libimhex/include/hex/api/event.hpp b/lib/libimhex/include/hex/api/event.hpp index 99511cddf..4543e36c3 100644 --- a/lib/libimhex/include/hex/api/event.hpp +++ b/lib/libimhex/include/hex/api/event.hpp @@ -206,6 +206,7 @@ namespace hex { EVENT_DEF(EventWindowInitialized); EVENT_DEF(EventBookmarkCreated, ImHexApi::Bookmarks::Entry&); EVENT_DEF(EventPatchCreated, u64, u8, u8); + EVENT_DEF(EventPatternEvaluating); EVENT_DEF(EventPatternExecuted, const std::string&); EVENT_DEF(EventPatternEditorChanged, const std::string&); EVENT_DEF(EventStoreContentDownloaded, const std::fs::path&); diff --git a/lib/libimhex/source/api/content_registry.cpp b/lib/libimhex/source/api/content_registry.cpp index 134da5737..8242d4edc 100644 --- a/lib/libimhex/source/api/content_registry.cpp +++ b/lib/libimhex/source/api/content_registry.cpp @@ -66,6 +66,11 @@ namespace hex { } void store() { + // During a crash settings can be empty, causing them to be overwritten. + if(getSettingsData().empty()) { + return; + } + for (const auto &dir : fs::getDefaultPaths(fs::ImHexPath::Config)) { wolv::io::File file(dir / SettingsFile, wolv::io::File::Mode::Create); diff --git a/plugins/builtin/include/content/views/view_pattern_data.hpp b/plugins/builtin/include/content/views/view_pattern_data.hpp index ee07723ab..c6c0a3947 100644 --- a/plugins/builtin/include/content/views/view_pattern_data.hpp +++ b/plugins/builtin/include/content/views/view_pattern_data.hpp @@ -20,8 +20,6 @@ namespace hex::plugin::builtin { private: ui::PatternDrawer m_patternDrawer; - bool m_shouldReset = false; - u64 m_lastRunId = 0; }; } \ No newline at end of file diff --git a/plugins/builtin/include/ui/pattern_drawer.hpp b/plugins/builtin/include/ui/pattern_drawer.hpp index 9cc491fbf..7935f0937 100644 --- a/plugins/builtin/include/ui/pattern_drawer.hpp +++ b/plugins/builtin/include/ui/pattern_drawer.hpp @@ -96,6 +96,7 @@ namespace hex::plugin::builtin::ui { std::vector m_filter; std::vector m_currPatternPath; std::map, std::unique_ptr> m_favorites; + std::map>> m_groups; bool m_showFavoriteStars = false; bool m_favoritesUpdated = false; bool m_showSpecName = false; diff --git a/plugins/builtin/source/content/views/view_pattern_data.cpp b/plugins/builtin/source/content/views/view_pattern_data.cpp index 551d26195..e0d981e01 100644 --- a/plugins/builtin/source/content/views/view_pattern_data.cpp +++ b/plugins/builtin/source/content/views/view_pattern_data.cpp @@ -20,6 +20,14 @@ namespace hex::plugin::builtin { this->m_patternDrawer.reset(); }); + EventManager::subscribe(this, [this]{ + this->m_patternDrawer.reset(); + }); + + EventManager::subscribe(this, [this](auto){ + this->m_patternDrawer.reset(); + }); + // Handle jumping to a pattern's location when it is clicked this->m_patternDrawer.setSelectionCallback([](Region region){ ImHexApi::HexEditor::setSelection(region); }); } @@ -27,6 +35,8 @@ namespace hex::plugin::builtin { ViewPatternData::~ViewPatternData() { EventManager::unsubscribe(this); EventManager::unsubscribe(this); + EventManager::unsubscribe(this); + EventManager::unsubscribe(this); } void ViewPatternData::drawContent() { @@ -36,20 +46,10 @@ namespace hex::plugin::builtin { // Make sure the runtime has finished evaluating and produced valid patterns auto &runtime = ContentRegistry::PatternLanguage::getRuntime(); if (!runtime.arePatternsValid()) { - // If the runtime is still evaluating, reset the pattern drawer - this->m_shouldReset = true; - this->m_patternDrawer.reset(); this->m_patternDrawer.draw({ }); } else { // If the runtime has finished evaluating, draw the patterns if (TRY_LOCK(ContentRegistry::PatternLanguage::getRuntimeLock())) { - auto runId = runtime.getRunId(); - if (this->m_shouldReset || this->m_lastRunId != runId) { - this->m_lastRunId = runId; - this->m_patternDrawer.reset(); - this->m_shouldReset = false; - } - this->m_patternDrawer.draw(runtime.getPatterns(), &runtime); } } diff --git a/plugins/builtin/source/content/views/view_pattern_editor.cpp b/plugins/builtin/source/content/views/view_pattern_editor.cpp index cd7dc63c4..a5abba3cd 100644 --- a/plugins/builtin/source/content/views/view_pattern_editor.cpp +++ b/plugins/builtin/source/content/views/view_pattern_editor.cpp @@ -923,6 +923,8 @@ namespace hex::plugin::builtin { } void ViewPatternEditor::evaluatePattern(const std::string &code, prv::Provider *provider) { + EventManager::post(); + auto lock = std::scoped_lock(ContentRegistry::PatternLanguage::getRuntimeLock()); this->m_runningEvaluators++; diff --git a/plugins/builtin/source/ui/pattern_drawer.cpp b/plugins/builtin/source/ui/pattern_drawer.cpp index 684cc2a02..238a38dcd 100644 --- a/plugins/builtin/source/ui/pattern_drawer.cpp +++ b/plugins/builtin/source/ui/pattern_drawer.cpp @@ -37,7 +37,7 @@ namespace hex::plugin::builtin::ui { namespace { - std::mutex s_favoritesMutex; + std::mutex s_resetDrawMutex; constexpr auto DisplayEndDefault = 50U; @@ -1006,6 +1006,8 @@ namespace hex::plugin::builtin::ui { } void PatternDrawer::draw(const std::vector> &patterns, pl::PatternLanguage *runtime, float height) { + std::scoped_lock lock(s_resetDrawMutex); + const auto treeStyleButton = [this](auto icon, TreeStyle style, const char *tooltip) { bool pushed = false; @@ -1089,12 +1091,20 @@ namespace hex::plugin::builtin::ui { this->m_favoritesUpdateTask = TaskManager::createTask("hex.builtin.pattern_drawer.updating"_lang, TaskManager::NoProgress, [this, patterns](auto &task) { size_t updatedFavorites = 0; - std::scoped_lock lock(s_favoritesMutex); for (auto &pattern : patterns) { std::vector patternPath; traversePatternTree(*pattern, patternPath, [&, this](pl::ptrn::Pattern &pattern) { if (pattern.hasAttribute("hex::favorite")) this->m_favorites.insert({ patternPath, pattern.clone() }); + + if (const auto &args = pattern.getAttributeArguments("hex::group"); !args.empty()) { + auto groupName = args.front().toString(); + + if (!this->m_groups.contains(groupName)) + this->m_groups.insert({groupName, std::vector>()}); + + this->m_groups[groupName].push_back(pattern.clone()); + } }); if (updatedFavorites == this->m_favorites.size()) @@ -1133,11 +1143,13 @@ namespace hex::plugin::builtin::ui { this->m_showFavoriteStars = false; if (!this->m_favoritesUpdateTask.isRunning()) { + int id = 1; + bool doTableNextRow = false; if (!this->m_favorites.empty() && !patterns.empty()) { ImGui::TableNextColumn(); ImGui::TableNextColumn(); - ImGui::PushID(1); + ImGui::PushID(id); if (ImGui::TreeNodeEx("hex.builtin.pattern_drawer.favorites"_lang, ImGuiTreeNodeFlags_SpanFullWidth)) { for (auto &[path, pattern] : this->m_favorites) { if (pattern == nullptr) @@ -1151,11 +1163,43 @@ namespace hex::plugin::builtin::ui { ImGui::TreePop(); } ImGui::PopID(); + + id += 1; + doTableNextRow = true; + } + + if (!this->m_groups.empty() && !patterns.empty()) { + for (auto &[groupName, groupPatterns]: this->m_groups) { + if(doTableNextRow) { + ImGui::TableNextRow(); + } + + ImGui::TableNextColumn(); + ImGui::TableNextColumn(); + ImGui::PushID(id); + if (ImGui::TreeNodeEx(groupName.c_str(), ImGuiTreeNodeFlags_SpanFullWidth)) { + for (auto &groupPattern: groupPatterns) { + if (groupPattern == nullptr) + continue; + + ImGui::PushID(id); + this->draw(*groupPattern); + ImGui::PopID(); + + id += 1; + } + + ImGui::TreePop(); + } + ImGui::PopID(); + + id += 1; + doTableNextRow = true; + } } this->m_showFavoriteStars = true; - int id = 2; for (auto &pattern : this->m_sortedPatterns) { ImGui::PushID(id); this->draw(*pattern); @@ -1174,6 +1218,8 @@ namespace hex::plugin::builtin::ui { } void PatternDrawer::reset() { + std::scoped_lock lock(s_resetDrawMutex); + this->resetEditing(); this->m_displayEnd.clear(); this->m_visualizedPatterns.clear(); @@ -1184,9 +1230,14 @@ namespace hex::plugin::builtin::ui { this->m_favoritesUpdateTask.interrupt(); - std::scoped_lock lock(s_favoritesMutex); for (auto &[path, pattern] : this->m_favorites) pattern = nullptr; + for (auto &[groupName, patterns]: this->m_groups) + for (auto &pattern: patterns) + pattern = nullptr; + + this->m_groups.clear(); + this->m_favoritesUpdated = false; } }