Cleaning up memory leaks

[Ook]

I have a set<Item*> that I use in a class, Item being one of my classes. In
the destructor, I want to iterate through the set and remove the elements so
there are no memory leaks. Is this the correct way to do it?

for( set<Item*>::iterator iterator = ItemSet.begin(); iterator !=
ItemSet.end(); iterator++)
{
delete *iterator;
}

 

[Kai-Uwe Bux]

"Ook" <Ook Don't send me any freakin' spam at zootal dot com delete the

I have a set<Item*> that I use in a class, Item being one of my classes.
In the destructor, I want to iterate through the set and remove the
elements so there are no memory leaks. Is this the correct way to do it?

for( set<Item*>::iterator iterator = ItemSet.begin(); iterator !=
ItemSet.end(); iterator++)
{
delete *iterator;
}

Looks good to me. One possible problem arises if you use a custom comparison
predicate for the set that defines the order in terms of the pointees and
not in terms of the pointers. In that case, erasing a pointer from the set
is best done as follows: save the pointer, erase it from the set, and then
delete the saved pointer. Did you encounter any problems with the code
above? e.g., does valgrind complain?


Best

Kai-Uwe Bux

 

[Ook]

"Ook" <Ook Don't send me any freakin' spam at zootal dot com delete the


Looks good to me. One possible problem arises if you use a custom
comparison
predicate for the set that defines the order in terms of the pointees and
not in terms of the pointers. In that case, erasing a pointer from the set
is best done as follows: save the pointer, erase it from the set, and then
delete the saved pointer. Did you encounter any problems with the code
above? e.g., does valgrind complain?


Best

Kai-Uwe Bux

Actually, when I ran it with one set it worked, when I ran it with the other
set it gave an unhandled exception error in ~Item. I don't have a destructor
for the Item class, maybe I need one there because some Item classes have
sets in them, and I need to delete there sets in the Item destructor?

Can I just do set.clear()? Or should I do set.clear, and then delete the
set?

 

[Jim Langston]

"Ook" <Ook Don't send me any freakin' spam at zootal dot com delete the
Don't send me any freakin' spam> wrote in message

Actually, when I ran it with one set it worked, when I ran it with the
other set it gave an unhandled exception error in ~Item. I don't have a
destructor for the Item class, maybe I need one there because some Item
classes have sets in them, and I need to delete there sets in the Item
destructor?

Can I just do set.clear()? Or should I do set.clear, and then delete the
set?

Well, as long as your Item class doesn't allocate memory itself with new or
malloc then you shouldn't need a user defined destructor. The default
destructor will destroy the set (as long as it's a set not of pointers
itself you should be okay).

I don't use sets, but I delete vectors exactly the way you describe with no
problems, but vectors aren't sorted. Why, however, would you use a set of
pointers? What good is it sorting by pointer type, and you with proper
coding you shouldn't ever try to push the same pointer twice. So maybe you
are using a custom comparison predicate as Kai-Uwe suggests?

 

[benben]

Actually, when I ran it with one set it worked, when I ran it with the other
set it gave an unhandled exception error in ~Item.

One possible cause to your problem is that you have pointers in each set
and they point to the same object. So if you try to free all pointers in
both sets the object actually gets double deletion, which is UB.

I don't have a destructor
for the Item class, maybe I need one there because some Item classes have
sets in them, and I need to delete there sets in the Item destructor?

If you don't have a destructor then the compiler will generate one for
you. You don't have to explicitly deallocate the sets in Items as long
as they are not sets of pointers.

Can I just do set.clear()? Or should I do set.clear, and then delete the
set?

No. If you do it, it'll be a memory leak.

And as the last point let me strongly advise you to consider smart
pointers, such as boost::shared_ptr<>. This may alleviate your problem.

Regards,
Ben

 

[Ook]

One possible cause to your problem is that you have pointers in each set
and they point to the same object. So if you try to free all pointers in
both sets the object actually gets double deletion, which is UB.


