Skip to content

Commit 56bdef9

Browse files
committedNov 8, 2024
Fix deadlocks related to ClassDB queries about global classes
`ClassDB::can_instantiate()` and other reflection methods deadlock if the type is an script global class, when such script indirectly uses a not-yet-registered class. The reason is the `ClassDB` read lock is still held when invoking the `ResourceLoader` to load the class script, which may in turn need to lock for writing (for the class registration). In particular, this happens with some types related to animation tree, that aren't registered at engine startup, but can happen with others, especially ones from the user. Registration statements are also added for the animation-related types that were lacking them.
1 parent 1bffd6c commit 56bdef9

File tree

2 files changed

+64
-43
lines changed

2 files changed

+64
-43
lines changed
 

‎core/object/class_db.cpp

+61-43
Original file line numberDiff line numberDiff line change
@@ -750,69 +750,87 @@ void ClassDB::set_object_extension_instance(Object *p_object, const StringName &
750750
}
751751

752752
bool ClassDB::can_instantiate(const StringName &p_class) {
753-
OBJTYPE_RLOCK;
753+
String script_path;
754+
{
755+
OBJTYPE_RLOCK;
754756

755-
ClassInfo *ti = classes.getptr(p_class);
756-
if (!ti) {
757-
if (!ScriptServer::is_global_class(p_class)) {
758-
ERR_FAIL_V_MSG(false, vformat("Cannot get class '%s'.", String(p_class)));
757+
ClassInfo *ti = classes.getptr(p_class);
758+
if (!ti) {
759+
if (!ScriptServer::is_global_class(p_class)) {
760+
ERR_FAIL_V_MSG(false, vformat("Cannot get class '%s'.", String(p_class)));
761+
}
762+
script_path = ScriptServer::get_global_class_path(p_class);
763+
goto use_script; // Open the lock for resource loading.
759764
}
760-
String path = ScriptServer::get_global_class_path(p_class);
761-
Ref<Script> scr = ResourceLoader::load(path);
762-
return scr.is_valid() && scr->is_valid() && !scr->is_abstract();
763-
}
764765
#ifdef TOOLS_ENABLED
765-
if ((ti->api == API_EDITOR || ti->api == API_EDITOR_EXTENSION) && !Engine::get_singleton()->is_editor_hint()) {
766-
return false;
767-
}
766+
if ((ti->api == API_EDITOR || ti->api == API_EDITOR_EXTENSION) && !Engine::get_singleton()->is_editor_hint()) {
767+
return false;
768+
}
768769
#endif
769-
return _can_instantiate(ti);
770+
return _can_instantiate(ti);
771+
}
772+
773+
use_script:
774+
Ref<Script> scr = ResourceLoader::load(script_path);
775+
return scr.is_valid() && scr->is_valid() && !scr->is_abstract();
770776
}
771777

772778
bool ClassDB::is_abstract(const StringName &p_class) {
773-
OBJTYPE_RLOCK;
779+
String script_path;
780+
{
781+
OBJTYPE_RLOCK;
774782

775-
ClassInfo *ti = classes.getptr(p_class);
776-
if (!ti) {
777-
if (!ScriptServer::is_global_class(p_class)) {
778-
ERR_FAIL_V_MSG(false, vformat("Cannot get class '%s'.", String(p_class)));
783+
ClassInfo *ti = classes.getptr(p_class);
784+
if (!ti) {
785+
if (!ScriptServer::is_global_class(p_class)) {
786+
ERR_FAIL_V_MSG(false, vformat("Cannot get class '%s'.", String(p_class)));
787+
}
788+
script_path = ScriptServer::get_global_class_path(p_class);
789+
goto use_script; // Open the lock for resource loading.
779790
}
780-
String path = ScriptServer::get_global_class_path(p_class);
781-
Ref<Script> scr = ResourceLoader::load(path);
782-
return scr.is_valid() && scr->is_valid() && scr->is_abstract();
783-
}
784791

785-
if (ti->creation_func != nullptr) {
786-
return false;
787-
}
788-
if (!ti->gdextension) {
789-
return true;
790-
}
792+
if (ti->creation_func != nullptr) {
793+
return false;
794+
}
795+
if (!ti->gdextension) {
796+
return true;
797+
}
791798
#ifndef DISABLE_DEPRECATED
792-
return ti->gdextension->create_instance2 == nullptr && ti->gdextension->create_instance == nullptr;
799+
return ti->gdextension->create_instance2 == nullptr && ti->gdextension->create_instance == nullptr;
793800
#else
794-
return ti->gdextension->create_instance2 == nullptr;
801+
return ti->gdextension->create_instance2 == nullptr;
795802
#endif // DISABLE_DEPRECATED
803+
}
804+
805+
use_script:
806+
Ref<Script> scr = ResourceLoader::load(script_path);
807+
return scr.is_valid() && scr->is_valid() && scr->is_abstract();
796808
}
797809

798810
bool ClassDB::is_virtual(const StringName &p_class) {
799-
OBJTYPE_RLOCK;
811+
String script_path;
812+
{
813+
OBJTYPE_RLOCK;
800814

801-
ClassInfo *ti = classes.getptr(p_class);
802-
if (!ti) {
803-
if (!ScriptServer::is_global_class(p_class)) {
804-
ERR_FAIL_V_MSG(false, vformat("Cannot get class '%s'.", String(p_class)));
815+
ClassInfo *ti = classes.getptr(p_class);
816+
if (!ti) {
817+
if (!ScriptServer::is_global_class(p_class)) {
818+
ERR_FAIL_V_MSG(false, vformat("Cannot get class '%s'.", String(p_class)));
819+
}
820+
script_path = ScriptServer::get_global_class_path(p_class);
821+
goto use_script; // Open the lock for resource loading.
805822
}
806-
String path = ScriptServer::get_global_class_path(p_class);
807-
Ref<Script> scr = ResourceLoader::load(path);
808-
return scr.is_valid() && scr->is_valid() && scr->is_abstract();
809-
}
810823
#ifdef TOOLS_ENABLED
811-
if ((ti->api == API_EDITOR || ti->api == API_EDITOR_EXTENSION) && !Engine::get_singleton()->is_editor_hint()) {
812-
return false;
813-
}
824+
if ((ti->api == API_EDITOR || ti->api == API_EDITOR_EXTENSION) && !Engine::get_singleton()->is_editor_hint()) {
825+
return false;
826+
}
814827
#endif
815-
return (_can_instantiate(ti) && ti->is_virtual);
828+
return (_can_instantiate(ti) && ti->is_virtual);
829+
}
830+
831+
use_script:
832+
Ref<Script> scr = ResourceLoader::load(script_path);
833+
return scr.is_valid() && scr->is_valid() && scr->is_abstract();
816834
}
817835

818836
void ClassDB::_add_class2(const StringName &p_class, const StringName &p_inherits) {

‎scene/register_scene_types.cpp

+3
Original file line numberDiff line numberDiff line change
@@ -512,6 +512,9 @@ void register_scene_types() {
512512
GDREGISTER_CLASS(AnimationNodeStateMachine);
513513
GDREGISTER_CLASS(AnimationNodeStateMachinePlayback);
514514

515+
GDREGISTER_INTERNAL_CLASS(AnimationNodeStartState);
516+
GDREGISTER_INTERNAL_CLASS(AnimationNodeEndState);
517+
515518
GDREGISTER_CLASS(AnimationNodeSync);
516519
GDREGISTER_CLASS(AnimationNodeStateMachineTransition);
517520
GDREGISTER_CLASS(AnimationNodeOutput);

0 commit comments

Comments
 (0)