Merge pull request #127 from gilzoide/bugfix/luascriptproperty-leak

Fix memory leak from cyclic reference of LuaScriptProperty <-> LuaState

Author: gilzoide

CHANGELOG.md

@@ -9,6 +9,7 @@
 
 ### Fixed
 - Fixed cyclic references from `LuaScriptInstance` <-> `LuaState`, avoiding leaks of `LuaScript`s
+- Fixed cyclic references from `LuaScriptProperty` <-> `LuaState`, avoiding memory leaks
 
 
 ## [0.5.0](https://github.com/gilzoide/lua-gdextension/releases/tag/0.5.0)

src/script-language/LuaScriptProperty.cpp

@@ -80,14 +80,14 @@ static LuaScriptProperty lua_property(sol::stack_object value) {
 			property.usage &= ~PROPERTY_USAGE_STORAGE;
 		}
 		else if (auto getter = table->get<sol::optional<sol::protected_function>>("get")) {
-			property.getter = LuaObject::wrap_object<LuaFunction>(*getter);
+			property.getter = *getter;
 			property.usage &= ~PROPERTY_USAGE_STORAGE;
 		}
 		if (auto setter_name = table->get<sol::optional<StringName>>("set")) {
 			property.setter_name = *setter_name;
 		}
 		else if (auto setter = table->get<sol::optional<sol::protected_function>>("set")) {
-			property.setter = LuaObject::wrap_object<LuaFunction>(*setter);
+			property.setter = *setter;
 		}
 	}
 	else if (auto type = value.as<sol::optional<VariantType>>()) {
@@ -118,7 +118,7 @@ LuaScriptProperty::LuaScriptProperty(const Variant& value, const StringName& nam
 }
 
 bool LuaScriptProperty::get_value(LuaScriptInstance *self, Variant& r_value) const {
-	if (getter.is_valid()) {
+	if (getter.valid()) {
 		r_value = LuaFunction::invoke_lua(getter, VariantArguments(self->owner, nullptr, 0), false);
 		return true;
 	}
@@ -141,7 +141,7 @@ Variant LuaScriptProperty::instantiate_default_value() const {
 }
 
 bool LuaScriptProperty::set_value(LuaScriptInstance *self, const Variant& value) const {
-	if (setter.is_valid()) {
+	if (setter.valid()) {
 		LuaCoroutine::invoke_lua(setter, Array::make(self->owner, value), false);
 		return true;
 	}

src/script-language/LuaScriptProperty.hpp

@@ -49,8 +49,10 @@ struct LuaScriptProperty {
 
 	StringName getter_name;
 	StringName setter_name;
-	Ref<LuaFunction> getter;  // Variant getter(self)
-	Ref<LuaFunction> setter;  // void setter(self, Variant value)
+	// we use sol::protected_function instead of Ref<LuaFunction> here because LuaScriptProperty lives inside Lua
+	// this avoids cyclic references from LuaScriptPropert to/from its owning LuaState
+	sol::protected_function getter;  // Variant getter(self)
+	sol::protected_function setter;  // void setter(self, Variant value)
 
 	bool get_value(LuaScriptInstance *self, Variant& r_value) const;
 	bool set_value(LuaScriptInstance *self, const Variant& value) const;