From 781c8683f7021ceb27dc83c19f7207322f2227d1 Mon Sep 17 00:00:00 2001 From: Mattes D Date: Fri, 20 Mar 2015 15:13:33 +0100 Subject: Added cPluginLua::cResettable interface, used for scheduled tasks. This allows plugins to register objects that can "survive" the plugin unloading - they will simply bail out if the plugin is already unloaded, instead of referencing bad plugin data. Fixes #1556. --- src/Bindings/ManualBindings.cpp | 32 +++++++++++----- src/Bindings/PluginLua.cpp | 77 +++++++++++++++++++++++++++++-------- src/Bindings/PluginLua.h | 84 +++++++++++++++++++++++++---------------- src/Blocks/BlockPiston.cpp | 6 +-- src/World.cpp | 10 ++--- src/World.h | 20 ++++------ 6 files changed, 150 insertions(+), 79 deletions(-) diff --git a/src/Bindings/ManualBindings.cpp b/src/Bindings/ManualBindings.cpp index 035be55ae..cdc01ae09 100644 --- a/src/Bindings/ManualBindings.cpp +++ b/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(*Plugin, FnRef)); + auto task = std::make_shared(*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(*Plugin, FnRef); + Plugin->AddResettable(task); + World->ScheduleTask(DelayTicks, task); return 0; } diff --git a/src/Bindings/PluginLua.cpp b/src/Bindings/PluginLua.cpp index 263d1f005..d133c091a 100644 --- a/src/Bindings/PluginLua.cpp +++ b/src/Bindings/PluginLua.cpp @@ -6,10 +6,11 @@ #include "Globals.h" // NOTE: MSVC stupidness requires this to be the same across all modules #ifdef __APPLE__ -#define LUA_USE_MACOSX + #define LUA_USE_MACOSX #else -#define LUA_USE_POSIX + #define LUA_USE_POSIX #endif + #include "PluginLua.h" #include "../CommandOutput.h" #include "PluginManager.h" @@ -52,24 +53,35 @@ cPluginLua::~cPluginLua() void cPluginLua::Close(void) { - if (m_LuaState.IsValid()) + cCSLock Lock(m_CriticalSection); + + // If already closed, bail out: + if (!m_LuaState.IsValid()) { - // Release all the references in the hook map: - for (cHookMap::iterator itrH = m_HookMap.begin(), endH = m_HookMap.end(); itrH != endH; ++itrH) - { - for (cLuaRefs::iterator itrR = itrH->second.begin(), endR = itrH->second.end(); itrR != endR; ++itrR) - { - delete *itrR; - } // for itrR - itrH->second[] - } // for itrH - m_HookMap[] - m_HookMap.clear(); - - m_LuaState.Close(); + ASSERT(m_Resettables.empty()); + ASSERT(m_HookMap.empty()); + return; } - else + + // Notify and remove all m_Resettables: + for (auto resettable: m_Resettables) { - ASSERT(m_HookMap.empty()); + 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) + { + for (cLuaRefs::iterator itrR = itrH->second.begin(), endR = itrH->second.end(); itrR != endR; ++itrR) + { + delete *itrR; + } // for itrR - itrH->second[] + } // for itrH - m_HookMap[] + m_HookMap.clear(); + + // Close the Lua engine: + m_LuaState.Close(); } @@ -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; +} + + + + diff --git a/src/Bindings/PluginLua.h b/src/Bindings/PluginLua.h index 4f3529fc9..e358693bc 100644 --- a/src/Bindings/PluginLua.h +++ b/src/Bindings/PluginLua.h @@ -59,6 +59,38 @@ public: /** RAII lock for m_Plugin.m_CriticalSection */ cCSLock m_Lock; } ; + + + + /** 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 cResettablePtr; + typedef std::vector cResettablePtrs; cPluginLua(const AString & a_PluginDirectory); @@ -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 with 0 args */ - template bool Call(FnT a_Fn) - { - cCSLock Lock(m_CriticalSection); - return m_LuaState.Call(a_Fn); - } - - /** Call a Lua function with 1 arg */ - template bool Call(FnT a_Fn, ArgT0 a_Arg0) - { - cCSLock Lock(m_CriticalSection); - return m_LuaState.Call(a_Fn, a_Arg0); - } - - /** Call a Lua function with 2 args */ - template bool Call(FnT a_Fn, ArgT0 a_Arg0, ArgT1 a_Arg1) + /** Call a Lua function residing in the plugin. */ + template + bool Call(FnT a_Fn, Args && ... a_Args) { cCSLock Lock(m_CriticalSection); - return m_LuaState.Call(a_Fn, a_Arg0, a_Arg1); + return m_LuaState.Call(a_Fn, a_Args...); } - /** Call a Lua function with 3 args */ - template 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 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); - } + /** Adds the specified cResettable instance to m_Resettables, so that it is notified when the plugin is being closed. */ + void AddResettable(cResettablePtr a_Resettable); 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 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 diff --git a/src/Blocks/BlockPiston.cpp b/src/Blocks/BlockPiston.cpp index 8d245cabe..1cc5524f0 100644 --- a/src/Blocks/BlockPiston.cpp +++ b/src/Blocks/BlockPiston.cpp @@ -169,7 +169,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(ScheduledBlocks)); } @@ -219,7 +219,7 @@ void cBlockPistonHandler::RetractPiston(int a_BlockX, int a_BlockY, int a_BlockZ std::vector 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(ScheduledBlocks)); return; } } @@ -229,7 +229,7 @@ void cBlockPistonHandler::RetractPiston(int a_BlockX, int a_BlockY, int a_BlockZ std::vector 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(ScheduledBlocks)); } diff --git a/src/World.cpp b/src/World.cpp index 292ff9b94..03f09c028 100644 --- a/src/World.cpp +++ b/src/World.cpp @@ -3167,14 +3167,14 @@ void cWorld::SaveAllChunks(void) void cWorld::QueueSaveAllChunks(void) { - QueueTask(make_unique()); + QueueTask(std::make_shared()); } -void cWorld::QueueTask(std::unique_ptr 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 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(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(TargetTick, a_Task)); + m_ScheduledTasks.insert(itr, cScheduledTaskPtr(new cScheduledTask(TargetTick, a_Task))); return; } } - m_ScheduledTasks.push_back(make_unique(TargetTick, a_Task)); + m_ScheduledTasks.push_back(cScheduledTaskPtr(new cScheduledTask(TargetTick, a_Task))); } diff --git a/src/World.h b/src/World.h index 0decc8c6e..f7d5cdc60 100644 --- a/src/World.h +++ b/src/World.h @@ -106,7 +106,8 @@ public: virtual void Run(cWorld & a_World) = 0; } ; - typedef std::vector> cTasks; + typedef SharedPtr cTaskPtr; + typedef std::vector 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 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. - The task object will be deleted once the task is finished */ - void ScheduleTask(int a_DelayTicks, cTask * a_Task); + /** Queues a task onto the tick thread, with the specified delay. */ + void ScheduleTask(int a_DelayTicks, cTaskPtr 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() - { - delete m_Task; - m_Task = nullptr; - } + virtual ~cScheduledTask() {} }; typedef std::unique_ptr cScheduledTaskPtr; -- cgit v1.2.3