Avoid cyclic resource loading of any type, fixes #22673

core/io/resource_loader.cpp

@@ -53,6 +53,12 @@ Error ResourceInteractiveLoader::wait() {
 	return err;
 }
 
+ResourceInteractiveLoader::~ResourceInteractiveLoader() {
+	if (path_loading != String()) {
+		ResourceLoader::_remove_from_loading_map(path_loading);
+	}
+}
+
 bool ResourceFormatLoader::recognize_path(const String &p_path, const String &p_for_type) const {
 
 	String extension = p_path.get_extension();
@@ -280,6 +286,39 @@ RES ResourceLoader::_load(const String &p_path, const String &p_original_path, c
 	return RES();
 }
 
+bool ResourceLoader::_add_to_loading_map(const String &p_path) {
+
+	bool success;
+	if (loading_map_mutex) {
+		loading_map_mutex->lock();
+	}
+
+	if (loading_map.has(p_path)) {
+		success = false;
+	} else {
+		loading_map[p_path] = true;
+		success = true;
+	}
+
+	if (loading_map_mutex) {
+		loading_map_mutex->unlock();
+	}
+
+	return success;
+}
+
+void ResourceLoader::_remove_from_loading_map(const String &p_path) {
+	if (loading_map_mutex) {
+		loading_map_mutex->lock();
+	}
+
+	loading_map.erase(p_path);
+
+	if (loading_map_mutex) {
+		loading_map_mutex->unlock();
+	}
+}
+
 RES ResourceLoader::load(const String &p_path, const String &p_type_hint, bool p_no_cache, Error *r_error) {
 
 	if (r_error)
@@ -292,6 +331,15 @@ RES ResourceLoader::load(const String &p_path, const String &p_type_hint, bool p
 		local_path = ProjectSettings::get_singleton()->localize_path(p_path);
 
 	if (!p_no_cache) {
+
+		{
+			bool success = _add_to_loading_map(local_path);
+			if (!success) {
+				ERR_EXPLAIN("Resource: '" + local_path + "' is already being loaded. Cyclic reference?");
+				ERR_FAIL_V(RES());
+			}
+		}
+
 		//lock first if possible
 		if (ResourceCache::lock) {
 			ResourceCache::lock->read_lock();
@@ -310,7 +358,7 @@ RES ResourceLoader::load(const String &p_path, const String &p_type_hint, bool p
 				if (ResourceCache::lock) {
 					ResourceCache::lock->read_unlock();
 				}
-				print_verbose("Loading resource: " + local_path + " (cached)");
+				_remove_from_loading_map(local_path);
 				return res;
 			}
 		}
@@ -322,12 +370,21 @@ RES ResourceLoader::load(const String &p_path, const String &p_type_hint, bool p
 	bool xl_remapped = false;
 	String path = _path_remap(local_path, &xl_remapped);
 
-	ERR_FAIL_COND_V(path == "", RES());
+	if (path == "") {
+		if (!p_no_cache) {
+			_remove_from_loading_map(local_path);
+		}
+		ERR_EXPLAIN("Remapping '" + local_path + "'failed.");
+		ERR_FAIL_V(RES());
+	}
 
 	print_verbose("Loading resource: " + path);
 	RES res = _load(path, local_path, p_type_hint, p_no_cache, r_error);
 
 	if (res.is_null()) {
+		if (!p_no_cache) {
+			_remove_from_loading_map(local_path);
+		}
 		return RES();
 	}
 	if (!p_no_cache)
@@ -346,6 +403,10 @@ RES ResourceLoader::load(const String &p_path, const String &p_type_hint, bool p
 	}
 #endif
 
+	if (!p_no_cache) {
+		_remove_from_loading_map(local_path);
+	}
+
 	if (_loaded_callback) {
 		_loaded_callback(res, p_path);
 	}
@@ -394,19 +455,36 @@ Ref<ResourceInteractiveLoader> ResourceLoader::load_interactive(const String &p_
 	else
 		local_path = ProjectSettings::get_singleton()->localize_path(p_path);
 
-	if (!p_no_cache && ResourceCache::has(local_path)) {
+	if (!p_no_cache) {
 
-		print_verbose("Loading resource: " + local_path + " (cached)");
-		Ref<Resource> res_cached = ResourceCache::get(local_path);
-		Ref<ResourceInteractiveLoaderDefault> ril = Ref<ResourceInteractiveLoaderDefault>(memnew(ResourceInteractiveLoaderDefault));
+		bool success = _add_to_loading_map(local_path);
+		if (!success) {
+			ERR_EXPLAIN("Resource: '" + local_path + "' is already being loaded. Cyclic reference?");
+			ERR_FAIL_V(RES());
+		}
 
-		ril->resource = res_cached;
-		return ril;
+		if (ResourceCache::has(local_path)) {
+
+			print_verbose("Loading resource: " + local_path + " (cached)");
+			Ref<Resource> res_cached = ResourceCache::get(local_path);
+			Ref<ResourceInteractiveLoaderDefault> ril = Ref<ResourceInteractiveLoaderDefault>(memnew(ResourceInteractiveLoaderDefault));
+
+			ril->resource = res_cached;
+			ril->path_loading = local_path;
+			return ril;
+		}
 	}
 
 	bool xl_remapped = false;
 	String path = _path_remap(local_path, &xl_remapped);
-	ERR_FAIL_COND_V(path == "", Ref<ResourceInteractiveLoader>());
+	if (path == "") {
+		if (!p_no_cache) {
+			_remove_from_loading_map(local_path);
+		}
+		ERR_EXPLAIN("Remapping '" + local_path + "'failed.");
+		ERR_FAIL_V(RES());
+	}
+
 	print_verbose("Loading resource: " + path);
 
 	bool found = false;
@@ -418,14 +496,21 @@ Ref<ResourceInteractiveLoader> ResourceLoader::load_interactive(const String &p_
 		Ref<ResourceInteractiveLoader> ril = loader[i]->load_interactive(path, local_path, r_error);
 		if (ril.is_null())
 			continue;
-		if (!p_no_cache)
+		if (!p_no_cache) {
 			ril->set_local_path(local_path);
+			ril->path_loading = local_path;
+		}
+
 		if (xl_remapped)
 			ril->set_translation_remapped(true);
 
 		return ril;
 	}
 
+	if (!p_no_cache) {
+		_remove_from_loading_map(local_path);
+	}
+
 	if (found) {
 		ERR_EXPLAIN("Failed loading resource: " + path);
 	} else {
@@ -833,6 +918,27 @@ void ResourceLoader::remove_custom_loaders() {
 	}
 }
 
+Mutex *ResourceLoader::loading_map_mutex = NULL;
+HashMap<String, int> ResourceLoader::loading_map;
+
+void ResourceLoader::initialize() {
+#ifndef NO_THREADS
+	loading_map_mutex = Mutex::create();
+#endif
+}
+
+void ResourceLoader::finalize() {
+#ifndef NO_THREADS
+	const String *K = NULL;
+	while ((K = loading_map.next(K))) {
+		ERR_PRINTS("Exited while resource is being loaded: " + *K);
+	}
+	loading_map.clear();
+	memdelete(loading_map_mutex);
+	loading_map_mutex = NULL;
+#endif
+}
+
 ResourceLoadErrorNotify ResourceLoader::err_notify = NULL;
 void *ResourceLoader::err_notify_ud = NULL;
 

core/io/resource_loader.h

@@ -40,6 +40,8 @@
 class ResourceInteractiveLoader : public Reference {
 
 	GDCLASS(ResourceInteractiveLoader, Reference);
+	friend class ResourceLoader;
+	String path_loading;
 
 protected:
 	static void _bind_methods();
@@ -54,6 +56,7 @@ public:
 	virtual Error wait();
 
 	ResourceInteractiveLoader() {}
+	~ResourceInteractiveLoader();
 };
 
 class ResourceFormatLoader : public Reference {
@@ -110,12 +113,18 @@ class ResourceLoader {
 	static SelfList<Resource>::List remapped_list;
 
 	friend class ResourceFormatImporter;
+	friend class ResourceInteractiveLoader;
 	//internal load function
 	static RES _load(const String &p_path, const String &p_original_path, const String &p_type_hint, bool p_no_cache, Error *r_error);
 
 	static ResourceLoadedCallback _loaded_callback;
 
 	static Ref<ResourceFormatLoader> _find_custom_resource_format_loader(String path);
+	static Mutex *loading_map_mutex;
+	static HashMap<String, int> loading_map;
+
+	static bool _add_to_loading_map(const String &p_path);
+	static void _remove_from_loading_map(const String &p_path);
 
 public:
 	static Ref<ResourceInteractiveLoader> load_interactive(const String &p_path, const String &p_type_hint = "", bool p_no_cache = false, Error *r_error = NULL);
@@ -170,6 +179,9 @@ public:
 	static void remove_custom_resource_format_loader(String script_path);
 	static void add_custom_loaders();
 	static void remove_custom_loaders();
+
+	static void initialize();
+	static void finalize();
 };
 
 #endif

core/register_core_types.cpp

@@ -99,6 +99,7 @@ void register_core_types() {
 	_global_mutex = Mutex::create();
 
 	StringName::setup();
+	ResourceLoader::initialize();
 
 	register_global_constants();
 	register_variant_methods();
@@ -269,6 +270,8 @@ void unregister_core_types() {
 	if (ip)
 		memdelete(ip);
 
+	ResourceLoader::finalize();
+
 	ObjectDB::cleanup();
 
 	unregister_variant_methods();