List of smart pointers - Managing object lifetime and pointer validity

Question

I have a list of smart pointers where each pointer points to a separate Entity class.

std::list<std::unique_ptr<Entity>> m_entities;

I would like the constructor to handle the assigning of each pointer to a std::list class as it is "automatically" handled by the code on class instantiation. However, if this design is bad then I would welcome a better alternative as it only makes sense to me coming from a C# background.

Entity::Entity(Game &game) 
    : m_game(game),             
    m_id(m_game.g_idGenerator->generateNewID())             
{
    m_game.m_entities.push_back(std::unique_ptr<Entity>(this));
}

The main problem I have encountered with this method is that the Entity class' lifetime is unmanaged by the Entity class.

For example if I allocate an Entity class on the stack it will call the Entity destructor after leaving the method in which it was allocated and the pointer will no longer be valid.

I therefore considered the alternative of creating a smart pointer, allocating the Entity class to the heap and then explicitly adding the pointer to the list.

std::unique_ptr<Entity> b(new Entity(*this));
m_entities.push_back(b);   // ERROR

This produces the following error

error C2664: 'void std::list<_Ty>::push_back(_Ty &&)' : cannot convert parameter 1 from 'std::unique_ptr<_Ty>' to 'std::unique_ptr<_Ty> &&'

What would be considered the best approach for allocating each pointer to the list and is a constructor based version possible?

I'm currently thinking that it is the list of smart pointers that should handle the lifetime for each Entity class and that assigning pointers in a constructor is not a good design choice. In that case I should probably create a CreateEntity method that adds the pointer to list rather than let the constructor handle it. Is this better?

I considered what type of smart pointer would be appropriate for this operation after reading through questions found here, here and here (offsite). It is difficult to get an exact answer based on what I've read so far though as they all offer somewhat conflicting advice.

Why does your code use `std::unique_ptr` but the error message use `std::auto_ptr`? — Dietmar Kühl, Sep 20 '12 at 22:39
Is this your real code? `std::unique_ptr` and `std::auto_ptr` are different unrelated classes, and the code you showed is using `std::unque_ptr` but the error message says `std::auto_ptr` instead. And how is `Boundary` related to `Entity`? — Remy Lebeau, Sep 20 '12 at 22:41
The wrong error was copied by mistake as I was changing between pointer types to see the difference they made. Apologies. It has been edited to show the correct error. — user1423893, Sep 20 '12 at 22:49
Related http://stackoverflow.com/questions/8114276/how-do-i-pass-a-unique-ptr-argument-to-a-constructor-or-a-function — R. Martinho Fernandes, Sep 20 '12 at 22:55

score 5 · Accepted Answer · edited Sep 20 '12 at 23:27

Using constructor this way is definitely not good idea because constructor has no information about how object is created and controlled - on the stack, statically, dynamically by some smart pointer, dynamically by dumb pointer?

To solve this problem you could use static factory method to create Entity instances:

class Entity
{
public:

   // Variant with unique ownership
   static void CreateGameEntity(Game& game)
   {
     std::unique_ptr<Entity> p(new Entity());
     game.m_entities.push_back(std::move(p));
   }

   // OR (you cannot use both)

   // Variant with shared ownership
   static std::shared_ptr<Entity> CreateGameEntity(Game& game)
   {
     std::shared_ptr<Entity> p(new Entity());
     game.m_entities.push_back(p);
     return p;
   }

private:

   // Declare ctors private to avoid possibility to create Entity instances
   // without CreateGameEntity() method, e.g. on stack.
   Entity();
   Entity(const Entity&);
};

Which smart pointer to use? Well, this depends on your design. If Game object solely owns Entity instances and completely manages their lifetime, using std::unique_ptr is OK. If you need some kind of shared ownership (e.g. you have several Game objects that can share same Entity objects) you shall use std::shared_ptr.

Also in case of unique ownership you may use Boost Pointer Container library. It contains specialized owning pointer containers like ptr_vector, ptr_list, ptr_map etc.

_If you need some kind of shared ownership (e.g. you have several Game objects that can share same Entity objects) you shall use std::shared_ptr_ Other classes will access the pointer via the std:list that owns/holds all the pointers. These other classes may get or set exposed Entity parameters. Do they need to own the object to do this? — user1423893, Sep 20 '12 at 23:35
@user1423893 : "Own" in this context means to control (or extend) an object's lifetime. — ildjarn, Sep 20 '12 at 23:38

score 3 · Answer 2 · answered Sep 20 '12 at 22:39

3

I won't comment on your design questions, but to fix your error, change your code to either:

m_entities.push_back(std::unique_ptr<Boundary>(new Boundary(*this, body)));

or:

std::unique_ptr<Boundary> b(new Boundary(*this, body));
m_entities.push_back(std::move(b));

The reason is that b in your code is an lvalue, but std::unique_ptr<> is a move-only type (i.e. has no copy constructor).

answered Sep 20 '12 at 22:39

ildjarn

62,044
9
127
211

This answer deserves more visibility. – vaeVictis May 01 '22 at 11:33

score 3 · Answer 3 · answered Sep 20 '12 at 22:42

The problem in your code is that you try to move a std::unique_ptr<T> from an l-value. The instantiations of std::unique_ptr<T> are non-copyable and are only movable. To move from an l-value you need to explicitly do so:

this->m_entities.push_back(std::move(b));

The call to std::move() won't really move anything but it does yield a type which indicates to the compiler that the object can be moved.

score 0 · Answer 4 · answered Sep 20 '12 at 22:41

To address the issue with the stack-created instance, you could simply add a parameter to the constructor that tells it to not add the new instance to the list, eg:

Entity::Entity(Game &game, bool AddToList = true)  
    : m_game(game),              
      m_id(m_game.g_idGenerator->generateNewID())              
{ 
    if (AddToList) m_game.m_entities.push_back(this); 
}

.

{
    ...
    Entity e(game, false);
    ...
}

Another option might be to add a destructor to Entity that removes it from the list if it is still present, but that might get a little complex trying to avoid conflicts between direct Entity destructions and unique_ptr destructions.

List of smart pointers - Managing object lifetime and pointer validity

4 Answers4