Merge pull request #23094 from RandomShaper/fix-crash-signal-to-danling-obj

Fix crash on signal/resume to dangling target

4 files changed, 46 insertions, 43 deletions

diff --git a/modules/gdscript/gdscript_compiler.cpp b/modules/gdscript/gdscript_compiler.cpp
index 310c4e21f2..45319c59e7 100644
--- a/modules/gdscript/gdscript_compiler.cpp
+++ b/modules/gdscript/gdscript_compiler.cpp
@@ -1603,13 +1603,13 @@ Error GDScriptCompiler::_parse_block(CodeGen &codegen, const GDScriptParser::Blo
 	return OK;
 }
 
-Error GDScriptCompiler::_parse_function(GDScript *p_script, const GDScriptParser::ClassNode *p_class, const GDScriptParser::FunctionNode *p_func, bool p_for_ready) {
+Error GDScriptCompiler::_parse_function(Ref<GDScript> p_script, const GDScriptParser::ClassNode *p_class, const GDScriptParser::FunctionNode *p_func, bool p_for_ready) {
 
 	Vector<int> bytecode;
 	CodeGen codegen;
 
 	codegen.class_node = p_class;
-	codegen.script = p_script;
+	codegen.script = p_script.ptr();
 	codegen.function_node = p_func;
 	codegen.stack_max = 0;
 	codegen.current_line = 0;
@@ -1853,7 +1853,7 @@ Error GDScriptCompiler::_parse_function(GDScript *p_script, const GDScriptParser
 	return OK;
 }
 
-Error GDScriptCompiler::_parse_class_level(GDScript *p_script, GDScript *p_owner, const GDScriptParser::ClassNode *p_class, bool p_keep_state) {
+Error GDScriptCompiler::_parse_class_level(Ref<GDScript> p_script, Ref<GDScript> p_owner, const GDScriptParser::ClassNode *p_class, bool p_keep_state) {
 
 	if (p_class->owner && p_class->owner->owner) {
 		// Owner is not root
@@ -1887,7 +1887,7 @@ Error GDScriptCompiler::_parse_class_level(GDScript *p_script, GDScript *p_owner
 	p_script->initializer = NULL;
 
 	p_script->subclasses.clear();
-	p_script->_owner = p_owner;
+	p_script->_owner = p_owner.ptr();
 	p_script->tool = p_class->tool;
 	p_script->name = p_class->name;
 
@@ -1994,7 +1994,7 @@ Error GDScriptCompiler::_parse_class_level(GDScript *p_script, GDScript *p_owner
 
 		StringName name = p_class->_signals[i].name;
 
-		GDScript *c = p_script;
+		GDScript *c = p_script.ptr();
 
 		while (c) {
 
@@ -2054,7 +2054,7 @@ Error GDScriptCompiler::_parse_class_level(GDScript *p_script, GDScript *p_owner
 	return OK;
 }
 
-Error GDScriptCompiler::_parse_class_blocks(GDScript *p_script, const GDScriptParser::ClassNode *p_class, bool p_keep_state) {
+Error GDScriptCompiler::_parse_class_blocks(Ref<GDScript> p_script, const GDScriptParser::ClassNode *p_class, bool p_keep_state) {
 	//parse methods
 
 	bool has_initializer = false;
@@ -2159,7 +2159,7 @@ Error GDScriptCompiler::_parse_class_blocks(GDScript *p_script, const GDScriptPa
 	return OK;
 }
 
-void GDScriptCompiler::_make_scripts(const GDScript *p_script, const GDScriptParser::ClassNode *p_class, bool p_keep_state) {
+void GDScriptCompiler::_make_scripts(const Ref<GDScript> p_script, const GDScriptParser::ClassNode *p_class, bool p_keep_state) {
 
 	Map<StringName, Ref<GDScript> > old_subclasses;
 
@@ -2178,20 +2178,20 @@ void GDScriptCompiler::_make_scripts(const GDScript *p_script, const GDScriptPar
 			subclass.instance();
 		}
 
-		subclass->_owner = const_cast<GDScript *>(p_script);
+		subclass->_owner = const_cast<GDScript *>(p_script.ptr());
 		class_map.insert(name, subclass);
 
 		_make_scripts(subclass.ptr(), p_class->subclasses[i], p_keep_state);
 	}
 }
 
-Error GDScriptCompiler::compile(const GDScriptParser *p_parser, GDScript *p_script, bool p_keep_state) {
+Error GDScriptCompiler::compile(const GDScriptParser *p_parser, Ref<GDScript> p_script, bool p_keep_state) {
 
 	err_line = -1;
 	err_column = -1;
 	error = "";
 	parser = p_parser;
-	main_script = p_script;
+	main_script = p_script.ptr();
 	const GDScriptParser::Node *root = parser->get_parse_tree();
 	ERR_FAIL_COND_V(root->type != GDScriptParser::Node::TYPE_CLASS, ERR_INVALID_DATA);
 
diff --git a/modules/gdscript/gdscript_compiler.h b/modules/gdscript/gdscript_compiler.h
index 55f5e2b48e..23c6450aa7 100644
--- a/modules/gdscript/gdscript_compiler.h
+++ b/modules/gdscript/gdscript_compiler.h
@@ -148,17 +148,17 @@ class GDScriptCompiler {
 	int _parse_assign_right_expression(CodeGen &codegen, const GDScriptParser::OperatorNode *p_expression, int p_stack_level);
 	int _parse_expression(CodeGen &codegen, const GDScriptParser::Node *p_expression, int p_stack_level, bool p_root = false, bool p_initializer = false);
 	Error _parse_block(CodeGen &codegen, const GDScriptParser::BlockNode *p_block, int p_stack_level = 0, int p_break_addr = -1, int p_continue_addr = -1);
-	Error _parse_function(GDScript *p_script, const GDScriptParser::ClassNode *p_class, const GDScriptParser::FunctionNode *p_func, bool p_for_ready = false);
-	Error _parse_class_level(GDScript *p_script, GDScript *p_owner, const GDScriptParser::ClassNode *p_class, bool p_keep_state);
-	Error _parse_class_blocks(GDScript *p_script, const GDScriptParser::ClassNode *p_class, bool p_keep_state);
-	void _make_scripts(const GDScript *p_script, const GDScriptParser::ClassNode *p_class, bool p_keep_state);
+	Error _parse_function(Ref<GDScript> p_script, const GDScriptParser::ClassNode *p_class, const GDScriptParser::FunctionNode *p_func, bool p_for_ready = false);
+	Error _parse_class_level(Ref<GDScript> p_script, Ref<GDScript> p_owner, const GDScriptParser::ClassNode *p_class, bool p_keep_state);
+	Error _parse_class_blocks(Ref<GDScript> p_script, const GDScriptParser::ClassNode *p_class, bool p_keep_state);
+	void _make_scripts(const Ref<GDScript> p_script, const GDScriptParser::ClassNode *p_class, bool p_keep_state);
 	int err_line;
 	int err_column;
 	StringName source;
 	String error;
 
 public:
-	Error compile(const GDScriptParser *p_parser, GDScript *p_script, bool p_keep_state = false);
+	Error compile(const GDScriptParser *p_parser, Ref<GDScript> p_script, bool p_keep_state = false);
 
 	String get_error() const;
 	int get_error_line() const;
diff --git a/modules/gdscript/gdscript_function.cpp b/modules/gdscript/gdscript_function.cpp
index f09ff224e8..31115a4bd9 100644
--- a/modules/gdscript/gdscript_function.cpp
+++ b/modules/gdscript/gdscript_function.cpp
@@ -275,7 +275,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 #endif
 
 	uint32_t alloca_size = 0;
-	GDScript *_class;
+	GDScript *script;
 	int ip = 0;
 	int line = _initial_line;
 
@@ -286,7 +286,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 		line = p_state->line;
 		ip = p_state->ip;
 		alloca_size = p_state->stack.size();
-		_class = p_state->_class;
+		script = p_state->script.ptr();
 		p_instance = p_state->instance;
 		defarg = p_state->defarg;
 		self = p_state->self;
@@ -368,9 +368,9 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 			} else {
 				self = p_instance->owner;
 			}
-			_class = p_instance->script.ptr();
+			script = p_instance->script.ptr();
 		} else {
-			_class = _script;
+			script = this->_script.ptr();
 		}
 	}
 
@@ -395,7 +395,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 
 #define GET_VARIANT_PTR(m_v, m_code_ofs)                                                       \
 	Variant *m_v;                                                                              \
-	m_v = _get_variant(_code_ptr[ip + m_code_ofs], p_instance, _class, self, stack, err_text); \
+	m_v = _get_variant(_code_ptr[ip + m_code_ofs], p_instance, script, self, stack, err_text); \
 	if (unlikely(!m_v))                                                                        \
 		OPCODE_BREAK;
 
@@ -404,7 +404,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 #define CHECK_SPACE(m_space)
 #define GET_VARIANT_PTR(m_v, m_code_ofs) \
 	Variant *m_v;                        \
-	m_v = _get_variant(_code_ptr[ip + m_code_ofs], p_instance, _class, self, stack, err_text);
+	m_v = _get_variant(_code_ptr[ip + m_code_ofs], p_instance, script, self, stack, err_text);
 
 #endif
 
@@ -1185,7 +1185,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 
 				GET_VARIANT_PTR(dst, argc + 3);
 
-				const GDScript *gds = _script;
+				const GDScript *gds = _script.ptr();
 
 				const Map<StringName, GDScriptFunction *>::Element *E = NULL;
 				while (gds->base.ptr()) {
@@ -1256,11 +1256,10 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 				gdfs->state.stack_size = _stack_size;
 				gdfs->state.self = self;
 				gdfs->state.alloca_size = alloca_size;
-				gdfs->state._class = _class;
+				gdfs->state.script = _script;
 				gdfs->state.ip = ip + ipofs;
 				gdfs->state.line = line;
 				gdfs->state.instance_id = (p_instance && p_instance->get_owner()) ? p_instance->get_owner()->get_instance_id() : 0;
-				gdfs->state.script_id = _class->get_instance_id();
 				//gdfs->state.result_pos=ip+ipofs-1;
 				gdfs->state.defarg = defarg;
 				gdfs->state.instance = p_instance;
@@ -1549,8 +1548,8 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 		String err_file;
 		if (p_instance)
 			err_file = p_instance->script->path;
-		else if (_class)
-			err_file = _class->path;
+		else if (script)
+			err_file = script->path;
 		if (err_file == "")
 			err_file = "<built-in>";
 		String err_func = name;
@@ -1765,15 +1764,20 @@ GDScriptFunction::~GDScriptFunction() {
 Variant GDScriptFunctionState::_signal_callback(const Variant **p_args, int p_argcount, Variant::CallError &r_error) {
 
 #ifdef DEBUG_ENABLED
-	if (state.instance_id && !ObjectDB::get_instance(state.instance_id)) {
-		ERR_EXPLAIN("Resumed after yield, but class instance is gone");
+	// Checking this first since it's faster
+	if (!state.script.is_valid()) {
+		ERR_EXPLAIN("Resumed after yield, but script is gone");
 		ERR_FAIL_V(Variant());
 	}
 
-	if (state.script_id && !ObjectDB::get_instance(state.script_id)) {
-		ERR_EXPLAIN("Resumed after yield, but script is gone");
+	if (state.instance_id && !ObjectDB::get_instance(state.instance_id)) {
+		ERR_EXPLAIN("Resumed after yield, but class instance is gone");
 		ERR_FAIL_V(Variant());
 	}
+#else
+	if (!is_valid()) {
+		return Variant();
+	}
 #endif
 
 	Variant arg;
@@ -1841,12 +1845,12 @@ bool GDScriptFunctionState::is_valid(bool p_extended_check) const {
 		return false;
 
 	if (p_extended_check) {
+		//script gone? (checking this first since it's faster)
+		if (!state.script.is_valid())
+			return false;
 		//class instance gone?
 		if (state.instance_id && !ObjectDB::get_instance(state.instance_id))
 			return false;
-		//script gone?
-		if (state.script_id && !ObjectDB::get_instance(state.script_id))
-			return false;
 	}
 
 	return true;
@@ -1856,13 +1860,14 @@ Variant GDScriptFunctionState::resume(const Variant &p_arg) {
 
 	ERR_FAIL_COND_V(!function, Variant());
 #ifdef DEBUG_ENABLED
-	if (state.instance_id && !ObjectDB::get_instance(state.instance_id)) {
-		ERR_EXPLAIN("Resumed after yield, but class instance is gone");
+	// Checking this first since it's faster
+	if (!state.script.is_valid()) {
+		ERR_EXPLAIN("Resumed after yield, but script is gone");
 		ERR_FAIL_V(Variant());
 	}
 
-	if (state.script_id && !ObjectDB::get_instance(state.script_id)) {
-		ERR_EXPLAIN("Resumed after yield, but script is gone");
+	if (state.instance_id && !ObjectDB::get_instance(state.instance_id)) {
+		ERR_EXPLAIN("Resumed after yield, but class instance is gone");
 		ERR_FAIL_V(Variant());
 	}
 #endif
diff --git a/modules/gdscript/gdscript_function.h b/modules/gdscript/gdscript_function.h
index bfb6d673f1..a47070de4f 100644
--- a/modules/gdscript/gdscript_function.h
+++ b/modules/gdscript/gdscript_function.h
@@ -225,7 +225,7 @@ private:
 	bool _static;
 	MultiplayerAPI::RPCMode rpc_mode;
 
-	GDScript *_script;
+	Ref<GDScript> _script;
 
 	StringName name;
 	Vector<Variant> constants;
@@ -272,15 +272,13 @@ private:
 public:
 	struct CallState {
 
-		ObjectID instance_id; //by debug only
-		ObjectID script_id;
-
+		ObjectID instance_id;
 		GDScriptInstance *instance;
 		Vector<uint8_t> stack;
 		int stack_size;
 		Variant self;
 		uint32_t alloca_size;
-		GDScript *_class;
+		Ref<GDScript> script;
 		int ip;
 		int line;
 		int defarg;
@@ -299,7 +297,7 @@ public:
 	int get_default_argument_addr(int p_idx) const;
 	GDScriptDataType get_return_type() const;
 	GDScriptDataType get_argument_type(int p_idx) const;
-	GDScript *get_script() const { return _script; }
+	GDScript *get_script() const { return const_cast<GDScript *>(_script.ptr()); }
 	StringName get_source() const { return source; }
 
 	void debug_get_stack_member_state(int p_line, List<Pair<StringName, int> > *r_stackvars) const;