Refactor core Lua state and add thread safety assertions

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

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, Lua::Core::State, init_run_script, &data);
+    bool ok = Lua::RunCoreQueryLoop(out, DFHack::Core::getInstance().getLuaState(true), init_run_script, &data);
 
     return ok ? CR_OK : CR_FAILURE;
 }
@@ -370,7 +370,7 @@ static command_result enableLuaScript(color_ostream &out, std::string name, bool
     data.pcmd = &name;
     data.pstate = state;
 
-    bool ok = Lua::RunCoreQueryLoop(out, Lua::Core::State, init_enable_script, &data);
+    bool ok = Lua::RunCoreQueryLoop(out, DFHack::Core::getInstance().getLuaState(), init_enable_script, &data);
 
     return ok ? CR_OK : CR_FAILURE;
 }
@@ -399,7 +399,7 @@ command_result Core::runCommand(color_ostream &out, const std::string &command)
 
 bool is_builtin(color_ostream &con, const std::string &command) {
     CoreSuspender suspend;
-    auto L = Lua::Core::State;
+    auto L = DFHack::Core::getInstance().getLuaState();
     Lua::StackUnwinder top(L);
 
     if (!lua_checkstack(L, 1) ||
@@ -427,7 +427,7 @@ void get_commands(color_ostream &con, std::vector<std::string> &commands) {
         return;
     }
 
-    auto L = Lua::Core::State;
+    auto L = DFHack::Core::getInstance().getLuaState();
     Lua::StackUnwinder top(L);
 
     if (!lua_checkstack(L, 1) ||
@@ -632,7 +632,7 @@ void help_helper(color_ostream &con, const std::string &entry_name) {
         con.printerr("Failed Lua call to helpdb.help (could not acquire core lock).\n");
         return;
     }
-    auto L = Lua::Core::State;
+    auto L = DFHack::Core::getInstance().getLuaState();
     Lua::StackUnwinder top(L);
 
     if (!lua_checkstack(L, 2) ||
@@ -650,7 +650,7 @@ void help_helper(color_ostream &con, const std::string &entry_name) {
 
 void tags_helper(color_ostream &con, const std::string &tag) {
     CoreSuspender suspend;
-    auto L = Lua::Core::State;
+    auto L = DFHack::Core::getInstance().getLuaState();
     Lua::StackUnwinder top(L);
 
     if (!lua_checkstack(L, 1) ||
@@ -687,7 +687,7 @@ void ls_helper(color_ostream &con, const std::vector<std::string> &params) {
     }
 
     CoreSuspender suspend;
-    auto L = Lua::Core::State;
+    auto L = DFHack::Core::getInstance().getLuaState();
     Lua::StackUnwinder top(L);
 
     if (!lua_checkstack(L, 5) ||
@@ -1366,7 +1366,7 @@ static void run_dfhack_init(color_ostream &out, Core *core)
     loadScriptFiles(core, out, prefixes, CONFIG_PATH + "init");
 
     // show the terminal if requested
-    auto L = Lua::Core::State;
+    auto L = DFHack::Core::getInstance().getLuaState();
     Lua::CallLuaModuleFunction(out, L, "dfhack", "getHideConsoleOnStartup", 0, 1,
         Lua::DEFAULT_LUA_LAMBDA, [&](lua_State* L) {
             if (!lua_toboolean(L, -1))
@@ -1805,7 +1805,10 @@ bool Core::InitSimulationThread()
     loadScriptPaths(con);
 
     // initialize common lua context
-    if (!Lua::Core::Init(con))
+    // Calls InitCoreContext after checking IsCoreContext
+    State = luaL_newstate();
+    State = Lua::Open(con, State);
+    if (!State)
     {
         fatal("Lua failed to initialize");
         return false;
@@ -2319,7 +2322,7 @@ void Core::onStateChange(color_ostream &out, state_change_event event)
         Persistence::Internal::load(out);
         plug_mgr->doLoadWorldData(out);
         loadModScriptPaths(out);
-        auto L = Lua::Core::State;
+        auto L = DFHack::Core::getInstance().getLuaState();
         Lua::StackUnwinder top(L);
         Lua::CallLuaModuleFunction(con, "script-manager", "reload", std::make_tuple(true));
         if (world && world->cur_savegame.save_dir.size())

library/LuaTools.cpp

@@ -71,8 +71,6 @@ distribution.
 using namespace DFHack;
 using namespace DFHack::LuaWrapper;
 
-lua_State *DFHack::Lua::Core::State = NULL;
-
 void dfhack_printerr(lua_State *S, const std::string &str);
 
 inline bool is_null_userdata(lua_State *L, int idx)
@@ -523,7 +521,7 @@ static void interrupt_hook (lua_State *L, lua_Debug *ar)
 
 bool DFHack::Lua::Interrupt (bool force)
 {
-    lua_State *L = Lua::Core::State;
+    lua_State *L = DFHack::Core::getInstance().getLuaState();
     if (L->hook != interrupt_hook && !force)
         return false;
     if (force)
@@ -1492,8 +1490,8 @@ bool Lua::IsCoreContext(lua_State *state)
     // This uses a private field of the lua state to
     // evaluate the condition without accessing the lua
     // stack, and thus requiring a lock on the core state.
-    return state && Lua::Core::State &&
-           state->l_G == Lua::Core::State->l_G;
+    return state && DFHack::Core::getInstance().getLuaState() &&
+           state->l_G == DFHack::Core::getInstance().getLuaState()->l_G;
 }
 
 static const luaL_Reg dfhack_funcs[] = {
@@ -2029,7 +2027,7 @@ int dfhack_timeout_active(lua_State *L)
 
 static void cancel_timers(std::multimap<int,int> &timers)
 {
-    using Lua::Core::State;
+    auto State = DFHack::Core::getInstance().getLuaState();
 
     Lua::StackUnwinder frame(State);
     lua_rawgetp(State, LUA_REGISTRYINDEX, &DFHACK_TIMEOUTS_TOKEN);
@@ -2044,6 +2042,7 @@ static void cancel_timers(std::multimap<int,int> &timers)
 }
 
 void DFHack::Lua::Core::onStateChange(color_ostream &out, int code) {
+    auto State = DFHack::Core::getInstance().getLuaState();
     if (!State) return;
 
     switch (code)
@@ -2084,6 +2083,7 @@ static void run_timers(color_ostream &out, lua_State *L,
 
 void DFHack::Lua::Core::onUpdate(color_ostream &out)
 {
+    auto State = DFHack::Core::getInstance().getLuaState();
     using df::global::world;
 
     if (frame_timers.empty() && tick_timers.empty())
@@ -2098,21 +2098,9 @@ void DFHack::Lua::Core::onUpdate(color_ostream &out)
         run_timers(out, State, tick_timers, frame[1], world->frame_counter);
 }
 
-bool DFHack::Lua::Core::Init(color_ostream &out)
-{
-    if (State) {
-        out.printerr("state already exists\n");
-        return false;
-    }
-
-    State = luaL_newstate();
-
-    // Calls InitCoreContext after checking IsCoreContext
-    return (Lua::Open(out, State) != NULL);
-}
-
 static void Lua::Core::InitCoreContext(color_ostream &out)
 {
+    auto State = DFHack::Core::getInstance().getLuaState();
     lua_newtable(State);
     lua_rawsetp(State, LUA_REGISTRYINDEX, &DFHACK_TIMEOUTS_TOKEN);
 
@@ -2150,6 +2138,7 @@ static void Lua::Core::InitCoreContext(color_ostream &out)
 void DFHack::Lua::Core::Reset(color_ostream &out, const char *where)
 {
     // This can happen if DFHack fails to initialize.
+    auto State = DFHack::Core::getInstance().getLuaState();
     if (!State)
         return;
 

library/PluginManager.cpp

@@ -704,7 +704,7 @@ void Plugin::index_lua(DFLibrary *lib)
             cmd->event = evlist->event;
             if (cmd->active)
             {
-                cmd->event->bind(Lua::Core::State, cmd);
+                cmd->event->bind(DFHack::Core::getInstance().getLuaState(), cmd);
                 if (cmd->count > 0)
                     cmd->event->on_count_changed(cmd->count, 0);
             }
@@ -830,7 +830,7 @@ void Plugin::open_lua(lua_State *state, int table)
 
             it->second->active = true;
             if (it->second->event)
-                it->second->event->bind(Lua::Core::State, it->second);
+                it->second->event->bind(DFHack::Core::getInstance().getLuaState(), it->second);
 
             lua_setfield(state, table, it->first.c_str());
         }

library/RemoteTools.cpp

@@ -757,7 +757,7 @@ command_result CoreService::RunLua(color_ostream &stream,
                                    const dfproto::CoreRunLuaRequest *in,
                                    StringListMessage *out)
 {
-    auto L = Lua::Core::State;
+    auto L = DFHack::Core::getInstance().getLuaState();
     LuaFunctionData data = { CR_FAILURE, in, out };
 
     lua_pushcfunction(L, doRunLuaFunction);

library/include/Core.h

@@ -25,6 +25,7 @@ distribution.
 #pragma once
 
 #include "Console.h"
+#include "CoreDefs.h"
 #include "Export.h"
 #include "Hooks.h"
 
@@ -48,6 +49,7 @@ distribution.
 #define DFH_MOD_ALT 4
 
 struct WINDOW;
+struct lua_State;
 
 namespace df
 {
@@ -75,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:
@@ -243,6 +220,11 @@ namespace DFHack
 
         PerfCounters perf_counters;
 
+        lua_State* getLuaState(bool bypass_assertion = false) {
+            assert(bypass_assertion || isSuspended());
+            return State;
+        }
+
     private:
         DFHack::Console con;
 
@@ -350,6 +332,8 @@ namespace DFHack
         std::thread::id df_render_thread;
         std::thread::id df_simulation_thread;
 
+        lua_State* State;
+
         friend class CoreService;
         friend class ServerConnection;
         friend class CoreSuspender;
@@ -395,9 +379,9 @@ 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);
             parent_t::unlock();
         }
 

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
+    };
+
+}

library/include/LuaTools.h

@@ -472,28 +472,43 @@ namespace DFHack {namespace Lua {
      * All accesses must be done under CoreSuspender.
      */
     namespace Core {
-        DFHACK_EXPORT extern lua_State *State;
+//        DFHACK_EXPORT extern lua_State *State;
 
         // Not exported; for use by the Core class
-        bool Init(color_ostream &out);
+        lua_State* Init(color_ostream &out);
         DFHACK_EXPORT void Reset(color_ostream &out, const char *where);
 
         // Events signalled by the core
         void onStateChange(color_ostream &out, int code);
         // Signals timers
         void onUpdate(color_ostream &out);
 
-        template<class T> inline void Push(T &arg) { Lua::Push(State, arg); }
-        template<class T> inline void Push(const T &arg) { Lua::Push(State, arg); }
-        template<class T> inline void PushVector(const T &arg) { Lua::PushVector(State, arg); }
+        template<class T> inline void Push(T &arg)
+        {
+            auto State = DFHack::Core::getInstance().getLuaState();
+            Lua::Push(State, arg);
+        }
+        template<class T> inline void Push(const T &arg)
+        {
+            auto State = DFHack::Core::getInstance().getLuaState();
+            Lua::Push(State, arg);
+        }
+        template<class T> inline void PushVector(const T &arg)
+        {
+            auto State = DFHack::Core::getInstance().getLuaState();
+            Lua::PushVector(State, arg);
+        }
 
         inline bool SafeCall(color_ostream &out, int nargs, int nres, bool perr = true) {
+            auto State = DFHack::Core::getInstance().getLuaState();
             return Lua::SafeCall(out, State, nargs, nres, perr);
         }
         inline bool PushModule(color_ostream &out, const char *module) {
+            auto State = DFHack::Core::getInstance().getLuaState();
             return Lua::PushModule(out, State, module);
         }
         inline bool PushModulePublic(color_ostream &out, const char *module, const char *name) {
+            auto State = DFHack::Core::getInstance().getLuaState();
             return Lua::PushModulePublic(out, State, module, name);
         }
     }
@@ -507,7 +522,7 @@ namespace DFHack {namespace Lua {
         color_ostream &out, const char* module_name, const char* fn_name, std::tuple<aT...>&& args = {},
         size_t nres = 0, Lua::LuaLambda && res_lambda = Lua::DEFAULT_LUA_LAMBDA)
     {
-        auto L = Lua::Core::State;
+        auto L = DFHack::Core::getInstance().getLuaState();
         bool ok;
 
         ok = Lua::CallLuaModuleFunction(out, L, module_name, fn_name, sizeof...(aT), nres,
@@ -523,7 +538,7 @@ namespace DFHack {namespace Lua {
         color_ostream &out, const char* module_name, const char* fn_name, const std::vector<aT> &args,
         size_t nres = 0, Lua::LuaLambda && res_lambda = Lua::DEFAULT_LUA_LAMBDA)
     {
-        auto L = Lua::Core::State;
+        auto L = DFHack::Core::getInstance().getLuaState();
         bool ok;
 
         ok = Lua::CallLuaModuleFunction(out, L, module_name, fn_name, args.size(), nres,