# Is the way I remove objects causing a memory leak?

The game I'm working on consumes memory, and the memory used in megabytes increases around 2MB every 4 seconds.

It starts at 5MB and grows to hundreds of megabytes in minutes.

I was careful, and have little to no new variable declarations in the main game loop (and the existing ones are integers and floats, not SDL_Surface's and images).

However, I keep creating new instances of classes, such as bullets and enemies, but I'm also careful with those:

/* Is a bullet out of the screen? Remove it from the list! */
for (i = 0; i < bullets.size(); i++) {
if (!bullets[i].is_on_screen(screen))
bullets.erase(bullets.begin() + i);
}


I remove them from the respective vectors when they are no longer needed, and with enemies too:

/* Is the enemy death? Put it out of the array! */
for (i = 0; i < enemies.size(); i++) {
if (enemies[i].health <= 0) enemies.erase(enemies.begin() + i);
}


Also, I create new Enemy(ies) with an image, but I only load the image on the beginning of the game, and then pass a pointer of it to the newly created class, so that it can draw it. I'm not creating hundreds of new images of enemies and bullets, am I?

When I erase the instances from the vectors, am I not fully deleting them from RAM?

• I think nic's answer is correct, but also make sure that if your bullets are created on the heap (that is, you use malloc() or the new keyword when creating them) that you also free them once you're done with them. Feb 27 '12 at 19:26

for (i = 0; i < bullets.size(); i++) {
if (!bullets[i].is_on_screen(screen))
bullets.erase(bullets.begin() + i);
}


Let's say you have 5 bullets.

• First iteration: Remove bullet at index 0. Now bullets.size() is 4
• Second iteration: Remove bullet at index 1. Now bullets.size() is 3
• Third iteration: Remove bullet at index 2. Now bullets.size() is 2.
• And now there will be no more iterations because i is larger than bullets.size(). Two of the 5 bullets will not be deleted properly.

for (i = bullets.size() - 1; i >= 0; --i) {
if (!bullets[i].is_on_screen(screen)) {
bullets.erase(i);
}
}

• First: Remove bullet at index 4. Now bullets.size() is 4. Decrement i to 3 at end of loop.
• Second: Remove bullet at index 3. Now bullets.size() is 3. Decrement i to 2.
• Third: Remove bullet at index 2. Now bullets.size() is 2. Decrement i to 1.
• Fourth: Remove bullet at index 1. Now bullets.size() is 1. Decrement i to 0.
• Fifth: Remove bullet at index 0. Now bullets.size() is 0. Decrement i to -1, which no longer evaluates to true in the for loop's conditional statement.

And we're done, now you can see all 5 bullets were removed. By iterating through the bullets in reverse you're able to alter the contents of the bullets container without affecting your loop. Some languages, like C#, will throw an exception when a container is altered while you're looping through from begin to end, for reasons like what you're experiencing here, reversing through the container should almost always be what you want to do when altering the container as you go through it.

There may very well be other leaks in your program as well. As others have mentioned already, a profiler might help you catch these leaks. Also, in every game I've made I always create a display UI that will show me the number of objects I have in certain containers, this way I can see in real-time if the number continues to rise and never go down. For example, if your is_on_screen method wasn't working properly, and was considering some bullets to always be on screen, they may never get erased. Having a UI to display the bullet count might help you spot this more quickly and easily.

• std::remove_if may be an easier, and more efficient way of doing this. cplusplus.com/reference/algorithm/remove_if Feb 27 '12 at 1:15
• Yes, I was just trying to keep it simple so he/she could easily see what was being done wrong. +1 for the good comment though. Feb 27 '12 at 4:55

You probably have a memory leak somewhere. Can't help you with that without thoroughly inspecting your code. Are you sure you are calling the destructors after removing them from the list? C++ doesn't have a garbage collector.

However in general you shouldn't, create, delete and then create lots of small objects. It's expensive and fragments your memory (which can also lead to some increase in memory usage). For objects like bullets you could use a pool. A pool is just a big list of instances of bullets that can be re-used. Instead of constructing a bullet when it's fired, and destructing when it hits something, you grab a bullet from the pool and use it, until it hits something when it is returned from the active actors list back to the pool.

A good article on this is: http://wiki.andengine.org/ObjectPool

• Roy T. is right. You are deleting them from vector (list), but you are not deleting them from memory. You have to delete them before removing them from vector (list). So try: if (!bullets[i].is_on_screen(screen)) { delete bullets[i]; bullets.erase(bullets.begin() + i); } Feb 27 '12 at 11:39
• And he is also right with pools. Feb 27 '12 at 11:41

In addition to Nic's solution, it is a good idea to add a profiler/Heap checker to your debug build, like Google profiler or Visual Leak Detector, to detected ASAP memory problems.

I found the answer! I had to use SDL_FreeSurface(); to free my SDL_Surface's!

• This shouldn't really be an answer... you should have put this as a comment or edit IMHO. Its to localized and not really relevant to your question (despite fixing the problem at hand). Feb 27 '12 at 22:54

use iterators like that

for(vector<Object>::iterator it = v.begin(); it != v.end(); /* no ++it! */ ) {
if(ToBeDeleted(*it))
it = v.erase(it);
else
++it;
}