Merge pull request #41931 from RandomShaper/fix_gdscript_leaks_3.2

Fix leaks in GDScript (3.2)
godotengine · Sep 24, 2020 · bca2633 · bca2633
2 parents 26512dd + 8e64969
commit bca2633
Show file tree

Hide file tree

Showing 6 changed files with 37 additions and 13 deletions.
diff --git a/modules/gdscript/gdscript.cpp b/modules/gdscript/gdscript.cpp
@@ -2244,6 +2244,22 @@ GDScriptLanguage::~GDScriptLanguage() {
 	if (_call_stack) {
 		memdelete_arr(_call_stack);
 	}
+
+	// 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;
 }
 

diff --git a/modules/gdscript/gdscript_compiler.cpp b/modules/gdscript/gdscript_compiler.cpp
@@ -111,7 +111,7 @@ bool GDScriptCompiler::_create_binary_operator(CodeGen &codegen, const GDScriptP
 	return true;
 }
 
-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.has_type) {
 		return GDScriptDataType();
 	}
@@ -130,12 +130,12 @@ 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::GDSCRIPT: {
 			result.kind = GDScriptDataType::GDSCRIPT;
-			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: {
@@ -159,7 +159,7 @@ GDScriptDataType GDScriptCompiler::_gdtype_from_datatype(const GDScriptParser::D
 			}
 
 			result.kind = GDScriptDataType::GDSCRIPT;
-			result.script_type = script;
+			result.script_type = Ref<Script>(script).ptr();
 			result.native_type = script->get_instance_base_type();
 		} break;
 		default: {
@@ -168,6 +168,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;
 }
 
@@ -1684,9 +1690,9 @@ Error GDScriptCompiler::_parse_function(GDScript *p_script, const GDScriptParser
 		gdfunc->rpc_mode = p_func->rpc_mode;
 		gdfunc->argument_types.resize(p_func->argument_types.size());
 		for (int i = 0; i < p_func->argument_types.size(); i++) {
-			gdfunc->argument_types.write[i] = _gdtype_from_datatype(p_func->argument_types[i]);
+			gdfunc->argument_types.write[i] = _gdtype_from_datatype(p_func->argument_types[i], p_script);
 		}
-		gdfunc->return_type = _gdtype_from_datatype(p_func->return_type);
+		gdfunc->return_type = _gdtype_from_datatype(p_func->return_type, p_script);
 	} else {
 		gdfunc->_static = false;
 		gdfunc->rpc_mode = MultiplayerAPI::RPC_MODE_DISABLED;
@@ -1869,7 +1875,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();
 			p_script->member_indices = base->member_indices;
@@ -1902,7 +1908,7 @@ Error GDScriptCompiler::_parse_class_level(GDScript *p_script, const GDScriptPar
 		minfo.setter = p_class->variables[i].setter;
 		minfo.getter = p_class->variables[i].getter;
 		minfo.rpc_mode = p_class->variables[i].rpc_mode;
-		minfo.data_type = _gdtype_from_datatype(p_class->variables[i].data_type);
+		minfo.data_type = _gdtype_from_datatype(p_class->variables[i].data_type, 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
@@ -142,7 +142,7 @@ class GDScriptCompiler {
 	bool _create_unary_operator(CodeGen &codegen, const GDScriptParser::OperatorNode *on, Variant::Operator op, int p_stack_level);
 	bool _create_binary_operator(CodeGen &codegen, const GDScriptParser::OperatorNode *on, Variant::Operator op, int p_stack_level, bool p_initializer = false, int p_index_addr = 0);
 
-	GDScriptDataType _gdtype_from_datatype(const GDScriptParser::DataType &p_datatype) const;
+	GDScriptDataType _gdtype_from_datatype(const GDScriptParser::DataType &p_datatype, GDScript *p_owner = NULL) const;
 
 	int _parse_assign_right_expression(CodeGen &codegen, const GDScriptParser::OperatorNode *p_expression, int p_stack_level, int p_index_addr = 0);
 	int _parse_expression(CodeGen &codegen, const GDScriptParser::Node *p_expression, int p_stack_level, bool p_root = false, bool p_initializer = false, int p_index_addr = 0);

diff --git a/modules/gdscript/gdscript_editor.cpp b/modules/gdscript/gdscript_editor.cpp
@@ -644,7 +644,7 @@ static GDScriptCompletionIdentifier _type_from_gdtype(const GDScriptDataType &p_
 	ci.type.has_type = true;
 	ci.type.builtin_type = p_gdtype.builtin_type;
 	ci.type.native_type = p_gdtype.native_type;
-	ci.type.script_type = p_gdtype.script_type;
+	ci.type.script_type = Ref<Script>(p_gdtype.script_type);
 
 	switch (p_gdtype.kind) {
 		case GDScriptDataType::UNINITIALIZED: {

diff --git a/modules/gdscript/gdscript_function.h b/modules/gdscript/gdscript_function.h
@@ -53,7 +53,8 @@ struct GDScriptDataType {
 	} kind;
 	Variant::Type builtin_type;
 	StringName native_type;
-	Ref<Script> script_type;
+	Script *script_type;
+	Ref<Script> script_type_ref;
 
 	bool is_type(const Variant &p_variant, bool p_allow_implicit_conversion = false) const {
 		if (!has_type) return true; // Can't type check
@@ -149,7 +150,8 @@ struct GDScriptDataType {
 	GDScriptDataType() :
 			has_type(false),
 			kind(UNINITIALIZED),
-			builtin_type(Variant::NIL) {}
+			builtin_type(Variant::NIL),
+			script_type(NULL) {}
 };
 
 class GDScriptFunction {

diff --git a/modules/gdscript/gdscript_parser.cpp b/modules/gdscript/gdscript_parser.cpp
@@ -6055,7 +6055,7 @@ GDScriptParser::DataType GDScriptParser::_type_from_gdtype(const GDScriptDataTyp
 	result.has_type = true;
 	result.builtin_type = p_gdtype.builtin_type;
 	result.native_type = p_gdtype.native_type;
-	result.script_type = p_gdtype.script_type;
+	result.script_type = Ref<Script>(p_gdtype.script_type);
 
 	switch (p_gdtype.kind) {
 		case GDScriptDataType::UNINITIALIZED: {