Sign in to follow this  
lord_balron

Releasing an Object from a Vector

Recommended Posts

Ok, so I have a vector (C++):
std::vector<Object*> Children;
And I want to delete the Object from memory and remove it from the vector (otherwise there are crashes as its accessing a non-existent place in memory). The Objects have a variable containing their location in the vector so you can access it with.
Children[PlaceInVector];
This is given to it with this code...
void Particle::AddChild(Object* Child)
{
	Children.push_back(Child);
	int i = 0;
	//for (int i = 0; i < Children.size(); i++)
	while (Children[i] != Child)
	{
		Child->PositionInVector = i;
		i++;
	}
	Child->Parent = this;
}
What problem I'm having is that when I go to release it with:
Place = Place - 1;
Children.erase(Children.begin() + Place);
I get an error (through the visual studio realtime debugger), that my program is "Access violation reading location 0x00000000" I have played around with:
Place = Place - 1;
Children.erase(Children.begin() + Place);

//To this
Children.erase(Children.begin() + (Place - 1));

//And this
Children.erase(Children.begin() + Place);

But am still getting that error. I have no idea what to do.

Share this post


Link to post
Share on other sites
Quote:
Original post by lord_balron
The Objects have a variable containing their location in the vector so you can access it with.

That is a bad idea. As soon as you remove something from the vector, things start getting out of sync. Why exactly do those objects need to know their position in the vector? When are you removing these childs - can you show that code? Who is removing it? Who owns those child objects (so, who is responsible for deallocating them)?

Share this post


Link to post
Share on other sites
There are some problems with the approach you're taking here, but I think the first thing to do would be to tell us what Place is. (Also, perhaps you could post the functions from which the last two code excerpts were taken.)

Share this post


Link to post
Share on other sites
I'd guess that you have that code inside a member function and you're calling that member function on a null pointer.

Share this post


Link to post
Share on other sites
	///
///@brief Adds a child object to the Object's children vector
///@param Child The given child to be put in the children vector.
///
void Object::AddChild(Object* Child)
{
Children.push_back(Child);
int i = 0;
//for (int i = 0; i < Children.size(); i++)
while (Children[i] != Child)
{
i++;
Child->PositionInVector = i;
}
Child->Parent = this;
}

void Object::ReleaseChild(int Place)
{
delete Children[Place];
//Children.erase(Children.begin());
}

///
///@brief Releases the object and all of its children from memory.
///Goes through the vector of children and calls their Release functions,
///it then deletes itself.
///
void Object::Release()
{
for (int Iter = 0; Iter < Children.size(); Iter++)
{
Children[Iter]->Release();
}
Parent->ReleaseChild(PositionInVector);
}


This is the Object code (these are virtual functions as other objects derive from it). PlaceInVector is the location in the vector so I can access it to remove it from the vector, Children[PlaceInVector], it's given to it when its added to the vector, but I can see now what could be wrong with it. My problem is that I wanted to be able to remove an object from the vector when I no longer needed it, say as in a particle that has a given lifetime that after that time has passed the particle would remove itself from the vector and from memory.

Share this post


Link to post
Share on other sites
Is "PositionInVector" used for anything except deciding which child to remove? Because you could just use something like this:

class Object
{
typedef std::vector<Object *> Children;

public:
void addChild(Object *child)
{
children.push_back(child);
child->parent = this;
}

// use: parent->removeChild(this);
void removeChild(Object *child)
{
Children::iterator it = std::find(children.begin(), children.end(), child);
if(it != children.end())
{
children.erase(it);
}
}

private:
Children children;
};



Be careful though. It appears that with your original code the child is deleted inside the removeChild() function, which looks very dangerous. If you had some explicit way to manage the ownership, such as smart pointers, it would probably be better.

An approach like the above or your original runs the risk of pointer related bugs, especially for a beginner.


A totally different approach would be to let the parent handle the lifetime of the Objects. For example, if the objects have a boolean function isDead(), you could do this:

class Object
{
typedef boost::shared_ptr<Object> ObjectPtr;
typedef std::vector<ObjectPtr> Children;

static bool dead(const ObjectPtr &ptr) { return ptr->isDead(); }
public:

bool isDead() const; // could be virtual or whatever

void addChild(ObjectPtr child) { children.push_back(child); }

void update()
{
// remove dead children
Children::iterator it = std::remove_if(children.begin, children.end(), dead);
children.erase(it, children.end());

// ...
}
};


