[SOLVED] Loading resource after ReleaseRef() segfaults

In a previous question of mine (see this thread) I was told it was possible to call cache->GetResource(“thingy.xml”)->ReleaseRef();. It appears if you ReleaseRef() a resource after loading and using it, loading another resource of the same type will segfault.

void App::Start() { ResourceCache* cache = GetSubsystem<ResourceCache>(); for(int i = 0; i < 10; i++) { XMLFile* f = cache->GetResource<XMLFile>("UI/DefaultStyle.xml"); f->GetRoot(); // segfaults for i=1, works at i=0 f->ReleaseRef(); } }

Another, simpler example even:

void App::Start() { ResourceCache* cache = GetSubsystem<ResourceCache>(); for(int i = 0; i < 10; i++) { XMLFile* f = cache->GetResource<XMLFile>("UI/DefaultStyle.xml"); // segfaults for i=1, works at i=0 f->ReleaseRef(); } }

In the first case, GetRoot() fails because document_ is NULL:

[code]XMLElement XMLFile::GetRoot(const String& name)
{
pugi::xml_node root = document_->first_child(); // document_ is NULL.

// SNIP

}[/code]

In the second case, GetResource() fails because refCount_ is pointing to garbage:

[code]void RefCounted::ReleaseRef()
{
assert(refCount_->refs_ > 0); // refCount_ is 0x642f74656d6f002f (this is a garbage value)

// SNIP

}[/code]

After some analysis, the reason is simple: ResourceCache isn’t notified of the resource being destroyed when manually calling ReleaseRef() on it, which means ResourceCache will continue to hold an invalid pointer to the destroyed resource.

So what is the correct way of destroying a resource from the cache manually?

Calling ReleaseRef() manually when something is managed by shared pointers (such as the resource map of ResourceCache) is a bad idea, because it disturbs the pointers’ automated refcount management.

The ResourceCache has the family of ReleaseResource() functions, please use these. In extreme situation use ResourceCache::GetAllResources() and const cast it as necessary so that you can modify the resource map directly.

Also you can simply use the cache->GetTempResource() which won’t store the resource in the cache but only in a smart pointer.So once it goes out of scope it will be deleted.

void App::Start() { ResourceCache* cache = GetSubsystem<ResourceCache>(); for(int i = 0; i < 10; i++) { SharedPtr<XMLFile> f = cache->GetTempResource<XMLFile>("UI/DefaultStyle.xml"); // Do something with f // f will be deleted once goes out of scope... nothing to release here } }

Well that’s exacly what should happen if you releaseRef().You should ALWAYS Release those refs that were prevoiusly AddRef()'d.

[quote] XMLFile* f = cache->GetResource(“UI/DefaultStyle.xml”);
f->ReleaseRef();[/quote]
This is wrong since the only ref is stored by the ResourceCache and you releasing it.

It was my fault… in the other topic i didn’t explaied Urho’s ref counting very well.
So

1. Call ReleaseRef() only if you AddRef() previously.
2. The easiest way to get rid of unused resources is call cache->RemoveAllResources(). This will remove all resources with refCount == 1 (referenced only by the cache , not used elesewhere)