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?

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

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

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

Answer 4

If you throw in a constructor, you should clean up everything that came before the call to throw. If you are using inheritance or throwing in a destructor, you really shouldn't be. The behaviour is odd (don't have my standard handy, but it might be undefined?).

Answer 5

Yes, that code will leak memory. Blocks of memory allocated using "new" are not freed when an exception is raised. This is part of the motivation behind RAII.

To avoid the memory leak, try something like this:

psomeclass = NULL;
pint = NULL;
/* So on for any pointers you allocate */

try {
    objsomeclass = someclass();
    psomeclass = new someclass();
    pint = new int(); 
    throw "constructor failed";
    a = 43;
 }
 catch (...)
 {
     delete psomeclass;
     delete pint;
     throw;
 }

---

Answer 6

Everything you "new" needs to be deleted, or you'll cause a memory leak. So these two lines:

psomeclass = new someclass();
pint = new int(); 

Will cause memory leaks, because you need to do:

delete pint;
delete psomeclass;

In a finally block to avoid them being leaked.

Also, this line:

base temp = base();

Is unnecessary. You just need to do:

base temp;

Adding the "= base()" is unnecessary.

Answer 7

you need to delete psomeclass... Its not necessary to clean up the integer...

RWendi