Fix Lua reference leaks (fixes #638)

The-Powder-Toy · Apr 9, 2019 · 06e2836 · 06e2836
1 parent 783310d
commit 06e2836
Show file tree

Hide file tree

Showing 21 changed files with 311 additions and 349 deletions.
diff --git a/src/graphics/Renderer.cpp b/src/graphics/Renderer.cpp
@@ -18,6 +18,7 @@
 #ifdef LUACONSOLE
 #include "lua/LuaScriptInterface.h"
 #include "lua/LuaScriptHelper.h"
+#include "lua/LuaSmartRef.h"
 #endif
 #include "hmap.h"
 #ifdef OGLR

diff --git a/src/gui/interface/Window.cpp b/src/gui/interface/Window.cpp
@@ -80,6 +80,7 @@ void Window::RemoveComponent(Component* c)
 			Components.erase(Components.begin() + i);
 
 			// we're done
+			c->SetParentWindow(NULL);
 			return;
 		}
 	}

diff --git a/src/lua/LegacyLuaAPI.cpp b/src/lua/LegacyLuaAPI.cpp
@@ -8,6 +8,7 @@
 #include "Format.h"
 #include "LuaScriptInterface.h"
 #include "LuaScriptHelper.h"
+#include "LuaSmartRef.h"
 #include "Platform.h"
 #include "PowderToy.h"
 
@@ -136,7 +137,7 @@ int luacon_partsread(lua_State* l)
 	if (i < 0 || i >= NPART)
 		return luaL_error(l, "array index out of bounds");
 
-	lua_rawgeti(l, LUA_REGISTRYINDEX, tptPart);
+	lua_rawgeti(l, LUA_REGISTRYINDEX, *tptPart);
 	cIndex = i;
 	return 1;
 }
