1

I implemented a simple ResourceManager - everything was fine until I tried to implement its destructor for (emergency) clean up(for example in case of fatal exception). I was not able to find a single way to do it:

template<typename T>
class ResourceManager
{       
public: 
    std::unordered_map<std::string, std::weak_ptr<T> > resource_map;
    std::shared_ptr<T> getResource(std::string name) 
    {
        std::shared_ptr<T> res = resource_map[name].lock();
        if(!res)
        {
            res = std::shared_ptr<T>(new T(this, name));
            resource_map[name] = res;
        }
        return std::move(res);
    }

    void Release(std::string name)
    {
        resource_map.erase(name);
    };
    ~ResourceManager(void) { /* ???? */}
};

class Texture
{
private:
    ResourceManager<Texture> * manager_;
    std::string name_;

public:
    Texture(ResourceManager<Texture> * manager, std::string& name) 
          : manager_(manager), name_(name) { }
    ~Texture(void){ 
        manager_->Release(name_);
    }
};

Obviously, I have to iterate over all active resources...but how can I free them if the ResourceManager is not technically (sole)owner of the resources? This might be flaw of design, if that is the case please suggest alternative.

EDIT: in response to answers, to define "resource manager" I imagine authoritative cache - storage for references to resources that can look up resources(=no duplicates) and manages their state in memory (in-memory, description-only(=path+type) and freed), all above as automated as possible. (there should be separate ResourceLoaders, but that does not change much for this question)

wondra
  • 3,271
  • 3
  • 29
  • 48
  • Personally, I don't see this as a good design. What's the point of shared ownership if someone can destroy the underlaying object arbitrarily? I don't think it is a good idea to twist the concept of shared ownership like this. Consider returning just a reference, weak_ptr or a *resource handle* instead, and let the resource manager retain ownership. – glampert Jan 30 '15 at 02:42
  • [This](http://scottbilas.com/publications/gem-resmgr/) might also be useful to you. – glampert Jan 30 '15 at 02:45
  • @glampert well, it is article from year 2000 - it implements in-house `weak_ptr` using handles, as they were not part of std before C++11(at least google sais that). It is almost the same like handing out `weak_ptr`s and `.lock()`ing them. But that is what I originally tried to avoid - necessity to check resource before every use. – wondra Jan 30 '15 at 10:41
  • Anyway, thanks for the comment - I assume you would store `shared_ptr`s and hand out `weak_ptr`s then, right? – wondra Jan 30 '15 at 10:48
  • 1
    Yes, sorry I couldn't provide a more recent reference. But anyway, the concept of "resource managers" has been around for quite some time and hasn't changed much since... If there is going to be a possibility that the resource gets evicted while references to it still exits outside the manager, then I don't think the weak/shared setup would do. The advantage of an opaque handle that you have to lock via the resource manager is that you can rebind the underlaying object at any time, without messing with the handles that exist outside. – glampert Jan 30 '15 at 17:31

2 Answers2

2

So, your code isn't doing a whole lot to illuminate your overall design, here, but...

As implemented, your resource manager appears to only have weak pointers to the resources. This indicates that it isn't responsible for the lifetime of the actual resources themselves, and therefore should not be cleaning those resources up in its destructor.

If you want the resource manager to be the authoritative owner of the resource data, you'll want to change its design/implementation. For example, you can have it store shared_ptrs to the resources themselves, and only pass out weak_ptrs to clients. Or just store unique_ptrs and pass out bare pointers to the client code.

But as written, you don't need to (and really can't) do anything to clean up the resources in ~ResourceManager().

  • Well, if I hand out `weak_ptr`, I have to check if the resource is in the memory every time I decide to use it, dont I? Can that be automated/synchronized in a way so I keep the pros of handing out `shared_ptr`s? (= no need to check for every use and have idea how many active users of resource are there?) – wondra Jan 29 '15 at 23:01
  • Yes, clients have to lock() the weak pointer. There is really no way to avoid a check if you want the client to be able to know if their pointer is dead *and* use a centralized ownership model instead of a shared one. –  Jan 30 '15 at 15:55
2

To suggest alternatives, we would need to know what purpose your resource manager is supposed to have.

  1. Do you want it to function as a "resource collection", i.e. keeping the resources alive until they are explicitly released?
  2. Or do you want it to be a "resource cache", keeping resources alive until it decides that some should be released to free up memory?
  3. Or do you really want it to NOT keep any resources alive, but simply keep a list of resources that are possibly held alive by something else?

Remember that shared_ptrs in C++ do not work like a GC works. I.e. if you destroy/reset the last shared_ptr to an object, that object will be deleted immediately, even if there are weak_ptrs to it.

So the approaches (1) and (2) can make a lot of sense. (3) however, which is what you currently have, only very rarely makes sense (if at all).

Paul Groke
  • 6,259
  • 2
  • 31
  • 32
  • Originally, I was tempted with fully automated freeing. Later I realized it is not a miraculous solution. Would be own GC with `weal_ptr`s a good idea? I tried implementing own aging gc, but there MUST be a resean why everybody does similar managers to the one above. – wondra Jan 30 '15 at 10:15