Does the "delete" statement doubly free an object?

Question

Hello C++/STL gurus:

Does the "delete" statement below "doubly free" an object?

(...object_list is a global vector<object*>...)

vector< object * >::iterator     it, eit, iter;
object *p_object;
vector< object * >   dead_objects;

it  = object_list_.begin();
eit = object_list_.end();

//---collect pointers of all dead objects to dead_objects vector
for ( ; it != eit; it++ )
{
    p_object = *it;
    if ( p_object->is_dead() == false )
        continue;

    dead_objects.push_back( p_object );
}

//---free every dead object from the global object_list
for ( iter = dead_objects.begin(); iter != dead_objects.end(); iter++ )
{
    p_object = *iter;

    it  = object_list_.begin();
    eit = object_list_.end();

    for ( ; it != eit; it++ )
    {
        if ( *it != p_object )
            continue;

        object_list_.erase( it );
        delete p_object;
        break;
    }
}

I ask the question because the erase() statement above should have called the destructor of an object and freed it already, shouldn't it?

Answer 1

It would appear not; if you have a vector of pointers to objects, calling erase() to remove one of them simply removes the pointer from the vector. You are still required to delete it yourself - this is because STL containers are designed primarily to collect objects by value.

Just a couple of suggestions - IMO your code would be clearer if you used STL algorithms like std::find instead of looping over all your vectors by hand. I'm not sure what the point of dead_objects vs object_list is - you don't seem to gain anything by storing them in the temporary vector, but something may have been lost in copying the code to SO. And std::vector is not optimal for lots of random erasures like this since erase runs in linear time - std::remove followed by erase would be a more efficient approach. For example:

for(vector<object*>::iterator it = object_list.begin(); it != object_list.end(); ++it) {
    if((*it)->is_dead()) {
        delete *it;
        *it = NULL;
    }
}
object_list.erase(std::remove(object_list.begin(), object_list.end(), NULL), object_list.end());

Answer 2

erase() does call the destructor on the object, but the destructor of a pointer type (such as object * here) does nothing -- it does NOT call delete on the pointer. If you want it to call delete, you need to use some object (such as auto_ptr<object *>) which does call delete.