• Announcements

    • khawk

      Download the Game Design and Indie Game Marketing Freebook   07/19/17

      GameDev.net and CRC Press have teamed up to bring a free ebook of content curated from top titles published by CRC Press. The freebook, Practices of Game Design & Indie Game Marketing, includes chapters from The Art of Game Design: A Book of Lenses, A Practical Guide to Indie Game Marketing, and An Architectural Approach to Level Design. The GameDev.net FreeBook is relevant to game designers, developers, and those interested in learning more about the challenges in game development. We know game development can be a tough discipline and business, so we picked several chapters from CRC Press titles that we thought would be of interest to you, the GameDev.net audience, in your journey to design, develop, and market your next game. The free ebook is available through CRC Press by clicking here. The Curated Books The Art of Game Design: A Book of Lenses, Second Edition, by Jesse Schell Presents 100+ sets of questions, or different lenses, for viewing a game’s design, encompassing diverse fields such as psychology, architecture, music, film, software engineering, theme park design, mathematics, anthropology, and more. Written by one of the world's top game designers, this book describes the deepest and most fundamental principles of game design, demonstrating how tactics used in board, card, and athletic games also work in video games. It provides practical instruction on creating world-class games that will be played again and again. View it here. A Practical Guide to Indie Game Marketing, by Joel Dreskin Marketing is an essential but too frequently overlooked or minimized component of the release plan for indie games. A Practical Guide to Indie Game Marketing provides you with the tools needed to build visibility and sell your indie games. With special focus on those developers with small budgets and limited staff and resources, this book is packed with tangible recommendations and techniques that you can put to use immediately. As a seasoned professional of the indie game arena, author Joel Dreskin gives you insight into practical, real-world experiences of marketing numerous successful games and also provides stories of the failures. View it here. An Architectural Approach to Level Design This is one of the first books to integrate architectural and spatial design theory with the field of level design. The book presents architectural techniques and theories for level designers to use in their own work. It connects architecture and level design in different ways that address the practical elements of how designers construct space and the experiential elements of how and why humans interact with this space. Throughout the text, readers learn skills for spatial layout, evoking emotion through gamespaces, and creating better levels through architectural theory. View it here. Learn more and download the ebook by clicking here. Did you know? GameDev.net and CRC Press also recently teamed up to bring GDNet+ Members up to a 20% discount on all CRC Press books. Learn more about this and other benefits here.
Sign in to follow this  
Followers 0
bean.

Cleaning up a vector of pointers?

27 posts in this topic

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.

[code]
vector _mazeMap;

_mazeMap.push_back(new Wall(x, y));
[/code]

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:

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

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.
0

Share this post


Link to post
Share on other sites
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.

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

for (iter = _mazeMap.begin(); iter != _mazeMap.end(); ++iter)
{
delete *iter;
}
_mazeMap.clear();
}
[/CODE]
0

Share this post


Link to post
Share on other sites
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.
1

Share this post


Link to post
Share on other sites
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.

[code]

// 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++;
}
}
}
[/code]


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.
0

Share this post


Link to post
Share on other sites
[quote name='jnmacd' timestamp='1336663054' post='4939010']
What is your wall destructor?
[/quote]

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.
0

Share this post


Link to post
Share on other sites
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.
0

Share this post


Link to post
Share on other sites
[quote name='clb' timestamp='1336664347' post='4939018']
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.
[/quote]

This is the output I get when that printf statement executes: [b]0x0xc00000001012f520[/b]
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.
0

Share this post


Link to post
Share on other sites
Instead of "delete *iter; ", try this:
[code]if (*iter) {delete *iter; *iter = NULL;}[/code]

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

Share this post


Link to post
Share on other sites
[quote name='mhagain' timestamp='1336666026' post='4939032']
Instead of "delete *iter; ", try this:
[code]if (*iter) {delete *iter; *iter = NULL;}[/code]

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

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.

[code]

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)
[/code]

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.
[img]http://oi48.tinypic.com/2ppelnb.jpg[/img]

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.
0

Share this post


Link to post
Share on other sites
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
0

Share this post


Link to post
Share on other sites
[quote name='jnmacd' timestamp='1336671477' post='4939058']
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?
[/quote]

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.

