[C++] memory leak with std::string when using std::list<std::string>?

Question

I'm working with std::list 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;
    }

}

So: why does my string list cause a memory leak? Shouldn't deleting the list cause the ~Operators to be called on each contained string? I hope someone can help...

Answer 1

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.

Answer 2

    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;

You forgot to delete s . You new'ed it, you have to delete it. As you're copying objects around(By stuffing them in a list) while managing memory in your Line class, you also have to provide a copy constructor and assignment operator for your Line class.

Answer 3

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

Answer 4

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.

Answer 5

Others have addressed specifically why you have your leak - deleting a list of pointers does not delete the objects that are pointed to, and should not as a simple pointer gives no indication whether it was the only reference to that object ), but there are more ways than having to make sure you iterate the list on deletion to delete the pointers.

---

As far as the example here shows theres no reason to use pointers to anything at all, since you're using them when they enter the scope and discarding them when they leave the scope - simply create everything on the stack instead and the compiler will properly dispose of everything on exiting the scopes. Eg.

while (1==1) {
    std::list<std::string> sl;
    std::string s = std::string("XXXXXXXXXXXXXXXXXXXXXXX");
    sl.push_back(s);
}

---

If you do need the pointer behaviour (to avoid having to duplicate objects that are linked to by many things etc. etc.) you should take a look at smart pointers, as these will remove many of the pitfalls as they can automatically handle the reference counting and semantics you require. (Specifically take a look at the boost smart pointers)

There are many types of smart pointer you can use depending on specific need and ownership semantic to represent.

The std::auto_ptr has strict ownership - if the pointer is "copied" the original is nulled and ownership transfered - there is only ever be one valid auto_ptr to the object. The object pointed to is deleted whenever the smart pointer with ownership goes out of scope.

Theres also boost shared pointers and weak pointers using reference counting to know when to free the object being pointed to. With Shared pointers each copy of the pointer increases a reference count, and the object pointed to is deleted whenever all the shared pointers go out of scope. A weak pointer points to an object managed by a shared pointer but does not increase the reference count, if all the parent shared pointers are deleted attempting to dereference a weak pointer will throw an easily catchable exception.

Obviously theres a lot more to the range of smart pointers, but I highly suggest taking a look at them as a solution to help with managing your memory.