How to delete raw pointers from a unique_ptr

Question

I'm writing a game framework, I have a vector<unique_ptr<Object>> list and I distribute pointers from that list by calling object.get() and sending that out. Before that I send out references instead of raw pointers but that resulted in other weird problem so I was told this is better. However when I remove a unique_ptr<Object>from the list, the raw pointers remains. I also can't deallocate them manually, I get an exception saying the pointer is not allocated.

So my question would be:

How do I delete raw pointers from removed unique_ptr's?

and is also a more general question:

Am I on the right track structure wise of passing pointers instead of references?

PxlObject* PxlFramework::AddObject(PxlObject* obj)
{
    std::unique_ptr<PxlObject> u_ptr(obj);
    objects_iterator = objects.insert(objects.end(), std::move(u_ptr));
    return obj;
}

void PxlFramework::DeleteObject(PxlObject* obj) {
    for(objects_iterator = objects.begin(); objects_iterator != objects.end(); ++objects_iterator)
    {
        if((*objects_iterator)->get_id() == obj->get_id())
        {
            //attempting to delete the raw pointer here but it will result in an error
            delete obj;

            //removing the unique_ptr<PxlObject> from the list here
            std::swap((*objects_iterator), objects.back());
            objects.pop_back();
            break;
        }
    }
}

You don't delete pointers that belong to smart pointers, they do that on their own, it's their whole and only goal in life, and doing it yourself would break their heart and your code. — user703016, May 27 '14 at 17:13
If you need shared ownership you should use shared_ptr. unique_ptr is there to be the unique owner of the object, so you are not supposed to have copies of its content laying around after its lifetime ended. — Matteo Italia, May 27 '14 at 17:31
One problem is your `AddObject` function. It passes a random pointer to a `unique_ptr`. This is a bug. Only pointers created with `new` should be passed to `unique_ptr` because `unique_ptr` will call `delete` on it. A fix would be to make `AddObject` take a `unique_ptr &&`. Another fix would be to take a `PxlObject &&`. — nwp, May 27 '14 at 17:56

score 2 · Answer 1 · answered May 27 '14 at 18:08

2

The whole point of std::unique_ptr is that it "owns" the object and it manages deletion automatically when the unique_ptr is destroyed. As such, you should not delete either a unique_ptr nor anything that a unique_ptr owns. To avoid this confusion, references are more common. Additionally, you have the oddity that your AddObject returns a pointer to a PxlObject that is not the one just added.

Something like this might be a little cleaner:

template<class Us...>
PxlObject& PxlFramework::AddObject(Us&&... obj)
{
    std::unique_ptr<PxlObject> u_ptr(new PxlObject(std::forward<Us>(obj)...));
    objects_iterator = objects.insert(objects.end(), std::move(u_ptr));
    return **objects_iterator;
}

void PxlFramework::DeleteObject(PxlObject& obj) {
    auto finder = [](std::unique_ptr<PxlObject>& p)->bool 
            {return obj.get_id()==p->get_id();};
    auto it = find_if(objects.begin(), objects,end(), finder);
    if (it != objects.end())
        objects.erase(it);
    else
        throw ...;
}

answered May 27 '14 at 18:08

Mooing Duck

64,318
19
100
158

but the problem I'm having with that is that even after I called DeleteObject with a reference, that reference will still exist and carry the same data. – user3680720 May 27 '14 at 18:33
@user3680720: The reference exists, but the data is gone. It's referring to where the data used to be. Simply don't use those references after that point. If that's not possible, then the other places in your code will also need to share in the ownership of the PxlObject, in which case you probably want a different design, with a `std::shared_ptr` instead. – Mooing Duck May 27 '14 at 18:37
ok so the AddObject method would return a reference but a reference can't be null, so what do I return in for example a FindObject method if the object isn't found or just throw an error? – user3680720 May 27 '14 at 20:15
Most containers return an iterator to the `end()` if the element is not found. – Mooing Duck May 27 '14 at 20:34

score 0 · Answer 2 · answered May 27 '14 at 18:41

You don't need delete the raw pointer directly you can use vector.erase instead. Here you have a simple example:

#include <iostream>
#include <algorithm>
#include <memory>
#include <vector>

using namespace std;

typedef vector<unique_ptr<int>> ptr_list_t;

void remove_number(int x, ptr_list_t& ptr_list)
{
    for (ptr_list_t::iterator it = ptr_list.begin(); it != ptr_list.end(); ++it)
        if (*(it->get()) == x) {
            ptr_list.erase(it);     // Use vector.erase for deleting objects from a vector.
                                    // since it points to a unique_ptr, the object owned by it
                                    // will be destroyed automatically.
            break;
        }
}

int main()
{
    ptr_list_t ptr_list;

    // Generating the pointer to numbers. 0 - 9
    for (int i = 0; i < 10; i++)
        ptr_list.push_back(unique_ptr<int>(new int(i)));

    // Remove the number 3.
    remove_number(3, ptr_list);

    // Printing the list. The number 3 will not appear.
    for (ptr_list_t::iterator it = ptr_list.begin(); it != ptr_list.end(); ++it)
        cout << *(it->get()) << endl;

    return 0;
}

Other thing, I'm agreed with @MooingDuck: you should not delete either a unique_ptr nor anything that a unique_ptr owns. But you sure can. Take a look on unique_ptr.release. This function frees the ownership of the managed object.

How to delete raw pointers from a unique_ptr

2 Answers2