[code]

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;

};
[/code]

[code]

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;
};
[/code]
0

Share this post


Link to post
Share on other sites
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.
1

Share this post


Link to post
Share on other sites
[quote name='madRenEGadE' timestamp='1336675026' post='4939078']
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.
[/quote]

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.

[quote name='bean.' timestamp='1336675002' post='4939077']
Here are the 2 class definitions.
[/quote]
Can you also post the Wall and VisibleGameObject constructors and destructors?
0

Share this post


Link to post
Share on other sites
[quote name='KymikoLoco' timestamp='1336678139' post='4939098']
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.
[/quote]

You are totally right! The hard way is sometimes better (that's why we c++ nerds are often better programmers ;)
0

Share this post


Link to post
Share on other sites
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.

[code]

// 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()
{

}
[/code]

[code]

// 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;
}
}
[/code]

Edit: Sorry about that. I'm not sure what happened. Code fixed. Edited by bean.
0

Share this post


Link to post
Share on other sites
1. Your last post isn't complete. ( I realized a little late you said your dtors were empty, lol )
2. [url="http://www.touch-code-magazine.com/how-to-debug-exc_bad_access/"]http://www.touch-code-magazine.com/how-to-debug-exc_bad_access/[/url]

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
[CODE]vector _mazeMap;
std::vector::iterator iter = _mazeMap.begin();[/CODE]
which don't seem like correct syntax.
[CODE]vector< Wall * > _mazeMap;
std::vector< Wall* >::iterator iter = _mazeMap.begin();[/CODE]
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.
1

Share this post


Link to post
Share on other sites
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.
0

Share this post


Link to post
Share on other sites
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.)
[CODE]

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

for (iter = _mazeMap.begin(); iter != _mazeMap.end(); ++iter)
{
delete *iter;
}
}
[/CODE]
0

Share this post


Link to post
Share on other sites
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
1

Share this post


Link to post
Share on other sites
I don't think you need to use 'new Maze(int x, int y)' to allocate memory for a wall, because the vector automatically allocates memory for an object when you use push_back().

Try this instead:

//Create a local variable of type Wall before you push back
Wall myWall(_some_X_, _some_Y_);

//Push back myWall without using new
_mazeMap.push_back(myWall);

after this, you don't need to delete the pointers, just use _mazeMap.pop_back() until the vector is cleared.

And, there's a thing with vectors, that when a vector size is adjusted (you push_back more objects than the reserved size), all the objects on the vector are reallocated, so the old adresses will be lost. I can't explain very well, but if you read about vectors, you'll know more.

Hope i helped.
0

Share this post


Link to post
Share on other sites
Whenever I hear this same problem description, my first advice is to google for "C++ rule of three" -- the first hit probably has the solution to your problem.

As 3333360 pointed out, you could also try switching to using a value container instead of a pointer container.
1

Share this post


Link to post
Share on other sites
In order to make sure the destructor of Map is called only once, maybe you could put a break point on its first line to check this?
Does the program crash when you use clear() on your vector<> at the end of the destructor ?
My 2 cents
0

Share this post


Link to post
Share on other sites
[quote name='Dragonsoulj' timestamp='1336805153' post='4939513']
Could it be his iterator itself is trying to access what got deleted?
[/quote]
No. It could be him attempting to access an iterator after he's invalidated it, but iterators by themselves will not do anything.

[quote name='3333360' timestamp='1336818710' post='4939532']
after this, you don't need to delete the pointers, just use _mazeMap.pop_back() until the vector is cleared.
[/quote]
or just call _mazeMap.clear();

[quote name='3333360' timestamp='1336818710' post='4939532']
And, there's a thing with vectors, that when a vector size is adjusted (you push_back more objects than the reserved size), all the objects on the vector are reallocated, so the old adresses will be lost. I can't explain very well, but if you read about vectors, you'll know more.
[/quote]
That has nothing to do with the memory he's allocated using new.

My suggestion would be to use [url="http://www.boost.org/doc/libs/1_49_0/libs/ptr_container/doc/ptr_vector.html"]boost::ptr_vector[/url] if you're going to insist on doing your own memory management. Edited by Washu
0

Share this post


Link to post
Share on other sites

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  
Followers 0