views:

1634

answers:

6
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;
    }
}
main(){
    base temp();
}

in the above code constructor fails.Which objects will be leaked and how to avoid memory leak

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

what about in the above code and how to avoid memory leak when constructor fails

A: 

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.

Colen
No such thing as a "finally" block in C++
John Millikin
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.
Colen
Your remark about the extra initialization is wrong. The resulting object will only be initialized once, and will not be copied.
John Millikin
A: 

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;
 }


John Millikin
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.
yesraaj
smart pointers are better. Also replace 'raise'; with 'throw;' To rethrow the current exception.
Martin York
A: 

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

hazzen
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.
John Millikin
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.
KTC
even though the default behavior can be overridden, your version still can't return back to the application, it still has to exit.
Greg Rogers
+4  A: 

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

KTC
shared_ptr<> is overkill if you own the object and are never going to give shared ownership up. Simplify with std::auto_ptr<>
Martin York
//Changed the question to have base *temp = new base();
yesraaj
And boost::scoped_ptr<> might be even better than auto_ptr<> which has it's own can of worms.
Michael Burr
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.
KTC
+28  A: 

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.
Michael Burr
Isn't hauling in Boost just go get memory management pretty silly?
John Millikin
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.
Michael Burr
will the dtor of base class will be called even if i have once?what will happen to below line base *temp = new base();
yesraaj
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).
Michael Burr
then y should one care to release the memory of individual member?
yesraaj
See the summary I added which I think should make the reasons why a little more clear.
Michael Burr
Note: The destructors of FULLY constructed members will be called.
Martin York
A: 

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

RWendi

RWendi
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.
RWendi
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.
Chris Cleeland