Jump to content

  • Log In with Google      Sign In   
  • Create Account


Cleaning up a vector of pointers?


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.

  • You cannot reply to this topic
27 replies to this topic

#1 bean.   Members   -  Reputation: 102

Like
0Likes
Like

Posted 10 May 2012 - 08:46 AM

Hey everybody. I've run into a snag, and I can't quite figure it out. I'm making a maze game with c++ and SFML.

My problem is with my Map class. My map class parses a text file (which is a text version of the map grid), and reads all 1s as Walls and all 0s as blank spaces. The 0s are ignored, and for each 1 that it encounters, it takes the coordinates of the '1' and creates a pointer to a Wall object with those coordinates, and adds it to my _mazeMap vector.

vector _mazeMap;

_mazeMap.push_back(new Wall(x, y));

is the gist of it.

My problem is cleaning up the memory. In my Map class' destructor I figure I need to delete all the pointers that were created in the vector, correct? So this is the code I wrote to try and do it:

Map::~Map()
{
    std::vector::iterator iter = _mazeMap.begin();
    
    for (iter = _mazeMap.begin(); iter != _mazeMap.end(); ++iter)
    {
        delete *iter;
    }
}

However, every time I run the game, when I shut it down, and the Destructor gets run, I get this error:

Thread 1: Program received signal: "EXC_BAD_ACCESS"

I have no idea what I'm doing wrong. Any help? Thanks so much.

Sponsor:

#2 jnmacd   Members   -  Reputation: 197

Like
0Likes
Like

Posted 10 May 2012 - 08:57 AM

What line is it actually crashing at?
Unless one of these pointers already got deleted somewhere else, there is nothing wrong with your loop.
However, it is always a good idea to also empty the vector when you are done... but this shouldn't be a problem here.

Map::~Map()
{
    std::vector::iterator iter = _mazeMap.begin();
   
    for (iter = _mazeMap.begin(); iter != _mazeMap.end(); ++iter)
    {
	    delete *iter;
    }
    _mazeMap.clear();
}


#3 clb   Members   -  Reputation: 1777

Like
1Likes
Like

Posted 10 May 2012 - 09:14 AM

You have confirmed that commenting out only the 'delete *iter;' line removes the crash?

In that case, I suspect you are either doing double delete, or the 'this' pointer in Map dtor is bad to start with. Check that you don't make value copies of the Map object, or that if you do, you have proper copy-ctor and assignment operators defined, since otherwise making copies of _mazeMap containers will cause duplicates of the pointers to be created, leading to a double delete on the long run.
Me+PC=clb.demon.fi | C++ Math and Geometry library: MathGeoLib, test it live! | C++ Game Networking: kNet | 2D Bin Packing: RectangleBinPack | Use gcc/clang/emcc from VS: vs-tool | Resume+Portfolio | gfxapi, test it live!

#4 bean.   Members   -  Reputation: 102

Like
0Likes
Like

Posted 10 May 2012 - 09:15 AM

It gives me the bad access error on the
delete *iter
line.


I'm still not sure what's wrong. It's still happening, and I can't really understand it.
Here's my code that Loads the Map/Vector in case that would help at all.


// Loads the map from a text file.
void Map::Load()
{

	// Opens the file.
	std::ifstream loadedFile;
	loadedFile.open("images/level.txt");

	// Variable that will hold each line for parsing.
	std::string line;

	// Counts how many rows have been completed.
	int rowcounter = 0;

	// Makes sure the file has opened.
	if(loadedFile.is_open())
	{
		// Remains true until the file has ended.
		// Loops through each line.
		while(std::getline(loadedFile, line))
		{
			// Once the line has been put into the string variable,
			// this will go through each character an check for walls.
			for (int i = 0; i < LEVELWIDTH; i++) {

				// If a wall (a '1' character in the text file) is found
				// then this will put it into the vector.
				if (line[i] == '1') {
					_mazeMap.push_back(new Wall(i, rowcounter));
				}
			}
			// After the line has been parsed, this iterates the counter.
			rowcounter++;
		}
	}
}


Edit:

Yes, commenting out the (delete *iter) line causes the bug to disappear.
As far as I'm aware, I don't make any copies of _mazeMap or Map objects. I have a single static Map object in my Game Global, and that's it.

Edited by bean., 10 May 2012 - 09:25 AM.


#5 jnmacd   Members   -  Reputation: 197

Like
0Likes
Like

Posted 10 May 2012 - 09:17 AM

What is your wall destructor?

#6 bean.   Members   -  Reputation: 102

Like
0Likes
Like

Posted 10 May 2012 - 09:27 AM

What is your wall destructor?


It's empty. Is that where my problem lies? I didn't give it much thought since it just has a few member variables and a draw function.

#7 clb   Members   -  Reputation: 1777

Like
0Likes
Like

Posted 10 May 2012 - 09:39 AM

Can you print out 'printf("0x%p", *iter);' just before calling 'delete *iter;' to confirm that double-delete doesn't occur?

Also, going for more exotic causes, can you check that you use the matching implementations of operator new and delete, and e.g. don't straddle the calls across module boundaries, or don't have any custom (debug-mode?) new/delete implementations going on.
Me+PC=clb.demon.fi | C++ Math and Geometry library: MathGeoLib, test it live! | C++ Game Networking: kNet | 2D Bin Packing: RectangleBinPack | Use gcc/clang/emcc from VS: vs-tool | Resume+Portfolio | gfxapi, test it live!

#8 bean.   Members   -  Reputation: 102

Like
0Likes
Like

Posted 10 May 2012 - 09:59 AM

