From 8bce5114dae38f96f562c3fe85c8a2e9129a2638 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Tue, 30 Jul 2013 23:20:38 +0200 Subject: [PATCH] Deleting all resource loaders before unloading any resource data. Previously was done per type (i.e. for each type delete loader and then the data), wasn't usable when the loader stored different resource type (which is the only case where it is useful). --- src/AbstractResourceLoader.h | 7 ++++- src/ResourceManager.h | 24 +++++++++++--- src/Test/ResourceManagerTest.cpp | 54 ++++++++++++++++++++------------ 3 files changed, 59 insertions(+), 26 deletions(-) diff --git a/src/AbstractResourceLoader.h b/src/AbstractResourceLoader.h index 64fcbc8c2..994633a5e 100644 --- a/src/AbstractResourceLoader.h +++ b/src/AbstractResourceLoader.h @@ -78,7 +78,12 @@ class MeshResourceLoader: public AbstractResourceLoader { }; @endcode -You can then add it to resource manager instance like this: +You can then add it to resource manager instance like this. Note that the +manager automatically deletes the all loaders on destruction before unloading +all resources. It allows you to use resources in the loader itself without +having to delete the loader explicitly to ensure proper resource unloading. In +the following code, however, the loader destroys itself (and removes itself +from the manager) before the manager is destroyed. @code MyResourceManager manager; MeshResourceLoader loader; diff --git a/src/ResourceManager.h b/src/ResourceManager.h index 6038d32dd..1c800f0c6 100644 --- a/src/ResourceManager.h +++ b/src/ResourceManager.h @@ -125,6 +125,8 @@ template class ResourceManagerData { AbstractResourceLoader* loader() { return _loader; } const AbstractResourceLoader* loader() const { return _loader; } + void freeLoader(); + void setLoader(AbstractResourceLoader* loader); protected: @@ -367,6 +369,8 @@ template class ResourceManager: private Implementation::Resource * @return Pointer to self (for method chaining) * * See AbstractResourceLoader documentation for more information. + * @attention The loader is deleted on destruction before unloading + * all resources. */ template ResourceManager* setLoader(AbstractResourceLoader* loader) { this->Implementation::ResourceManagerData::setLoader(loader); @@ -380,6 +384,12 @@ template class ResourceManager: private Implementation::Resource } template void freeInternal() const {} + template void freeLoaders() { + Implementation::ResourceManagerData::freeLoader(); + freeLoaders(); + } + template void freeLoaders() const {} + static ResourceManager*& internalInstance(); }; @@ -393,12 +403,8 @@ template ResourceManager*& ResourceManager:: namespace Implementation { template ResourceManagerData::~ResourceManagerData() { + /* Loaders are already deleted via freeLoaders() from ResourceManager */ delete _fallback; - - if(_loader) { - _loader->manager = nullptr; - delete _loader; - } } template std::size_t ResourceManagerData::referenceCount(const ResourceKey key) const { @@ -499,6 +505,13 @@ template void ResourceManagerData::setLoader(AbstractResourceLoader< if((_loader = loader)) _loader->manager = this; } +template void ResourceManagerData::freeLoader() { + if(!_loader) return; + + _loader->manager = nullptr; + delete _loader; +} + template void ResourceManagerData::decrementReferenceCount(ResourceKey key) { auto it = _data.find(key); @@ -546,6 +559,7 @@ template ResourceManager::ResourceManager() { } template ResourceManager::~ResourceManager() { + freeLoaders(); CORRADE_INTERNAL_ASSERT(internalInstance() == this); internalInstance() = nullptr; } diff --git a/src/Test/ResourceManagerTest.cpp b/src/Test/ResourceManagerTest.cpp index 11e9c8f8c..ccfe57b8f 100644 --- a/src/Test/ResourceManagerTest.cpp +++ b/src/Test/ResourceManagerTest.cpp @@ -229,6 +229,7 @@ void ResourceManagerTest::manualPolicy() { void ResourceManagerTest::loader() { class IntResourceLoader: public AbstractResourceLoader { public: + IntResourceLoader(): resource(ResourceManager::instance()->get("data")) {} void load() { set("hello", new Int(773), ResourceDataState::Final, ResourcePolicy::Resident); @@ -242,32 +243,45 @@ void ResourceManagerTest::loader() { if(key == ResourceKey("hello")) return "hello"; return ""; } + + /* To verify that the loader is destroyed before unloading + _all types of_ resources */ + Resource resource; }; auto rm = new ResourceManager; auto loader = new IntResourceLoader; rm->setLoader(loader); - Resource data = rm->get("data"); - Resource hello = rm->get("hello"); - Resource world = rm->get("world"); - CORRADE_COMPARE(data.state(), ResourceState::NotLoaded); - CORRADE_COMPARE(hello.state(), ResourceState::Loading); - CORRADE_COMPARE(world.state(), ResourceState::Loading); - - CORRADE_COMPARE(loader->requestedCount(), 2); - CORRADE_COMPARE(loader->loadedCount(), 0); - CORRADE_COMPARE(loader->notFoundCount(), 0); - CORRADE_COMPARE(loader->name(ResourceKey("hello")), "hello"); - - loader->load(); - CORRADE_COMPARE(hello.state(), ResourceState::Final); - CORRADE_COMPARE(*hello, 773); - CORRADE_COMPARE(world.state(), ResourceState::NotFound); - - CORRADE_COMPARE(loader->requestedCount(), 2); - CORRADE_COMPARE(loader->loadedCount(), 1); - CORRADE_COMPARE(loader->notFoundCount(), 1); + { + Resource data = rm->get("data"); + Resource hello = rm->get("hello"); + Resource world = rm->get("world"); + CORRADE_COMPARE(data.state(), ResourceState::NotLoaded); + CORRADE_COMPARE(hello.state(), ResourceState::Loading); + CORRADE_COMPARE(world.state(), ResourceState::Loading); + + CORRADE_COMPARE(loader->requestedCount(), 2); + CORRADE_COMPARE(loader->loadedCount(), 0); + CORRADE_COMPARE(loader->notFoundCount(), 0); + CORRADE_COMPARE(loader->name(ResourceKey("hello")), "hello"); + + loader->load(); + CORRADE_COMPARE(hello.state(), ResourceState::Final); + CORRADE_COMPARE(*hello, 773); + CORRADE_COMPARE(world.state(), ResourceState::NotFound); + + CORRADE_COMPARE(loader->requestedCount(), 2); + CORRADE_COMPARE(loader->loadedCount(), 1); + CORRADE_COMPARE(loader->notFoundCount(), 1); + + /* Verify that the loader is deleted at proper time */ + rm->set("data", new Data); + CORRADE_COMPARE(Data::count, 1); + } + + delete rm; + CORRADE_COMPARE(Data::count, 0); } }}