From ca31a280a29ca1fb24864de6cbb18c82ea0a32d3 Mon Sep 17 00:00:00 2001 From: Kelly Kinkade Date: Mon, 2 Dec 2024 21:55:46 -0600 Subject: [PATCH] adjustments to core suspender this is an attempted step to improve semantics around core suspenders. this does not actually resolve #5074 but it is intended as a step toward doing so --- library/Core.cpp | 18 ++++++++++++++++-- library/include/Core.h | 41 +++++++++++++++++++++++++++++++++++------ 2 files changed, 51 insertions(+), 8 deletions(-) diff --git a/library/Core.cpp b/library/Core.cpp index ca9199c56c..7900faa346 100644 --- a/library/Core.cpp +++ b/library/Core.cpp @@ -439,7 +439,15 @@ bool is_builtin(color_ostream &con, const std::string &command) { } void get_commands(color_ostream &con, std::vector &commands) { - CoreSuspender suspend; + // FIXME: this should be a conditional lock but that functionality doesn't work yet + CoreSuspender suspend{}; + + if (!suspend.owns_lock()) { + con.printerr("Cannot acquire core lock in helpdb.get_commands\n"); + commands.clear(); + return; + } + auto L = Lua::Core::State; Lua::StackUnwinder top(L); @@ -641,7 +649,13 @@ static std::string sc_event_name (state_change_event id) { } void help_helper(color_ostream &con, const std::string &entry_name) { - CoreSuspender suspend; + // FIXME: this should be a conditional lock but that functionality doesn't work yet + CoreSuspender suspend{ }; + + if (!suspend.owns_lock()) { + con.printerr("Failed Lua call to helpdb.help (could not acquire core lock).\n"); + return; + } auto L = Lua::Core::State; Lua::StackUnwinder top(L); diff --git a/library/include/Core.h b/library/include/Core.h index 88ed02ce4d..9671cb1892 100644 --- a/library/include/Core.h +++ b/library/include/Core.h @@ -31,6 +31,7 @@ distribution. #include "modules/Graphic.h" #include +#include #include #include #include @@ -353,15 +354,17 @@ namespace DFHack friend struct CoreSuspendReleaseMain; }; - class CoreSuspenderBase : protected std::unique_lock { + class CoreSuspenderBase : protected std::unique_lock< decltype(Core::CoreSuspendMutex) > { protected: - using parent_t = std::unique_lock; + size_t lock_count = 0; + using mutex_type = decltype(Core::CoreSuspendMutex); + using parent_t = std::unique_lock< mutex_type >; std::thread::id tid; CoreSuspenderBase(std::defer_lock_t d) : CoreSuspenderBase{&Core::getInstance(), d} {} CoreSuspenderBase(Core* core, std::defer_lock_t) : - /* Lock the core */ + /* Lock the core (jk not really) */ parent_t{core->CoreSuspendMutex,std::defer_lock}, /* Mark this thread to be the core owner */ tid{} @@ -373,6 +376,20 @@ namespace DFHack parent_t::lock(); tid = core.ownerThread.exchange(std::this_thread::get_id(), std::memory_order_acquire); + lock_count++; + } + + bool try_lock() + { + auto& core = Core::getInstance(); + bool locked = parent_t::try_lock(); + if (locked) + { + tid = core.ownerThread.exchange(std::this_thread::get_id(), + std::memory_order_acquire); + lock_count++; + } + return locked; } void unlock() @@ -383,15 +400,17 @@ namespace DFHack if (tid == std::thread::id{}) Lua::Core::Reset(core.getConsole(), "suspend"); parent_t::unlock(); + lock_count--; } bool owns_lock() const noexcept { - return parent_t::owns_lock(); + return lock_count > 0; +// return parent_t::owns_lock(); } ~CoreSuspenderBase() { - if (owns_lock()) + while (lock_count > 0) unlock(); } friend class MainThread; @@ -443,6 +462,15 @@ namespace DFHack parent_t::lock(); } + bool try_lock() + { + auto& core = Core::getInstance(); + bool locked = parent_t::try_lock(); + if (locked) + core.toolCount.fetch_add(1, std::memory_order_relaxed); + return locked; + } + void unlock() { auto& core = Core::getInstance(); @@ -457,7 +485,7 @@ namespace DFHack } ~CoreSuspender() { - if (owns_lock()) + while (lock_count > 0) unlock(); } }; @@ -483,4 +511,5 @@ namespace DFHack }; using CoreSuspendClaimer = CoreSuspender; + }