Memory leak - arghh!

[richiebabes]

This code creates a memory leak:

CShape *shape = new CShape();
	shape->m_sName = newShape.m_sShapeName;
	shape->m_sPoints = newShape.m_sShapePoints;
	shape->m_nSides = newShape.m_nSides;
	shapes->Add(shape);
	CMainFrame* pFrame = (CMainFrame*)AfxGetMainWnd();
	pFrame->m_TreeView->AddShapeToTree(shape);

If I delete shape after this code then it messes up my shapes array (CArray <CShape *, CShape *>). I do delete the shapes array when the program closes.

What am I doing wrong?

Bit more info. Here's where I create the array:

shapes = new CArray<CShape *, CShape *>();

And here's deleting it:

for (int x = 0; x < theApp.shapes->GetCount(); x++)
		delete theApp.shapes->GetAt(x);

	theApp.shapes->RemoveAll();

I am missing something obvious. Why do I still get a memory leak?

[ahoodin]

Look if you delete that you are deallocating your memory.

Then your shapes is just a map of pointers (addresses of allocated memeory)

When you are *done* using your shapes map, just go through and deallocate the memory one at a time. Dont do this before you are done with it, that is what you were trying to do.

HTH,

[sunny_sz]

Additional ahoodin. You'd better delete shape when not use it, otherewise that may cause something wrong.

[richiebabes]

Hmm, maybe I should explain again.

1. If I delete shape after creating it then when I try and use the shapes array the relevant shape is missing. Somehow by deleting the temporary shape I created it meant that the array was corrupted.

2. I do deallocate my shapes map only when I have finished using it!!!

[richiebabes]

Look if you delete that you are deallocating your memory.

Then your shapes is just a map of pointers (addresses of allocated memeory)

When you are *done* using your shapes map, just go through and deallocate the memory one at a time. Dont do this before you are done with it, that is what you were trying to do.

HTH,

I really don't know how this addresses my question.

[ahoodin]

If you delete your shape right after you call this code, you delete your memory before you actaully use it. Thats bad.

CShape *shape = new CShape();
	shape->m_sName = newShape.m_sShapeName;
	shape->m_sPoints = newShape.m_sShapePoints;
	shape->m_nSides = newShape.m_nSides;
	shapes->Add(shape);
	CMainFrame* pFrame = (CMainFrame*)AfxGetMainWnd();
	pFrame->m_TreeView->AddShapeToTree(shape);

just like you said:

If I delete shape after this code then it messes up my shapes array

[richiebabes]

OK, thanks for the clarification - yes, I thought so.

So how come when I deallocate the shapes array at the end I still get a memory leak? When should I delete shape? Doesn't deleting the array take care of it?

[ahoodin]

No you will have to call delete on each pointer to deallocate the shape objects.

Hope that answers your question.

[Paul McKenzie]

Doesn't deleting the array take care of it?

The CArray has no idea whether those shape pointers are pointing to dynamically allocated memory, or even it did, what function allocated the memory. So there is no way that CArray can call delete automatically, since it would be wrong. Example:

Shape myShape;
shapes->Add(&myShape);

Calling "delete" on that shape pointer that was added will likely cause a crash, since it was not pointing to dynamically allocated memory. The only one who knows where or how those shape objects are created is you, that's why you're responsible for deleting them.

Regards,

Paul McKenzie

[richiebabes]

OK, but how does any of that help.

WHERE SHOULD I DELETE THE shape VARIABLE?????????

[Lindley]

The concern is not with deleting particular variables. The concern is with deleting all allocated objects.

When you allocate an object, you initially store its location in your shape pointer. At some point you copy this pointer into your array, but (presumably) no copy is made of the object, only the pointer to it.

Conceptually it is probably best to think of the array as "taking ownership" of the object, even though the array is not automatically responsible for deleting its elements. (If it were an array of smart pointers, it would be.) Thus, there is no need to do anything special to the shape pointer. The object can be deleted just prior to the array's own deletion with no trouble.

This sort of confusion is the reason why pointer over-use is discouraged in C++ programs. For instance, is there any particular reason why "shapes" is a pointer rather than just a CArray on the stack? And do its contents *need* to be CShape pointers, or can they just be CShapes? I don't know, but these are things you should think about.

[Paul McKenzie]

OK, but how does any of that help.

It helps in answering your next question...

WHERE SHOULD I DELETE THE shape VARIABLE?????????

Whenever that particular object is no longer necessary. Again, you and only you know when that object is no longer needed.

Bottom line -- you know when you created those objects, you know when those objects are in use, and you know when those objects are no longer used and should be removed from memory. That is where you delete the memory.

With what little code you posted, that is the proper answer to give you.

Regards,

Paul McKenzie

[Paul McKenzie]

I am missing something obvious. Why do I still get a memory leak?

The code you posted is in bits and pieces. There is no coherent path that we can follow as to what gets called where.

Please post a complete, but small example of what you are doing.

Regards,

Paul McKenzie

[richiebabes]

This is embarrassing - but I am rusty! The memory leak comes from this line of code:

char *chpts = new char[sPoints.GetLength() + 1];

How can I delete it? A simple delete chpts causes things to blow up. Methinks I need to traverse through each element and delete it? Or what?

[Lindley]

When you are done with that array, you would do

delete[] arr;

where arr is whatever variable holds the address of the array at that time.

Alternatively, you could do

std::vector<char> chpts(sPoints.GetLength() + 1);
//or
std::string chpoints(sPoints.GetLength() + 1);

and thus avoid the need for any explicit deletes.