[Core] Disconnect one-shot signals before calling callbacks

This prevents infinite recursion with one-shot connections emitting
themselves
This commit is contained in:
A Thousand Ships 2024-03-13 17:57:22 +01:00
parent fe01776f05
commit db455e5bee
No known key found for this signature in database
GPG key ID: 2033189A662F8BD7
5 changed files with 76 additions and 41 deletions

View file

@ -1100,11 +1100,6 @@ bool Object::_has_user_signal(const StringName &p_name) const {
return signal_map[p_name].user.name.length() > 0;
}
struct _ObjectSignalDisconnectData {
StringName signal;
Callable callable;
};
Error Object::_emit_signal(const Variant **p_args, int p_argcount, Callable::CallError &r_error) {
if (unlikely(p_argcount < 1)) {
r_error.error = Callable::CallError::CALL_ERROR_TOO_FEW_ARGUMENTS;
@ -1153,26 +1148,43 @@ Error Object::emit_signalp(const StringName &p_name, const Variant **p_args, int
// 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;
// Ensure that disconnecting the signal or even deleting the object
// will not affect the signal calling.
LocalVector<Connection> slot_conns;
slot_conns.resize(s->slot_map.size());
{
uint32_t idx = 0;
for (const KeyValue<Callable, SignalData::Slot> &slot_kv : s->slot_map) {
slot_conns[idx++] = slot_kv.value.conn;
Callable *slot_callables = (Callable *)alloca(sizeof(Callable) * s->slot_map.size());
uint32_t *slot_flags = (uint32_t *)alloca(sizeof(uint32_t) * s->slot_map.size());
uint32_t slot_count = 0;
for (const KeyValue<Callable, SignalData::Slot> &slot_kv : s->slot_map) {
memnew_placement(&slot_callables[slot_count], Callable(slot_kv.value.conn.callable));
slot_flags[slot_count] = slot_kv.value.conn.flags;
++slot_count;
}
DEV_ASSERT(slot_count == s->slot_map.size());
// Disconnect all one-shot connections before emitting to prevent recursion.
for (uint32_t i = 0; i < slot_count; ++i) {
bool disconnect = slot_flags[i] & CONNECT_ONE_SHOT;
#ifdef TOOLS_ENABLED
if (disconnect && (slot_flags[i] & CONNECT_PERSIST) && Engine::get_singleton()->is_editor_hint()) {
// This signal was connected from the editor, and is being edited. Just don't disconnect for now.
disconnect = false;
}
#endif
if (disconnect) {
_disconnect(p_name, slot_callables[i]);
}
DEV_ASSERT(idx == s->slot_map.size());
}
OBJ_DEBUG_LOCK
Error err = OK;
for (const Connection &c : slot_conns) {
if (!c.callable.is_valid()) {
for (uint32_t i = 0; i < slot_count; ++i) {
const Callable &callable = slot_callables[i];
const uint32_t &flags = slot_flags[i];
if (!callable.is_valid()) {
// Target might have been deleted during signal callback, this is expected and OK.
continue;
}
@ -1180,51 +1192,34 @@ Error Object::emit_signalp(const StringName &p_name, const Variant **p_args, int
const Variant **args = p_args;
int argc = p_argcount;
if (c.flags & CONNECT_DEFERRED) {
MessageQueue::get_singleton()->push_callablep(c.callable, args, argc, true);
if (flags & CONNECT_DEFERRED) {
MessageQueue::get_singleton()->push_callablep(callable, args, argc, true);
} else {
Callable::CallError ce;
_emitting = true;
Variant ret;
c.callable.callp(args, argc, ret, ce);
callable.callp(args, argc, ret, ce);
_emitting = false;
if (ce.error != Callable::CallError::CALL_OK) {
#ifdef DEBUG_ENABLED
if (c.flags & CONNECT_PERSIST && Engine::get_singleton()->is_editor_hint() && (script.is_null() || !Ref<Script>(script)->is_tool())) {
if (flags & CONNECT_PERSIST && Engine::get_singleton()->is_editor_hint() && (script.is_null() || !Ref<Script>(script)->is_tool())) {
continue;
}
#endif
Object *target = c.callable.get_object();
Object *target = callable.get_object();
if (ce.error == Callable::CallError::CALL_ERROR_INVALID_METHOD && target && !ClassDB::class_exists(target->get_class_name())) {
//most likely object is not initialized yet, do not throw error.
} else {
ERR_PRINT("Error calling from signal '" + String(p_name) + "' to callable: " + Variant::get_callable_error_text(c.callable, args, argc, ce) + ".");
ERR_PRINT("Error calling from signal '" + String(p_name) + "' to callable: " + Variant::get_callable_error_text(callable, args, argc, ce) + ".");
err = ERR_METHOD_NOT_FOUND;
}
}
}
bool disconnect = c.flags & CONNECT_ONE_SHOT;
#ifdef TOOLS_ENABLED
if (disconnect && (c.flags & CONNECT_PERSIST) && Engine::get_singleton()->is_editor_hint()) {
//this signal was connected from the editor, and is being edited. just don't disconnect for now
disconnect = false;
}
#endif
if (disconnect) {
_ObjectSignalDisconnectData dd;
dd.signal = p_name;
dd.callable = c.callable;
disconnect_data.push_back(dd);
}
}
while (!disconnect_data.is_empty()) {
const _ObjectSignalDisconnectData &dd = disconnect_data.front()->get();
_disconnect(dd.signal, dd.callable);
disconnect_data.pop_front();
for (uint32_t i = 0; i < slot_count; ++i) {
slot_callables[i].~Callable();
}
return err;

View file

@ -0,0 +1,12 @@
# https://github.com/godotengine/godot/issues/89439
extends Node
signal my_signal
func async_func():
await my_signal
my_signal.emit()
func test():
async_func()
my_signal.emit()

View file

@ -0,0 +1 @@
GDTEST_OK

View file

@ -0,0 +1,22 @@
# https://github.com/godotengine/godot/issues/89439
signal my_signal
func foo():
print("Foo")
my_signal.emit()
func bar():
print("Bar")
func baz():
print("Baz")
func test():
@warning_ignore("return_value_discarded")
my_signal.connect(foo, CONNECT_ONE_SHOT)
@warning_ignore("return_value_discarded")
my_signal.connect(bar, CONNECT_ONE_SHOT)
@warning_ignore("return_value_discarded")
my_signal.connect(baz)
my_signal.emit()

View file

@ -0,0 +1,5 @@
GDTEST_OK
Foo
Baz
Bar
Baz