From 8e3141c9c3edc5a58283a8f5cef854e7fe3c4e6c Mon Sep 17 00:00:00 2001 From: HolonProduction Date: Fri, 7 Feb 2025 10:32:05 +0100 Subject: [PATCH] GDScript: Cancel suspended functions when reloading a script --- modules/gdscript/gdscript.cpp | 37 +++++++++++-------- modules/gdscript/gdscript.h | 3 ++ modules/gdscript/gdscript_compiler.cpp | 2 + modules/gdscript/gdscript_function.h | 7 ++++ .../reload_suspended_function.notest.gd | 12 ++++++ .../errors/reload_suspended_function.out | 2 + ...reload_suspended_function_helper.notest.gd | 3 ++ 7 files changed, 51 insertions(+), 15 deletions(-) create mode 100644 modules/gdscript/tests/scripts/runtime/errors/reload_suspended_function.notest.gd create mode 100644 modules/gdscript/tests/scripts/runtime/errors/reload_suspended_function.out create mode 100644 modules/gdscript/tests/scripts/runtime/errors/reload_suspended_function_helper.notest.gd diff --git a/modules/gdscript/gdscript.cpp b/modules/gdscript/gdscript.cpp index 8ff0c63bd451..9eb283a77189 100644 --- a/modules/gdscript/gdscript.cpp +++ b/modules/gdscript/gdscript.cpp @@ -1625,6 +1625,27 @@ void GDScript::clear(ClearData *p_clear_data) { } } +void GDScript::cancel_pending_functions(bool warn) { + MutexLock lock(GDScriptLanguage::get_singleton()->mutex); + + while (SelfList *E = pending_func_states.first()) { + // Order matters since clearing the stack may already cause + // the GDScriptFunctionState to be destroyed and thus removed from the list. + pending_func_states.remove(E); + GDScriptFunctionState *state = E->self(); +#ifdef DEBUG_ENABLED + if (warn) { + WARN_PRINT("Canceling suspended execution of \"" + state->get_readable_function() + "\" due to a script reload."); + } +#endif + ObjectID state_id = state->get_instance_id(); + state->_clear_connections(); + if (ObjectDB::get_instance(state_id)) { + state->_clear_stack(); + } + } +} + GDScript::~GDScript() { if (destructing) { return; @@ -1640,21 +1661,7 @@ GDScript::~GDScript() { clear(); - { - MutexLock lock(GDScriptLanguage::get_singleton()->mutex); - - while (SelfList *E = pending_func_states.first()) { - // Order matters since clearing the stack may already cause - // the GDScriptFunctionState to be destroyed and thus removed from the list. - pending_func_states.remove(E); - GDScriptFunctionState *state = E->self(); - ObjectID state_id = state->get_instance_id(); - state->_clear_connections(); - if (ObjectDB::get_instance(state_id)) { - state->_clear_stack(); - } - } - } + cancel_pending_functions(false); { MutexLock lock(GDScriptLanguage::get_singleton()->mutex); diff --git a/modules/gdscript/gdscript.h b/modules/gdscript/gdscript.h index fa141d65ac5e..1052477dbba6 100644 --- a/modules/gdscript/gdscript.h +++ b/modules/gdscript/gdscript.h @@ -244,6 +244,9 @@ class GDScript : public Script { void clear(GDScript::ClearData *p_clear_data = nullptr); + // Cancels all functions of the script that are are waiting to be resumed after using await. + void cancel_pending_functions(bool warn); + virtual bool is_valid() const override { return valid; } virtual bool is_abstract() const override { return false; } // GDScript does not support abstract classes. diff --git a/modules/gdscript/gdscript_compiler.cpp b/modules/gdscript/gdscript_compiler.cpp index e52486e209e5..02d21636d920 100644 --- a/modules/gdscript/gdscript_compiler.cpp +++ b/modules/gdscript/gdscript_compiler.cpp @@ -2660,6 +2660,8 @@ Error GDScriptCompiler::_prepare_compilation(GDScript *p_script, const GDScriptP p_script->clearing = true; + p_script->cancel_pending_functions(true); + p_script->native = Ref(); p_script->base = Ref(); p_script->_base = nullptr; diff --git a/modules/gdscript/gdscript_function.h b/modules/gdscript/gdscript_function.h index 6433072b5560..4f073aa3de58 100644 --- a/modules/gdscript/gdscript_function.h +++ b/modules/gdscript/gdscript_function.h @@ -616,6 +616,13 @@ class GDScriptFunctionState : public RefCounted { bool is_valid(bool p_extended_check = false) const; Variant resume(const Variant &p_arg = Variant()); +#ifdef DEBUG_ENABLED + // Returns a human-readable representation of the function. + String get_readable_function() { + return state.function_name; + } +#endif + void _clear_stack(); void _clear_connections(); diff --git a/modules/gdscript/tests/scripts/runtime/errors/reload_suspended_function.notest.gd b/modules/gdscript/tests/scripts/runtime/errors/reload_suspended_function.notest.gd new file mode 100644 index 000000000000..7ac8f99446c7 --- /dev/null +++ b/modules/gdscript/tests/scripts/runtime/errors/reload_suspended_function.notest.gd @@ -0,0 +1,12 @@ +# TODO: This test is currently disabled since it triggers some complex memory leaks. Try enabling it again once GH-101830 is fixed. + +signal finished + +const scr: GDScript = preload("reload_suspended_function_helper.notest.gd") + +func test(): + @warning_ignore("UNSAFE_METHOD_ACCESS") + scr.test(self) + @warning_ignore("RETURN_VALUE_DISCARDED") + scr.reload(true) + finished.emit() diff --git a/modules/gdscript/tests/scripts/runtime/errors/reload_suspended_function.out b/modules/gdscript/tests/scripts/runtime/errors/reload_suspended_function.out new file mode 100644 index 000000000000..d86e986852fb --- /dev/null +++ b/modules/gdscript/tests/scripts/runtime/errors/reload_suspended_function.out @@ -0,0 +1,2 @@ +GDTEST_RUNTIME_ERROR +>> WARNING: Canceling suspended execution of "test" due to a script reload. diff --git a/modules/gdscript/tests/scripts/runtime/errors/reload_suspended_function_helper.notest.gd b/modules/gdscript/tests/scripts/runtime/errors/reload_suspended_function_helper.notest.gd new file mode 100644 index 000000000000..cc876f769c55 --- /dev/null +++ b/modules/gdscript/tests/scripts/runtime/errors/reload_suspended_function_helper.notest.gd @@ -0,0 +1,3 @@ +static func test(a): + await a.finished + pass