Will the below code cause memory leak in c++

Question

class someclass {};

class base
{
    int a;
    int *pint;
    someclass objsomeclass;
    someclass* psomeclass;
public:
    base()
    {
        objsomeclass = someclass();
        psomeclass = new someclass();
        pint = new int(); 
        throw "constructor failed";
        a = 43;
    }
}

int main()
{
    base temp();
}

In the above code, the constructor throws. Which objects will be leaked, and how can the memory leaks be avoided?

int main()
{
    base *temp = new base();
}

How about in the above code? How can the memory leaks be avoided after the constructor throws?

Solution 1

Yes it will leak memory. When the constructor throws, no destructor will be called (in this case you don't show a destructor that frees the dynamically allocated objects, but lets assume you had one).

This is a major reason to use smart pointers - since the smart poitners are full fledged objects, they will get destructors called during the exception's stack unwind and have the opportunity to free the memory.

If you use something like Boost's scoped_ptr<> template, your class could look more like:

class base{
    int a;
    scoped_ptr<int> pint;
    someclass objsomeclass;
    scoped_ptr<someclass> psomeclass;
    base() : 
       pint( new int),
       objsomeclass( someclass()),
       psomeclass( new someclass())

    {
        throw "constructor failed";
        a = 43;
    }
}

And you would have no memory leaks (and the default dtor would also clean up the dynamic memory allocations).

---

To sum up (and hopefully this also answers the question about the

base* temp = new base();

statement):

When an exception is thrown inside a constructor there are several things that you should take note of in terms of properly handling resource allocations that may have occured in the aborted construction of the object:

1. the destructor for the object being constructed will not be called.
2. destructors for member objects contained in that object's class will be called
3. the memory for the object that was being constructed will be freed.

This means that if your object owns resources, you have 2 methods available to clean up those resources that might have already been acquired when the constructor throws:

1. catch the exception, release the resources, then rethrow. This can be difficult to get correct and can become a maintenance problem.
2. use objects to manage the resource lifetimes (RAII) and use those objects as the members. When the constructor for your object throws an exception, the member objects will have desctructors called and will have an opportunity to free the resource whose lifetimes they are responsible for.

Solution 2

Both new's will be leaked.

Assign the address of the heap created objects to named smart pointers so that it will be deleted inside the smart pointers destructor that get call when the exception is thrown - (RAII).

class base {
    int a;
    boost::shared_ptr<int> pint;
    someclass objsomeclass;
    boost::shared_ptr<someclass> psomeclass;

    base() :
        objsomeclass( someclass() ),
        boost::shared_ptr<someclass> psomeclass( new someclass() ),
        boost::shared_ptr<int> pint( new int() )
    {
        throw "constructor failed";
        a = 43;
    }
};

Now psomeclass & pint destructors will be called when the stack unwind when the exception is thrown in the constructor, and those destructors will deallocate the allocated memory.

int main(){
    base *temp = new base();
}

For ordinary memory allocation using (non-plcaement) new, memory allocated by the operator new is freed automatically if the constructor throws an exception. In terms of why bother freeing individual members (in response to comments to Mike B's answer), the automatic freeing only applies when an exception is thrown in a constructor of an object being new'ly allocated, not in other cases. Also, the memory that is freed is those allocated for the object members, not any memory you might have allocated say inside the constructor. i.e. It would free the memory for the member variables a, pint, objsomeclass, and psomeclass, but not the memory allocated from new someclass() and new int().

Solution 3

I believe that the top answer is wrong and would still leak memory. The destructor for the class members will not be called if the constructor throws an exception (because it never completed its initialization, and perhaps some members have never reached their constructor calls). Their destructors are only called during the class's destructor call. That only makes sense.

This simple program demonstrates it.

#include <stdio.h>


class A
{
    int x;

public:
    A(int x) : x(x) { printf("A constructor [%d]\n", x); }
    ~A() { printf("A destructor [%d]\n", x); }
};


class B
{
    A a1;
    A a2;

public:
    B()
    :   a1(3),
        a2(5)
    {
        printf("B constructor\n");
        throw "failed";
    }
    ~B() { printf("B destructor\n"); }
};


int main()
{
    B b;

    return 0;
}

With the following output (using g++ 4.5.2):

A constructor [3]
A constructor [5]
B constructor
terminate called after throwing an instance of 'char const*'
Aborted

If your constructor fails partway then it is your responsibility to deal with it. Worse, the exception may be thrown from your base class' constructor! The way to deal with these cases is by employing a "function try block" (but even then you must carefully code the destruction of your partially initialized object).

The correct approach to your problem would then be something like this:

#include <stdio.h>


class A
{
    int x;

public:
    A(int x) : x(x) { printf("A constructor [%d]\n", x); }
    ~A() { printf("A destructor [%d]\n", x); }
};


class B
{
    A * a1;
    A * a2;

public:
    B()
    try  // <--- Notice this change
    :   a1(NULL),
        a2(NULL)
    {
        printf("B constructor\n");
        a1 = new A(3);
        throw "fail";
        a2 = new A(5);
    }
    catch ( ... ) {   // <--- Notice this change
        printf("B Cleanup\n");
        delete a2;  // It's ok if it's NULL.
        delete a1;  // It's ok if it's NULL.
    }

    ~B() { printf("B destructor\n"); }
};


int main()
{
    B b;

    return 0;
}

If you run it you will get the expected output where only the allocated objects are destroyed and freed.

B constructor
A constructor [3]
B Cleanup
A destructor [3]
terminate called after throwing an instance of 'char const*'
Aborted

You can still work it out with smart shared pointers if you want to, with additional copying. Writing a constructor similar to this:

class C
{
    std::shared_ptr<someclass> a1;
    std::shared_ptr<someclass> a2;

public:
    C()
    {
        std::shared_ptr<someclass> new_a1(new someclass());
        std::shared_ptr<someclass> new_a2(new someclass());

        // You will reach here only if both allocations succeeded. Exception will free them both since they were allocated as automatic variables on the stack.
        a1 = new_a1;
        a2 = new_a2;
    }
}

Good luck, Tzvi.

Comments

[John Millikin]

No such thing as a "finally" block in C++

[Colen]

True, you may or may not have access to it depending on your C++ flavour - if not, you'll have to make sure the allocations get deleted regardless of the code path taken.

[John Millikin]

Your remark about the extra initialization is wrong. The resulting object will only be initialized once, and will not be copied.

[John Millikin]

Not sure if it's actually undefined, but is certainly very dangerous because destructors are called during stack unwinding on the event of a raised exception. If you raise an exception while another has been raised, every C++ runtime I know will terminate the application.

[KTC]

An uncaught exception in a destructor raised during exception handling causes std::terminate() to be called, which by default call std::abort(). The default behaviour can be over-ridden.

[yesraaj]

rather than using pointers using object(smart pointer) will make things better?Since when ever an exception is thrown in a block automatic objects are cleared.

[Martin York]

smart pointers are better. Also replace 'raise'; with 'throw;' To rethrow the current exception.

[Martin York]

shared_ptr<> is overkill if you own the object and are never going to give shared ownership up. Simplify with std::auto_ptr<>

[yesraaj]

//Changed the question to have base *temp = new base();

[Greg Rogers]

even though the default behavior can be overridden, your version still can't return back to the application, it still has to exit.

[John Millikin]

Isn't hauling in Boost just go get memory management pretty silly?

[Michael Burr]

And boost::scoped_ptr<> might be even better than auto_ptr<> which has it's own can of worms.

[Michael Burr]

Maybe, but scoped_ptr is in TR1 and will be in C++09, so it's something that should be learned anyway. And the part of Boost that has scoped_ptr is just a bunch of headers. Finally, you could use auto_ptr instead for this simple example, but auto_ptr is probably something that should be avoided.

[yesraaj]

will the dtor of base class will be called even if i have once? what will happen to below line base *temp = new base();

[Michael Burr]

Nothing will happen to the "base* temp = new base;" line - the exception will cause the memory for the attempted new base object allocation to be freed (even thought the dtor will not be called).

[yesraaj]

then y should one care to release the memory of individual member?

[Michael Burr]

See the summary I added which I think should make the reasons why a little more clear.

[Martin York]

Note: The destructors of FULLY constructed members will be called.

[KTC]

It was a (sorta) random pick in terms of smart pointers as examples. It's general enough that one doesn't have to worry about explaining when it shouldn't be used in quick examples like this. But yes, if a simpler smart pointers can be used, then do.

[kasia]

Can you please elaborate Dave Moore? Is it about the "not necessary to clean up the integer" part? The reason behind this is that Int memory pointer doesn't cost much in comparison to class memory pointer, that's why I said its not necessary to clean it.

[Chris Cleeland]

They both leak; cost isn't an issue. The question was whether it leaked or not. And if that chunk of code executes thousands or millions of times, that little cost adds up. Even if "cost" were relevant, it's not the pointer's size that makes a difference, but rather the size of the pointed-to entity. For example, it's possible for sizeof(someclass) == sizeof(int). And you're not deleting the pointer--you're deleting the pointed-to entity.

[sourcenouveau]

The exception in your first example is not caught, so stack unwinding does not occur and no destructors are called. If you wrap B b; in a try catch then the destructors are called as expected.

[Nathan]

@Loki Can you define "FULLY constructed"? Wouldn't all members except for a be constructed at the time that the throw line is executed?

[Martin York]

@Nathan: Fully: The thread of execution has exited the constructor. In this case yes all members are fully constructed. But if a member throws an exception during its construction (ie during the initializer list) not all other members may be constructed. So if you use an object of base as a member of another class you may have that issue.