How to organize object ownership for class that lives lesser time than owner of the object?

Question

I have the following situation: there is class of GraphicsContext:

class GraphicsContext {
    ...
private:
    std::unique_ptr<Renderer> m_renderer;
}

And there is a class of application that uses the GraphicsContext:

class Application {
   ...
private:
    std::unique_ptr<GraphicsContext> m_graphicsContext;
}

And there are sub-level classes those are used in Application class and those uses Renderer from the GraphicsContext. I need to store pointer to the renderer in these classes, but how should I do that?

class SubLevelClass {
public:
    SubLevelClass(Renderer* renderer);
    ...
    void drawSomething();
private:
    Renderer* m_renderer; // this class is not owner of Renderer but should can ability to refer to it
}

Such sub-level classes does not semantically own the Renderer and therefore I think it't not good idea to use shared_ptr instead of unique_ptr. But how to organize such ownership if it's garanteed that objects of sub-level classes live lesser time than Application object? Can I store and return from GraphicsContext a raw pointer to Renderer or it's semantically wrong idea?

Possible duplicate : [Is it a good practice to always use smart pointers?](https://stackoverflow.com/questions/2454214/is-it-a-good-practice-to-always-use-smart-pointers) — François Andrieux, Aug 23 '19 at 17:36
François, but does not it lead to the possibility of memory leaks in comparison with using of shared pointer in such situations? — Nikolai, Aug 23 '19 at 18:37
Is `SubLevelClass` ever responsible for `delete`ing the renderer? My understanding is that no, it never is. So a raw pointer is fine, you are not responsible for it's lifetime. Whoever *is* responsible will be the one with a smart pointer like `std::unique_ptr`. If more than one object may be responsible, then you have shared ownership and there you use `shared_ptr`. It looks here like `Renderer` is not implicated with the cleanup at all so not implicated with the ownership. But maybe I misunderstood your question. — François Andrieux, Aug 23 '19 at 18:47
`SubLevelClass` is not responsible for `delete`ing the renderer. I mean the situation when GraphicsContext decide to `delete` the renderer before it will be used in `SubLevelClass`. In this case we will get using of already freed pointer. — Nikolai, Aug 23 '19 at 20:03
To ensure that `Renderer* m_renderer;` is not a dangling pointer, you can use assertion. Whenever an instance of `Renderer` is delete - check whether there are any `SubLevelClass` still store it. Some additional datastructure (e.g. map) are required to keep track. — javaLover, Aug 24 '19 at 02:16
javaLover, if I understand correctly, this approach requires to notify all `SubLevelClasses` about the deleting `Renderer` instance? And I I doubt the applicability of the assertions because in C++ I can not check validity of raw pointer in runtime. I only can check it for non-zero value, but it is not enough in this case. — Nikolai, Aug 24 '19 at 21:14
Sorry, I didn't get notification from your reply. You can add "@" like "@JavaLover" in comment to ping me. :) — javaLover, Sep 01 '19 at 04:50

javaLover · Accepted Answer · 2019-09-01T04:58:57.523

There are some ways that can solve it.

These codes are not tested but should be enough to show the idea.

Solution 1 : plainly keep track

Solution 1A :-

class Renderer{
    std::vector<SubLevelClass*> whoThatLinkBackToMe; //or std::unordered_set
    public: ~Renderer(){
        //assert that whoThatLinkBackToMe.size() == 0
    }
};
class SubLevelClass{
    SubLevelClass(Renderer* renderer){
        renderer->whoThatLinkBackToMe.push_back(this);
    }
    //.. destructor should remove "this" from "renderer->whoThatLinkBackToMe" too
};

Solution 1B :-

class CentralizeSystem{
    public: static std::unordered_map<Renderer*,SubLevelClass*> map;
};
class Renderer{
    public: ~Renderer(){
        //assert that CentralizeSystem::map[this].size() == 0
    }
};
class SubLevelClass{
    SubLevelClass(Renderer* renderer){
        CentralizeSystem::map.add(renderer,this);
    }
    //.. destructor should remove "this" from "CentralizeSystem::map" too
};

Solution 2 : Entity Component System (ECS)

It is a revolution in design that require a huge commitment :-

Make Renderer a System in ECS. Thus, it is automatically deleted last.
Make SubLevelClass a Component in ECS. Try to store all information (field, cache) in SubLevelClass - not Renderer. These 2 things alone should solve your issue.
However, if it is very unlucky e.g. you need to make Renderer not singleton (become Component):

3.1 Create a new component Component_CheckDelete e.g. :-
```
class Component_CheckDelete : public Component{
    public: bool ToBeDeleted=false;
};
```
3.2 Whenever a Renderer is to be deleted, just mark its Component_CheckDelete::ToBeDeleted=true.
Then, in the end of time-step, check every instance of SubLevelClass.
If there are some SubLevelClass that refer to the Renderer that has convertToComponent<Component_CheckDelete>(rendererPtr)->ToBeDeleted==true, throw assert fail.

Solution 3

Just ignore the whole issue.
It is an error at the user's side. Engine creator is not supposed to catch every user's mistake.

Bullet Physics (one of the best Physics Engine) uses this approach a lot - if I delete its boardphase module and still use its main engine, I can get an irresponsible access violation.

My opinion : I usually picks Solution 3, sometimes Solution 2, rarely pick Solution 1A.

score 0 · Answer 2 · answered Sep 01 '19 at 05:00

std::shared_ptr is the correct solution here imho.

Such sub-level classes does not semantically own the Renderer and therefore I think it't not good idea to use shared_ptr

If sub-level classes need to prolong the lifetime of the Renderer object to match their own, then they share ownership of it.

How to organize object ownership for class that lives lesser time than owner of the object?

2 Answers2

Solution 1 : plainly keep track

Solution 2 : Entity Component System (ECS)

Solution 3