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

Question

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

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.

Answer 2

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 3

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 4

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 5

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

Answer 6

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.