Search code examples
c++iterator

is it safe to call delete inside a remove_if predicate?


I have a vector

std::vector<Object*> objects;

And a method that removes an object if it's found:

void Remove(Object *o)
{
    objects.erase(
        std::remove_if(
            objects.begin(), objects.end(),
            [&o](Object *_object) {
                if (o == _object)
                {
                    delete _object;
                    return true;
                }
                return false;
            }
        ),
        objects.end()
    );
}

Is this safe? Should I not call delete? But then would erase call delete for me? I'm a bit confused. Would this invalidate the iterator or leak memory?


Solution

  • Would this invalidate the iterator or leak memory?

    Memory doesn't get leaked by individual bits of code; it gets leaked by complete programs. Someone has to be responsible for deallocation, but just because it doesn't happen here, doesn't mean it won't happen at all. Not calling delete here would also not "leak memory"; what leaks memory is the entire program. Code like this cannot stand on its own.

    It's difficult to figure out where the responsibility lies, except by following established patterns. That's why tools like std::unique_ptr<T>, std::shared_ptr<T> and std::weak_ptr<T> exist. Please use them.

    The iterator will not be invalidated. The iterator is iterating over elements of the container, which are the pointers themselves. delete does not affect the pointers it's used with. delete affects the pointed-at memory.

    Would erase call delete for me?

    No.

    Is this safe?

    Code like this risks a double deallocation (undefined behaviour) if any other code could possibly attempt to delete any of the same pointed-at elements from the container.

    Again, this is not a local risk, it is a whole-program risk.

    Again, figuring out memory management across a whole-program context is difficult in general. Please use standard library tools like std::unique_ptr<T>, std::shared_ptr<T> and std::weak_ptr<T>, as appropriate to the situation, in appropriate ways. A proper tutorial on memory management in C++ is beyond the scope of a Stack Overflow question.