CODEWITHSUNDEEP
c++stlunique-ptr
heyufool1
heyufool1

Reputation: 63

std::unique_ptr and its effect on matching pointers in STL containers

I made a factory class for lights in my game where a function "create" creates a unique_ptr and stores it in a STL vector, so I can collect the light data and send it to a shader, then it returns a reference to that unique_ptr in the vector like so:

std::unique_ptr<Light>& create() {
    myVector.push_back(std::unique_ptr<Light>(new Light));
    return &myVector.back();
}

However, when I add more lights, the unique_ptr reference (not the raw pointer) gets invalidated because it was referencing the unique_ptr object in the vector. So, passing the unique_ptr reference after creating it lead to errors. So, I came up with this solution:

std::unique_ptr<Light> create() {
    Light* light = new Light();
    myVector.push_back(light);
    return std::unique_ptr<Light>(light);
}

Which solved the invalidation issue, but now, when the unique_ptr went out of scope, it replaced its raw pointer in myVector with the next pointer in the vector. So, if I had a vector of pointers: [1, 2, 3] then after unique_ptr went out of scope the vector became [2, 2, 3]. Naturally, I didn't want redundant data so I replaced the vector with a STL set. Now, when the unique_ptr goes out of scope the set finds the redundant data and removes the one copy, so the set correctly contains pointers: [2, 3]. This works and I don't receive any errors, but I don't understand why or how the unique_ptr changes the matching pointer value in the STL container. Is this expected behavior? If so, why/how does it work?

PS: I understand that the purpose of a unique_ptr is to make the pointer owned by one single object. As a result, I prefer having the sole ownership kept in the object that calls the create() function. So, the factory simply creates the light and stores references to the lights it created for easy collection, though the storage done by the factory itself is just a temporary measure. However, I love the nature of the STL set automatically being maintained when the unique_ptr automatically releases itself when it goes out of scope. So, I'd like to keep that generic design, only if it isn't some fluke with the unique_ptr.

Upvotes: -1

Views: 739

Answers (4)

Ben L
Ben L

Reputation: 1321

Smart pointers are about solving the resource ownership problem. This doesn't mean you always use shared_ptr and never use "raw" pointers.

It appears that you want to have a vector of lights where this vector is responsible for the resources. For that you don't even need pointers because the vector handles the pointers internally.

vector<Light> myVector;

You can get a handle to this resource from the vector or from the return of this create function. 

const Light& create() {
      Light light;
      myVector.push_back(light);
      return myVector.back();
}

What is the consumer of this resource allowed to do? Can it delete the light?

And what guarantees can the vector make to the consumers? Will these pointers be used passed the lifetime of the list.

  • Passing around unique_ptr means passing around the ownership.
  • Creating shared_ptr's means that when the last reference is deleted, the resource is cleaned up.
  • weak_ptr's can be used for observers that don't care if the resource is cleaned up.
  • raw pointers and references don't care about anything. But if you are just acquiring a handle in place and read and writing to that instance, you don't need to deal with the overhead of a share_ptr.

Upvotes: 0

Dietmar K&#252;hl
Dietmar K&#252;hl

Reputation: 153840

Since the std::vector<std::unique_ptr<Light>> is the owner of the Lights, there is little use in returning references to these rather than pointers to the Lights. The references are only useful if you need to replace the value. I'd just travel in terms of Light* or Light const* where the objects are used rather than owned. You can get the pointer held by a std::unique_ptr<T> with the get() method.

Instead of returning a Light* you could return a simple_ptr<Light> where simple_ptr<T> essentially just has a constructor taking a T* and operators operator->() which returns a T* and operator*() which returns a T&:

template <typename T>
class simple_ptr {
    T* ptr;
public:
    explicit simple_ptr(T* ptr): ptr(ptr) {}
    T* operator->() const { return this->ptr; }
    T& operator*() const { return *this->ptr; }
};

The generated functions just do the Right Thing (i.e. copy the pointer as needed). Without jumping through hoops it wouldn't be possible delete this pointer. If you give your Light type a custom unary operator&() you could make it even harder to explicitly get to the pointer. Tt won't be impossible, though, as you can always call ptr.operator->() to get it. However, you should only defend against Murphy, not against Machiavelli.

Upvotes: 2

Drew Dormann
Drew Dormann

Reputation: 63775

You're contradicting the design of std::unique_ptr by trying to have multiple copies of it.

This could be solved by using std::shared_ptr instead.

std::shared_ptr<Light> create() {
    myVector.push_back(std::make_shared<Light>());
    return myVector.back();
}

Or if you want your vector to be primarily responsible for the lifetime of these lights, return a std::weak_ptr, which knows when it's dangling.

std::weak_ptr<Light> create() {
    myVector.push_back(std::make_shared<Light>());
    return myVector.back();
}

Upvotes: 0

The Paramagnetic Croissant
The Paramagnetic Croissant

Reputation: 9343

Since you don't want to delete the object while either the caller of your factory function or the vector has a pointer to it, why not use an std::shared_ptr?

Upvotes: 0

Related Questions