adjustments to core suspender

this is an attempted step to improve semantics around core suspenders. this does not actually resolve DFHack#5074 but it is intended as a step toward doing so
ab9rf · Dec 14, 2024 · ca31a28 · ca31a28
1 parent fcc83fe
commit ca31a28
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 8 deletions.
diff --git 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<std::string> &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
@@ -31,6 +31,7 @@ distribution.
 #include "modules/Graphic.h"
 
 #include <atomic>
+#include <chrono>
 #include <condition_variable>
 #include <map>
 #include <memory>
@@ -353,15 +354,17 @@ namespace DFHack
         friend struct CoreSuspendReleaseMain;
     };
 
-    class CoreSuspenderBase  : protected std::unique_lock<std::recursive_mutex> {
+    class CoreSuspenderBase : protected std::unique_lock< decltype(Core::CoreSuspendMutex) > {
     protected:
-        using parent_t = std::unique_lock<std::recursive_mutex>;
+        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;
+
 }