Browse Source

Fix edge cases of object lifetime when signals involved

Pedro J. Estébanez 2 years ago
parent
commit
2f4168daeb

+ 4 - 0
core/object/object.cpp

@@ -1013,6 +1013,10 @@ Error Object::emit_signalp(const StringName &p_name, const Variant **p_args, int
 		return ERR_UNAVAILABLE;
 	}
 
+	// If this is a ref-counted object, prevent it from being destroyed during signal emission,
+	// which is needed in certain edge cases; e.g., https://github.com/godotengine/godot/issues/73889.
+	Ref<RefCounted> rc = Ref<RefCounted>(Object::cast_to<RefCounted>(this));
+
 	List<_ObjectSignalDisconnectData> disconnect_data;
 
 	//copy on write will ensure that disconnecting the signal or even deleting the object will not affect the signal calling.

+ 20 - 1
modules/gdscript/gdscript_byte_codegen.cpp

@@ -142,7 +142,9 @@ void GDScriptByteCodeGenerator::pop_temporary() {
 	if (slot.type == Variant::NIL) {
 		// Avoid keeping in the stack long-lived references to objects,
 		// which may prevent RefCounted objects from being freed.
-		write_assign_false(Address(Address::TEMPORARY, slot_idx));
+		// However, the cleanup will be performed an the end of the
+		// statement, to allow object references to survive chaining.
+		temporaries_pending_clear.push_back(slot_idx);
 	}
 	temporaries_pool[slot.type].push_back(slot_idx);
 	used_temporaries.pop_back();
@@ -1752,6 +1754,23 @@ void GDScriptByteCodeGenerator::end_block() {
 	pop_stack_identifiers();
 }
 
+void GDScriptByteCodeGenerator::clean_temporaries() {
+	List<int>::Element *E = temporaries_pending_clear.front();
+	while (E) {
+		// The temporary may have been re-used as something else than an object
+		// since it was added to the list. In that case, there's no need to clear it.
+		int slot_idx = E->get();
+		const StackSlot &slot = temporaries[slot_idx];
+		if (slot.type == Variant::NIL) {
+			write_assign_false(Address(Address::TEMPORARY, slot_idx));
+		}
+
+		List<int>::Element *next = E->next();
+		E->erase();
+		E = next;
+	}
+}
+
 GDScriptByteCodeGenerator::~GDScriptByteCodeGenerator() {
 	if (!ended && function != nullptr) {
 		memdelete(function);

+ 2 - 0
modules/gdscript/gdscript_byte_codegen.h

@@ -88,6 +88,7 @@ class GDScriptByteCodeGenerator : public GDScriptCodeGenerator {
 	Vector<StackSlot> locals;
 	Vector<StackSlot> temporaries;
 	List<int> used_temporaries;
+	List<int> temporaries_pending_clear;
 	RBMap<Variant::Type, List<int>> temporaries_pool;
 
 	List<GDScriptFunction::StackDebug> stack_debug;
@@ -463,6 +464,7 @@ public:
 	virtual uint32_t add_or_get_name(const StringName &p_name) override;
 	virtual uint32_t add_temporary(const GDScriptDataType &p_type) override;
 	virtual void pop_temporary() override;
+	virtual void clean_temporaries() override;
 
 	virtual void start_parameters() override;
 	virtual void end_parameters() override;

+ 1 - 0
modules/gdscript/gdscript_codegen.h

@@ -72,6 +72,7 @@ public:
 	virtual uint32_t add_or_get_name(const StringName &p_name) = 0;
 	virtual uint32_t add_temporary(const GDScriptDataType &p_type) = 0;
 	virtual void pop_temporary() = 0;
+	virtual void clean_temporaries() = 0;
 
 	virtual void start_parameters() = 0;
 	virtual void end_parameters() = 0;

+ 3 - 0
modules/gdscript/gdscript_compiler.cpp

@@ -1665,6 +1665,7 @@ Error GDScriptCompiler::_parse_block(CodeGen &codegen, const GDScriptParser::Sui
 	Error err = OK;
 	GDScriptCodeGenerator *gen = codegen.generator;
 
+	gen->clean_temporaries();
 	codegen.start_block();
 
 	if (p_add_locals) {
@@ -1967,6 +1968,8 @@ Error GDScriptCompiler::_parse_block(CodeGen &codegen, const GDScriptParser::Sui
 				}
 			} break;
 		}
+
+		gen->clean_temporaries();
 	}
 
 	codegen.end_block();