tullhead
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;
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;
ASKER CERTIFIED SOLUTION
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
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.
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()
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()
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.
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.
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?
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?
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?
ASKER
Each Chunk is created as
Chunk* pC = new Chunk();
and then later I stick it into the array...
pCA->SetAt(I, pC);
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.
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?
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).
ASKER
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 ;) .
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 ;) .
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.
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.
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.
would release all memory,
to add a new Chunk and get access to the new Chunk added you can do
Sara
#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();
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();
Sara
Of course, you can achieve the same using unique_ptr.
ASKER
Thanks!
ASKER