Two threading bugs in LuaMod I ran into while chasing #1328. Both are in the shared code, not platform specific, so they should apply to Windows too. I have fixes running locally, happy to PR either if you want them.
1. The shared hook thread can be GC'd while still in use
ensure_hook_thread_exists() creates the mod's hook thread on first use, anchors it in the registry, and returns that ref to the caller:
if (mod->m_hook_lua == nullptr)
{
mod->m_hook_lua = &mod->lua().new_thread();
int thread_ref = luaL_ref(mod->lua().get_lua_state(), LUA_REGISTRYINDEX);
return thread_ref;
}The thread is shared by every hook of that mod, but the anchor gets handed to whoever created it first, and that caller ends up owning it. When that one callback is torn down (LuaMod.cpp:256, and :6535 where a NotifyOnNewObject callback returning true "releases the thread_ref to GC") it unrefs the only anchor. Lua then collects the lua_State while mod->m_hook_lua still points at it and is never reset, so every other hook keeps using freed memory.
What I did: anchor the thread once for the mod's lifetime and always return LUA_REFNIL. luaL_unref ignores negative refs so the existing teardown call sites become no-ops and don't need touching.
2. Lua runs concurrently from the async thread and the game thread
main_lua, hook_lua and async_lua are all lua_newthread() coroutines off one lua_State, so they share one global_State and one GC. But process_delayed_actions calls call_function after releasing the actions lock, so the async thread executes Lua while the game thread is executing Lua in its hooks. Two threads in one global_State, no lock.
What I did: a mutex serialising every Lua execution point. Lock order matters, it has to be taken before m_thread_actions_mutex, because ExecuteInGameThread touches Lua while holding that one and inverting them deadlocks. It's a band aid, the TODO in the code already says the real answer is separate lua_newstate per thread.
For context these were not the cause of the crash I was originally chasing (that one turned out to be a box64 bug), but they're real and easy to hit with a mod that registers hooks and does async work.
Two threading bugs in LuaMod I ran into while chasing #1328. Both are in the shared code, not platform specific, so they should apply to Windows too. I have fixes running locally, happy to PR either if you want them.
1. The shared hook thread can be GC'd while still in use
ensure_hook_thread_exists()creates the mod's hook thread on first use, anchors it in the registry, and returns that ref to the caller:The thread is shared by every hook of that mod, but the anchor gets handed to whoever created it first, and that caller ends up owning it. When that one callback is torn down (LuaMod.cpp:256, and :6535 where a NotifyOnNewObject callback returning true "releases the thread_ref to GC") it unrefs the only anchor. Lua then collects the lua_State while
mod->m_hook_luastill points at it and is never reset, so every other hook keeps using freed memory.What I did: anchor the thread once for the mod's lifetime and always return LUA_REFNIL.
luaL_unrefignores negative refs so the existing teardown call sites become no-ops and don't need touching.2. Lua runs concurrently from the async thread and the game thread
main_lua,hook_luaandasync_luaare alllua_newthread()coroutines off one lua_State, so they share one global_State and one GC. Butprocess_delayed_actionscallscall_functionafter releasing the actions lock, so the async thread executes Lua while the game thread is executing Lua in its hooks. Two threads in one global_State, no lock.What I did: a mutex serialising every Lua execution point. Lock order matters, it has to be taken before
m_thread_actions_mutex, becauseExecuteInGameThreadtouches Lua while holding that one and inverting them deadlocks. It's a band aid, the TODO in the code already says the real answer is separatelua_newstateper thread.For context these were not the cause of the crash I was originally chasing (that one turned out to be a box64 bug), but they're real and easy to hit with a mod that registers hooks and does async work.