Can you print out 'printf("0x%p", *iter);' just before calling 'delete *iter;' to confirm that double-delete doesn't occur?

Also, going for more exotic causes, can you check that you use the matching implementations of operator new and delete, and e.g. don't straddle the calls across module boundaries, or don't have any custom (debug-mode?) new/delete implementations going on.


This is the output I get when that printf statement executes: 0x0xc00000001012f520
And that's all I get. I'm not extremely experienced at programming, just enough to know what I'm doing, I'd say. However, I'm not sure how to go about checking my implementations of new and delete. I haven't overloaded them, if that's what you're asking? How should I go about checking what implementations of new and delete I'm using? Thanks for the help.

Edit:
I commented out the (delete *iter) line with the printf statement still inserted. It gave me a ton of memory addresses. So, for some reason the program is crashing as soon as it tries to delete the first pointer.

Edited by bean., 10 May 2012 - 10:01 AM.


#9 mhagain   Crossbones+   -  Reputation: 7448

Like
0Likes
Like

Posted 10 May 2012 - 10:07 AM

Instead of "delete *iter; ", try this:
if (*iter) {delete *iter; *iter = NULL;}

Does the crash go away? If so you've definitely got double-delete, if not then you've got something a little more insidious.

It appears that the gentleman thought C++ was extremely difficult and he was overjoyed that the machine was absorbing it; he understood that good C++ is difficult but the best C++ is well-nigh unintelligible.


#10 bean.   Members   -  Reputation: 102

Like
0Likes
Like

Posted 10 May 2012 - 10:59 AM

Instead of "delete *iter; ", try this:

if (*iter) {delete *iter; *iter = NULL;}

Does the crash go away? If so you've definitely got double-delete, if not then you've got something a little more insidious.


The crash did not go away when I replaced the code.

Here's my debugger output if that sheds light on anything? I'm just lost. I can't figure out what I'm doing wrong.


GNU gdb 6.3.50-20050815 (Apple version gdb-1708) (Thu Nov  3 21:59:02 UTC 2011)
Copyright 2004 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB.  Type "show warranty" for details.
This GDB was configured as "x86_64-apple-darwin".tty /dev/ttys000
warning: Unable to read symbols for @executable_path/../Frameworks/sfml-audio.framework/Versions/A/sfml-audio (file not found).
warning: Unable to read symbols from "sfml-audio" (not yet mapped into memory).
warning: Unable to read symbols for @executable_path/../Frameworks/sfml-graphics.framework/Versions/A/sfml-graphics (file not found).
warning: Unable to read symbols from "sfml-graphics" (not yet mapped into memory).
warning: Unable to read symbols for @executable_path/../Frameworks/sfml-network.framework/Versions/A/sfml-network (file not found).
warning: Unable to read symbols from "sfml-network" (not yet mapped into memory).
warning: Unable to read symbols for @executable_path/../Frameworks/sfml-system.framework/Versions/A/sfml-system (file not found).
warning: Unable to read symbols from "sfml-system" (not yet mapped into memory).
warning: Unable to read symbols for @executable_path/../Frameworks/sfml-window.framework/Versions/A/sfml-window (file not found).
warning: Unable to read symbols from "sfml-window" (not yet mapped into memory).
warning: Unable to read symbols for @executable_path/../Frameworks/SFML.framework/Versions/A/SFML (file not found).
warning: Unable to read symbols from "SFML" (not yet mapped into memory).
[Switching to process 49627 thread 0x0]
0x0x1000000000000000sharedlibrary apply-load-rules all
Warning: the current language does not match this frame.
Current language:  auto; currently c++
(gdb)

Update: Does this help at all? It's a picture of my debug area, and I think it's showing what's going on underneath the iterator. I'm not sure I particularly understand it though.
Posted Image

Update 2:
Could my issue have anything to do with the fact that class Wall extends VisibleGameObject? I have a blank virtual destructor in VisibleGameObject and a blank destructor in Wall.

Edited by bean., 10 May 2012 - 11:14 AM.


#11 jnmacd   Members   -  Reputation: 197

Like
0Likes
Like

Posted 10 May 2012 - 11:37 AM

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?

Edited by jnmacd, 10 May 2012 - 11:39 AM.


#12 bean.   Members   -  Reputation: 102

Like
0Likes
Like

Posted 10 May 2012 - 12:36 PM

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;
};


#13 madRenEGadE   Members   -  Reputation: 190

Like
1Likes
Like

Posted 10 May 2012 - 12:37 PM

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.

#14 KymikoLoco   Members   -  Reputation: 195

Like
0Likes
Like

Posted 10 May 2012 - 01:28 PM

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?

#15 madRenEGadE   Members   -  Reputation: 190

Like
0Likes
Like

Posted 10 May 2012 - 01:33 PM

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 ;)

#16 bean.   Members   -  Reputation: 102

Like
0Likes
Like

Posted 10 May 2012 - 01:57 PM

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.

Edited by bean., 10 May 2012 - 08:50 PM.


#17 KymikoLoco   Members   -  Reputation: 195

Like
1Likes
Like

Posted 10 May 2012 - 02:58 PM

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.

#18 bean.   Members   -  Reputation: 102

Like
0Likes
Like

Posted 11 May 2012 - 02:33 PM

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.

#19 KymikoLoco   Members   -  Reputation: 195

Like
0Likes
Like

Posted 11 May 2012 - 03:06 PM

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;
    }
}


#20 Trienco   Crossbones+   -  Reputation: 2054

Like
1Likes
Like

Posted 12 May 2012 - 12:34 AM

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.

Edited by Trienco, 12 May 2012 - 12:38 AM.

f@dzhttp://festini.device-zero.de




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.



PARTNERS