Merge pull request #136 from ElunaLuaEngine/hookmgr_cleanup

Hookmgr cleanup

Includes:
 - Locking
 - Removed macros
 - Added more checking to calling hooks
 - Replacing of an argument on hook call

ElunaBinding.h

@@ -17,7 +17,7 @@ extern "C"
 #include "lauxlib.h"
 };
 
-class ElunaBind
+class ElunaBind : public ElunaUtil::RWLockable
 {
 public:
     struct Binding
@@ -71,6 +71,8 @@ public:
     // unregisters all registered functions and clears all registered events from the bind std::maps (reset)
     void Clear() override
     {
+        WriteGuard guard(GetLock());
+
         for (EventToFunctionsMap::iterator itr = Bindings.begin(); itr != Bindings.end(); ++itr)
         {
             FunctionRefVector& funcrefvec = itr->second;
@@ -83,6 +85,8 @@ public:
 
     void Clear(uint32 event_id)
     {
+        WriteGuard guard(GetLock());
+
         for (FunctionRefVector::iterator itr = Bindings[event_id].begin(); itr != Bindings[event_id].end(); ++itr)
             delete *itr;
         Bindings[event_id].clear();
@@ -91,6 +95,8 @@ public:
     // Pushes the function references and updates the counters on the binds and erases them if the counter would reach 0
     void PushFuncRefs(lua_State* L, int event_id)
     {
+        WriteGuard guard(GetLock());
+
         for (FunctionRefVector::iterator it = Bindings[event_id].begin(); it != Bindings[event_id].end();)
         {
             FunctionRefVector::iterator it_old = it++;
@@ -115,12 +121,15 @@ public:
 
     void Insert(int eventId, int funcRef, uint32 shots) // Inserts a new registered event
     {
+        WriteGuard guard(GetLock());
         Bindings[eventId].push_back(new Binding(E, funcRef, shots));
     }
 
     // Checks if there are events for ID
-    bool HasEvents(T eventId) const
+    bool HasEvents(T eventId)
     {
+        ReadGuard guard(GetLock());
+
         if (Bindings.empty())
             return false;
         if (Bindings.find(eventId) == Bindings.end())
@@ -144,6 +153,8 @@ public:
     // unregisters all registered functions and clears all registered events from the bindmap
     void Clear() override
     {
+        WriteGuard guard(GetLock());
+
         for (EntryToEventsMap::iterator itr = Bindings.begin(); itr != Bindings.end(); ++itr)
         {
             EventToFunctionsMap& funcmap = itr->second;
@@ -161,6 +172,8 @@ public:
 
     void Clear(uint32 entry, uint32 event_id)
     {
+        WriteGuard guard(GetLock());
+
         for (FunctionRefVector::iterator itr = Bindings[entry][event_id].begin(); itr != Bindings[entry][event_id].end(); ++itr)
             delete *itr;
         Bindings[entry][event_id].clear();
@@ -169,6 +182,8 @@ public:
     // Pushes the function references and updates the counters on the binds and erases them if the counter would reach 0
     void PushFuncRefs(lua_State* L, int event_id, uint32 entry)
     {
+        WriteGuard guard(GetLock());
+
         for (FunctionRefVector::iterator it = Bindings[entry][event_id].begin(); it != Bindings[entry][event_id].end();)
         {
             FunctionRefVector::iterator it_old = it++;
@@ -196,12 +211,15 @@ public:
 
     void Insert(uint32 entryId, int eventId, int funcRef, uint32 shots) // Inserts a new registered event
     {
+        WriteGuard guard(GetLock());
         Bindings[entryId][eventId].push_back(new Binding(E, funcRef, shots));
     }
 
     // Returns true if the entry has registered binds
-    bool HasEvents(uint32 entryId, int eventId) const
+    bool HasEvents(T eventId, uint32 entryId)
     {
+        ReadGuard guard(GetLock());
+
         if (Bindings.empty())
             return false;
 
@@ -212,8 +230,10 @@ public:
         return itr->second.find(eventId) != itr->second.end();
     }
 
-    bool HasEvents(uint32 entryId) const
+    bool HasEvents(uint32 entryId)
     {
+        ReadGuard guard(GetLock());
+
         if (Bindings.empty())
             return false;
 

ElunaEventMgr.cpp

@@ -26,7 +26,7 @@ LuaEvent::~LuaEvent()
 
 void LuaEvent::Execute()
 {
-    ELUNA_LOCK(*events->E);
+    LOCK_ELUNA;
     // In multithread get map from object and the map's lua state
     lua_rawgeti((*events->E)->L, LUA_REGISTRYINDEX, funcRef);
     Eluna::Push((*events->E)->L, funcRef);
@@ -45,7 +45,7 @@ ElunaEventProcessor::ElunaEventProcessor(Eluna** _E, WorldObject* _obj) : m_time
 {
     if (obj)
     {
-        EventMgr::WriteGuard lock((*E)->eventMgr->GetLock());
+        EventMgr::WriteGuard guard((*E)->eventMgr->GetLock());
         (*E)->eventMgr->processors.insert(this);
     }
 }
@@ -56,7 +56,7 @@ ElunaEventProcessor::~ElunaEventProcessor()
 
     if (obj && Eluna::initialized)
     {
-        EventMgr::WriteGuard lock((*E)->eventMgr->GetLock());
+        EventMgr::WriteGuard guard((*E)->eventMgr->GetLock());
         (*E)->eventMgr->processors.erase(this);
     }
 }
@@ -133,7 +133,7 @@ EventMgr::EventMgr(Eluna** _E) : globalProcessor(new ElunaEventProcessor(_E, NUL
 EventMgr::~EventMgr()
 {
     {
-        ReadGuard lock(GetLock());
+        ReadGuard guard(GetLock());
         if (!processors.empty())
             for (ProcessorSet::const_iterator it = processors.begin(); it != processors.end(); ++it) // loop processors
                 (*it)->RemoveEvents_internal();
@@ -145,7 +145,7 @@ EventMgr::~EventMgr()
 
 void EventMgr::RemoveEvents()
 {
-    ReadGuard lock(GetLock());
+    ReadGuard guard(GetLock());
     if (!processors.empty())
         for (ProcessorSet::const_iterator it = processors.begin(); it != processors.end(); ++it) // loop processors
             (*it)->RemoveEvents();
@@ -154,7 +154,7 @@ void EventMgr::RemoveEvents()
 
 void EventMgr::RemoveEvent(int eventId)
 {
-    ReadGuard lock(GetLock());
+    ReadGuard guard(GetLock());
     if (!processors.empty())
         for (ProcessorSet::const_iterator it = processors.begin(); it != processors.end(); ++it) // loop processors
             (*it)->RemoveEvent(eventId);

ElunaUtility.h

@@ -119,9 +119,9 @@ namespace ElunaUtil
     /*
      * Usage:
      * Inherit this class, then when needing lock, use
-     * ReadGuard lock(_lock);
+     * ReadGuard guard(GetLock());
      * or
-     * WriteGuard lock(_lock);
+     * WriteGuard guard(GetLock());
      *
      * The lock is automatically released at end of scope
      */

LuaEngine.cpp

@@ -36,6 +36,7 @@ std::string Eluna::lua_requirepath;
 Eluna* Eluna::GEluna = NULL;
 bool Eluna::reload = false;
 bool Eluna::initialized = false;
+Eluna::LockType Eluna::lock;
 
 extern void RegisterFunctions(Eluna* E);
 
@@ -88,13 +89,13 @@ void Eluna::ReloadEluna()
 
     EventMgr::ProcessorSet oldProcessors;
     {
-        EventMgr::ReadGuard lock(sEluna->eventMgr->GetLock());
+        EventMgr::ReadGuard guard(sEluna->eventMgr->GetLock());
         oldProcessors = sEluna->eventMgr->processors;
     }
     Uninitialize();
     Initialize();
     {
-        EventMgr::WriteGuard lock(sEluna->eventMgr->GetLock());
+        EventMgr::WriteGuard guard(sEluna->eventMgr->GetLock());
         sEluna->eventMgr->processors.insert(oldProcessors.begin(), oldProcessors.end());
     }
 
@@ -118,6 +119,7 @@ Eluna::Eluna() :
 L(luaL_newstate()),
 
 event_level(0),
+push_counter(0),
 
 eventMgr(NULL),
 
@@ -431,19 +433,31 @@ void Eluna::report(lua_State* luastate)
 void Eluna::ExecuteCall(int params, int res)
 {
     int top = lua_gettop(L);
-    int type = lua_type(L, top - params);
 
+    // Expected: function, [parameters]
+    ASSERT(top > params);
+
+    // Check function type
+    int type = lua_type(L, top - params);
     if (type != LUA_TFUNCTION)
     {
-        lua_pop(L, params + 1);  // Cleanup the stack.
         ELUNA_LOG_ERROR("[Eluna]: Cannot execute call: registered value is %s, not a function.", lua_typename(L, type));
-        return;
+        ASSERT(false);
     }
 
+    // Objects are invalidated when event level hits 0
     ++event_level;
-    if (lua_pcall(L, params, res, 0))
+    int result = lua_pcall(L, params, res, 0);
-        report(L);
     --event_level;
+
+    // lua_pcall returns 0 on success.
+    // On error we report errors and push nils for expected amount of returned values
+    if (result)
+    {
+        report(L);
+        for (int i = 0; i < res; ++i)
+            lua_pushnil(L);
+    }
 }
 
 void Eluna::Push(lua_State* luastate)

LuaEngine.h

@@ -109,6 +109,44 @@ private:
     Eluna(Eluna const&);
     Eluna& operator=(const Eluna&);
 
+    // Some helpers for hooks to call event handlers.
+    template<typename T> int SetupStack(EventBind<T>* event_bindings, EntryBind<T>* entry_bindings, T event_id, uint32 entry, int number_of_arguments);
+                         void CleanUpStack(int number_of_arguments);
+    template<typename T> void ReplaceArgument(T value, uint8 index);
+                         int CallOneFunction(int number_of_functions, int number_of_arguments, int number_of_results);
+    template<typename T> void CallAllFunctions(EventBind<T>* event_bindings, EntryBind<T>* entry_bindings, T event_id, uint32 entry);
+    template<typename T> bool CallAllFunctionsBool(EventBind<T>* event_bindings, EntryBind<T>* entry_bindings, T event_id, uint32 entry, bool default_value);
+
+    // Convenient overloads for Setup. Use these in hooks instead of original.
+    template<typename T> int SetupStack(EventBind<T>* event_bindings, T event_id, int number_of_arguments)
+    {
+        return SetupStack(event_bindings, (EntryBind<T>*)NULL, event_id, 0, number_of_arguments);
+    }
+    template<typename T> int SetupStack(EntryBind<T>* entry_bindings, T event_id, uint32 entry, int number_of_arguments)
+    {
+        return SetupStack((EventBind<T>*)NULL, entry_bindings, event_id, entry, number_of_arguments);
+    }
+
+    // Convenient overloads for CallAllFunctions. Use these in hooks instead of original.
+    template<typename T> void CallAllFunctions(EventBind<T>* event_bindings, T event_id)
+    {
+        CallAllFunctions(event_bindings, (EntryBind<T>*)NULL, event_id, 0);
+    }
+    template<typename T> void CallAllFunctions(EntryBind<T>* entry_bindings, T event_id, uint32 entry)
+    {
+        CallAllFunctions((EventBind<T>*)NULL, entry_bindings, event_id, entry);
+    }
+
+    // Convenient overloads for CallAllFunctionsBool. Use these in hooks instead of original.
+    template<typename T> bool CallAllFunctionsBool(EventBind<T>* event_bindings, T event_id, bool default_value = false)
+    {
+        return CallAllFunctionsBool(event_bindings, (EntryBind<T>*)NULL, event_id, 0, default_value);
+    }
+    template<typename T> bool CallAllFunctionsBool(EntryBind<T>* entry_bindings, T event_id, uint32 entry, bool default_value = false)
+    {
+        return CallAllFunctionsBool((EventBind<T>*)NULL, entry_bindings, event_id, entry, default_value);
+    }
+
 public:
     typedef std::list<LuaScript> ScriptList;
 
@@ -118,13 +156,13 @@ public:
 
 #ifdef TRINITY
     typedef std::recursive_mutex LockType;
-    typedef std::lock_guard<LockType> ElunaGuard;
+    typedef std::lock_guard<LockType> Guard;
 #else
     typedef ACE_Recursive_Thread_Mutex LockType;
-    typedef ACE_Guard<LockType> ElunaGuard;
+    typedef ACE_Guard<LockType> Guard;
 #endif
 
-    LockType elunaLock;
+    static LockType lock;
 
     lua_State* L;
     uint32 event_level;
@@ -168,7 +206,7 @@ public:
     void RunScripts();
     void InvalidateObjects();
 
-    // Pushes
+    // Static pushes, can be used by anything, including methods.
     static void Push(lua_State* luastate); // nil
     static void Push(lua_State* luastate, const long long);
     static void Push(lua_State* luastate, const unsigned long long);
@@ -191,6 +229,26 @@ public:
     static void Push(lua_State* luastate, Pet const* pet);
     static void Push(lua_State* luastate, TempSummon const* summon);
 
+    // When a hook pushes arguments to be passed to event handlers
+    //   this is used to keep track of how many arguments were pushed.
+    uint8 push_counter;
+
+    // Non-static pushes, to be used in hooks.
+    // These just call the correct static version with the main thread's Lua state.
+    void Push()                                 { Push(L); ++push_counter; }
+    void Push(const long long value)            { Push(L, value); ++push_counter; }
+    void Push(const unsigned long long value)   { Push(L, value); ++push_counter; }
+    void Push(const long value)                 { Push(L, value); ++push_counter; }
+    void Push(const unsigned long value)        { Push(L, value); ++push_counter; }
+    void Push(const int value)                  { Push(L, value); ++push_counter; }
+    void Push(const unsigned int value)         { Push(L, value); ++push_counter; }
+    void Push(const bool value)                 { Push(L, value); ++push_counter; }
+    void Push(const float value)                { Push(L, value); ++push_counter; }
+    void Push(const double value)               { Push(L, value); ++push_counter; }
+    void Push(const std::string& value)         { Push(L, value); ++push_counter; }
+    void Push(const char* value)                { Push(L, value); ++push_counter; }
+    template<typename T> void Push(T const* ptr){ Push(L, ptr); ++push_counter; }
+
     // Checks
     template<typename T> static T CHECKVAL(lua_State* luastate, int narg);
     template<typename T> static T CHECKVAL(lua_State* luastate, int narg, T def)
@@ -401,5 +459,5 @@ template<> WorldObject* Eluna::CHECKOBJ<WorldObject>(lua_State* L, int narg, boo
 template<> ElunaObject* Eluna::CHECKOBJ<ElunaObject>(lua_State* L, int narg, bool error);
 
 #define sEluna Eluna::GEluna
-#define ELUNA_LOCK(E) Eluna::ElunaGuard elunaGuard((E)->elunaLock);
+#define LOCK_ELUNA Eluna::Guard __guard(Eluna::lock)
 #endif