Will this lead to a memory leak in C++?

Question

Hello, I have a C++ memory management doubt, that's (obviously) related to references and pointers. Suppose I have a class Class with a method my_method:

OtherClass& Class::my_method( ... ) {
    OtherClass* other_object = new OtherClass( ... );
    return *other_object;
}

Meanwhile in a nearby piece of code:

{
    Class m( ... );
    OtherClass n;
    n = m.my_method( ... );
}

So, I know that there's a general rule about pointers (~ "anything new-ed, must be delete-d") to avoid memory leaks. But basicly I'm taking a reference to my heap-allocated object, so when n goes out of scope, shouldn't the destructor of OtherClass be called thus freeing the memory previously pointed by other_object? So in the end the real question is: will this lead to a memory leak?

Thanks.

Answer 1

Calling the destructor and freeing the memory are two distinct things in C++.

delete does both call the destructor and free the memory. delete[] calls the destructor for the allocated number of elements, then frees the memory.

When OtherClass goes out of scope, the destructor is called, but the memory is not freed.

---

As a suggestion, when you feel you have thoroughly understood pointers in C++, look into smart pointers, e.g. boost smart pointers to ease your memory management life in C++. (e.g. see the article here for an introduction)

Answer 2

Yes that will lead to a memory leak.

What you'll do is, in the return statement, dereference the new object you created. The compiler will invoke the assignment operator as part of the returning and copy the CONTENTS of your new object to the object it's assigned to in the calling method.

The new object will be left on the heap, and its pointer cleared from the stack, thus creating a memory leak.

Why not return a pointer and manage it that way?

Answer 3

You have 2 OtherClass objects:

One is n, which is created on the stack, and successfully deleted when goes out of scope.

The other one is the one that you create on the heap, inside my_method. This one is never deleted, and it will lead to memory leak.

Answer 4

Usually, when a local object gets out of scope, it's memory is freed only because it's allocated on the stack and the stack gets cleaned up automatically. Since your object is allocated on the heap, there's no way it can get automatically freed. The only way to free it is to call delete explicitly.

I think you probably can do this:

Declare another class DummyClass, which contain a public member that is a pointer to OtherClass object. In the constructor of DummyClass, clear the pointer to NULL. In your function, declare an object of type DummyClass, and its member pointer to create another object of type OtherClass. Then in the destructor of DummyClass, check if the pointer is NULL, if it is not, delete it. In this way your object will be cleaned up automatically when DummyClass object goes out of scope.

Answer 5

If possible you can consider std::auto_ptr or boost/c0x shared_ptr to ease the memory management.

Answer 6

It's fairly obvious that you want to return a new object to the caller that you do not need to keep any reference to. For this purpose, the simplest thing to do is to return the object by value.

OtherClass Class::my_method( ... ) {
    return OtherClass( ... );
}

Then in the calling code you can construct the new object like this.

{
    Class m( ... );
    OtherClass n( m.mymethod( ... ) );
}

This avoids any worries about returning reference to temporaries or requiring the client to manager deletion of a returned pointer. Note, that this does require your object to be copyable, but it is a legal and commonly implemented optimization for the copy to be avoided when returning by value.

You would only need to consider a shared pointer or similar if you need shared ownership or for the object to have a lifetime outside the scope of the calling function. In this latter case you can leave this decision up to the client and still return by value.

E.g.

{
    Class m( ... );

    // Trust me I know what I'm doing, I'll delete this object later...
    OtherClass* n = new OtherClass( m.mymethod( ... ) );
}

Answer 7

If you insist on stack-allocation, don't use the new operator in my_method() and pass a reference to n instead, ie:

void Class::my_method( OtherClass& other_object, ... ) {
    other_object.init( ... );
}

Then call my_method() like this:

{
    Class m( ... );
    OtherClass n;
    m.my_method( n, ... );
}

For this pattern to work, Class must implement some kind of init() method which allows to correctly initialize objects without calling the constructor.