I'm using a vector of pointers to objects. These objects are derived from a base class, and are being dynamically allocated and stored.
For example, I have something like:
vector<Enemy*> Enemies;
and I'll be deriving from the Enemy class and then dynamically allocating memory for the derived class, like this:
enemies.push_back(new Monster());
What are things I need to be aware of to avoid memory leaks and other problems?
std::vector will manage the memory for you, like always, but this memory will be of pointers, not objects.
What this means is that your classes will be lost in memory once your vector goes out of scope. For example:
#include <vector>
struct base
{
virtual ~base() {}
};
struct derived : base {};
typedef std::vector<base*> container;
void foo()
{
container c;
for (unsigned i = 0; i < 100; ++i)
c.push_back(new derived());
} // leaks here! frees the pointers, doesn't delete them (nor should it)
int main()
{
foo();
}
What you'd need to do is make sure you delete all the objects before the vector goes out of scope:
#include <algorithm>
#include <vector>
struct base
{
virtual ~base() {}
};
struct derived : base {};
typedef std::vector<base*> container;
template <typename T>
void delete_pointed_to(T* const ptr)
{
delete ptr;
}
void foo()
{
container c;
for (unsigned i = 0; i < 100; ++i)
c.push_back(new derived());
// free memory
std::for_each(c.begin(), c.end(), delete_pointed_to<base>);
}
int main()
{
foo();
}
This is difficult to maintain, though, because we have to remember to perform some action. More importantly, if an exception were to occur in-between the allocation of elements and the deallocation loop, the deallocation loop would never run and you're stuck with the memory leak anyway! This is called exception safety and it's a critical reason why deallocation needs to be done automatically.
Better would be if the pointers deleted themselves. Theses are called smart pointers, and the standard library provides std::unique_ptr and std::shared_ptr.
std::unique_ptr represents a unique (unshared, single-owner) pointer to some resource. This should be your default smart pointer, and overall complete replacement of any raw pointer use.
auto myresource = /*std::*/make_unique<derived>(); // won't leak, frees itself
std::make_unique is missing from the C++11 standard by oversight, but you can make one yourself. To directly create a unique_ptr (not recommended over make_unique if you can), do this:
std::unique_ptr<derived> myresource(new derived());
Unique pointers have move semantics only; they cannot be copied:
auto x = myresource; // error, cannot copy
auto y = std::move(myresource); // okay, now myresource is empty
And this is all we need to use it in a container:
#include <memory>
#include <vector>
struct base
{
virtual ~base() {}
};
struct derived : base {};
typedef std::vector<std::unique_ptr<base>> container;
void foo()
{
container c;
for (unsigned i = 0; i < 100; ++i)
c.push_back(make_unique<derived>());
} // all automatically freed here
int main()
{
foo();
}
shared_ptr has reference-counting copy semantics; it allows multiple owners sharing the object. It tracks how many shared_ptrs exist for an object, and when the last one ceases to exist (that count goes to zero), it frees the pointer. Copying simply increases the reference count (and moving transfers ownership at a lower, almost free cost). You make them with std::make_shared (or directly as shown above, but because shared_ptr has to internally make allocations, it's generally more efficient and technically more exception-safe to use make_shared).
#include <memory>
#include <vector>
struct base
{
virtual ~base() {}
};
struct derived : base {};
typedef std::vector<std::shared_ptr<base>> container;
void foo()
{
container c;
for (unsigned i = 0; i < 100; ++i)
c.push_back(std::make_shared<derived>());
} // all automatically freed here
int main()
{
foo();
}
Remember, you generally want to use std::unique_ptr as a default because it's more lightweight. Additionally, std::shared_ptr can be constructed out of a std::unique_ptr (but not vice versa), so it's okay to start small.
Alternatively, you could use a container created to store pointers to objects, such as a boost::ptr_container:
#include <boost/ptr_container/ptr_vector.hpp>
struct base
{
virtual ~base() {}
};
struct derived : base {};
// hold pointers, specially
typedef boost::ptr_vector<base> container;
void foo()
{
container c;
for (int i = 0; i < 100; ++i)
c.push_back(new Derived());
} // all automatically freed here
int main()
{
foo();
}
While boost::ptr_vector<T> had obvious use in C++03, I can't speak of the relevance now because we can use std::vector<std::unique_ptr<T>> with probably little to no comparable overhead, but this claim should be tested.
Regardless, never explicitly free things in your code. Wrap things up to make sure resource management is dealt with automatically. You should have no raw owning pointers in your code.
As a default in a game, I would probably go with std::vector<std::shared_ptr<T>>. We expect sharing anyway, it's fast enough until profiling says otherwise, it's safe, and it's easy to use.
The trouble with using vector<T*> is that, whenever the vector goes out of scope unexpectedly (like when an exception is thrown), the vector cleans up after yourself, but this will only free the memory it manages for holding the pointer, not the memory you allocated for what the pointers are referring to. So GMan's delete_pointed_to function is of limited value, as it only works when nothing goes wrong.
What you need to do is to use a smart pointer:
vector< std::tr1::shared_ptr<Enemy> > Enemies;
(If your std lib comes without TR1, use boost::shared_ptr instead.)
Except for very rare corner cases (circular references) this simply removes the trouble of object lifetime.
Edit: Note that GMan, in his detailed answer, mentions this, too.
I am assuming following:
Following things come to my mind:
One thing to be very careful is IF there are two Monster() DERIVED objects whose contents are identical in value. Suppose that you wanted to remove the DUPLICATE Monster objects from your vector (BASE class pointers to DERIVED Monster objects). If you used the standard idiom for removing duplicates (sort, unique, erase: see LINK #2], you will run into memory leak issues, and/or duplicate delete problems, possibly leading to SEGMENTATION VOIOLATIONS (I have personally seen these problems on LINUX machine).
THe problem with the std::unique() is that the duplicates in the [duplicatePosition,end) range [inclusive, exclusive) at the end of the vector are undefined as ?. What can happen is that those undefined ((?) items might be extra duplicate or a missing duplicate.
The problem is that std::unique() isn't geared to handle a vector of pointers properly. The reason is that std::unique copies uniques from the end of the vector "down" toward the beginning of the vector. For a vector of plain objects this invokes the COPY CTOR, and if the COPY CTOR is written properly, there is no problem of memory leaks. But when its a vector of pointers, there is no COPY CTOR other than "bitwise copy", and so the pointer itself is simply copied.
THere are ways to solve these memory leak other than using a smart pointer. One way to write your own slightly modified version of std::unique() as "your_company::unique()". The basic trick is that instead of copying an element, you would swap two elements. And you would have to be sure that the instead of comparing two pointers, you call BinaryPredicate that follows the two pointers to the object themselves, and compare the contents of those two "Monster" derived objects.
1) @SEE_ALSO: http://www.cplusplus.com/reference/algorithm/unique/
2) @SEE_ALSO: What's the most efficient way to erase duplicates and sort a vector?
2nd link is excellently written, and will work for a std::vector but has memory leaks, duplicate frees (sometimes resulting in SEGMENTATION violations) for a std::vector
3) @SEE_ALSO: valgrind(1). THis "memory leak" tool on LINUX is amazing in what it can find! I HIGHLY recommend using it!
I hope to post a nice version of "my_company::unique()" in a future post. Right now, its not perfect, because I want the 3-arg version having BinaryPredicate to work seamlessly for either a function pointer or a FUNCTOR, and I'm having some problems handling both properly. IF I cannot solve those problems, I'll post what I have, and let the community have a go at improving on what I have done so far.
