Merge pull request #1822 from mc-server/ResettableLuaCallback

Added cPluginLua::cResettable interface, used for scheduled tasks.

src/Bindings/ManualBindings.cpp

@@ -1299,23 +1299,27 @@ tolua_lerror:
 
 
 class cLuaWorldTask :
-	public cWorld::cTask
+	public cWorld::cTask,
+	public cPluginLua::cResettable
 {
 public:
 	cLuaWorldTask(cPluginLua & a_Plugin, int a_FnRef) :
-		m_Plugin(a_Plugin),
+		cPluginLua::cResettable(a_Plugin),
 		m_FnRef(a_FnRef)
 	{
 	}
 
 protected:
-	cPluginLua & m_Plugin;
 	int m_FnRef;
 	
 	// cWorld::cTask overrides:
 	virtual void Run(cWorld & a_World) override
 	{
-		m_Plugin.Call(m_FnRef, &a_World);
+		cCSLock Lock(m_CSPlugin);
+		if (m_Plugin != nullptr)
+		{
+			m_Plugin->Call(m_FnRef, &a_World);
+		}
 	}
 } ;
 
@@ -1354,7 +1358,9 @@ static int tolua_cWorld_QueueTask(lua_State * tolua_S)
 		return lua_do_error(tolua_S, "Error in function call '#funcname#': Could not get function reference of parameter #1");
 	}
 
-	self->QueueTask(make_unique<cLuaWorldTask>(*Plugin, FnRef));
+	auto task = std::make_shared<cLuaWorldTask>(*Plugin, FnRef);
+	Plugin->AddResettable(task);
+	self->QueueTask(task);
 	return 0;
 }
 
@@ -1363,23 +1369,27 @@ static int tolua_cWorld_QueueTask(lua_State * tolua_S)
 
 
 class cLuaScheduledWorldTask :
-	public cWorld::cTask
+	public cWorld::cTask,
+	public cPluginLua::cResettable
 {
 public:
 	cLuaScheduledWorldTask(cPluginLua & a_Plugin, int a_FnRef) :
-		m_Plugin(a_Plugin),
+		cPluginLua::cResettable(a_Plugin),
 		m_FnRef(a_FnRef)
 	{
 	}
 
 protected:
-	cPluginLua & m_Plugin;
 	int m_FnRef;
 	
 	// cWorld::cTask overrides:
 	virtual void Run(cWorld & a_World) override
 	{
-		m_Plugin.Call(m_FnRef, &a_World);
+		cCSLock Lock(m_CSPlugin);
+		if (m_Plugin != nullptr)
+		{
+			m_Plugin->Call(m_FnRef, &a_World);
+		}
 	}
 };
 
@@ -1425,7 +1435,9 @@ static int tolua_cWorld_ScheduleTask(lua_State * tolua_S)
 	
 	int DelayTicks = (int)tolua_tonumber(tolua_S, 2, 0);
 
-	World->ScheduleTask(DelayTicks, new cLuaScheduledWorldTask(*Plugin, FnRef));
+	auto task = std::make_shared<cLuaScheduledWorldTask>(*Plugin, FnRef);
+	Plugin->AddResettable(task);
+	World->ScheduleTask(DelayTicks, task);
 	return 0;
 }
 

src/Bindings/PluginLua.cpp

@@ -10,6 +10,7 @@
 #else
 	#define LUA_USE_POSIX
 #endif
+
 #include "PluginLua.h"
 #include "../CommandOutput.h"
 #include "PluginManager.h"
@@ -52,8 +53,23 @@ cPluginLua::~cPluginLua()
 
 void cPluginLua::Close(void)
 {
-	if (m_LuaState.IsValid())
+	cCSLock Lock(m_CriticalSection);
+
+	// If already closed, bail out:
+	if (!m_LuaState.IsValid())
 	{
+		ASSERT(m_Resettables.empty());
+		ASSERT(m_HookMap.empty());
+		return;
+	}
+
+	// Notify and remove all m_Resettables:
+	for (auto resettable: m_Resettables)
+	{
+		resettable->Reset();
+	}
+	m_Resettables.clear();
+
 	// Release all the references in the hook map:
 	for (cHookMap::iterator itrH = m_HookMap.begin(), endH = m_HookMap.end(); itrH != endH; ++itrH)
 	{
@@ -64,13 +80,9 @@ void cPluginLua::Close(void)
 	}  // for itrH - m_HookMap[]
 	m_HookMap.clear();
 
+	// Close the Lua engine:
 	m_LuaState.Close();
 }
