Merge pull request #113600 from HolonProduction/cache-fixup

GDScript: Ensure correct caching of cyclic references

Author: akien-mga

modules/gdscript/gdscript.cpp

@@ -3054,11 +3054,7 @@ Ref<GDScript> GDScriptLanguage::get_script_by_fully_qualified_name(const String
 Ref<Resource> ResourceFormatLoaderGDScript::load(const String &p_path, const String &p_original_path, Error *r_error, bool p_use_sub_threads, float *r_progress, CacheMode p_cache_mode) {
 	Error err;
 	bool ignoring = p_cache_mode == CACHE_MODE_IGNORE || p_cache_mode == CACHE_MODE_IGNORE_DEEP;
-	Ref<GDScript> scr = GDScriptCache::get_full_script_no_resource_cache(p_original_path, err, "", ignoring);
-	// Reset `path_cache` so that when resource loader uses `set_path()` later, the script gets added to the cache.
-	if (scr.is_valid()) {
-		scr->set_path_cache(String());
-	}
+	Ref<GDScript> scr = GDScriptCache::get_full_script(p_original_path, err, "", ignoring);
 
 	if (err && scr.is_valid()) {
 		// If !scr.is_valid(), the error was likely from scr->load_source_code(), which already generates an error.

modules/gdscript/gdscript_cache.cpp

@@ -339,17 +339,6 @@ Ref<GDScript> GDScriptCache::get_shallow_script(const String &p_path, Error &r_e
 }
 
 Ref<GDScript> GDScriptCache::get_full_script(const String &p_path, Error &r_error, const String &p_owner, bool p_update_from_disk) {
-	Ref<GDScript> uncached_script = get_full_script_no_resource_cache(p_path, r_error, p_owner, p_update_from_disk);
-
-	// Resources don't know whether they are cached, so using `set_path()` after `set_path_cache()` does not add the resource to the cache if the path is the same.
-	// We reset the cached path from `get_shallow_script()` so that the subsequent call to `set_path()` caches everything correctly.
-	uncached_script->set_path_cache(String());
-	uncached_script->set_path(p_path, true);
-
-	return uncached_script;
-}
-
-Ref<GDScript> GDScriptCache::get_full_script_no_resource_cache(const String &p_path, Error &r_error, const String &p_owner, bool p_update_from_disk) {
 	MutexLock lock(singleton->mutex);
 
 	if (!p_owner.is_empty() && p_path != p_owner) {
@@ -403,6 +392,12 @@ Ref<GDScript> GDScriptCache::get_full_script_no_resource_cache(const String &p_p
 	singleton->full_gdscript_cache[p_path] = script;
 	singleton->shallow_gdscript_cache.erase(p_path);
 
+	// Add the script to the resource cache. Usually ResourceLoader would take care of it, but cyclic references can break that sometimes so we do it ourselves.
+	// Resources don't know whether they are cached, so using `set_path()` after `set_path_cache()` does not add the resource to the cache if the path is the same.
+	// We reset the cached path from `get_shallow_script()` so that the subsequent call to `set_path()` caches everything correctly.
+	script->set_path_cache(String());
+	script->set_path(p_path, true);
+
 	return script;
 }
 

modules/gdscript/gdscript_cache.h

@@ -121,14 +121,6 @@ class GDScriptCache {
 	static String get_source_code(const String &p_path);
 	static Vector<uint8_t> get_binary_tokens(const String &p_path);
 	static Ref<GDScript> get_shallow_script(const String &p_path, Error &r_error, const String &p_owner = String());
-	/**
-	 * Returns a fully loaded GDScript using an already cached script if one exists.
-	 *
-	 * If a new script is created, the resource will only have its patch_cache set, so it won't be present in the ResourceCache.
-	 * Mismatches between GDScriptCache and ResourceCache might trigger complex issues so when using this method ensure
-	 * that the script is added to the resource cache or removed from the GDScript cache.
-	 */
-	static Ref<GDScript> get_full_script_no_resource_cache(const String &p_path, Error &r_error, const String &p_owner = String(), bool p_update_from_disk = false);
 	/**
 	 * Returns a fully loaded GDScript using an already cached script if one exists.
 	 *