tNode* deleteTemp = NULL;
while(!m_Closed.empty())
{
deleteTemp = m_Closed.back();
m_Closed.pop_back();
delete deleteTemp;
}
[Solved]STD Vector and Dynamic Memory Clean up
Author
One thing after another tonight...
I'm doing an exercise with Dijkstra and A*, so I have an STD vector of path nodes. I'm trying to clean up memory since I create a new path node as I explore the terrain. At then end of the program I basically have a vector that is filled with small ~ 16byte nodes that need to be deleted.
My current loop to clean up the memory is this:
This works great till I get to the last item in the vector, then it causes a _BLOCK_TYPE_IS_VALID debug assertion and crashes(only in Debug of course).
While I can just shrug this off, since it works in release mode, I'd rather just try and fix the problem.
What is a good way to delete dynamic memory that is contained within an STD vector? Is my way alright? Is there a better method? Is this even the reason for the crash(the debugger says it is)?
[Edited by - ChaosPhoenix on November 7, 2006 8:08:41 AM]
Your way should work, I think... Give this a whirl though and see if it still crashes:
Definitely do _NOT_ ignore memory crashes in debug. Just because it doesn't crash in Release doesn't mean there's no bug; in Release you have a silent bug. It's not crashing but it's likely over-writing or leaking memory; both of which are amongst the hardest to find and most deadly of bugs; second only to thread bugs.
-me
//assuming m_Closed declared as follows//std::vector<tNode*> m_Closed;std::vector<tNode*>::iterator ite = m_Closed.end();for( std::vector<tNode*>::iterator it = m_Closed.begin(); it != ite; ++it ){ delete *it;}m_Closed.clear();Definitely do _NOT_ ignore memory crashes in debug. Just because it doesn't crash in Release doesn't mean there's no bug; in Release you have a silent bug. It's not crashing but it's likely over-writing or leaking memory; both of which are amongst the hardest to find and most deadly of bugs; second only to thread bugs.
-me
Or even easier, don't make the vector of tNode not tNode*. No need to make it a pointer, unless you NEED a pointer, don't use one. You can always get the pointer if you need it later, there is no valid reason why you need tNode*.
This way you don't have any memory leaks.
This way you don't have any memory leaks.
Author
Still crashing and I don't understand why. I actually tried a method like the one you suggested earlier.
The node it keeps blow up on is declared as:
It's then placed in an "Open" list, starts the path finding loop, and is instantly removed and placed into the closed list where it sits till the end of the program. I don't *think* that node is the problem, I find it odd that all elements of the vector can be deleted fine, except for the final node.
The node it keeps blow up on is declared as:
// Create our root node. tNode* root = new tNode; root->pParent = NULL; // No parent root->x = pMap->GetStartingPointX(); // Origin X root->y = pMap->GetStartingPointY(); // Origin Y root->tCost = 0; // No costIt's then placed in an "Open" list, starts the path finding loop, and is instantly removed and placed into the closed list where it sits till the end of the program. I don't *think* that node is the problem, I find it odd that all elements of the vector can be deleted fine, except for the final node.
Author
Quote:Original post by Anonymous Poster
Or even easier, don't make the vector of tNode not tNode*. No need to make it a pointer, unless you NEED a pointer, don't use one. You can always get the pointer if you need it later, there is no valid reason why you need tNode*.
This way you don't have any memory leaks.
The path finding solution uses a list of tNodes to traverse and find the path, so having them dynamically declared makes life much easier (except for the clean up, ofcourse).
Quote:Original post by ChaosPhoenix
the program. I don't *think* that node is the problem, I find it odd that all elements of the vector can be deleted fine, except for the final node.
Maybe you already deleted root somewhere in your program?
Quote:Original post by Anonymous Poster
Or even easier, don't make the vector of tNode not tNode*. No need to make it a pointer, unless you NEED a pointer, don't use one. You can always get the pointer if you need it later, there is no valid reason why you need tNode*.
This way you don't have any memory leaks.
It looks like his tNodes have a pointer to a parent tNode (which is presumably in the same list)... so that pretty much requires using a pointers to dynamically allocated memory.
He could redesign his tNode to be more efficent also. tNode.mParent (please don't use hungarian notation like p for pointers..) can point to a spot in the vector instead of a pointer.
It would be easier for the OP to slightly redesign his class to be safer instead of working around an issue.
In all honesty, pointers should be a last resort, when there is no way around using one. It eliminates mistakes. This is why languages like C# really don't use them.
It would be easier for the OP to slightly redesign his class to be safer instead of working around an issue.
In all honesty, pointers should be a last resort, when there is no way around using one. It eliminates mistakes. This is why languages like C# really don't use them.
Quote:Original post by Anonymous Poster
He could redesign his tNode to be more efficent also. tNode.mParent (please don't use hungarian notation like p for pointers..) can point to a spot in the vector instead of a pointer.
IMO, that would be a terrible design. It means a node can really only exist in the context of a vector. It can only have a parent if its parent is in a vector. If anything was added to the vector, the index (or iterator) would now point to something different. Basically, you could never more anything around, w/o having to scan every node and update its parent. Almost certainly, this would lead to more mistakes, and more difficult coding, than simply using pointers.
That design would only make sense in a very limited context.
Using pointers isn't hard - learn to use them - they are the proper way to go for a case like this. It sounds like you understand what to do anyway - the problem is mostly somewhere else in your program.
If you need to, consider using one of the smart pointers in the boost library that works when used in containers (shared_ptr I think?).
If not, then here is some convenient code:
struct delete_object{ template <typename T> void operator()(T *ptr){ delete ptr;}};then you can just do
std::for_each( m_Closed.begin(), m_Closed.end(), delete_object());[Edited by - phil_t on November 6, 2006 9:40:27 PM]
Also, ChaosPhoenix, do you understand who "owns" the pointers to the tNode objects? e.g. if the vector "owns" them, then you should call delete on each item in the vector as you are doing - in that case though, you need to make sure your tNode destructor doesn't call delete on the pParent member!
Although I have never used it, shared_ptr would probably eliminate this responsibility.
http://www.boost.org/libs/smart_ptr/shared_ptr.htm
Although I have never used it, shared_ptr would probably eliminate this responsibility.
http://www.boost.org/libs/smart_ptr/shared_ptr.htm
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
