Browse Source

Fix memory leak in Lua

Michael Ragazzon 6 years ago
parent
commit
ae753676ac

+ 1 - 1
Include/RmlUi/Core/Factory.h

@@ -157,7 +157,7 @@ public:
 	/// @param[in] instancer The instancer to be called.
 	/// @param[in] instancer The instancer to be called.
 	/// @lifetime The instancer must be kept alive until after the call to Core::Shutdown.
 	/// @lifetime The instancer must be kept alive until after the call to Core::Shutdown.
 	static void RegisterEventInstancer(EventInstancer* instancer);
 	static void RegisterEventInstancer(EventInstancer* instancer);
-	/// Instance and event object
+	/// Instance an event object
 	/// @param[in] target Target element of this event.
 	/// @param[in] target Target element of this event.
 	/// @param[in] name Name of this event.
 	/// @param[in] name Name of this event.
 	/// @param[in] parameters Additional parameters for this event.
 	/// @param[in] parameters Additional parameters for this event.

+ 2 - 2
Source/Core/Lua/LuaDocument.cpp

@@ -43,13 +43,13 @@ LuaDocument::LuaDocument(const String& tag) : ElementDocument(tag)
 void LuaDocument::LoadScript(Stream* stream, const String& source_name)
 void LuaDocument::LoadScript(Stream* stream, const String& source_name)
 {
 {
     //if it is loaded from a file
     //if it is loaded from a file
-    if(source_name != "")
+    if(!source_name.empty())
     {
     {
         Interpreter::LoadFile(source_name);
         Interpreter::LoadFile(source_name);
     }
     }
     else
     else
     {
     {
-        String buffer = "";
+        String buffer;
         stream->Read(buffer,stream->Length()); //just do the whole thing
         stream->Read(buffer,stream->Length()); //just do the whole thing
         Interpreter::DoString(buffer, this->GetSourceURL());
         Interpreter::DoString(buffer, this->GetSourceURL());
     }
     }

+ 18 - 7
Source/Core/Lua/LuaEventListener.cpp

@@ -76,9 +76,9 @@ LuaEventListener::LuaEventListener(const String& code, Element* element) : Event
 
 
     attached = element;
     attached = element;
 	if(element)
 	if(element)
-		parent = element->GetOwnerDocument();
+		owner_document = element->GetOwnerDocument();
 	else
 	else
-		parent = nullptr;
+		owner_document = nullptr;
     strFunc = function;
     strFunc = function;
     lua_settop(L,top);
     lua_settop(L,top);
 }
 }
@@ -101,24 +101,35 @@ LuaEventListener::LuaEventListener(lua_State* L, int narg, Element* element)
 
 
 	attached = element;
 	attached = element;
 	if(element)
 	if(element)
-		parent = element->GetOwnerDocument();
+		owner_document = element->GetOwnerDocument();
 	else
 	else
-		parent = nullptr;
+		owner_document = nullptr;
     lua_settop(L,top);
     lua_settop(L,top);
 }
 }
 
 
 LuaEventListener::~LuaEventListener()
 LuaEventListener::~LuaEventListener()
 {
 {
+	// Remove the Lua function from its table
+	lua_State* L = Interpreter::GetLuaState();
+	lua_getglobal(L, "EVENTLISTENERFUNCTIONS");
+	luaL_unref(L, -1, luaFuncRef);
+	lua_pop(L, 1); // pop table
+}
 
 
+void LuaEventListener::OnDetach(Element* element)
+{
+	// We consider this listener owned by its element, so we must delete ourselves when
+	// we detach (probably because element was removed).
+	delete this;
 }
 }
 
 
 /// Process the incoming Event
 /// Process the incoming Event
 void LuaEventListener::ProcessEvent(Event& event)
 void LuaEventListener::ProcessEvent(Event& event)
 {
 {
     //not sure if this is the right place to do this, but if the element we are attached to isn't a document, then
     //not sure if this is the right place to do this, but if the element we are attached to isn't a document, then
-    //the 'parent' variable will be nullptr, because element->ower_document hasn't been set on the construction. We should
+    //the 'owner_document' variable will be nullptr, because element->ower_document hasn't been set on the construction. We should
     //correct that
     //correct that
-    if(!parent && attached) parent = attached->GetOwnerDocument();
+    if(!owner_document && attached) owner_document = attached->GetOwnerDocument();
     lua_State* L = Interpreter::GetLuaState();
     lua_State* L = Interpreter::GetLuaState();
     int top = lua_gettop(L); 
     int top = lua_gettop(L); 
 
 
@@ -127,7 +138,7 @@ void LuaEventListener::ProcessEvent(Event& event)
     lua_rawgeti(L,-1,luaFuncRef);
     lua_rawgeti(L,-1,luaFuncRef);
     LuaType<Event>::push(L,&event,false);
     LuaType<Event>::push(L,&event,false);
 	LuaType<Element>::push(L,attached,false);
 	LuaType<Element>::push(L,attached,false);
-    LuaType<Document>::push(L,parent,false);
+    LuaType<Document>::push(L,owner_document,false);
     
     
     Interpreter::ExecuteCall(3,0); //call the function at the top of the stack with 3 arguments
     Interpreter::ExecuteCall(3,0); //call the function at the top of the stack with 3 arguments
 
 

+ 8 - 3
Source/Core/Lua/LuaEventListener.h

@@ -52,13 +52,18 @@ public:
 
 
     virtual ~LuaEventListener();
     virtual ~LuaEventListener();
 
 
-    /// Process the incoming Event
+	// Deletes itself, which also unreferences the Lua function.
+	void OnDetach(Element* element) override;
+
+	// Calls the associated Lua function.
 	void ProcessEvent(Event& event) override;
 	void ProcessEvent(Event& event) override;
+
 private:
 private:
     //the lua-side function to call when ProcessEvent is called
     //the lua-side function to call when ProcessEvent is called
-    int luaFuncRef = 0;
+    int luaFuncRef = -1;
+
     Element* attached = nullptr;
     Element* attached = nullptr;
-    ElementDocument* parent = nullptr;
+    ElementDocument* owner_document = nullptr;
     String strFunc; //for debugging purposes
     String strFunc; //for debugging purposes
 };
 };