If you don't have a destructor then the compiler will generate one for
you. You don't have to explicitly deallocate the sets in Items as long as
they are not sets of pointers.


No. If you do it, it'll be a memory leak.

And as the last point let me strongly advise you to consider smart
pointers, such as boost::shared_ptr<>. This may alleviate your problem.

Regards,
Ben

Thanks everyone for your advice. FWIW, It's a school assignment. Which is
why it's like it is . I have to take it as-is, and make sure there are not
leaks in it.

 

[Ook]

If I set.clear(), and then delete set, will that cause a memory leak? Do I
have to delete the items in the set instead of doing a set.clear()?

 

[Gianni Mariani]

If I set.clear(), and then delete set, will that cause a memory leak? Do I
have to delete the items in the set instead of doing a set.clear()?

No need to set.clear() if you delete the set.

If your set item is a pointer and the only place that points to the
objects pointed to by the set object are in the set, then you will need
to delete the objects before delete the set.

 

[Ook]

No need to set.clear() if you delete the set.

If your set item is a pointer and the only place that points to the
objects pointed to by the set object are in the set, then you will need to
delete the objects before delete the set.

I have determined that the remaining leaks are being caused by populating
strings in the class. Specifically, if I remove from the constructor:

_title = title;

Where _title is the only string, then my leak goes away. Is there anything
special that has to be done to release memory used by strings before you
release the class instance?

 

[Ook]

"Ook" <Ook Don't send me any freakin' spam at zootal dot com delete the
Don't send me any freakin' spam> wrote in message

I have determined that the remaining leaks are being caused by populating
strings in the class. Specifically, if I remove from the constructor:

_title = title;

Where _title is the only string, then my leak goes away. Is there anything
special that has to be done to release memory used by strings before you
release the class instance?

More specifically - if the value I put into the string is more then 15
charactors, it leaks. If less, no leak. Am I missing something here? Does
this sound familiar?

 

[Gianni Mariani]

Ook wrote:
....

More specifically - if the value I put into the string is more then 15
charactors, it leaks. If less, no leak. Am I missing something here? Does
this sound familiar?

Is it a std::string ? What kind of string is it and how is it allocated
in the first place ?

 

[tragomaskhalos]

More specifically - if the value I put into the string is more then 15
charactors, it leaks. If less, no leak. Am I missing something here? Does
this sound familiar?- Hide quoted text -

Some std::string implementations use a "small string optimisation",
whereby the string contains a small fixed-sized buffer where it stores
the characters, only going to the heap if the string contains more
characters than the buffer's size. That doesn't fully explain why
you're seeing a leak though, unless (unlikely!) there's a bug in your
std::string.

 

[red floyd]

Some std::string implementations use a "small string optimisation",
whereby the string contains a small fixed-sized buffer where it stores
the characters, only going to the heap if the string contains more
characters than the buffer's size. That doesn't fully explain why
you're seeing a leak though, unless (unlikely!) there's a bug in your
std::string.

Exactly. The 15 char limit indicates SSO. What I'd like to know is why
the OP thinks there's a leak.

 

[Ook]

Exactly. The 15 char limit indicates SSO. What I'd like to know is why
the OP thinks there's a leak.

I have built a VS2005 project that demonstrates this. I'm using the leak
detection features of VS2005 to determine if there is a leak or not, and I'm
not 100% convinced that it's output is reliable. I have posted a VS2005
project here:

http://emberts.no-ip.info/stuff/vs2005leakdemo/memleakdemo.zip

In the code are two places where you can comment out a line and see the
leaks go away. They are:

1) Storing a value in a string that is longer then 15 characters causes a
leak.
2) Defining a set in a subclass causes a leak, even if you don't do anything
with it.

It's possible I'm doing something wrong/stupid, but I don't know what it is.
If any kind soul would like to look at this and comment on why the indicated
lines of code cause a leak to be detected, I would be very gratefull.

FWIW, this is for a school assignment, which is why it's structured the way
it is.

 

[Ook]

Problem solved. I needed a virtual destructor in my Item class - and now we
have zero leaks