From a25846507df8ccc6ab7846878919a483b7047e11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20J=2E=20Est=C3=A9banez?= Date: Tue, 19 Aug 2025 13:35:37 +0200 Subject: [PATCH] Fix regression in mechanism to hold objects while emitting --- core/object/object.cpp | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/core/object/object.cpp b/core/object/object.cpp index f553872060..265c2f120d 100644 --- a/core/object/object.cpp +++ b/core/object/object.cpp @@ -1235,10 +1235,6 @@ 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 rc = Ref(Object::cast_to(this)); - if (s->slot_map.size() > MAX_SLOTS_ON_STACK) { slot_callables = (Callable *)memalloc(sizeof(Callable) * s->slot_map.size()); slot_flags = (uint32_t *)memalloc(sizeof(uint32_t) * s->slot_map.size()); @@ -1271,6 +1267,13 @@ Error Object::emit_signalp(const StringName &p_name, const Variant **p_args, int OBJ_DEBUG_LOCK + // If this is a ref-counted object, prevent it from being destroyed during signal + // emission, which is needed in certain edge cases; e.g., GH-73889 and GH-109471. + // Moreover, since signals can be emitted from constructors (classic example being + // notify_property_list_changed), we must be careful not to do the ref init ourselves, + // which would lead to the object being destroyed at the end of this function. + bool pending_unref = Object::cast_to(this) ? ((RefCounted *)this)->reference() : false; + Error err = OK; for (uint32_t i = 0; i < slot_count; ++i) { @@ -1320,6 +1323,15 @@ Error Object::emit_signalp(const StringName &p_name, const Variant **p_args, int memfree(slot_flags); } + if (pending_unref) { + // We have to do the same Ref would do. We can't just use Ref + // because it would do the init ref logic, which is something this function + // shouldn't do, as explained above. + if (((RefCounted *)this)->unreference()) { + memdelete(this); + } + } + return err; }