Cleaning up a vector of pointers?

Started by
26 comments, last by Storyyeller 11 years, 11 months ago
Use your debugger and step through the deletion.
Lots of stuff happens when you free memory. Not only does the destructor of Wall get called, but also the destructors of any object that Wall contained and VisibleGameObject.

Post the class definition of both Wall and VisibleGameObject

Edit: Does GDB not automatically break execution when it crashes?? The debugger should catch this. What IDE are you using?
Advertisement

Use your debugger and step through the deletion.
Lots of stuff happens when you free memory. Not only does the destructor of Wall get called, but also the destructors of any object that Wall contained and VisibleGameObject.

Post the class definition of both Wall and VisibleGameObject

Edit: Does GDB not automatically break execution when it crashes?? The debugger should catch this. What IDE are you using?


I use xcode (developing on a macbook). I don't have much experience with my debugger, so I'm going to go do some research and figure out how to properly use it. Here are the 2 class definitions.



class Wall : public VisibleGameObject
{
public:

// Functions explained in implementation.
Wall(int x, int y);
~Wall();

int GetCellX();
int GetCellY();

void Draw(sf::RenderWindow& rw);

private:

// X and Y position member variables.
int _cellPosX;
int _cellPosY;

};




class VisibleGameObject
{
public:
VisibleGameObject();
virtual ~VisibleGameObject();

// Loads an image for the object.
virtual void Load(std::string filename);
// Draws the object to the screen.
virtual void Draw(sf::RenderWindow& window);
// Updates the object's position.
virtual void Update(float elapsedTime);

// Sets the position of the sprite.
virtual void SetPosition(float x, float y);
// Returns a Vector for the position of the sprite.
virtual sf::Vector2f GetPosition() const;
// Boolean value for the image/sprite being loaded
virtual bool IsLoaded() const;

protected:
// Provides access to the sprite
sf::Sprite& GetSprite();

private:
// Member variables
sf::Sprite _sprite;
sf::Image _image;
std::string _filename;
bool _isLoaded;
};
If you want to avoid manual deletion of the pointers you could use a vector of smart pointers instead of raw pointers. C++11 offers you std::shared_ptr.

If you want to avoid manual deletion of the pointers you could use a vector of smart pointers instead of raw pointers. C++11 offers you std::shared_ptr.


Seeing as OP has commented multiple times they are not very strong with programming, this may be pointing them to a completely different direction, especially when I am under the impression that if they figure out this problem, they will learn more about memory management than they will from the moving to the new C++ standard.


Here are the 2 class definitions.

Can you also post the Wall and VisibleGameObject constructors and destructors?

Seeing as OP has commented multiple times they are not very strong with programming, this may be pointing them to a completely different direction, especially when I am under the impression that if they figure out this problem, they will learn more about memory management than they will from the moving to the new C++ standard.


You are totally right! The hard way is sometimes better (that's why we c++ nerds are often better programmers ;)
Thanks for all the help so far! Yeah, I'm just trying to tackle this right now, cause if I can get this to work, then my game (final project for my 1st programming course) will be done. Here's the constructors and destructors of Wall and VisibleGameObject, and an extra function or two that might be necessary to see what's going on.



// Loads the wall from an image. Sets its position.
Wall::Wall(int x, int y) : _cellPosX(x), _cellPosY(y)
{
Load("images/Wall.png");
SetPosition(_cellPosX * Game::CELL_SIZE, _cellPosY * Game::CELL_SIZE);
assert(IsLoaded());
}

Wall::~Wall()
{

}




// When an object is created, it is not yet loaded.
VisibleGameObject::VisibleGameObject()
{
_isLoaded=false;
}

VisibleGameObject::~VisibleGameObject()
{

}

// Loads an image and sets it as the sprite.
void VisibleGameObject::Load(std::string filename)
{
// Handles the image not loading.
if(_image.LoadFromFile(filename) == false)
{
_filename = "";
_isLoaded = false;
}
// Sets the image for the sprite.
else
{
_filename = filename;
_sprite.SetImage(_image);
_isLoaded = true;
}
}


Edit: Sorry about that. I'm not sure what happened. Code fixed.
1. Your last post isn't complete. ( I realized a little late you said your dtors were empty, lol )
2. http://www.touch-code-magazine.com/how-to-debug-exc_bad_access/

This might help a little. It seems as though something is accessing some allocated memory after it was released, so your delete/clean up steps are most likely correct, but somewhere down the line something ELSE is trying to access memory you've already freed. Since commenting out the delete *iter stopped the error from happening, the culprit is most likely _mazeMap.

3. Since I am not familiar with xcode or GDB debugger, what is _mazeMap's and iter's REAL definitions? You have
vector _mazeMap;
std::vector::iterator iter = _mazeMap.begin();

which don't seem like correct syntax.
vector< Wall * > _mazeMap;
std::vector< Wall* >::iterator iter = _mazeMap.begin();

It's better to post *exactly* what you have, instead of cleaning it up for a post.

I would also try the _mazeMap.clear(). If you don't, you basically have _mazeMap full of garbage pointers.

4. Get familiar with breakpoints. They will be your best friend in situations like this.
I learned a good deal about debugging today, a lot more than I knew before. However, all I was able to gather was that something was releasing my pointers before the destructor of Map was being called. So, I just took out the destructor, and used Instruments to run it and check it for Memory Leaks. It didn't detect any, so, I'm not sure what's happening. I tried using NSZombie to help, but it didn't get me too far. Using breakpoints, I noticed, that right before delete *iter is called, the value of *iter is non existant, as it does not have the X and Y values that a *Wall should have.

Is there any way that pointers could get released before calling delete on them? I'm not using smart pointers. Unless my pointers are magically smarter than I thought.
I would set breakpoints on every line of code that has _mazeMap on it, and make sure it has what you think it does on every line.

And is this code EXACTLY what you have in your Map destructor? (My guess is no, if that is the case, post the actual dtor.)


Map::~Map()
{
std::vector::iterator iter = _mazeMap.begin();

for (iter = _mazeMap.begin(); iter != _mazeMap.end(); ++iter)
{
delete *iter;
}
}
Not to mention that you better make sure that there is absolutely no location in your code that creates a copy of the map or calls a function where you pass it by value. It at any point you create a temporary copy of that map, all your pointers will be invalid and your walls deleted.

If that is the case, it might be time to google for the Rule of Three.

Did you place a breakpoint in the destructor and make sure it only gets called once? Debug output might actually work better, as some debuggers are notoriously bad with breakpoints in destructors.

If in doubt, place a breakpoint in the destructor of your Wall class and check the call stack when you hit it to see where it's getting called.

Things don't get magically deleted on their own. A more far fetched explanation would be writing out of bounds and screwing up the internal pointer of the vector.
f@dzhttp://festini.device-zero.de

This topic is closed to new replies.

Advertisement