Sign in to follow this  

Memory leak using STL List

This topic is 3744 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hi, I am writing a simple 2d game demo and I store all of my projectiles in a list called ProjectileList and I check to see if they have gone off screen, in which I erase them from the list. I have noticed however that when I fire a projectile in game, the amount of memory the program takes up keeps growing and never goes down. So I believe it is most likely to do with how I am getting rid of my projectiles... Here is my code... that should help
std::list<Projectile*> ProjectileList;
...
...
void Player::fireProjectile()
{
	Projectile *projectile;
	projectile = new Projectile(5000);

	D3DXVECTOR3 playerPos = GetPosition();

	playerPos.x += 64;
	playerPos.y += 32;

	projectile->Init("projectile.tga", 16, D3DXVECTOR3(playerPos.x, playerPos.y, 0), true);

	ProjectileList.push_back(projectile);
}
...
...
// Remove projectiles
for( std::list<Projectile*>::iterator node = ProjectileList.begin(); node != ProjectileList.end(); )
{
	if((*node)->IsDead())
	{
		(*node) = NULL;
		node = ProjectileList.erase(node);
	}
	else
	        ++node;
	}
I do know the remove projectiles area is being called as my projectiles and other objects I use similar code will stop rendering and updating when the erase has been called.

Share this post


Link to post
Share on other sites
Quote:
Original post by matth
I tried something like...

(*node) = NULL;
delete (*node);
node = ProjectileList.erase(node);

But didn't seem to make a difference.


You're NULLing the pointer before you delete it, so the memory still isn't getting deallocated.

Try:
delete (*node);
node = ProjectileList.erase(node);

There's really no need to make the pointer NULL at all.

Share this post


Link to post
Share on other sites
It's worth noting that task manager isn't a good indication of how much memory you're using. It's unlikely to go down much because the runtime library will keep blocks of memory around in case you want to allocate memory again.

Although, if it keeps going up, it's a rough indication that you're leaking memory somewhere.

Share this post


Link to post
Share on other sites
Yea, basically I just have a simple game coded directly in Direct X 9 using LPDIRECT3DTEXTURE9's for my player and projectiles. Here is the Sprite class that handles them both.


... deleted my code since i narrowed down where leak was occuring


[Edited by - matth on September 15, 2007 3:58:02 PM]

Share this post


Link to post
Share on other sites
I've narrowed down where the leak was occurring, which again goes to a list function.

I create the projSwap and enemySwap because I had trouble feeding the iterator into the SpriteCollisionTest functions. However, if I am to delete projSwap and enemySwap after, I am getting a crash - potentially because it is deleting the object at the address which is the same object that the iterator is storing the address of?


// Check for Collisions
for( std::list<Projectile*>::iterator projNode = ProjectileList.begin(); projNode != ProjectileList.end(); ++projNode )
{
Projectile *projSwap;
projSwap = new Projectile();
projSwap = (*projNode);

if(SpriteCollisionTest(dynamic_cast<Sprite&>(*projSwap), dynamic_cast<Sprite&>(*playerMothra)))
{
(*projNode)->Kill();
}
else
{
for( std::list<Enemy*>::iterator enemyNode = EnemyList.begin(); enemyNode != EnemyList.end(); ++enemyNode )
{
Enemy *enemySwap;
enemySwap = new Enemy();
enemySwap = (*enemyNode);

if(SpriteCollisionTest(dynamic_cast<Sprite&>(*projSwap), dynamic_cast<Sprite&>(*enemySwap)))
{
(*projNode)->Kill();
(*enemyNode)->Hit();
}
delete enemySwap;
}
}
delete projSwap;
}

Share this post


Link to post
Share on other sites
Here:
Quote:

projSwap = new Projectile();
projSwap = (*projNode);


And here:
Quote:

enemySwap = new Enemy();
enemySwap = (*enemyNode);


You overwrite the pointers so you cannot call delete them.

I think you should investigate smart pointers. boost::shared_ptr<> is an excellent example.

Also, this:
Quote:
if(SpriteCollisionTest(dynamic_cast<Sprite&>(*projSwap), dynamic_cast<Sprite&>(*playerMothra)))