-	else
-	{
-		ASSERT(m_HookMap.empty());
-	}
-}
 
 
 
@@ -1709,6 +1721,16 @@ int cPluginLua::CallFunctionFromForeignState(
 
 
 
+void cPluginLua::AddResettable(cPluginLua::cResettablePtr a_Resettable)
+{
+	cCSLock Lock(m_CriticalSection);
+	m_Resettables.push_back(a_Resettable);
+}
+
+
+
+
+
 AString cPluginLua::HandleWebRequest(const HTTPRequest * a_Request)
 {
 	cCSLock Lock(m_CriticalSection);
@@ -1826,3 +1848,26 @@ void cPluginLua::CallbackWindowSlotChanged(int a_FnRef, cWindow & a_Window, int
 
 
 
+
+////////////////////////////////////////////////////////////////////////////////
+// cPluginLua::cResettable:
+
+cPluginLua::cResettable::cResettable(cPluginLua & a_Plugin):
+	m_Plugin(&a_Plugin),
+	m_CSPlugin(a_Plugin.m_CriticalSection)
+{
+}
+
+
+
+
+
+void cPluginLua::cResettable::Reset(void)
+{
+	cCSLock Lock(m_CSPlugin);
+	m_Plugin = nullptr;
+}
+
+
+
+

src/Bindings/PluginLua.h

@@ -61,6 +61,38 @@ public:
 	} ;
 
 
+
+	/** A base class that represents something related to a plugin
+	The plugin can reset this class so that the instance can continue to exist but will not engage the (possibly non-existent) plugin anymore.
+	This is used for scheduled tasks etc., so that they can be queued and reset when the plugin is terminated, without removing them from the queue. */
+	class cResettable
+	{
+	public:
+		/** Creates a new instance bound to the specified plugin. */
+		cResettable(cPluginLua & a_Plugin);
+
+		// Force a virtual destructor in descendants:
+		virtual ~cResettable() {}
+
+		/** Resets the plugin instance stored within.
+		The instance will continue to exist, but should not call into the plugin anymore. */
+		virtual void Reset(void);
+
+	protected:
+		/** The plugin that this instance references.
+		If nullptr, the plugin has already unloaded and the instance should bail out any processing.
+		Protected against multithreaded access by m_CSPlugin. */
+		cPluginLua * m_Plugin;
+
+		/** The mutex protecting m_Plugin against multithreaded access.
+		Actually points to m_Plugin's internal m_CriticalSection in order to prevent deadlocks. */
+		cCriticalSection & m_CSPlugin;
+	};
+
+	typedef SharedPtr<cResettable> cResettablePtr;
+	typedef std::vector<cResettablePtr> cResettablePtrs;
+	
+	
 	cPluginLua(const AString & a_PluginDirectory);
 	~cPluginLua();
 
@@ -187,42 +219,16 @@ public:
 		int a_ParamEnd
 	);
 	
-	// The following templates allow calls to arbitrary Lua functions residing in the plugin:
+	/** Call a Lua function residing in the plugin. */
-	
+	template <typename FnT, typename... Args>
-	/** Call a Lua function with 0 args */
+	bool Call(FnT a_Fn, Args && ... a_Args)
-	template <typename FnT> bool Call(FnT a_Fn)
 	{
 		cCSLock Lock(m_CriticalSection);
-		return m_LuaState.Call(a_Fn);
+		return m_LuaState.Call(a_Fn, a_Args...);
 	}
 
-	/** Call a Lua function with 1 arg */
+	/** Adds the specified cResettable instance to m_Resettables, so that it is notified when the plugin is being closed. */
-	template <typename FnT, typename ArgT0> bool Call(FnT a_Fn, ArgT0 a_Arg0)
+	void AddResettable(cResettablePtr a_Resettable);
-	{
-		cCSLock Lock(m_CriticalSection);
-		return m_LuaState.Call(a_Fn, a_Arg0);
-	}
-
-	/** Call a Lua function with 2 args */
-	template <typename FnT, typename ArgT0, typename ArgT1> bool Call(FnT a_Fn, ArgT0 a_Arg0, ArgT1 a_Arg1)
-	{
-		cCSLock Lock(m_CriticalSection);
-		return m_LuaState.Call(a_Fn, a_Arg0, a_Arg1);
-	}
-
-	/** Call a Lua function with 3 args */
-	template <typename FnT, typename ArgT0, typename ArgT1, typename ArgT2> bool Call(FnT a_Fn, ArgT0 a_Arg0, ArgT1 a_Arg1, ArgT2 a_Arg2)
-	{
-		cCSLock Lock(m_CriticalSection);
-		return m_LuaState.Call(a_Fn, a_Arg0, a_Arg1, a_Arg2);
-	}
-
-	/** Call a Lua function with 4 args */
-	template <typename FnT, typename ArgT0, typename ArgT1, typename ArgT2, typename ArgT3> bool Call(FnT a_Fn, ArgT0 a_Arg0, ArgT1 a_Arg1, ArgT2 a_Arg2, ArgT3 a_Arg3)
-	{
-		cCSLock Lock(m_CriticalSection);
-		return m_LuaState.Call(a_Fn, a_Arg0, a_Arg1, a_Arg2, a_Arg3);
-	}
 
 protected:
 	/** Maps command name into Lua function reference */
@@ -234,15 +240,27 @@ protected:
 	/** Maps hook types into arrays of Lua function references to call for each hook type */
 	typedef std::map<int, cLuaRefs> cHookMap;
 	
+
+	/** The mutex protecting m_LuaState and each of the m_Resettables[] against multithreaded use. */
 	cCriticalSection m_CriticalSection;
+
+	/** The plugin's Lua state. */
 	cLuaState m_LuaState;
 	
+	/** Objects that need notification when the plugin is about to be unloaded. */
+	cResettablePtrs m_Resettables;
+
+	/** In-game commands that the plugin has registered. */
 	CommandMap m_Commands;
+
+	/** Console commands that the plugin has registered. */
 	CommandMap m_ConsoleCommands;
 	
+	/** Hooks that the plugin has registered. */
 	cHookMap m_HookMap;
 	
-	/** Releases all Lua references and closes the LuaState */
+
+	/** Releases all Lua references, notifies and removes all m_Resettables[] and closes the m_LuaState. */
 	void Close(void);
 } ;  // tolua_export
 

