From 9483459be4aa49deaf68950f2129af2e2ae605a3 Mon Sep 17 00:00:00 2001
From: Kelly Kinkade <kelly.lynn.martin@gmail.com>
Date: Mon, 30 Dec 2024 22:17:58 -0600
Subject: [PATCH] add thread safety assertions to `getLuaState`

split `CoreDefs.h` from `Core.h` to keep `Core` from being loaded into `RemoteClient`
---
 library/CMakeLists.txt                       |  1 +
 library/Core.cpp                             |  2 +-
 library/include/Core.h                       | 35 +++++--------------
 library/include/CoreDefs.h                   | 36 ++++++++++++++++++++
 library/include/RemoteClient.h               |  2 +-
 plugins/remotefortressreader/item_reader.cpp |  1 +
 6 files changed, 48 insertions(+), 29 deletions(-)
 create mode 100644 library/include/CoreDefs.h

diff --git a/library/CMakeLists.txt b/library/CMakeLists.txt
index c24a4cedbb..c05d2cb6d1 100644
--- a/library/CMakeLists.txt
+++ b/library/CMakeLists.txt
@@ -55,6 +55,7 @@ set(MAIN_HEADERS
     include/ColorText.h
     include/Console.h
     include/Core.h
+    include/CoreDefs.h
     include/DataDefs.h
     include/DataFuncs.h
     include/DataIdentity.h
diff --git a/library/Core.cpp b/library/Core.cpp
index 21fc69efd1..d9910c41c8 100644
--- a/library/Core.cpp
+++ b/library/Core.cpp
@@ -346,7 +346,7 @@ static command_result runLuaScript(color_ostream &out, std::string name, std::ve
     data.pcmd = &name;
     data.pargs = &args;
 
-    bool ok = Lua::RunCoreQueryLoop(out, DFHack::Core::getInstance().getLuaState(), init_run_script, &data);
+    bool ok = Lua::RunCoreQueryLoop(out, DFHack::Core::getInstance().getLuaState(true), init_run_script, &data);
 
     return ok ? CR_OK : CR_FAILURE;
 }
diff --git a/library/include/Core.h b/library/include/Core.h
index 06ee17d4a7..5b89b88a11 100644
--- a/library/include/Core.h
+++ b/library/include/Core.h
@@ -25,6 +25,7 @@ distribution.
 #pragma once
 
 #include "Console.h"
+#include "CoreDefs.h"
 #include "Export.h"
 #include "Hooks.h"
 
@@ -76,31 +77,6 @@ namespace DFHack
         struct Hide;
     }
 
-    enum command_result
-    {
-        CR_LINK_FAILURE = -3,    // RPC call failed due to I/O or protocol error
-        CR_NEEDS_CONSOLE = -2,   // Attempt to call interactive command without console
-        CR_NOT_IMPLEMENTED = -1, // Command not implemented, or plugin not loaded
-        CR_OK = 0,               // Success
-        CR_FAILURE = 1,          // Failure
-        CR_WRONG_USAGE = 2,      // Wrong arguments or ui state
-        CR_NOT_FOUND = 3         // Target object not found (for RPC mainly)
-    };
-
-    enum state_change_event
-    {
-        SC_UNKNOWN = -1,
-        SC_WORLD_LOADED = 0,
-        SC_WORLD_UNLOADED = 1,
-        SC_MAP_LOADED = 2,
-        SC_MAP_UNLOADED = 3,
-        SC_VIEWSCREEN_CHANGED = 4,
-        SC_CORE_INITIALIZED = 5,
-        SC_BEGIN_UNLOAD = 6,
-        SC_PAUSED = 7,
-        SC_UNPAUSED = 8
-    };
-
     class DFHACK_EXPORT PerfCounters
     {
     public:
@@ -244,7 +220,10 @@ namespace DFHack
 
         PerfCounters perf_counters;
 
-        lua_State* getLuaState() { return State; }
+        lua_State* getLuaState(bool bypass_assertion = false) {
+            assert(bypass_assertion || isSuspended());
+            return State;
+        }
 
     private:
         DFHack::Console con;
@@ -400,9 +379,11 @@ namespace DFHack
             if (!owns_lock())
                 return;
             /* 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");
+            core.ownerThread.store(tid, std::memory_order_release);
+            //if (tid == std::thread::id{})
+            //    Lua::Core::Reset(core.getConsole(), "suspend");
             parent_t::unlock();
         }
 
diff --git a/library/include/CoreDefs.h b/library/include/CoreDefs.h
new file mode 100644
index 0000000000..dd78649394
--- /dev/null
+++ b/library/include/CoreDefs.h
@@ -0,0 +1,36 @@
+#pragma once
+
+/**
+  * Split off from Core.h to keep unnecessary class definitions out of compilation units that do not need them
+  *
+  */
+
+namespace DFHack
+{
+
+    enum command_result
+    {
+        CR_LINK_FAILURE = -3,    // RPC call failed due to I/O or protocol error
+        CR_NEEDS_CONSOLE = -2,   // Attempt to call interactive command without console
+        CR_NOT_IMPLEMENTED = -1, // Command not implemented, or plugin not loaded
+        CR_OK = 0,               // Success
+        CR_FAILURE = 1,          // Failure
+        CR_WRONG_USAGE = 2,      // Wrong arguments or ui state
+        CR_NOT_FOUND = 3         // Target object not found (for RPC mainly)
+    };
+
+    enum state_change_event
+    {
+        SC_UNKNOWN = -1,
+        SC_WORLD_LOADED = 0,
+        SC_WORLD_UNLOADED = 1,
+        SC_MAP_LOADED = 2,
+        SC_MAP_UNLOADED = 3,
+        SC_VIEWSCREEN_CHANGED = 4,
+        SC_CORE_INITIALIZED = 5,
+        SC_BEGIN_UNLOAD = 6,
+        SC_PAUSED = 7,
+        SC_UNPAUSED = 8
+    };
+
+}
diff --git a/library/include/RemoteClient.h b/library/include/RemoteClient.h
index 449a743753..abcf61dc1e 100644
--- a/library/include/RemoteClient.h
+++ b/library/include/RemoteClient.h
@@ -25,7 +25,7 @@ distribution.
 #pragma once
 #include "Export.h"
 #include "ColorText.h"
-#include "Core.h"
+#include "CoreDefs.h"
 
 class CPassiveSocket;
 class CActiveSocket;
diff --git a/plugins/remotefortressreader/item_reader.cpp b/plugins/remotefortressreader/item_reader.cpp
index d06c2473eb..1353243b2b 100644
--- a/plugins/remotefortressreader/item_reader.cpp
+++ b/plugins/remotefortressreader/item_reader.cpp
@@ -53,6 +53,7 @@
 #include "modules/MapCache.h"
 #include "modules/Materials.h"
 #include "MiscUtils.h"
+#include "Core.h"
 
 
 using namespace DFHack;