Merge pull request #68281 from maximkulkin/resource-circular-references

Fix crash when saving resources with circular references
This commit is contained in:
Yuri Sizov 2023-07-14 21:27:58 +02:00
commit 4dc26bffeb
3 changed files with 57 additions and 3 deletions

View file

@ -1960,6 +1960,8 @@ void ResourceFormatSaverBinaryInstance::_find_resources(const Variant &p_variant
return;
}
resource_set.insert(res);
List<PropertyInfo> property_list;
res->get_property_list(&property_list);
@ -1983,7 +1985,6 @@ void ResourceFormatSaverBinaryInstance::_find_resources(const Variant &p_variant
}
}
resource_set.insert(res);
saved_resources.push_back(res);
} break;

View file

@ -1877,6 +1877,8 @@ void ResourceFormatSaverTextInstance::_find_resources(const Variant &p_variant,
return;
}
resource_set.insert(res);
List<PropertyInfo> property_list;
res->get_property_list(&property_list);
@ -1908,8 +1910,7 @@ void ResourceFormatSaverTextInstance::_find_resources(const Variant &p_variant,
I = I->next();
}
resource_set.insert(res); //saved after, so the children it needs are available when loaded
saved_resources.push_back(res);
saved_resources.push_back(res); // Saved after, so the children it needs are available when loaded
} break;
case Variant::ARRAY: {

View file

@ -109,6 +109,58 @@ TEST_CASE("[Resource] Saving and loading") {
loaded_child_resource_text->get_name() == "I'm a child resource",
"The loaded child resource name should be equal to the expected value.");
}
TEST_CASE("[Resource] Breaking circular references on save") {
Ref<Resource> resource_a = memnew(Resource);
resource_a->set_name("A");
Ref<Resource> resource_b = memnew(Resource);
resource_b->set_name("B");
Ref<Resource> resource_c = memnew(Resource);
resource_c->set_name("C");
resource_a->set_meta("next", resource_b);
resource_b->set_meta("next", resource_c);
resource_c->set_meta("next", resource_b);
const String save_path_binary = OS::get_singleton()->get_cache_path().path_join("resource.res");
const String save_path_text = OS::get_singleton()->get_cache_path().path_join("resource.tres");
ResourceSaver::save(resource_a, save_path_binary);
ResourceSaver::save(resource_a, save_path_text);
const Ref<Resource> &loaded_resource_a_binary = ResourceLoader::load(save_path_binary);
CHECK_MESSAGE(
loaded_resource_a_binary->get_name() == "A",
"The loaded resource name should be equal to the expected value.");
const Ref<Resource> &loaded_resource_b_binary = loaded_resource_a_binary->get_meta("next");
CHECK_MESSAGE(
loaded_resource_b_binary->get_name() == "B",
"The loaded child resource name should be equal to the expected value.");
const Ref<Resource> &loaded_resource_c_binary = loaded_resource_b_binary->get_meta("next");
CHECK_MESSAGE(
loaded_resource_c_binary->get_name() == "C",
"The loaded child resource name should be equal to the expected value.");
CHECK_MESSAGE(
!loaded_resource_c_binary->get_meta("next"),
"The loaded child resource circular reference should be NULL.");
const Ref<Resource> &loaded_resource_a_text = ResourceLoader::load(save_path_text);
CHECK_MESSAGE(
loaded_resource_a_text->get_name() == "A",
"The loaded resource name should be equal to the expected value.");
const Ref<Resource> &loaded_resource_b_text = loaded_resource_a_text->get_meta("next");
CHECK_MESSAGE(
loaded_resource_b_text->get_name() == "B",
"The loaded child resource name should be equal to the expected value.");
const Ref<Resource> &loaded_resource_c_text = loaded_resource_b_text->get_meta("next");
CHECK_MESSAGE(
loaded_resource_c_text->get_name() == "C",
"The loaded child resource name should be equal to the expected value.");
CHECK_MESSAGE(
!loaded_resource_c_text->get_meta("next"),
"The loaded child resource circular reference should be NULL.");
// Break circular reference to avoid memory leak
resource_c->remove_meta("next");
}
} // namespace TestResource
#endif // TEST_RESOURCE_H