Avatar of tullhead
tullhead
Flag 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
{
public:
      DECLARE_SERIAL(Chunk);
      void Serialize(CArchive& ar);

      Chunk()
      {
            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;
ProgrammingC++* MemoryR

Avatar of undefined
Last Comment
tullhead

8/22/2022 - Mon
ASKER CERTIFIED SOLUTION
pepr

Log in or sign up to see answer
Become an EE member today7-DAY FREE TRIAL
Members can start a 7-Day Free trial then enjoy unlimited access to the platform
Sign up - Free for 7 days
or
Learn why we charge membership fees
We get it - no one likes a content blocker. Take one extra minute and find out why we block content.
Not exactly the question you had in mind?
Sign up for an EE membership and get your own personalized solution. With an EE membership, you can ask unlimited troubleshooting, research, or opinion questions.
ask a question
tullhead

ASKER
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....
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?
evilrix

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.
Experts Exchange is like having an extremely knowledgeable team sitting and waiting for your call. Couldn't do my job half as well as I do without it!
James Murphy
tullhead

ASKER
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()
tullhead

ASKER
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.
evilrix

>>   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.
Get an unlimited membership to EE for less than $4 a week.
Unlimited question asking, solutions, articles and more.
evilrix

Incidentally, you can (and should) avoid memory leaks by using smart-pointers.
evilrix

>> 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?
evilrix

>> 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?
This is the best money I have ever spent. I cannot not tell you how many times these folks have saved my bacon. I learn so much from the contributors.
rwheeler23
tullhead

ASKER
Each Chunk is created as

Chunk* pC = new Chunk();

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

pCA->SetAt(I, pC);
evilrix

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.
tullhead

ASKER
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?
Get an unlimited membership to EE for less than $4 a week.
Unlimited question asking, solutions, articles and more.
evilrix

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).
tullhead

ASKER
OK. Thanks a million for your help.  I will try now...
evilrix

No worries.
I started with Experts Exchange in 2004 and it's been a mainstay of my professional computing life since. It helped me launch a career as a programmer / Oracle data analyst
William Peck
pepr

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 ;) .
tullhead

ASKER
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.
evilrix

You're very welcome.
Get an unlimited membership to EE for less than $4 a week.
Unlimited question asking, solutions, articles and more.
pepr

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.
sarabande

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.

ChunkArray.clear(); 

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


Sara
evilrix

Of course, you can achieve the same using unique_ptr.
Your help has saved me hundreds of hours of internet surfing.
fblack61
tullhead

ASKER
Thanks!