src/Blocks/BlockPiston.cpp

@@ -179,7 +179,7 @@ void cBlockPistonHandler::ExtendPiston(int a_BlockX, int a_BlockY, int a_BlockZ,
 
 	a_World->SetBlock(a_BlockX, a_BlockY, a_BlockZ, pistonBlock, pistonMeta | 0x8);
 	a_World->SetBlock(extx, exty, extz, E_BLOCK_PISTON_EXTENSION, pistonMeta | (IsSticky(pistonBlock) ? 8 : 0), false);
-	a_World->ScheduleTask(PISTON_TICK_DELAY, new cWorld::cTaskSendBlockToAllPlayers(ScheduledBlocks));
+	a_World->ScheduleTask(PISTON_TICK_DELAY, std::make_shared<cWorld::cTaskSendBlockToAllPlayers>(ScheduledBlocks));
 }
 
 
@@ -229,7 +229,7 @@ void cBlockPistonHandler::RetractPiston(int a_BlockX, int a_BlockY, int a_BlockZ
 			std::vector<Vector3i> ScheduledBlocks;
 			ScheduledBlocks.push_back(Vector3i(a_BlockX, a_BlockY, a_BlockZ));
 			ScheduledBlocks.push_back(Vector3i(tempx, tempy, tempz));
-			a_World->ScheduleTask(PISTON_TICK_DELAY + 1, new cWorld::cTaskSendBlockToAllPlayers(ScheduledBlocks));
+			a_World->ScheduleTask(PISTON_TICK_DELAY + 1, std::make_shared<cWorld::cTaskSendBlockToAllPlayers>(ScheduledBlocks));
 			return;
 		}
 	}
