Jump to content
  • Advertisement

Archived

This topic is now archived and is closed to further replies.

shadow12345

difficulty deleting pointers from std::vector

This topic is 5365 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

No matter what I do I cannot delete pointers from std::vectors. In this case, my std::vectors hold VWorldObject pointers. I am going through each leaf in my BSP destructor and trying to delete the entities from the appropriate std::vector if there are any.
	for(int i = 0; i < mNumLeafs; i++)
	{
		if(mpLeafs[i].entitylist.size() > 0)
		{
			std::for_each(mpLeafs[i].entitylist.begin(),mpLeafs[i].entitylist.end(),Deleter<VWorldObject>());
			mpLeafs[i].entitylist.clear();
		}

	}
here is the code for the Deleter structure:
template<class T>
struct Deleter 
{    
	void operator()(T* obj) 
	{        
		if(obj)
		{
			delete obj;    
		//	obj	=	NULL;

		}
	}
};
When I step through the code, it gives me an access violation in the destructor of VWorldObject. Specifically on this line when delete[] is called:
	if(mpBrushes)
	{
		delete[]	mpBrushes;
		mpBrushes	=	NULL;
		mnumOfBrushes = 0;
	}
The weird thing is the this pointer for the VWorldObject class is valid, so Im not sure why I am getting so many problems. EDIT: Did I even post enough information? I'm not sure if I'm being vague or not. [edited by - Shadow12345 on February 7, 2004 11:27:58 AM] [edited by - Shadow12345 on February 7, 2004 11:35:03 AM] [edited by - Shadow12345 on February 7, 2004 11:42:52 AM]

Share this post


Link to post
Share on other sites
Advertisement
Im gonna think that you want Deleter< VWorldObject* >() instead of Deleter< VWorldObject >(). Im making the assumption that you have VWorldObject pointers, otherwise you would not need to be deleting them, because they would automatically be deleted when the container object was deleted.

[EDIT]
Stupid HTML >_<


[edited by - porthios on February 7, 2004 11:38:13 AM]

Share this post


Link to post
Share on other sites
This is where it gets pretty complicated, but doing what you said doesn''t work, it gives errors (I''ve tried that before). Sorry about the HTML thing.

I am really stumped with this problem, I have *absolutely* no clue what is wrong. On the other hand, I''ve got a huge project, and I guess the problem could potentially be in a lot of different places. If anybody wants me to post more code I will, but I don''t want to post too much.

Share this post


Link to post
Share on other sites
The reason porthois' suggestion didn't work is because Delete is templated on class T and its operator() takes a parameter of type T*, so templating it on VWorldObject makes it delete VWorldObject*'s while templating it on VWorldObject* would make it delete VWorldObject**'s.

I don't think you've posted enough code for us to see where the problem is. Usually errors like this mean that either a) the WorldObject pointer is not valid or b) mpBrushes is not valid. Can you post the any code from VWorldObject that does anything with mpBrushes, including all constructors and assignment operators, and also the code where you populate your vector?

Enigma

EDIT: typo

[edited by - Enigma on February 7, 2004 11:56:41 AM]

Share this post


Link to post
Share on other sites
When I create a VWorldObject I set mpBrushes equal to NULL.
Here's the tEntBrush class and the tEntPlane classes that I am using.



struct tEntPlane
{
tEntPlane()
{
d = 0;
vNormal = Vector3(0,0,0);
}
float d;
Vector3 vNormal;
};

struct tEntBrush
{
tEntBrush()
{
numPlanes = 0;
mpPlanes = NULL;
textureID = 0;
}
~tEntBrush()
{
if(mpPlanes)
{
delete[] mpPlanes;
mpPlanes = NULL;
}
numPlanes = 0;
textureID = 0;
}

int numPlanes;
tEntPlane *mpPlanes;
int textureID; //Index into BSP's textures

};



I didn't know whether or not to put constructors/destructors in those structs, but I figured if I did I was just being safe. I'll probably edit and keep adding more code to this post.

EDIT:
I don't know what to post, so here is the full VWorldObject constructor/destructor. If this is TOO much I'll gladly take it down.


VWorldObject::VWorldObject()
{
this->mvStates = SOLID_ENT;// + ACTIVE_ENT; //Always solid by default

this->mMaxs = Vector3(0, 0, 0);
this->mMins = Vector3(0, 0, 0);
this->mPosition = Vector3(0, 0, 0);
this->mnumOfBrushes = 0;
this->mnumOfFaces = 0;
this->mfaceIndex = 0;
this->mbrushIndex = 0;
this->mpBrushes = NULL;
this->mDisplacement = Vector3(0,0,0);
this->mFrame = 0;
this->mFrameTouch = 0;
this->mpcTeamName = NULL;
this->mpNeighbor = NULL;
this->mVelocity = Velocity();
this->mTimeInAir = 0;
this->mFallVelocity.Dir = Vector3(0,-1,0);
this->mFallVelocity.Mag = 0;
this->mpModel = NULL;
mLeafs.clear();
meCollisionType = eBrush; //default

}

