From 95828161d4ffd01b99abc2155ffb89c728cebb2f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pedro=20J=2E=20Est=C3=A9banez?= <pedrojrulez@gmail.com>
Date: Thu, 10 Sep 2020 01:26:07 +0200
Subject: Prevent cyclic reference between script and its members

---
 modules/gdscript/gdscript_compiler.cpp | 24 +++++++++++++++---------
 modules/gdscript/gdscript_compiler.h   |  4 ++--
 modules/gdscript/gdscript_function.h   |  3 ++-
 3 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/modules/gdscript/gdscript_compiler.cpp b/modules/gdscript/gdscript_compiler.cpp
index c3d651ee79..bad450c9f9 100644
--- a/modules/gdscript/gdscript_compiler.cpp
+++ b/modules/gdscript/gdscript_compiler.cpp
@@ -76,7 +76,7 @@ void GDScriptCompiler::_set_error(const String &p_error, const GDScriptParser::N
 	}
 }
 
-GDScriptDataType GDScriptCompiler::_gdtype_from_datatype(const GDScriptParser::DataType &p_datatype) const {
+GDScriptDataType GDScriptCompiler::_gdtype_from_datatype(const GDScriptParser::DataType &p_datatype, GDScript *p_owner) const {
 	if (!p_datatype.is_set() || !p_datatype.is_hard_type()) {
 		return GDScriptDataType();
 	}
@@ -98,7 +98,7 @@ GDScriptDataType GDScriptCompiler::_gdtype_from_datatype(const GDScriptParser::D
 		} break;
 		case GDScriptParser::DataType::SCRIPT: {
 			result.kind = GDScriptDataType::SCRIPT;
-			result.script_type = p_datatype.script_type;
+			result.script_type = Ref<Script>(p_datatype.script_type).ptr();
 			result.native_type = result.script_type->get_instance_base_type();
 		} break;
 		case GDScriptParser::DataType::CLASS: {
@@ -124,11 +124,11 @@ GDScriptDataType GDScriptCompiler::_gdtype_from_datatype(const GDScriptParser::D
 						names.pop_back();
 					}
 					result.kind = GDScriptDataType::GDSCRIPT;
-					result.script_type = script;
+					result.script_type = script.ptr();
 					result.native_type = script->get_instance_base_type();
 				} else {
 					result.kind = GDScriptDataType::GDSCRIPT;
-					result.script_type = GDScriptCache::get_shallow_script(p_datatype.script_path, main_script->path);
+					result.script_type = GDScriptCache::get_shallow_script(p_datatype.script_path, main_script->path).ptr();
 					result.native_type = p_datatype.native_type;
 				}
 			}
@@ -149,6 +149,12 @@ GDScriptDataType GDScriptCompiler::_gdtype_from_datatype(const GDScriptParser::D
 		}
 	}
 
+	// Only hold strong reference to the script if it's not the owner of the
+	// element qualified with this type, to avoid cyclic references (leaks).
+	if (result.script_type && result.script_type != p_owner) {
+		result.script_type_ref = Ref<Script>(result.script_type);
+	}
+
 	return result;
 }
 
