View more

View more

View more

Image of the Day Submit

IOTD | Top Screenshots

The latest, straight to your Inbox.

Subscribe to GameDev.net Direct to receive the latest updates and exclusive content.

c++ : Deleting object (not just its pointer) from vector

Old topic!

Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

13 replies to this topic

#1chondee  Members

Posted 13 January 2012 - 02:07 AM

Hi Everyone,

I have recently started writing an engine in C++ / SDL for a horizontal scrolling shooter, and most of the functionality that I wanted for it is done.
Now I am trying to deal with the memory leaks, and with that I would appreciate some help.

I store my particles, stars, bullets, enemies in my World object, that has vectors for them to be contained, such as these:

vector<Enemy> v_enemies;
vector<Enemy>::iterator v_enemies_it;
vector<Particle> v_particles;
vector<Particle>::iterator v_particles_it;


new objects to these vectors are added in several different functions in the following way:

Particle a(xx, yy, 6);
v_particles.push_back(a);


when the vectors are dead and flagged to be deleted, I erase them from the vector with remove-erase idiom in the following way:
v_particles.erase(remove_if(v_particles.begin(), v_particles.end(), dead_p), v_particles.end());


I do understand, that the memory leak is because the fact that the vector.erase only deletes the pointer to the object, and not the object itself, however when I tried to step through the vector with its iterator manually in a loop, I have encountered crashes due to the fact that the vector iterator is invalidated when the vector is reallocated.

I would really appreciate it if someone could provide me a method of erasing dead object from the vector, in a way that the object itself is deleted, not only its pointer. Also, I suspect my way of adding the object to the vector might not be the best way either (should I use "new"?).

I hope I have not broken any forum rules. This is my first post, so please go easy on me .