And for future note: an hour is actually quite a short time for a forum. You should wait a *lot* longer before bumping. Certain times are busier than others, so someone will probably see your post before long.

Share this post


Link to post
Share on other sites
Well I tried what you said rip-off, but I'm getting the same thing, when it goes to release itself through,

Parent->ReleaseChild(this);

It fails in the exact same way. Visual studio pops up and says Unhandled exception at 0x0040173b in MesaApp.exe: 0xC0000005: Access violation reading location 0x00000057.

This has been what I've been getting all along, everything is fine until it goes to release itself.

I don't know how to fix it, this is how my Release works:

void Particle::AddChild(Object* Child)
{
Children.push_back(Child);
Child->Parent = this;
}

void Particle::ReleaseChild(Object* ToRemove)
{
std::vector<Object*>::iterator it = std::find(Children.begin(), Children.end(), ToRemove);
if(it != Children.end())
{
Children.erase(it);
}
}

void Particle::Release()
{
for (int Iter = 0; Iter < Children.size(); Iter++)
{
Children[Iter]->Release();
}
Parent->ReleaseChild(this);
delete this;
}



EDIT:
Quote:
Original post by rip-off
And for future note: an hour is actually quite a short time for a forum. You should wait a *lot* longer before bumping. Certain times are busier than others, so someone will probably see your post before long.


Yeah, I'm just antsy is all.

Share this post


Link to post
Share on other sites
So I've implemented your other suggestion, the one of using dead and such, but what is happening is the Objects aren't being removed, they are still being drawn which shouldn't be as they are only drawn if they are in the vector.

I don't have any crashing though.

Share this post


Link to post
Share on other sites
Why are you delete-ing 'this' in the Release function? Why aren't you using a destructor to release a particle's resources?

Take a look at this:
Particle::~Particle()
{
// Make sure my children are also removed when I am being removed
ClearAllChildren();
}

void Particle::ClearAllChildren()
{
// Delete all children (they should delete their children from within
// their destructors, just like Particle's destructor does)
for(int i = 0; i < Children.size(); i++)
delete Children[i];
Children.clear();
}

Now, when you call ClearAllChildren, you remove all of a Particle's children, and their children, while the particle itself stays alive. Calling it a second time is no problem, because it's Children vector has been cleared.

When a Particle object is removed (you new-ed it and are now delete-ing it, or it goes out of scope - either way, it's destructor is being called), it will also always remove it's children.

Share this post


Link to post
Share on other sites
Quote:

It fails in the exact same way. Visual studio pops up and says Unhandled exception at 0x0040173b in MesaApp.exe: 0xC0000005: Access violation reading location 0x00000057.

It is as SiCrane said, you are calling Release() on a null pointer. This is probably in the calling code.

Quote:

So I've implemented your other suggestion, the one of using dead and such, but what is happening is the Objects aren't being removed, they are still being drawn which shouldn't be as they are only drawn if they are in the vector.

Have you checked in the debugger to ensure that they are dead()? That when they are dead() that the call to remove them actually occurs.

We cannot diagnose issues in code we cannot see.

Share this post


Link to post
Share on other sites
Quote:
Original post by lord_balron
The Objects have a variable containing their location in the vector so you can access it with.


As mentioned, this is a very bad idea.

Quote:

This is given to it with this code...
void Particle::AddChild(Object* Child)
{
Children.push_back(Child);
int i = 0;
//for (int i = 0; i < Children.size(); i++)
while (Children[i] != Child)
{
Child->PositionInVector = i;
i++;
}
Child->Parent = this;
}


So every time you add a child, you re-number all the previous children, even though their places haven't changed. Then, because of how your while condition works, you don't actually number the child that was just added (note that the commented-out approach would have worked). You also depend on operator== working properly for Objects. And why is a Particle allowed to have any kind of Object as a child, anyway?

Quote:

What problem I'm having is that when I go to release it with:

Place = Place - 1;
Children.erase(Children.begin() + Place);


I get an error (through the visual studio realtime debugger), that my program is "Access violation reading location 0x00000000"


1) Have you tried verifying the value of Place?
2) After erasing an element like this, all the elements that followed it will get shifted down, and need to be renumbered. There's no code anywhere to do that.

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