@@ -239,7 +239,7 @@ void cBlockPistonHandler::RetractPiston(int a_BlockX, int a_BlockY, int a_BlockZ
 
 	std::vector<Vector3i> ScheduledBlocks;
 	ScheduledBlocks.push_back(Vector3i(a_BlockX, a_BlockY, a_BlockZ));
-	a_World->ScheduleTask(PISTON_TICK_DELAY + 1, new cWorld::cTaskSendBlockToAllPlayers(ScheduledBlocks));
+	a_World->ScheduleTask(PISTON_TICK_DELAY + 1, std::make_shared<cWorld::cTaskSendBlockToAllPlayers>(ScheduledBlocks));
 }
 
 

src/World.cpp

@@ -3167,14 +3167,14 @@ void cWorld::SaveAllChunks(void)
 
 void cWorld::QueueSaveAllChunks(void)
 {
-	QueueTask(make_unique<cWorld::cTaskSaveAllChunks>());
+	QueueTask(std::make_shared<cWorld::cTaskSaveAllChunks>());
 }
 
 
 
 
 
-void cWorld::QueueTask(std::unique_ptr<cTask> a_Task)
+void cWorld::QueueTask(cTaskPtr a_Task)
 {
 	cCSLock Lock(m_CSTasks);
 	m_Tasks.push_back(std::move(a_Task));
@@ -3184,7 +3184,7 @@ void cWorld::QueueTask(std::unique_ptr<cTask> a_Task)
 
 
 
-void cWorld::ScheduleTask(int a_DelayTicks, cTask * a_Task)
+void cWorld::ScheduleTask(int a_DelayTicks, cTaskPtr a_Task)
 {
 	Int64 TargetTick = a_DelayTicks + std::chrono::duration_cast<cTickTimeLong>(m_WorldAge).count();
 	
@@ -3194,11 +3194,11 @@ void cWorld::ScheduleTask(int a_DelayTicks, cTask * a_Task)
 	{
 		if ((*itr)->m_TargetTick >= TargetTick)
 		{
-			m_ScheduledTasks.insert(itr, make_unique<cScheduledTask>(TargetTick, a_Task));
+			m_ScheduledTasks.insert(itr, cScheduledTaskPtr(new cScheduledTask(TargetTick, a_Task)));
 			return;
 		}
 	}
-	m_ScheduledTasks.push_back(make_unique<cScheduledTask>(TargetTick, a_Task));
+	m_ScheduledTasks.push_back(cScheduledTaskPtr(new cScheduledTask(TargetTick, a_Task)));
 }
 
 

src/World.h

@@ -106,7 +106,8 @@ public:
 		virtual void Run(cWorld & a_World) = 0;
 	} ;
 	
-	typedef std::vector<std::unique_ptr<cTask>> cTasks;
+	typedef SharedPtr<cTask> cTaskPtr;
+	typedef std::vector<cTaskPtr> cTasks;
 
 	
 	class cTaskSaveAllChunks :
@@ -691,11 +692,10 @@ public:
 	void QueueSaveAllChunks(void);  // tolua_export
 	
 	/** Queues a task onto the tick thread. The task object will be deleted once the task is finished */
-	void QueueTask(std::unique_ptr<cTask> a_Task);  // Exported in ManualBindings.cpp
+	void QueueTask(cTaskPtr a_Task);  // Exported in ManualBindings.cpp
 	
-	/** Queues a task onto the tick thread, with the specified delay.
+	/** Queues a task onto the tick thread, with the specified delay. */
-	The task object will be deleted once the task is finished */
+	void ScheduleTask(int a_DelayTicks, cTaskPtr a_Task);
-	void ScheduleTask(int a_DelayTicks, cTask * a_Task);
 
 	/** Returns the number of chunks loaded	 */
 	int GetNumChunks() const;  // tolua_export
@@ -867,20 +867,16 @@ private:
 	{
 	public:
 		Int64 m_TargetTick;
-		cTask * m_Task;
+		cTaskPtr m_Task;
 		
 		/** Creates a new scheduled task; takes ownership of the task object passed to it. */
-		cScheduledTask(Int64 a_TargetTick, cTask * a_Task) :
+		cScheduledTask(Int64 a_TargetTick, cTaskPtr a_Task) :
 			m_TargetTick(a_TargetTick),
 			m_Task(a_Task)
 		{
 		}
 		
-		virtual ~cScheduledTask()
+		virtual ~cScheduledTask() {}
-		{
-			delete m_Task;
-			m_Task = nullptr;
-		}
 	};
 
 	typedef std::unique_ptr<cScheduledTask> cScheduledTaskPtr;