(btw here is a short clip of the game engine so far: http://www.youtube.com/watch?v=EjaTcaEIQSU)

Posted 13 January 2012 - 02:12 AM

You are putting copies of a particle object on the vector. This copy will be destructed within vector::erase(). If you have allocated pointers in the particle object itself, its up to you to delete them.

#3chondee  Members

Posted 13 January 2012 - 02:23 AM

Thanks for the quick answer.
Are you saying that the vector itself contains the objects, and when erase is called not only a pointer gets deleted, but the object itself?

Below, I create particle a, and I create a copy of particle a with the vector.push_back()?
Particle a(xx, yy, 6);
v_particles.push_back(a);


If that is the case the memory leak is because the first Particle a(xx,yy,6); line, right?

Could you help me how I could make sure that the object is created only once, and the vector.erase will delete it anyway, or that I could delete the first object itself after its copy has been pushed back to the vector container?

EDIT:
I have modified all the code with vector.push_back like this:
Particle a(xx, yy, 6);
v_particles.push_back(a);
a.~Particle();


This way if the memory leak's cause would be that only the copy gets deleted and the originally created "a" keeps accumulating, however it still seems to be leaking memory, with the same rate as before..

Edited by chondee, 13 January 2012 - 02:48 AM.

#4Kazuo5000  Members

Posted 13 January 2012 - 02:58 AM

Your vectors are not storing pointers. If they were storing pointers they would be declared something like this:

vector<particle*> v_pParticles; // store a pointer to a particle

whereas you are storing a particle object. When you call erase for your vector this will call the destructor for the particle and thus destroy your particle. If however, you have pointers to textures or other objects that are not released or destroyed then when your particle object is destroyed this will leave memory leaks.

The objects you are storing in the vectors is where you should be looking for your memory leaks (in your example the Particle class and the Enemy class to see if every new or new [] has a delete or delete[] and all pointers are not left dangling).

#5brx  Members

Posted 13 January 2012 - 03:03 AM

Actually, I do not see any way that one of the lines you posted could cause a memory leak (except when one of your classes allocates memory inside its contructor that is not freed in the destructor). Since you are not constructing objects on the heap using new, all objects cou create are created on the stack. Those will be deleted automatically when they go out of scope. The vector itself stores copies of your object:
Particle a(xx, yy, 6);  // creates a new Particle object on the STACK
v_particles.push_back(a); // creates a new Particle object inside push_back and uses the copy constructor of Particle to do so.


In order to find memory leaks you should look into tools that do that for you, e.g. valgrind.

#6chondee  Members

Posted 13 January 2012 - 03:14 AM

Thanks, so I am actually storing the objects.
All objects that are being created in enemy and the likes are added and erased the same way.
Neither new or delete are called anywhere in the code.

The only other thing that I could have thought of causing a memory leak are the SDL_Surface -es, however I load all the images and create surfaces only during initialization, all the created objects only use pointers to those surfaces.

Just to make sure, I freed for example the surface enemies used, but when the first enemy died, all the other enemies' surface disappeared as well, so it means the newly created objects don't create surfaces with them.

The memory leak is pretty big too, like 1MByte / 5 seconds...

Anyways, thanks for the help

EDIT: brx, thanks for the advice, I'll check out valgrind

Edited by chondee, 13 January 2012 - 03:15 AM.

#7brx  Members

Posted 13 January 2012 - 03:18 AM

EDIT:
I have modified all the code with vector.push_back like this:

Particle a(xx, yy, 6);
v_particles.push_back(a);
a.~Particle();


Do not call destructors explicitly except when you REALLY know what you are doing. Read this: http://www.parashift.com/c++-faq-lite/dtors.html#faq-11.5

#8chondee  Members

Posted 13 January 2012 - 03:25 AM

Thanks, I didn't know that.

Since a is recreated as a copy during vector.push_back(), only the one in the vector will get used, shown and eventually deleted.

What happens to the originally created "a" Particle? If it just stays there, and takes up memory, how am I supposed to get rid of it, without calling its destructor?
Since it wasn't created with "new", "delete" cannot be called either, right?

#9Brother Bob  Moderators

Posted 13 January 2012 - 03:42 AM

Thanks, I didn't know that.

Since a is recreated as a copy during vector.push_back(), only the one in the vector will get used, shown and eventually deleted.

What happens to the originally created "a" Particle? If it just stays there, and takes up memory, how am I supposed to get rid of it, without calling its destructor?
Since it wasn't created with "new", "delete" cannot be called either, right?

The object's lifetime is managed automatically. Sooner or later it will go out of scope, at which point its destructor is called and the storage is released.

#10chondee  Members

Posted 13 January 2012 - 02:05 PM

Alright, thanks for all the responses, all of you were right, and the memory leak did not come from where I thought it did.
Since then I looked over all the code, disabling everything possible, and enabling them gradually, to see when the memory leak kicks in, and I have found the source of it.

Even though this isn't related to the thread title anymore, if it is not necessary I wouldn't start another for this, maybe some of you will spot the problem right away, I am not too experienced in c++, at least not in memory management, as I have never worked on anything big enough to make me worry about memory.

This is what causes the memory leak, this is a HUD function, to display the number of collected red orbs on top of the screen, using SDL_ttf.h
For me to be able to use that to output a number, I had to convert int to string :

void Hud::show_red_orb_counter()
{
red_orb_exact = w->myPlayer.p_orb_count();
if (red_orb_exact > red_orb_counter){
red_orb_counter++;
red_orb_counter_changing = true;
//if (frame % 2 == 0)

}
else
{
red_orb_counter_changing = false;
}
if (red_orb_counter_changing)
{
if (red_orb_counter_color.r < 245)
red_orb_counter_color.r = red_orb_counter_color.r + 10;
}
else
{
if (red_orb_counter_color.r > 115)
red_orb_counter_color.r--;
}
char buffer[33];
_itoa (red_orb_counter, buffer, 10);
red_orb_counter_surf = TTF_RenderText_Solid( font, buffer, red_orb_counter_color );
apply_surface( 295, 5, red_orb_counter_surf, screen );

}


I guess only the last four lines are relevant, probably the stuff with the conversion. I have found this int to const char solution somewhere, and copied it as it is, I am not really familiar with _itoa either.

Could someone tell me why this causes memory leak, and if this is a bad implementation for int to const char conversion, could you provide an alternate way?

#11ApochPiQ  Moderators

Posted 13 January 2012 - 02:25 PM

Are you sure the int-to-string conversion is the problem, and not your text rendering itself?
Wielder of the Sacred Wands

#12chondee  Members

Posted 13 January 2012 - 02:36 PM

I'm not sure, you might be right.
maybe it creates another surface every frame, I'll look into that now

EDIT:
Yeah, that was it

calling SDL_FreeSurface( red_orb_counter_surf ); afterwards solves it.

Thank you!

Edited by chondee, 13 January 2012 - 02:41 PM.

#13taz0010  Members

Posted 13 January 2012 - 08:16 PM

You always need to read the documentation to make sure you aren't causing memory leaks when using API functions. Functions with "create" in their name typically allocate memory, and functions with "get" sometimes increment reference counts. The documentation will tell you if you need to call the corresponding "free" or "release" functions.

#14chondee  Members

Posted 14 January 2012 - 03:12 PM

You always need to read the documentation to make sure you aren't causing memory leaks when using API functions. Functions with "create" in their name typically allocate memory, and functions with "get" sometimes increment reference counts. The documentation will tell you if you need to call the corresponding "free" or "release" functions.

Yes, you are right, I ran into this whole thing a bit hastily, I was too excited

Old topic!

Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.