LuaState: Implemented proper locking for cCallback.

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_OnChunkAvailable = a_OnChunkAvailable;
-	m_LuaState = &m_Plugin.GetLuaState();
+	m_OnAllChunksAvailable = a_OnAllChunksAvailable;
-	m_OnChunkAvailable.RefStack(*m_LuaState, a_OnChunkAvailableStackPos);
-	m_OnAllChunksAvailable.RefStack(*m_LuaState, a_OnAllChunksAvailableStackPos);
 
 	// 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);
+		m_OnChunkAvailable->Call(a_ChunkX, a_ChunkZ);
-		Op().Call(static_cast<int>(m_OnChunkAvailable), 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);
+		m_OnAllChunksAvailable->Call();
-		Op().Call(static_cast<int>(m_OnAllChunksAvailable));
 
 		// Remove the callback references - they won't be needed anymore
-		m_OnChunkAvailable.UnRef();
+		m_OnChunkAvailable.reset();
-		m_OnAllChunksAvailable.UnRef();
+		m_OnAllChunksAvailable.reset();
 	}
 
 	// Disable the ChunkStay by returning true

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:

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)
 {
 }

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)
 {
 }

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_CS = &(canonState->m_CS);
-	m_Ref.RefStack(a_LuaState, a_StackPos);
+	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);
+		auto cs = m_CS;
-		if (!m_Ref.IsValid())
+		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<void *>(this)
 		);
 		return;

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
+any Lua object across several function calls. The class is RAII-like, with automatic resource management.
-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().
+	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
+	with a cCallbackPtr. Note that instances of this class are tracked in the canon LuaState instance, so that
-	when the LuaState is unloaded; due to multithreading issues they can only be tracked by-ptr, which has
+	they can be invalidated when the LuaState is unloaded; due to multithreading issues they can only be tracked
-	an unfortunate effect of disabling the copy and move constructors. */
+	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 <typename... Args>
 		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>(args)...);
+			return cLuaState(m_Ref.GetLuaState()).Call(m_Ref, std::forward<Args>(args)...);
-			return true;
 		}
 
 		/** 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<cCallback> 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 */

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())

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())

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

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);
+	cOperation op(*this);
-	if (!m_LuaState.IsValid())
+	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);
+	cOperation op(*this);
-	if (!m_LuaState.HasFunction("OnDisable"))
+	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);
+	cOperation op(*this);
-	if (!m_LuaState.IsValid())
+	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);
+	cOperation op(*this);
-	if (!m_LuaState.IsValid())
+	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);
+	cOperation op(*this);
-	if (!m_LuaState.IsValid())
+	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);
+	cOperation op(*this);
-	if (!m_LuaState.IsValid())
+	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);
+	cOperation op(*this);
-	if (!m_LuaState.IsValid())
+	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);
+	cOperation op(*this);
-	if (!m_LuaState.IsValid())
+	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);
+	cOperation op(*this);
-	if (!m_LuaState.IsValid())
+	if (!op().IsValid())
 	{
 		return false;
 	}
@@ -923,8 +921,8 @@ bool cPluginLua::OnUpdatingSign(
 	cPlayer * a_Player
 )
 {
-	cCSLock Lock(m_CriticalSection);
+	cOperation op(*this);
-	if (!m_LuaState.IsValid())
+	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);
+	cOperation op(*this);
-	if (!m_LuaState.IsValid())
+	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);
+	cOperation op(*this);
-	m_LuaState.Call(a_FnRef, &a_Window, a_SlotNum);
+	op().Call(a_FnRef, &a_Window, a_SlotNum);
 }
 
 

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 */
+		/** RAII lock for the Lua state. */
-		cCSLock m_Lock;
+		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 <typename FnT, typename... Args>
 	bool Call(FnT a_Fn, Args && ... a_Args)
 	{
-		cCSLock Lock(m_CriticalSection);
+		return cOperation(*this)().Call(a_Fn, a_Args...);
-		return m_LuaState.Call(a_Fn, a_Args...);
 	}
 
 protected:
@@ -204,9 +196,6 @@ protected:
 	typedef std::map<int, cLuaCallbacks> 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 <typename... Args>
 	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)