From 24853397ef4648155d886b112e00c3e2c3d1e900 Mon Sep 17 00:00:00 2001 From: Mattes D Date: Sun, 12 Jun 2016 16:53:24 +0200 Subject: [PATCH] LuaState: Implemented proper locking for cCallback. --- src/Bindings/LuaChunkStay.cpp | 22 +++---- src/Bindings/LuaChunkStay.h | 6 +- src/Bindings/LuaNameLookup.cpp | 2 +- src/Bindings/LuaServerHandle.cpp | 2 +- src/Bindings/LuaState.cpp | 58 +++++++++++++---- src/Bindings/LuaState.h | 57 ++++++++++++----- src/Bindings/LuaTCPLink.cpp | 2 +- src/Bindings/LuaUDPEndpoint.cpp | 2 +- src/Bindings/ManualBindings_World.cpp | 4 +- src/Bindings/PluginLua.cpp | 90 +++++++++++---------------- src/Bindings/PluginLua.h | 21 ++----- 11 files changed, 151 insertions(+), 115 deletions(-) diff --git a/src/Bindings/LuaChunkStay.cpp b/src/Bindings/LuaChunkStay.cpp index d8d7da5d0..819e10f19 100644 --- a/src/Bindings/LuaChunkStay.cpp +++ b/src/Bindings/LuaChunkStay.cpp @@ -113,12 +113,10 @@ void cLuaChunkStay::AddChunkCoord(cLuaState & L, int a_Index) -void cLuaChunkStay::Enable(cChunkMap & a_ChunkMap, int a_OnChunkAvailableStackPos, int a_OnAllChunksAvailableStackPos) +void cLuaChunkStay::Enable(cChunkMap & a_ChunkMap, cLuaState::cCallbackPtr a_OnChunkAvailable, cLuaState::cCallbackPtr a_OnAllChunksAvailable) { - // Get the references to the callback functions: - m_LuaState = &m_Plugin.GetLuaState(); - m_OnChunkAvailable.RefStack(*m_LuaState, a_OnChunkAvailableStackPos); - m_OnAllChunksAvailable.RefStack(*m_LuaState, a_OnAllChunksAvailableStackPos); + m_OnChunkAvailable = a_OnChunkAvailable; + m_OnAllChunksAvailable = a_OnAllChunksAvailable; // Enable the ChunkStay: super::Enable(a_ChunkMap); @@ -130,10 +128,9 @@ void cLuaChunkStay::Enable(cChunkMap & a_ChunkMap, int a_OnChunkAvailableStackPo void cLuaChunkStay::OnChunkAvailable(int a_ChunkX, int a_ChunkZ) { - if (m_OnChunkAvailable.IsValid()) + if (m_OnChunkAvailable != nullptr) { - cPluginLua::cOperation Op(m_Plugin); - Op().Call(static_cast(m_OnChunkAvailable), a_ChunkX, a_ChunkZ); + m_OnChunkAvailable->Call(a_ChunkX, a_ChunkZ); } } @@ -143,15 +140,14 @@ void cLuaChunkStay::OnChunkAvailable(int a_ChunkX, int a_ChunkZ) bool cLuaChunkStay::OnAllChunksAvailable(void) { - if (m_OnAllChunksAvailable.IsValid()) + if (m_OnAllChunksAvailable != nullptr) { // Call the callback: - cPluginLua::cOperation Op(m_Plugin); - Op().Call(static_cast(m_OnAllChunksAvailable)); + m_OnAllChunksAvailable->Call(); // Remove the callback references - they won't be needed anymore - m_OnChunkAvailable.UnRef(); - m_OnAllChunksAvailable.UnRef(); + m_OnChunkAvailable.reset(); + m_OnAllChunksAvailable.reset(); } // Disable the ChunkStay by returning true diff --git a/src/Bindings/LuaChunkStay.h b/src/Bindings/LuaChunkStay.h index a455d2502..51356d5b7 100644 --- a/src/Bindings/LuaChunkStay.h +++ b/src/Bindings/LuaChunkStay.h @@ -39,7 +39,7 @@ public: bool AddChunks(int a_ChunkCoordTableStackPos); /** Enables the ChunkStay for the specified chunkmap, with the specified Lua callbacks. */ - void Enable(cChunkMap & a_ChunkMap, int a_OnChunkAvailableStackPos, int a_OnAllChunksAvailableStackPos); + void Enable(cChunkMap & a_ChunkMap, cLuaState::cCallbackPtr a_OnChunkAvailable, cLuaState::cCallbackPtr a_OnAllChunksAvailable); protected: /** The plugin which has created the ChunkStay, via cWorld:ChunkStay() binding method. */ @@ -49,10 +49,10 @@ protected: cLuaState * m_LuaState; /** The Lua function to call in OnChunkAvailable. Only valid when enabled. */ - cLuaState::cRef m_OnChunkAvailable; + cLuaState::cCallbackPtr m_OnChunkAvailable; /** The Lua function to call in OnAllChunksAvailable. Only valid when enabled. */ - cLuaState::cRef m_OnAllChunksAvailable; + cLuaState::cCallbackPtr m_OnAllChunksAvailable; // cChunkStay overrides: diff --git a/src/Bindings/LuaNameLookup.cpp b/src/Bindings/LuaNameLookup.cpp index e52d8dbdc..3cbdbb5cf 100644 --- a/src/Bindings/LuaNameLookup.cpp +++ b/src/Bindings/LuaNameLookup.cpp @@ -12,7 +12,7 @@ cLuaNameLookup::cLuaNameLookup(const AString & a_Query, cPluginLua & a_Plugin, int a_CallbacksTableStackPos): m_Plugin(a_Plugin), - m_Callbacks(a_Plugin.GetLuaState(), a_CallbacksTableStackPos), + m_Callbacks(cPluginLua::cOperation(a_Plugin)(), a_CallbacksTableStackPos), m_Query(a_Query) { } diff --git a/src/Bindings/LuaServerHandle.cpp b/src/Bindings/LuaServerHandle.cpp index a84f894b5..9cc8ad350 100644 --- a/src/Bindings/LuaServerHandle.cpp +++ b/src/Bindings/LuaServerHandle.cpp @@ -14,7 +14,7 @@ cLuaServerHandle::cLuaServerHandle(UInt16 a_Port, cPluginLua & a_Plugin, int a_CallbacksTableStackPos): m_Plugin(a_Plugin), - m_Callbacks(a_Plugin.GetLuaState(), a_CallbacksTableStackPos), + m_Callbacks(cPluginLua::cOperation(a_Plugin)(), a_CallbacksTableStackPos), m_Port(a_Port) { } diff --git a/src/Bindings/LuaState.cpp b/src/Bindings/LuaState.cpp index 550737704..85c6c2e12 100644 --- a/src/Bindings/LuaState.cpp +++ b/src/Bindings/LuaState.cpp @@ -124,6 +124,15 @@ cLuaStateTracker & cLuaStateTracker::Get(void) //////////////////////////////////////////////////////////////////////////////// // cLuaState::cCallback: +cLuaState::cCallback::cCallback(void): + m_CS(nullptr) +{ +} + + + + + bool cLuaState::cCallback::RefStack(cLuaState & a_LuaState, int a_StackPos) { // Check if the stack contains a function: @@ -136,11 +145,12 @@ bool cLuaState::cCallback::RefStack(cLuaState & a_LuaState, int a_StackPos) Clear(); // Add self to LuaState's callback-tracking: - a_LuaState.TrackCallback(*this); + auto canonState = a_LuaState.QueryCanonLuaState(); + canonState->TrackCallback(*this); // Store the new callback: - cCSLock Lock(m_CS); - m_Ref.RefStack(a_LuaState, a_StackPos); + m_CS = &(canonState->m_CS); + m_Ref.RefStack(*canonState, a_StackPos); return true; } @@ -153,16 +163,24 @@ void cLuaState::cCallback::Clear(void) // Free the callback reference: lua_State * luaState = nullptr; { - cCSLock Lock(m_CS); - if (!m_Ref.IsValid()) + auto cs = m_CS; + if (cs != nullptr) { - return; + cCSLock Lock(*cs); + if (!m_Ref.IsValid()) + { + return; + } + luaState = m_Ref.GetLuaState(); + m_Ref.UnRef(); } - luaState = m_Ref.GetLuaState(); - m_Ref.UnRef(); } // Remove from LuaState's callback-tracking: + if (luaState == nullptr) + { + return; + } cLuaState(luaState).UntrackCallback(*this); } @@ -172,7 +190,12 @@ void cLuaState::cCallback::Clear(void) bool cLuaState::cCallback::IsValid(void) { - cCSLock lock(m_CS); + auto cs = m_CS; + if (cs == nullptr) + { + return false; + } + cCSLock lock(*cs); return m_Ref.IsValid(); } @@ -182,7 +205,12 @@ bool cLuaState::cCallback::IsValid(void) bool cLuaState::cCallback::IsSameLuaState(cLuaState & a_LuaState) { - cCSLock lock(m_CS); + auto cs = m_CS; + if (cs == nullptr) + { + return false; + } + cCSLock lock(*cs); if (!m_Ref.IsValid()) { return false; @@ -201,10 +229,16 @@ bool cLuaState::cCallback::IsSameLuaState(cLuaState & a_LuaState) void cLuaState::cCallback::Invalidate(void) { - cCSLock Lock(m_CS); + auto cs = m_CS; + if (cs == nullptr) + { + // Already invalid + return; + } + cCSLock Lock(*cs); if (!m_Ref.IsValid()) { - LOGD("%s: Invalidating an already invalid callback at %p, this should not happen", + LOGD("%s: Inconsistent callback at %p, has a CS but an invalid Ref. This should not happen", __FUNCTION__, reinterpret_cast(this) ); return; diff --git a/src/Bindings/LuaState.h b/src/Bindings/LuaState.h index faa43b6d6..88313f9fb 100644 --- a/src/Bindings/LuaState.h +++ b/src/Bindings/LuaState.h @@ -7,7 +7,7 @@ The contained lua_State can be either owned or attached. Owned lua_State is created by calling Create() and the cLuaState automatically closes the state Or, lua_State can be attached by calling Attach(), the cLuaState doesn't close such a state -Attaching a state will automatically close an owned state. +If owning a state, trying to attach a state will automatically close the previously owned state. Calling a Lua function is done internally by pushing the function using PushFunction(), then pushing the arguments and finally executing CallFunction(). cLuaState automatically keeps track of the number of @@ -16,8 +16,12 @@ the stack using GetStackValue(). All of this is wrapped in a templated function which is generated automatically by gen_LuaState_Call.lua script file into the LuaState_Call.inc file. Reference management is provided by the cLuaState::cRef class. This is used when you need to hold a reference to -any Lua object across several function calls; usually this is used for callbacks. The class is RAII-like, with -automatic resource management. +any Lua object across several function calls. The class is RAII-like, with automatic resource management. + +Callbacks management is provided by the cLuaState::cCallback class. Use a GetStackValue() with cCallbackPtr +parameter to store the callback, and then at any time you can use the cCallback's Call() templated function +to call the callback. The callback management takes care of cLuaState being destroyed - the cCallback object +stays valid but doesn't call into Lua code anymore, returning false for "failure" instead. */ @@ -39,6 +43,7 @@ extern "C" class cLuaServerHandle; class cLuaTCPLink; class cLuaUDPEndpoint; +class cPluginLua; @@ -49,6 +54,20 @@ class cLuaState { public: + /** Provides a RAII-style locking for the LuaState. + Used mainly by the cPluginLua internals to provide the actual locking for interface operations, such as callbacks. */ + class cLock + { + public: + cLock(cLuaState & a_LuaState): + m_Lock(a_LuaState.m_CS) + { + } + protected: + cCSLock m_Lock; + }; + + /** Used for storing references to object in the global registry. Can be bound (contains a reference) or unbound (doesn't contain reference). The reference can also be reset by calling RefStack(). */ @@ -127,15 +146,15 @@ public: /** Represents a callback to Lua that C++ code can call. Is thread-safe and unload-safe. When the Lua state is unloaded, the callback returns an error instead of calling into non-existent code. - To receive the callback instance from the Lua side, use RefStack() or (better) cLuaState::GetStackValue(). - Note that instances of this class are tracked in the canon LuaState instance, so that they can be invalidated - when the LuaState is unloaded; due to multithreading issues they can only be tracked by-ptr, which has - an unfortunate effect of disabling the copy and move constructors. */ + To receive the callback instance from the Lua side, use RefStack() or (better) cLuaState::GetStackValue() + with a cCallbackPtr. Note that instances of this class are tracked in the canon LuaState instance, so that + they can be invalidated when the LuaState is unloaded; due to multithreading issues they can only be tracked + by-ptr, which has an unfortunate effect of disabling the copy and move constructors. */ class cCallback { public: /** Creates an unbound callback instance. */ - cCallback(void) = default; + cCallback(void); ~cCallback() { @@ -148,13 +167,17 @@ public: template bool Call(Args &&... args) { - cCSLock Lock(m_CS); + auto cs = m_CS; + if (cs == nullptr) + { + return false; + } + cCSLock Lock(*cs); if (!m_Ref.IsValid()) { return false; } - cLuaState(m_Ref.GetLuaState()).Call(m_Ref, std::forward(args)...); - return true; + return cLuaState(m_Ref.GetLuaState()).Call(m_Ref, std::forward(args)...); } /** Set the contained callback to the function in the specified Lua state's stack position. @@ -175,7 +198,7 @@ public: friend class cLuaState; /** The mutex protecting m_Ref against multithreaded access */ - cCriticalSection m_CS; + cCriticalSection * m_CS; /** Reference to the Lua callback */ cRef m_Ref; @@ -185,10 +208,12 @@ public: Called only from cLuaState when closing the Lua state. */ void Invalidate(void); - /** This class cannot be copied, because it is tracked in the LuaState by-ptr. */ + /** This class cannot be copied, because it is tracked in the LuaState by-ptr. + Use cCallbackPtr for a copyable object. */ cCallback(const cCallback &) = delete; - /** This class cannot be moved, because it is tracked in the LuaState by-ptr. */ + /** This class cannot be moved, because it is tracked in the LuaState by-ptr. + Use cCallbackPtr for a copyable object. */ cCallback(cCallback &&) = delete; }; typedef SharedPtr cCallbackPtr; @@ -250,6 +275,8 @@ public: protected: lua_State * m_LuaState; + /** Used for debugging, Lua state's stack size is checked against this number to make sure + it is the same as when the value was pushed. */ int m_StackLen; // Remove the copy-constructor: @@ -538,6 +565,8 @@ public: protected: + cCriticalSection m_CS; + lua_State * m_LuaState; /** If true, the state is owned by this object and will be auto-Closed. False => attached state */ diff --git a/src/Bindings/LuaTCPLink.cpp b/src/Bindings/LuaTCPLink.cpp index deaba9d86..4b04a1c02 100644 --- a/src/Bindings/LuaTCPLink.cpp +++ b/src/Bindings/LuaTCPLink.cpp @@ -13,7 +13,7 @@ cLuaTCPLink::cLuaTCPLink(cPluginLua & a_Plugin, int a_CallbacksTableStackPos): m_Plugin(a_Plugin), - m_Callbacks(a_Plugin.GetLuaState(), a_CallbacksTableStackPos) + m_Callbacks(cPluginLua::cOperation(a_Plugin)(), a_CallbacksTableStackPos) { // Warn if the callbacks aren't valid: if (!m_Callbacks.IsValid()) diff --git a/src/Bindings/LuaUDPEndpoint.cpp b/src/Bindings/LuaUDPEndpoint.cpp index 8637eeb4e..ed8f4e87f 100644 --- a/src/Bindings/LuaUDPEndpoint.cpp +++ b/src/Bindings/LuaUDPEndpoint.cpp @@ -12,7 +12,7 @@ cLuaUDPEndpoint::cLuaUDPEndpoint(cPluginLua & a_Plugin, int a_CallbacksTableStackPos): m_Plugin(a_Plugin), - m_Callbacks(a_Plugin.GetLuaState(), a_CallbacksTableStackPos) + m_Callbacks(cPluginLua::cOperation(a_Plugin)(), a_CallbacksTableStackPos) { // Warn if the callbacks aren't valid: if (!m_Callbacks.IsValid()) diff --git a/src/Bindings/ManualBindings_World.cpp b/src/Bindings/ManualBindings_World.cpp index 3faf038aa..3f9f5f94d 100644 --- a/src/Bindings/ManualBindings_World.cpp +++ b/src/Bindings/ManualBindings_World.cpp @@ -103,7 +103,9 @@ static int tolua_cWorld_ChunkStay(lua_State * tolua_S) return 0; } - ChunkStay->Enable(*World->GetChunkMap(), 3, 4); + cLuaState::cCallbackPtr onChunkAvailable, onAllChunksAvailable; + L.GetStackValues(3, onChunkAvailable, onAllChunksAvailable); // Callbacks may be unassigned at all - as a request to load / generate chunks + ChunkStay->Enable(*World->GetChunkMap(), onChunkAvailable, onAllChunksAvailable); return 0; } diff --git a/src/Bindings/PluginLua.cpp b/src/Bindings/PluginLua.cpp index fd3e8bc69..04ea76f0a 100644 --- a/src/Bindings/PluginLua.cpp +++ b/src/Bindings/PluginLua.cpp @@ -45,7 +45,6 @@ cPluginLua::cPluginLua(const AString & a_PluginDirectory) : cPluginLua::~cPluginLua() { - cCSLock Lock(m_CriticalSection); Close(); } @@ -55,10 +54,9 @@ cPluginLua::~cPluginLua() void cPluginLua::Close(void) { - cCSLock Lock(m_CriticalSection); - + cOperation op(*this); // If already closed, bail out: - if (!m_LuaState.IsValid()) + if (!op().IsValid()) { ASSERT(m_HookMap.empty()); return; @@ -73,7 +71,7 @@ void cPluginLua::Close(void) m_HookMap.clear(); // Close the Lua engine: - m_LuaState.Close(); + op().Close(); } @@ -82,8 +80,8 @@ void cPluginLua::Close(void) bool cPluginLua::Load(void) { - cCSLock Lock(m_CriticalSection); - if (!m_LuaState.IsValid()) + cOperation op(*this); + if (!op().IsValid()) { m_LuaState.Create(); m_LuaState.RegisterAPILibs(); @@ -198,12 +196,12 @@ void cPluginLua::Unload(void) void cPluginLua::OnDisable(void) { - cCSLock Lock(m_CriticalSection); - if (!m_LuaState.HasFunction("OnDisable")) + cOperation op(*this); + if (!op().HasFunction("OnDisable")) { return; } - m_LuaState.Call("OnDisable"); + op().Call("OnDisable"); } @@ -257,8 +255,8 @@ bool cPluginLua::OnBrewingCompleting(cWorld & a_World, cBrewingstandEntity & a_B bool cPluginLua::OnChat(cPlayer & a_Player, AString & a_Message) { - cCSLock Lock(m_CriticalSection); - if (!m_LuaState.IsValid()) + cOperation op(*this); + if (!op().IsValid()) { return false; } @@ -380,8 +378,8 @@ bool cPluginLua::OnEntityChangedWorld(cEntity & a_Entity, cWorld & a_World) bool cPluginLua::OnExecuteCommand(cPlayer * a_Player, const AStringVector & a_Split, const AString & a_EntireCommand, cPluginManager::CommandResult & a_Result) { - cCSLock Lock(m_CriticalSection); - if (!m_LuaState.IsValid()) + cOperation op(*this); + if (!op().IsValid()) { return false; } @@ -404,8 +402,8 @@ bool cPluginLua::OnExecuteCommand(cPlayer * a_Player, const AStringVector & a_Sp bool cPluginLua::OnExploded(cWorld & a_World, double a_ExplosionSize, bool a_CanCauseFire, double a_X, double a_Y, double a_Z, eExplosionSource a_Source, void * a_SourceData) { - cCSLock Lock(m_CriticalSection); - if (!m_LuaState.IsValid()) + cOperation op(*this); + if (!op().IsValid()) { return false; } @@ -444,8 +442,8 @@ bool cPluginLua::OnExploded(cWorld & a_World, double a_ExplosionSize, bool a_Can bool cPluginLua::OnExploding(cWorld & a_World, double & a_ExplosionSize, bool & a_CanCauseFire, double a_X, double a_Y, double a_Z, eExplosionSource a_Source, void * a_SourceData) { - cCSLock Lock(m_CriticalSection); - if (!m_LuaState.IsValid()) + cOperation op(*this); + if (!op().IsValid()) { return false; } @@ -511,8 +509,8 @@ bool cPluginLua::OnHopperPushingItem(cWorld & a_World, cHopperEntity & a_Hopper, bool cPluginLua::OnKilled(cEntity & a_Victim, TakeDamageInfo & a_TDI, AString & a_DeathMessage) { - cCSLock Lock(m_CriticalSection); - if (!m_LuaState.IsValid()) + cOperation op(*this); + if (!op().IsValid()) { return false; } @@ -777,8 +775,8 @@ bool cPluginLua::OnPluginMessage(cClientHandle & a_Client, const AString & a_Cha bool cPluginLua::OnPluginsLoaded(void) { - cCSLock Lock(m_CriticalSection); - if (!m_LuaState.IsValid()) + cOperation op(*this); + if (!op().IsValid()) { return false; } @@ -835,8 +833,8 @@ bool cPluginLua::OnProjectileHitEntity(cProjectileEntity & a_Projectile, cEntity bool cPluginLua::OnServerPing(cClientHandle & a_ClientHandle, AString & a_ServerDescription, int & a_OnlinePlayersCount, int & a_MaxPlayersCount, AString & a_Favicon) { - cCSLock Lock(m_CriticalSection); - if (!m_LuaState.IsValid()) + cOperation op(*this); + if (!op().IsValid()) { return false; } @@ -923,8 +921,8 @@ bool cPluginLua::OnUpdatingSign( cPlayer * a_Player ) { - cCSLock Lock(m_CriticalSection); - if (!m_LuaState.IsValid()) + cOperation op(*this); + if (!op().IsValid()) { return false; } @@ -956,8 +954,8 @@ bool cPluginLua::OnWeatherChanged(cWorld & a_World) bool cPluginLua::OnWeatherChanging(cWorld & a_World, eWeather & a_NewWeather) { - cCSLock Lock(m_CriticalSection); - if (!m_LuaState.IsValid()) + cOperation op(*this); + if (!op().IsValid()) { return false; } @@ -999,6 +997,7 @@ bool cPluginLua::OnWorldTick(cWorld & a_World, std::chrono::milliseconds a_Dt, s bool cPluginLua::HandleCommand(const AStringVector & a_Split, cPlayer & a_Player, const AString & a_FullCommand) { ASSERT(!a_Split.empty()); + cOperation op(*this); CommandMap::iterator cmd = m_Commands.find(a_Split[0]); if (cmd == m_Commands.end()) { @@ -1006,9 +1005,8 @@ bool cPluginLua::HandleCommand(const AStringVector & a_Split, cPlayer & a_Player return false; } - cCSLock Lock(m_CriticalSection); bool res = false; - m_LuaState.Call(cmd->second, a_Split, &a_Player, a_FullCommand, cLuaState::Return, res); + op().Call(cmd->second, a_Split, &a_Player, a_FullCommand, cLuaState::Return, res); return res; } @@ -1019,6 +1017,7 @@ bool cPluginLua::HandleCommand(const AStringVector & a_Split, cPlayer & a_Player bool cPluginLua::HandleConsoleCommand(const AStringVector & a_Split, cCommandOutputCallback & a_Output, const AString & a_FullCommand) { ASSERT(!a_Split.empty()); + cOperation op(*this); CommandMap::iterator cmd = m_ConsoleCommands.find(a_Split[0]); if (cmd == m_ConsoleCommands.end()) { @@ -1028,10 +1027,9 @@ bool cPluginLua::HandleConsoleCommand(const AStringVector & a_Split, cCommandOut return false; } - cCSLock Lock(m_CriticalSection); bool res = false; AString str; - m_LuaState.Call(cmd->second, a_Split, a_FullCommand, cLuaState::Return, res, str); + op().Call(cmd->second, a_Split, a_FullCommand, cLuaState::Return, res, str); if (res && !str.empty()) { a_Output.Out(str); @@ -1045,7 +1043,7 @@ bool cPluginLua::HandleConsoleCommand(const AStringVector & a_Split, cCommandOut void cPluginLua::ClearCommands(void) { - cCSLock Lock(m_CriticalSection); + cOperation op(*this); // Unreference the bound functions so that Lua can GC them if (m_LuaState != nullptr) @@ -1064,7 +1062,7 @@ void cPluginLua::ClearCommands(void) void cPluginLua::ClearConsoleCommands(void) { - cCSLock Lock(m_CriticalSection); + cOperation op(*this); // Unreference the bound functions so that Lua can GC them if (m_LuaState != nullptr) @@ -1187,8 +1185,6 @@ const char * cPluginLua::GetHookFnName(int a_HookType) bool cPluginLua::AddHookCallback(int a_HookType, cLuaState::cCallbackPtr a_Callback) { - ASSERT(m_CriticalSection.IsLockedByCurrentThread()); // It probably has to be, how else would we have a LuaState? - m_HookMap[a_HookType].push_back(a_Callback); return true; } @@ -1204,10 +1200,10 @@ int cPluginLua::CallFunctionFromForeignState( int a_ParamEnd ) { - cCSLock Lock(m_CriticalSection); + cOperation op(*this); // Call the function: - int NumReturns = m_LuaState.CallFunctionWithForeignParams(a_FunctionName, a_ForeignState, a_ParamStart, a_ParamEnd); + int NumReturns = op().CallFunctionWithForeignParams(a_FunctionName, a_ForeignState, a_ParamStart, a_ParamEnd); if (NumReturns < 0) { // The call has failed, an error has already been output to the log, so just silently bail out with the same error @@ -1251,23 +1247,13 @@ void cPluginLua::BindConsoleCommand(const AString & a_Command, int a_FnRef) -void cPluginLua::Unreference(int a_LuaRef) -{ - cCSLock Lock(m_CriticalSection); - luaL_unref(m_LuaState, LUA_REGISTRYINDEX, a_LuaRef); -} - - - - - bool cPluginLua::CallbackWindowClosing(int a_FnRef, cWindow & a_Window, cPlayer & a_Player, bool a_CanRefuse) { ASSERT(a_FnRef != LUA_REFNIL); - cCSLock Lock(m_CriticalSection); + cOperation op(*this); bool res = false; - m_LuaState.Call(a_FnRef, &a_Window, &a_Player, a_CanRefuse, cLuaState::Return, res); + op().Call(a_FnRef, &a_Window, &a_Player, a_CanRefuse, cLuaState::Return, res); return res; } @@ -1279,8 +1265,8 @@ void cPluginLua::CallbackWindowSlotChanged(int a_FnRef, cWindow & a_Window, int { ASSERT(a_FnRef != LUA_REFNIL); - cCSLock Lock(m_CriticalSection); - m_LuaState.Call(a_FnRef, &a_Window, a_SlotNum); + cOperation op(*this); + op().Call(a_FnRef, &a_Window, a_SlotNum); } diff --git a/src/Bindings/PluginLua.h b/src/Bindings/PluginLua.h index e53fbaaa8..8e40ff584 100644 --- a/src/Bindings/PluginLua.h +++ b/src/Bindings/PluginLua.h @@ -47,7 +47,7 @@ public: public: cOperation(cPluginLua & a_Plugin) : m_Plugin(a_Plugin), - m_Lock(a_Plugin.m_CriticalSection) + m_Lock(a_Plugin.m_LuaState) { } @@ -56,8 +56,8 @@ public: protected: cPluginLua & m_Plugin; - /** RAII lock for m_Plugin.m_CriticalSection */ - cCSLock m_Lock; + /** RAII lock for the Lua state. */ + cLuaState::cLock m_Lock; } ; @@ -155,13 +155,6 @@ public: /** Binds the console command to call the function specified by a Lua function reference. Simply adds to CommandMap. */ void BindConsoleCommand(const AString & a_Command, int a_FnRef); - cLuaState & GetLuaState(void) { return m_LuaState; } - - cCriticalSection & GetCriticalSection(void) { return m_CriticalSection; } - - /** Removes a previously referenced object (luaL_unref()) */ - void Unreference(int a_LuaRef); - /** Calls the plugin-specified "cLuaWindow closing" callback. Returns true only if the callback returned true */ bool CallbackWindowClosing(int a_FnRef, cWindow & a_Window, cPlayer & a_Player, bool a_CanRefuse); @@ -189,8 +182,7 @@ public: template bool Call(FnT a_Fn, Args && ... a_Args) { - cCSLock Lock(m_CriticalSection); - return m_LuaState.Call(a_Fn, a_Args...); + return cOperation(*this)().Call(a_Fn, a_Args...); } protected: @@ -204,9 +196,6 @@ protected: typedef std::map cHookMap; - /** The mutex protecting m_LuaState against multithreaded use. */ - cCriticalSection m_CriticalSection; - /** The plugin's Lua state. */ cLuaState m_LuaState; @@ -232,7 +221,7 @@ protected: template bool CallSimpleHooks(int a_HookType, Args && ... a_Args) { - cCSLock lock(m_CriticalSection); + cOperation op(*this); auto & hooks = m_HookMap[a_HookType]; bool res = false; for (auto & hook: hooks)