@@ -1668,7 +1674,7 @@ Error GDScriptCompiler::_parse_function(GDScript *p_script, const GDScriptParser
 		func_name = p_func->identifier->name;
 		is_static = p_func->is_static;
 		rpc_mode = p_func->rpc_mode;
-		return_type = _gdtype_from_datatype(p_func->get_datatype());
+		return_type = _gdtype_from_datatype(p_func->get_datatype(), p_script);
 	} else {
 		if (p_for_ready) {
 			func_name = "_ready";
@@ -1685,7 +1691,7 @@ Error GDScriptCompiler::_parse_function(GDScript *p_script, const GDScriptParser
 	if (p_func) {
 		for (int i = 0; i < p_func->parameters.size(); i++) {
 			const GDScriptParser::ParameterNode *parameter = p_func->parameters[i];
-			GDScriptDataType par_type = _gdtype_from_datatype(parameter->get_datatype());
+			GDScriptDataType par_type = _gdtype_from_datatype(parameter->get_datatype(), p_script);
 			uint32_t par_addr = codegen.generator->add_parameter(parameter->identifier->name, parameter->default_value != nullptr, par_type);
 			codegen.parameters[parameter->identifier->name] = GDScriptCodeGenerator::Address(GDScriptCodeGenerator::Address::FUNCTION_PARAMETER, par_addr, par_type);
 
@@ -1830,7 +1836,7 @@ Error GDScriptCompiler::_parse_setter_getter(GDScript *p_script, const GDScriptP
 		return_type.kind = GDScriptDataType::BUILTIN;
 		return_type.builtin_type = Variant::NIL;
 	} else {
-		return_type = _gdtype_from_datatype(p_variable->get_datatype());
+		return_type = _gdtype_from_datatype(p_variable->get_datatype(), p_script);
 	}
 
 	codegen.generator->write_start(p_script, func_name, false, p_variable->rpc_mode, return_type);
@@ -1927,7 +1933,7 @@ Error GDScriptCompiler::_parse_class_level(GDScript *p_script, const GDScriptPar
 			p_script->native = native;
 		} break;
 		case GDScriptDataType::GDSCRIPT: {
-			Ref<GDScript> base = base_type.script_type;
+			Ref<GDScript> base = Ref<GDScript>(base_type.script_type);
 			p_script->base = base;
 			p_script->_base = base.ptr();
 
@@ -1994,7 +2000,7 @@ Error GDScriptCompiler::_parse_class_level(GDScript *p_script, const GDScriptPar
 						break;
 				}
 				minfo.rpc_mode = variable->rpc_mode;
-				minfo.data_type = _gdtype_from_datatype(variable->get_datatype());
+				minfo.data_type = _gdtype_from_datatype(variable->get_datatype(), p_script);
 
 				PropertyInfo prop_info = minfo.data_type;
 				prop_info.name = name;
diff --git a/modules/gdscript/gdscript_compiler.h b/modules/gdscript/gdscript_compiler.h
index 80fba6a934..db02079d26 100644
--- a/modules/gdscript/gdscript_compiler.h
+++ b/modules/gdscript/gdscript_compiler.h
@@ -84,7 +84,7 @@ class GDScriptCompiler {
 
 					Ref<Script> script = obj->get_script();
 					if (script.is_valid()) {
-						type.script_type = script;
+						type.script_type = script.ptr();
 						Ref<GDScript> gdscript = script;
 						if (gdscript.is_valid()) {
 							type.kind = GDScriptDataType::GDSCRIPT;
@@ -125,7 +125,7 @@ class GDScriptCompiler {
 	Error _create_binary_operator(CodeGen &codegen, const GDScriptParser::BinaryOpNode *on, Variant::Operator op, bool p_initializer = false, const GDScriptCodeGenerator::Address &p_index_addr = GDScriptCodeGenerator::Address());
 	Error _create_binary_operator(CodeGen &codegen, const GDScriptParser::ExpressionNode *p_left_operand, const GDScriptParser::ExpressionNode *p_right_operand, Variant::Operator op, bool p_initializer = false, const GDScriptCodeGenerator::Address &p_index_addr = GDScriptCodeGenerator::Address());
 
-	GDScriptDataType _gdtype_from_datatype(const GDScriptParser::DataType &p_datatype) const;
+	GDScriptDataType _gdtype_from_datatype(const GDScriptParser::DataType &p_datatype, GDScript *p_owner = nullptr) const;
 
 	GDScriptCodeGenerator::Address _parse_assign_right_expression(CodeGen &codegen, Error &r_error, const GDScriptParser::AssignmentNode *p_assignmentint, const GDScriptCodeGenerator::Address &p_index_addr = GDScriptCodeGenerator::Address());
 	GDScriptCodeGenerator::Address _parse_expression(CodeGen &codegen, Error &r_error, const GDScriptParser::ExpressionNode *p_expression, bool p_root = false, bool p_initializer = false, const GDScriptCodeGenerator::Address &p_index_addr = GDScriptCodeGenerator::Address());
diff --git a/modules/gdscript/gdscript_function.h b/modules/gdscript/gdscript_function.h
index d1c98a5456..c98ac09310 100644
--- a/modules/gdscript/gdscript_function.h
+++ b/modules/gdscript/gdscript_function.h
@@ -56,7 +56,8 @@ struct GDScriptDataType {
 	bool has_type = false;
 	Variant::Type builtin_type = Variant::NIL;
 	StringName native_type;
-	Ref<Script> script_type;
+	Script *script_type = nullptr;
+	Ref<Script> script_type_ref;
 
 	bool is_type(const Variant &p_variant, bool p_allow_implicit_conversion = false) const {
 		if (!has_type) {
-- 
cgit v1.2.3


From e25f5e791e709f44026ad933d5eada34a4715ef6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pedro=20J=2E=20Est=C3=A9banez?= <pedrojrulez@gmail.com>
Date: Thu, 10 Sep 2020 01:26:50 +0200
Subject: Ensure cyclic dependencies between scripts are broken at exit

---
 modules/gdscript/gdscript.cpp | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/modules/gdscript/gdscript.cpp b/modules/gdscript/gdscript.cpp
index 0263e32c5b..7f303a966d 100644
--- a/modules/gdscript/gdscript.cpp
+++ b/modules/gdscript/gdscript.cpp
@@ -1053,7 +1053,9 @@ GDScript::~GDScript() {
 		memdelete(E->get());
 	}
 
-	GDScriptCache::remove_script(get_path());
+	if (GDScriptCache::singleton) { // Cache may have been already destroyed at engine shutdown.
+		GDScriptCache::remove_script(get_path());
+	}
 
 	_save_orphaned_subclasses();
 
@@ -2035,7 +2037,23 @@ GDScriptLanguage::~GDScriptLanguage() {
 	if (_call_stack) {
 		memdelete_arr(_call_stack);
 	}
-	singleton = nullptr;
+
+	// Clear dependencies between scripts, to ensure cyclic references are broken (to avoid leaks at exit).
+	while (script_list.first()) {
+		GDScript *script = script_list.first()->self();
+		for (Map<StringName, GDScriptFunction *>::Element *E = script->member_functions.front(); E; E = E->next()) {
+			GDScriptFunction *func = E->get();
+			for (int i = 0; i < func->argument_types.size(); i++) {
+				func->argument_types.write[i].script_type_ref = Ref<Script>();
+			}
+			func->return_type.script_type_ref = Ref<Script>();
+		}
+		for (Map<StringName, GDScript::MemberInfo>::Element *E = script->member_indices.front(); E; E = E->next()) {
+			E->get().data_type.script_type_ref = Ref<Script>();
+		}
+	}
+
+	singleton = NULL;
 }
 
 void GDScriptLanguage::add_orphan_subclass(const String &p_qualified_name, const ObjectID &p_subclass) {
-- 
cgit v1.2.3