Will not work as you intend, as a bad dynamic_cast on a reference will throw an exception (it cannot return a "null" reference because no such thing exists). It is one of the few times it is preferable to use a pointer over a reference.

Share this post


Link to post
Share on other sites
Quote:
Original post by matth

Projectile *projSwap;
projSwap = new Projectile();//this is a memory leak
projSwap = (*projNode);

Enemy *enemySwap;
enemySwap = new Enemy(); //this is a memory leak
enemySwap = (*enemyNode);
}


You are allocating memory for a projectile object, then immediately assigning the pointer to a different block of memory. Don't do this.

Share this post


Link to post
Share on other sites
The following should work:



// Check for Collisions
for( std::list<Projectile*>::iterator projNode = ProjectileList.begin(); projNode != ProjectileList.end(); ++projNode )
{
Projectile *projSwap = (*projNode);

if(SpriteCollisionTest(dynamic_cast<Sprite&>(*projSwap), dynamic_cast<Sprite&>(*playerMothra)))
{
(*projNode)->Kill();
}
else
{
for( std::list<Enemy*>::iterator enemyNode = EnemyList.begin(); enemyNode != EnemyList.end(); ++enemyNode )
{
Enemy *enemySwap = (*enemyNode);

if(SpriteCollisionTest(dynamic_cast<Sprite&>(*projSwap), dynamic_cast<Sprite&>(*enemySwap)))
{
(*projNode)->Kill();
(*enemyNode)->Hit();
}
//delete enemySwap; <---remove
}
}
//delete projSwap; <---remove
}





Keep in mind smart pointers are NOT STL container safe. I normally store
plain old pointers in the container and when I need to move it around I throw
it into a smart pointer.

Share this post


Link to post
Share on other sites
Quote:
Original post by Moss
Keep in mind smart pointers are NOT STL container safe. I normally store
plain old pointers in the container and when I need to move it around I throw
it into a smart pointer.


std::auto_ptr isn't, boost::shared_ptr is.

Share this post


Link to post
Share on other sites
Quote:
Original post by Moss
The following should work:

*** Source Snippet Removed ***


Not really. You are deleting every item in the list, but still leaving them in there. I get the feeling that you shouldn't be deleting any of them, but if you do, remove them from the list so you don't access deleted memory later.

Share this post


Link to post
Share on other sites
Thanks for all the help guys - as usual I am able to find my errors a whole lot easier once I start asking questions.

I changed the following line :

if(SpriteCollisionTest(dynamic_cast<Sprite&>(*projSwap), dynamic_cast<Sprite&>(*enemySwap)))

over to:

if(SpriteCollisionTest(dynamic_cast<Sprite&>(**projNode), dynamic_cast<Sprite&>(**enemyNode)))

And took out all the unneeded enemySwap/projSwap stuff

Share this post


Link to post
Share on other sites
Quote:
Original post by matth
Thanks for all the help guys - as usual I am able to find my errors a whole lot easier once I start asking questions.

I changed the following line :

if(SpriteCollisionTest(dynamic_cast<Sprite&>(*projSwap), dynamic_cast<Sprite&>(*enemySwap)))

over to:

if(SpriteCollisionTest(dynamic_cast<Sprite&>(**projNode), dynamic_cast<Sprite&>(**enemyNode)))

And took out all the unneeded enemySwap/projSwap stuff


Did you miss the last bit of my post?

Quote:
Source: me [smile]
Quote:
if(SpriteCollisionTest(dynamic_cast<Sprite&>(*projSwap), dynamic_cast<Sprite&>(*playerMothra)))

Will not work as you intend, as a bad dynamic_cast on a reference will throw an exception (it cannot return a "null" reference because no such thing exists). It is one of the few times it is preferable to use a pointer over a reference.

Share this post


Link to post
Share on other sites
Is there any reason that you have to do all these dynamic allocations, anyway?

Why not a list<Projectile> instead of a list<Projectile *>?

It's probably just as efficient for small objects (which I assume Projectile is) and you won't have to worry about when to delete, it it's null-valued, what happens in the case of exceptions and adding deletes in the case of early returns and etc.

Edd

Share this post


Link to post
Share on other sites

This topic is 3744 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this