@@ -317,12 +318,9 @@ int luatpt_element_func(lua_State *l)
 	{
 		int element = luaL_optint(l, 2, 0);
 		int replace = luaL_optint(l, 3, 0);
-		int function;
-		lua_pushvalue(l, 1);
-		function = luaL_ref(l, LUA_REGISTRYINDEX);
 		if(element > 0 && element < PT_NUM)
 		{
-			lua_el_func[element] = function;
+			lua_el_func[element].Assign(1);
 			if (replace == 2)
 				lua_el_mode[element] = 3; //update before
 			else if (replace)
@@ -341,7 +339,7 @@ int luatpt_element_func(lua_State *l)
 		int element = luaL_optint(l, 2, 0);
 		if(element > 0 && element < PT_NUM)
 		{
-			lua_el_func[element] = 0;
+			lua_el_func[element].Clear();
 			lua_el_mode[element] = 0;
 		}
 		else
@@ -390,12 +388,9 @@ int luatpt_graphics_func(lua_State *l)
 	if(lua_isfunction(l, 1))
 	{
 		int element = luaL_optint(l, 2, 0);
-		int function;
-		lua_pushvalue(l, 1);
-		function = luaL_ref(l, LUA_REGISTRYINDEX);
 		if (element > 0 && element < PT_NUM)
 		{
-			lua_gr_func[element] = function;
+			lua_gr_func[element].Assign(1);
 			luacon_ren->graphicscache[element].isready = 0;
 			return 0;
 		}
@@ -409,7 +404,7 @@ int luatpt_graphics_func(lua_State *l)
 		int element = luaL_optint(l, 2, 0);
 		if (element > 0 && element < PT_NUM)
 		{
-			lua_gr_func[element] = 0;
+			lua_gr_func[element].Clear();
 			luacon_ren->graphicscache[element].isready = 0;
 			return 0;
 		}

diff --git a/src/lua/LuaButton.cpp b/src/lua/LuaButton.cpp
@@ -20,7 +20,7 @@ Luna<LuaButton>::RegType LuaButton::methods[] = {
 
 LuaButton::LuaButton(lua_State * l) :
 	LuaComponent(l),
-	actionFunction(0)
+	actionFunction(l)
 {
 	int posX = luaL_optinteger(l, 1, 0);
 	int posY = luaL_optinteger(l, 2, 0);
@@ -62,17 +62,7 @@ int LuaButton::enabled(lua_State * l)
 
 int LuaButton::action(lua_State * l)
 {
-	if(lua_type(l, 1) != LUA_TNIL)
-	{
-		luaL_checktype(l, 1, LUA_TFUNCTION);
-		lua_pushvalue(l, 1);
-		actionFunction = luaL_ref(l, LUA_REGISTRYINDEX);
-	}
-	else
-	{
-		actionFunction = 0;
-	}
-	return 0;
+	return actionFunction.CheckAndAssignArg1();
 }
 
 int LuaButton::text(lua_State * l)
@@ -96,7 +86,7 @@ void LuaButton::triggerAction()
 	if(actionFunction)
 	{
 		lua_rawgeti(l, LUA_REGISTRYINDEX, actionFunction);
-		lua_rawgeti(l, LUA_REGISTRYINDEX, UserData);
+		lua_rawgeti(l, LUA_REGISTRYINDEX, owner_ref);
 		if (lua_pcall(l, 1, 0, 0))
 		{
 			ci->Log(CommandInterface::LogError, ByteString(lua_tostring(l, -1)).FromUtf8());

diff --git a/src/lua/LuaButton.h b/src/lua/LuaButton.h
@@ -13,7 +13,7 @@ class LuaScriptInterface;
 class LuaButton: public LuaComponent
 {
 	ui::Button * button;
-	int actionFunction;
+	LuaComponentCallback actionFunction;
 	void triggerAction();
 	int action(lua_State * l);
 	int text(lua_State * l);

diff --git a/src/lua/LuaCheckbox.cpp b/src/lua/LuaCheckbox.cpp
@@ -20,7 +20,7 @@ Luna<LuaCheckbox>::RegType LuaCheckbox::methods[] = {
 
 LuaCheckbox::LuaCheckbox(lua_State * l) :
 	LuaComponent(l),
-	actionFunction(0)
+	actionFunction(l)
 {
 	int posX = luaL_optinteger(l, 1, 0);
 	int posY = luaL_optinteger(l, 2, 0);
@@ -60,17 +60,7 @@ int LuaCheckbox::checked(lua_State * l)
 
 int LuaCheckbox::action(lua_State * l)
 {
-	if(lua_type(l, 1) != LUA_TNIL)
-	{
-		luaL_checktype(l, 1, LUA_TFUNCTION);
-		lua_pushvalue(l, 1);
-		actionFunction = luaL_ref(l, LUA_REGISTRYINDEX);
-	}
-	else
-	{
-		actionFunction = 0;
-	}
-	return 0;
+	return actionFunction.CheckAndAssignArg1();
 }
 
 int LuaCheckbox::text(lua_State * l)
@@ -93,7 +83,7 @@ void LuaCheckbox::triggerAction()
 	if(actionFunction)
 	{
 		lua_rawgeti(l, LUA_REGISTRYINDEX, actionFunction);
-		lua_rawgeti(l, LUA_REGISTRYINDEX, UserData);
+		lua_rawgeti(l, LUA_REGISTRYINDEX, owner_ref);
 		lua_pushboolean(l, checkbox->GetChecked());
 		if (lua_pcall(l, 2, 0, 0))
 		{

diff --git a/src/lua/LuaCheckbox.h b/src/lua/LuaCheckbox.h
@@ -13,7 +13,7 @@ class LuaScriptInterface;
 class LuaCheckbox: public LuaComponent
 {
 	ui::Checkbox * checkbox;
-	int actionFunction;
+	LuaComponentCallback actionFunction;
 	void triggerAction();
 	int action(lua_State * l);
 	int checked(lua_State * l);

diff --git a/src/lua/LuaComponent.cpp b/src/lua/LuaComponent.cpp
@@ -5,8 +5,17 @@
 #include "LuaScriptInterface.h"
 #include "gui/interface/Component.h"
 
+int LuaComponentCallback::CheckAndAssignArg1()
+{
+	if (lua_type(l, 1) != LUA_TNIL)
+	{
+		luaL_checktype(l, 1, LUA_TFUNCTION);
+	}
+	LuaSmartRef::Assign(1);
+	return 0;
+}
 
-LuaComponent::LuaComponent(lua_State * l)
+LuaComponent::LuaComponent(lua_State * l) : owner_ref(LUA_REFNIL)
 {
 	this->l = l;
 

diff --git a/src/lua/LuaComponent.h b/src/lua/LuaComponent.h
@@ -1,6 +1,7 @@
 #pragma once
 
 #include "LuaLuna.h"
+#include "LuaSmartRef.h"
 
 namespace ui
 {
@@ -9,6 +10,13 @@ namespace ui
 
 class LuaScriptInterface;
 
+class LuaComponentCallback : public LuaSmartRef
+{
+public:
+	using LuaSmartRef::LuaSmartRef;
+	int CheckAndAssignArg1();
+};
+
 class LuaComponent
 {
 protected:
@@ -19,7 +27,7 @@ class LuaComponent
 	int visible(lua_State * l);
 public:
 	LuaScriptInterface * ci;
-	int UserData;
+	int owner_ref;
 
 	ui::Component * GetComponent() { return component; }
 	LuaComponent(lua_State * l);

diff --git a/src/lua/LuaLuna.h b/src/lua/LuaLuna.h
@@ -133,9 +133,6 @@ template <typename T> class Luna
 		userdataType *ud = static_cast<userdataType*>(lua_newuserdata(L, sizeof(userdataType)));
 		ud->pT = obj;  // store pointer to object in userdata
 
-		obj->UserData = luaL_ref(L, LUA_REGISTRYINDEX);
-		lua_rawgeti(L, LUA_REGISTRYINDEX, obj->UserData);
-
 		luaL_getmetatable(L, T::className);  // lookup metatable in Lua registry
 		lua_setmetatable(L, -2);
 		return 1;  // userdata containing pointer to T object

diff --git a/src/lua/LuaScriptHelper.h b/src/lua/LuaScriptHelper.h
@@ -11,13 +11,16 @@ extern Renderer * luacon_ren;
 extern bool *luacon_currentCommand;
 extern String *luacon_lastError;
 
-extern int *lua_el_func, *lua_el_mode, *lua_gr_func;
+class LuaSmartRef;
+extern int *lua_el_mode;
+extern LuaSmartRef *lua_el_func, *lua_gr_func;
 
 extern int getPartIndex_curIdx;
 extern int tptProperties; //Table for some TPT properties
 extern int tptPropertiesVersion;
 extern int tptElements; //Table for TPT element names
-extern int tptParts, tptPartsMeta, tptElementTransitions, tptPartsCData, tptPartMeta, tptPart, cIndex;
+extern int tptParts, tptPartsMeta, tptElementTransitions, tptPartsCData, tptPartMeta, cIndex;
+extern LuaSmartRef *tptPart;
 
 void luaopen_eventcompat(lua_State *l);
 void luacon_hook(lua_State *L, lua_Debug *ar);