Memory leak with std::string when using std::list<std::string>
I'm working with std::list<std::string> in my current project. But there is a memory leak somewhere connected with this. So I've tested the problematic code separately:
#include <iostream>
#include <string>
#include <list>
class Line {
public:
Line();
~Line();
std::string* mString;
};
Line::Line() {
mString = new std::string("XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX");
}
Line::~Line() {
//mString->clear(); // should not be neccessary
delete mString;
}
int main(int argc, char** argv)
{
// no memory leak
while (1==1) {
std::string *test = new std::string("XXXXXXXXXXXXXXXXXXXXXXXX");
delete test;
}
// LEAK!
// This causes a memory overflow, because the string thats added
// to the list is not deleted when the list is deleted.
while (1==1) {
std::list<std::string> *sl = new std::list<std::string>;
std::string *s = new std::string("XXXXXXXXXXXXXXXXXXXXXXX");
sl->push_back(*s);
//sl->pop_back(); //doesn't delete the string?- just the pointer
delete sl;
}
// LEAK!
// Here the string IS deleted, but the memory does still fill up
// but slower
while (1==1) {
std::list<Line> *sl = new std::list<Line>;
Line *s = new Line();
sl->push_back(*s);
//sl->pop_back(); //does delete the Line-Element
sl->clear();
delete sl;
}
return 0;
// this does not cause any noticable memory leak
while (1==1) {
std::list<int> *sl = new std::list<int>;
int i = 0xFFFF;
sl->push_back(i);
sl->clear();
delete sl;
}
return 0;
// This does not cause any overflow or leak
while (1==1) {
int *i;
i= new int [9999];
delete[] i;
}
}
Why does my string list cause a memory leak? Shouldn't deleting the list cause the destructors to be called on each contained string?
In the first case, the list class has no idea you allocated the string with new , and cannot delete it. In particular, the list only ever contains a copy of the string that you passed in.
Similarly, in the second case, you never free the line object s , and thus you leak memory. The reason why the internal string is deleted is because you have not correctly implemented a copy constructor. Thus, if you make a copy of a Line object, both of them will reference the same string pointer, and if you try to delete both of them, you are in trouble.
Your Line class needs a copy-ctor and an assignment operator that properly deal with the string pointer.
Alternatively, just have a std::string member rather than a pointer and let the string class handle the memory (that's what it's for).
Here's your leak:
while (1==1) {
std::list<Line> *sl = new std::list<Line>;
Line *s = new Line();
sl->push_back(*s);
//sl->pop_back(); //does delete the Line-Element
sl->clear();
delete sl;
}
STL collections store elements by value , allocating and releasing space for it. What you allocated you have to release explicitly. Just add delete s to the end of the loop.
If you have to store pointers, consider storing managed pointers like boost::shared_ptr , or look into Boost pointer container library.
On the second look, you don't need to allocate Line on the heap at all. Just change it to:
sl->push_back(Line());
And, as others noted, make sure Line 's pointer members are properly managed in copy-constructor, copy-assignment, and destructor.