Link to home
Start Free TrialLog in
Avatar of tullhead
tullheadFlag for United States of America

asked on

C++ Programming question ( simple? )

Found a memory leak when I delete a ChunkArray.   But I'm not sure how to fix it.  How to a deallocate or recover the memory used in "buffer" ?

class Chunk : public CObject
      void Serialize(CArchive& ar);

            used = 0;

      ~Chunk()   // 8.0.24
            // How to clean buffer?  

         BYTE buffer[0x2000];  // 8KB
      int used;             // How many bytes of "buffer" are used

// Define an Array type for Chunk
typedef CArray<Chunk*, Chunk*> ChunkArray;
Avatar of pepr

Link to home
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
Avatar of tullhead


Could you show me how to call "delete ptr" - I guess that's the part I don't know.   Normally I use an MFC collection type and not something like "BYTE buffer[ox2000]" but in this case I need to due to code by others.  So I just don't know how to delete the byte array....
Avatar of pepr

The buffer is wrapped inside the Chunk instance. When you delete the Chunk object, the buffer will be deleted automatically -- unless it contains also pointers to something than must be deleted earlier. Can you show some code that would clarify what you exactly do?
Buffer is just a stack array of bytes. It doesn't need to be deleted. It is an "auto" type variable, meaning it is automatically deleted for you. If you have a memory leak it is not specifically because you are not deleting buffer and, in fact, it would be wrong to do so since it is not allocated on the heap. As pepr said, you need to show/explain more.
If I have

ChunkArray* pCA;

and later, I just want to delete the whole thing with

delete pCA;

but this burns heap.  I think its because I don't do anything in ~Chunk()
evilrix -- yes, that's what I had thought when I wrote the code originally.  But somehow, it is burning heap.   Instead of  just doing

delete pCA;

do I have to iterate thru the array and delete each element first?  I had thought not.
>>   I think its because I don't do anything in ~Chunk()
You don't need to do anything in ~Chunk(), since you are not allocating any resources in that class that are not automatically being deallocated for you.
Incidentally, you can (and should) avoid memory leaks by using smart-pointers.
>> do I have to iterate thru the array and delete each element first?  I had thought not.
Unless you are allocating data into the array from the heap, no. Given that buffer is just declared as a stack array of bytes you should not need to do anything to deallocate it. Why do you think it is "burning" heap and why do you believe it's this bit of code rather than something else?
>> typedef CArray<Chunk*, Chunk*> ChunkArray;
Just out of interest, how are you allocating/deallocating memory for each Chunk? This is a more likely candidate for the leak. How is each Chuck that makes up ChunkArray being deallocated?
Each Chunk is created as

Chunk* pC = new Chunk();

and then later I stick it into the array...

pCA->SetAt(I, pC);
Okay, and I assume that each item added to the CArray is deallocated using "delete" before you destroy the array? Again, you should consider using a smart pointer. I'd also consider using std::vector as it's part of the C++ standard, whereas CArray isn't (it's part of Microsoft's MFC), but that's just me.
Right now, I'm simply doing
delete pCA;   // the array
So, you are saying that because I use "new" in creating the elements, I *do* have to iterate thru the array and call delete on each element?
Yes, everything you allocation using "new" must be deallocated using new. Sorry, when you referred to the array I thought you meant buffer, which is an actual array. ChunkArray is a CArray (which is a class that represents an array).
OK. Thanks a million for your help.  I will try now...
No worries.
To add, when you have ChunkArray* pCA; and later you delete pCA;, you are leaking not only the dynamically allocated elements of the ChunkArray, but you are also leaking the whole ChunkArray.

It seems you are used to Java or C# or the like language where the pointers are hidden behind the scene and where the garbage collector cleans automatically the "leaked" objects. I am not writing it you did not know, but you may be used to some style of work. In C++, the static creation of an object may be a better alternative to the dynamic (new) creation -- whenever it is possible. From my experience, you should also prefer references to pointers. And if nothing of this is possible, use smart pointers, as evilrix wrote.

I guess, you should continue in discussion to make it clear (for us and for yourself ;) .
OK, now its fixed: simply needed to iterate thru the array and delete each element before deleting the array.  I do this for dozens of other classes in my project -- my brain just went off track because I jumped to the false conclusion it had something to do with my use of "BYTE buffer[0x2000]" a sort of large allocation that I typically don't do.  But of course, it is simple: things created with "new" should later be recovered with "delete".  So, thanks a lot to both of you for helping and putting up with my dumbness.
You're very welcome.
It may be a good idea to create your own class that wraps the ChunkArray and knows how to destroy itself correctly -- especially when you need it on more than one place, or when you find yourself writing some code repeatedly.

The "hybrid" features of C++ (read it "more natural") allow also to write functions or templates to "purge" the object passed as the argument.
as suggested by evilrix you may consider to using std::vector as array container. i would try to store the Chunk elements as objects and not as pointers

#include <vector>

std::vector<Chunk> ChunkArray;

this would store an array of Chunk objects instead of pointers. the Advantage is that the array elements don't need to be deleted.


Open in new window

would release all memory,

to add a new Chunk and get access to the new Chunk added you can do

ChunkArray.resize(ChunkArray.size()+1); // creates a new empty Chunk
Chunk & newChunk = ChunkArray.back();

Open in new window

Of course, you can achieve the same using unique_ptr.