VWorldObject::~VWorldObject()
{
if(mpBrushes)
{
delete[] mpBrushes;
mpBrushes = NULL;
mnumOfBrushes = 0;
}
if(mpcTeamName)
{
delete[] mpcTeamName;
mpcTeamName = NULL;
}
if(mpModel)
{
delete mpModel;
mpModel = NULL;
}
mLeafs.clear();
}


One thing I legitimately am not sure about is how to deal with the mLeafs vector. It is an std::vector, and I am not sure if I should init it by doing mLeafs.clear() as I am doing above, or init it by doing mLeafs = std::vector(); They both seem to work. Keep in mind my program works perfectly with what I have, it is only when I try to delete my entities that I get errors (I can delete the BSP without deleting the entities and it works fine, but it takes up too much memory when I do that)

[edited by - Shadow12345 on February 7, 2004 12:07:49 PM]

[edited by - Shadow12345 on February 7, 2004 12:09:26 PM]

[edited by - Shadow12345 on February 7, 2004 12:10:09 PM]

[edited by - Shadow12345 on February 7, 2004 12:10:27 PM]

[edited by - Shadow12345 on February 7, 2004 12:10:49 PM]

Share this post


Link to post
Share on other sites
It sounds like that at some point you''re doing a shallow copy of the std::vector that contains your VWorldObject pointers. What does the class that holds that vector look like? Specifically the copy constructor or assignment operator.

Share this post


Link to post
Share on other sites
Shallow copies of the vector wouldn't cause this problem, because you'd have multiple pointers to the same object and when it was deleted the first time the member pointers would be set to null. Unless I'm misunderstanding what you're saying. EDIT: D'uh, wasn't thinking clearly, if the memory was reused between deletes then this problem might occur.

Do you ever copy VWorldObject objects (not pointers)? If you don't have a proper copy constructor and assignment operator then this will result in your program attempting to delete the same memory multiple times.

Oh, and while I'm at it, calls like this:
tEntPlane()
{
d = 0;
vNormal = Vector3(0,0,0);
}


would be better as:
tEntPlane()
:
vNormal(0, 0, 0)
{
d = 0;
}

which can avoid a temporary.

Enigma

[edited by - Enigma on February 7, 2004 12:35:43 PM]

Share this post


Link to post
Share on other sites
I think you're double-deleting memory. Try the following code for your Deleter:

template<class T>
struct Deleter
{
void operator()(T * & obj)
{
if(obj)
{
delete obj;
obj = NULL;
}
}
};


Note that "* &" in the function definition, which is a reference to a pointer (not a pointer to a reference). That way, you can do the "obj = NULL;" and have it actually NULL out the pointer so you don't re-delete the same memory.[/source]



[edited by - aprosenf on February 7, 2004 12:29:46 PM]

Share this post


Link to post
Share on other sites
I don't have a copy constructor or assignment operator for the class implemented.

Here is how I'm initializing the VWorldObjects. They are entities stored in a quake3 entity string.

I've got functions to parse each of the types of entities. Here is one that parses a func_door, which derives from VWorldObject



void BSP::ParseFuncDoor(char *temp, std::vector<VWorldObject*> *a)
{
//Allocate memory for a func_door

Door *door = new Door;
/*
--a ton of code taken out here
*/

LinkEntityToWorld(door); //adds pointer to bsp leaf entitylist std::vectors

a->push_back(door); //adds to GameExport std::vector

}


Note that I only try deleting the objects themselves in the BSP constructor. All entities in the GameExport std::vector are manually removed from the vector using std::vector::erase and iterators.
EDIT:
I tried what you suggested Aprosenf and it does not work for some reason. Why wouldn't the pointer be set to null in the original version? I suspect that for some reason the pointer isn't being set to null but I don't know why it isn't.

EDIT1:
quote:

Do you ever copy VWorldObject objects (not pointers)? If you don't have a proper copy constructor and assignment operator then this will result in your program attempting to delete the same memory multiple times.


No, I never attempt to copy VWorldObject objects. they are all pointers. The only possible exception would be the Player, but that too is also a pointer and also derives from VWorldObject.
[edited by - Shadow12345 on February 7, 2004 12:31:37 PM]

[edited by - Shadow12345 on February 7, 2004 12:33:46 PM]

[edited by - Shadow12345 on February 7, 2004 12:35:31 PM]

Share this post


Link to post
Share on other sites
Well, if your program gets an access violation during a delete statement, then 99.9% of the time you''re either trying to delete NULL memory (which you''re clearly not), you''re trying to delete an uninitialized pointer (which you''re also not), or you''re trying to delete memory that''s already been deleted. Make sure you also check the destructors of subobjects since calling delete [] objs; calls the destructors of objs before it trys to free the memory. The reason I think you''re deleting the same memory twice is because your original Deleter struct deleted the memory but didn''t NULL-out the pointer, which would cause that pointer to be deleted a second time at some later point in its life. However, you can''t simply just say obj = NULL in that case, because the pointer is passed by value. You could NULL out that pointer, but that is a copy of the original pointer, and the original pointer wouldn''t by NULL''ed out. That is why I added the "* &" so that NULLing out the pointer reference would NULL out the original. If doing those changes didn''t work, then I''m really baffled as to what''s happening.

Share this post


Link to post
Share on other sites

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!