Avoid locking CoreSuspender second time in main thread

library/Core.cpp

@@ -97,12 +97,15 @@ using df::global::world;
 static bool parseKeySpec(std::string keyspec, int *psym, int *pmod, std::string *pfocus = NULL);
 size_t loadScriptFiles(Core* core, color_ostream& out, const vector<std::string>& prefix, const std::string& folder);
 
-//! mainThreadSuspend keeps the main DF thread suspended from Core::Init to
-//! thread exit.
-template<typename M>
-static std::unique_lock<M>& mainThreadSuspend(M& mutex) {
-    static thread_local std::unique_lock<M> lock(mutex, std::defer_lock);
-    return lock;
+namespace DFHack {
+struct MainThread {
+    //! MainThread::suspend keeps the main DF thread suspended from Core::Init to
+    //! thread exit.
+    static CoreSuspenderBase& suspend() {
+        static thread_local CoreSuspenderBase lock(std::defer_lock);
+        return lock;
+    }
+};
 }
 
 struct Core::Private
@@ -1454,8 +1457,8 @@ void fIOthread(void * iodata)
 
 Core::~Core()
 {
-    if (mainThreadSuspend(CoreSuspendMutex).owns_lock())
-        mainThreadSuspend(CoreSuspendMutex).unlock();
+    if (MainThread::suspend().owns_lock())
+        MainThread::suspend().unlock();
 
     if (d->hotkeythread.joinable()) {
         std::lock_guard<std::mutex> lock(HotkeyMutex);
@@ -1544,7 +1547,7 @@ bool Core::Init()
 
     // Lock the CoreSuspendMutex until the thread exits or call Core::Shutdown
     // Core::Update will temporary unlock when there is any commands queued
-    mainThreadSuspend(CoreSuspendMutex).lock();
+    MainThread::suspend().lock();
 
     // Re-route stdout and stderr again - DF seems to set up stdout and
     // stderr.txt on Windows as of 0.43.05. Also, log before switching files to
@@ -2005,8 +2008,6 @@ int Core::Update()
     // Pretend this thread has suspended the core in the usual way,
     // and run various processing hooks.
     {
-        CoreSuspendClaimer suspend(true);
-
         // Initialize the core
         bool first_update = false;
 
@@ -2023,7 +2024,7 @@ int Core::Update()
     }
 
     // Let all commands run that require CoreSuspender
-    CoreWakeup.wait(mainThreadSuspend(CoreSuspendMutex),
+    CoreWakeup.wait(MainThread::suspend(),
             [this]() -> bool {return this->toolCount.load() == 0;});
 
     return 0;
@@ -2245,8 +2246,8 @@ int Core::Shutdown ( void )
     errorstate = 1;
 
     // Make sure we release main thread if this is called from main thread
-    if (mainThreadSuspend(CoreSuspendMutex).owns_lock())
-        mainThreadSuspend(CoreSuspendMutex).unlock();
+    if (MainThread::suspend().owns_lock())
+        MainThread::suspend().unlock();
 
     // Make sure the console thread shutdowns before clean up to avoid any
     // unlikely data races.

library/include/Core.h

@@ -299,14 +299,52 @@ namespace DFHack
         friend class CoreService;
         friend class ServerConnection;
         friend class CoreSuspender;
+        friend class CoreSuspenderBase;
         ServerMain *server;
     };
 
-    template<typename Derived>
-    struct ToolIncrement {
-        ToolIncrement(std::atomic<size_t>& toolCount) {
-            toolCount += 1;
+    class CoreSuspenderBase  : protected std::unique_lock<std::recursive_mutex> {
+    protected:
+        using parent_t = std::unique_lock<std::recursive_mutex>;
+        std::thread::id tid;
+
+        CoreSuspenderBase(std::defer_lock_t d) : CoreSuspenderBase{&Core::getInstance(), d} {}
+
+        CoreSuspenderBase(Core* core, std::defer_lock_t) :
+            /* Lock the core */
+            parent_t{core->CoreSuspendMutex,std::defer_lock},
+            /* Mark this thread to be the core owner */
+            tid{}
+        {}
+    public:
+        void lock()
+        {
+            auto& core = Core::getInstance();
+            parent_t::lock();
+            tid = core.ownerThread.exchange(std::this_thread::get_id(),
+                    std::memory_order_acquire);
+        }
+
+        void unlock()
+        {
+            auto& core = Core::getInstance();
+            /* Restore core owner to previous value */
+            core.ownerThread.store(tid, std::memory_order_release);
+            if (tid == std::thread::id{})
+                Lua::Core::Reset(core.getConsole(), "suspend");
+            parent_t::unlock();
+        }
+
+        bool owns_lock() const noexcept
+        {
+            return parent_t::owns_lock();
+        }
+
+        ~CoreSuspenderBase() {
+            if (owns_lock())
+                unlock();
         }
+        friend class MainThread;
     };
 
     /*!
@@ -332,36 +370,45 @@ namespace DFHack
      *   no more tools are queued trying to acquire the
      *   Core::CoreSuspenderMutex.
      */
-    class CoreSuspender : protected ToolIncrement<CoreSuspender>,
-                          public std::unique_lock<std::recursive_mutex> {
-        using parent_t = std::unique_lock<std::recursive_mutex>;
-        Core *core;
-        std::thread::id tid;
+    class CoreSuspender : public CoreSuspenderBase {
+        using parent_t = CoreSuspenderBase;
     public:
-        CoreSuspender() : CoreSuspender(&Core::getInstance()) { }
-        CoreSuspender(bool) : CoreSuspender(&Core::getInstance()) { }
-        CoreSuspender(Core* core, bool) : CoreSuspender(core) { }
+        CoreSuspender() : CoreSuspender{&Core::getInstance()} { }
+        CoreSuspender(std::defer_lock_t d) : CoreSuspender{&Core::getInstance(),d} { }
+        CoreSuspender(bool) : CoreSuspender{&Core::getInstance()} { }
+        CoreSuspender(Core* core, bool) : CoreSuspender{core} { }
         CoreSuspender(Core* core) :
-            /* Increment the wait count */
-            ToolIncrement{core->toolCount},
-            /* Lock the core */
-            parent_t{core->CoreSuspendMutex},
-            core{core},
-            /* Mark this thread to be the core owner */
-            tid{core->ownerThread.exchange(std::this_thread::get_id())}
-        { }
-        ~CoreSuspender() {
-            /* Restore core owner to previous value */
-            core->ownerThread.store(tid);
-            if (tid == std::thread::id{})
-                Lua::Core::Reset(core->getConsole(), "suspend");
+            CoreSuspenderBase{core, std::defer_lock}
+        {
+            lock();
+        }
+        CoreSuspender(Core* core, std::defer_lock_t) :
+            CoreSuspenderBase{core, std::defer_lock}
+        {}
+
+        void lock()
+        {
+            auto& core = Core::getInstance();
+            core.toolCount.fetch_add(1, std::memory_order_relaxed);
+            parent_t::lock();
+        }
+
+        void unlock()
+        {
+            auto& core = Core::getInstance();
+            parent_t::unlock();
             /* Notify core to continue when all queued tools have completed
              * 0 = None wants to own the core
              * 1+ = There are tools waiting core access
              * fetch_add returns old value before subtraction
              */
-            if (core->toolCount.fetch_add(-1) == 1)
-                core->CoreWakeup.notify_one();
+            if (core.toolCount.fetch_add(-1, std::memory_order_relaxed) == 1)
+                core.CoreWakeup.notify_one();
+        }
+
+        ~CoreSuspender() {
+            if (owns_lock())
+                unlock();
         }
     };