Will this leak?

[redskull]

For the C++ experts:

A 'foo' object constructor requires a pointer to a 'bar' object and a pointer to a 'baz' object:

Code Select Expand

foo::foo(bar* o1, baz* o2);

The code creates a new foo object on the stack, like so:

Code Select Expand

foo f = foo(new bar(), new baz());

baz() is constructed first and successfully, but then bar() throw an exception.  According to the spec, bar() will be deallocated, but what about baz()?

-r

[drizz]

If I say that "new = alloc memory, call constructor" and "delete = call destructor, free memory" (and "on stack = automatic free"), does that answer your question?

Do "bar","baz" have copy constructors or is the care for newly created objects given to "foo" class?
If "foo" does not handle deallocation then you have leaks.

The "exception" comes from something else.

According to the spec, bar() will be deallocated, but what about baz()?

What spec?

[redskull]

It is my understanding that the C++ spec says that a throw during a heap won't leak; that is, given

Code Select Expand


foo::foo()
{
     throw 1;
}


that the statment

Code Select Expand


foo* f = new foo();


doesn't leak any memory; i.e. if the ctor doesn't finish, then the deallocation is (should be) undone by the compiler.  So, if I do things seperately,

Code Select Expand


baz* b1 = new baz();    // constructs fine
bar* b2 = new bar();    // throws exception
foo* f =  new foo(b2, b1);    // never executes, stack or heap


b1 would *not* be deallocated, b2 would, and f would never be allocated in the first place.

So, if I put it altogether:

Code Select Expand


foo* f = new foo (new bar(), new baz())


I presume the same thing happens?  In which case, how would someone prevent against this?  Something like this wouldn't work:

Code Select Expand


try
{
    foo* f = new foo (new bar(), new baz());
}
catch
{
    // should be deallocating baz, but no pointer to do it with!
}


my next guess at an exception safe snippit would be something hacky like this, but I would anticipate scoping issues:

Code Select Expand


try
{
foo* f = new foo ( bar* t1 = new bar(), baz* t2 = new baz())
}
catch
{
    if (t1) delete t1;
    if (t2) delete t2;
}


Exception safety in ctors is much more subtle than I anticipated, so any suggestions are appreciated.

-r

[drizz]

Ok, more clear now.

Code Select Expand

foo* f = new foo (new bar(), new baz())
The question still stands, where are these two objects deallocated if it succeeds?

my next guess at an exception safe snippit would be something hacky like this, but I would anticipate scoping issues:

Code Select Expand


try
{
  foo* f = new foo ( bar* t1 = new bar(), baz* t2 = new baz())
}
catch
{
   if (t1) delete t1;
   if (t2) delete t2;
}


I wouldn't call your code hacky. "delete" accepts "NULL" so I don't see why something like this wouldn't work:

Code Select Expand

bar* t1 = NULL;
baz* t2 = NULL;
foo* f = NULL;
try
{
  t1 = new bar();
  t2 = new baz();
  f = new foo (t1, t2);
}
catch(...)
{
// if you want to return or abort delete here also
}
// finally { //  :)
delete t2;
delete t1;
delete f;
//}

And you anticipate scoping issues rightly, no stack vars or var declaration inside try{}.

[Antariy]

Have a look to this code.

Just simple class is used which holds pointer to a new'ed object, and may return it as the pointer to such class object.

With such approach you may forget about manual deletion of the function-local heap objects. As well, as with unwinding semantics used (/EHsc compiler switch) all the objects of the all the top-level functions will be freed on exception if they was allocated with such wrapper class, untill would be found handler of the exception.

I.e. something like

Code Select Expand


f0{

try{
f1();
}
catch(...){
}
}


}

f1{
SimplePtr<c1> o1;
f2();

}

f2{
SimplePtr<c2> o2;
f3();

}

f3{
SimplePtr<c3> o3;
f4();

}

f4{
throw(XXX);
}


will properly freed all the objects in all the levels (i.e. o3,o2,o1).

Results for

Code Select Expand


baz 00420020 ctor
Simple local pointes wrapper ctor, SP object: 0012FF40, new object: 00420020
bar 01830020 ctor (throw)
Simple local pointes wrapper dtor, SP object: 0012FF40, del object: 00420020
baz 00420020 dtor
An exception: 123


baz was properly allocated.
bar threw excection.
baz was properly deallocated.
return to the top level function due to it contains nearest handler.

Q.E.D. :bg

Here is the

Code Select Expand


#include "stdio.h"
#include "conio.h"


class bar {
public:
bar(){printf("bar %p ctor (throw)\n",this);throw((int)123);}
~bar(){printf("bar %p dtor\n",this);}
char buf[1024*1024*10];
};

class baz {
public:
baz(){printf("baz %p ctor\n",this);}
~baz(){printf("baz %p dtor\n",this);}
char buf[1024*1024*20];
};

class foo {
public:
foo(bar* br, baz* bz){
printf("foo %p ctor\tbar: %p, baz: %p\n",this,br,bz);
}
~foo(){printf("foo  %p dtor\n",this);}
};

template <class C>
class SimplePtr {
public:
SimplePtr(){
ptr=new C;
printf("Simple local pointes wrapper ctor, SP object: %p, new object: %p\n",this,ptr);
}
~SimplePtr(){
printf("Simple local pointes wrapper dtor, SP object: %p, del object: %p\n",this,ptr);
delete ptr;

}

operator C*(){
return ptr;
}

private:
C* ptr;
};

int test(){
foo fo=foo(SimplePtr<bar>(),SimplePtr<baz>());
printf("Just for making sure compiler doesn't strip this out... foo: %p",&fo);

return 0;
}

int main(){

try{
test();
}
catch(int i){
printf("An exception: %d",i);
}

_getch();
}


Any comments about formatting would be ignored and not accepted :green2

[redskull]

On close inspection, it appears that it might deallocate on its own.  The following code (where each ctor/dtor just prints a message) gives me these results:

This code:

Code Select Expand


bar* t1 = 0;
baz* t2 = 0;

    try
    {
        t2 = new baz();             //no throw
        t1 = new bar();             //throws
        foo f = foo(t1, t2);        //never happens
    }
    catch (int)
    {
        if(t1) delete t1;           //not needed
        if(t2) delete t2;           //needed
        cout << "Caught!" << endl;
    }
    return 0;
}


produces these results, as expected:

Code Select Expand


baz CTOR   
bar CTOR
baz DTOR
Caught!


However, this modification:

Code Select Expand


bar* t1 = 0;
baz* t2 = 0;

    try
    {
        foo f = foo( t1=new bar(), t2=new baz());
    }
    catch (int)
    {
        if(t1) delete t1;       //not needed
        if(t2) delete t2;       //not needed
        cout << "Caught!" << endl;
    }
    return 0;
}


produces this:

Code Select Expand


baz CTOR
bar CTOR
Caught!


So it would seem as though the compiler DOES take care of the deallocation on its own, and the temp pointers are not required.  This begs the question, however, of what would happen to any of the resources that the baz ctor aquires, since it completes succesfuly yet the destructor isn't (and apparently can never be) called...

[drizz]

So it would seem as though the compiler DOES take care of the deallocation on its own

If it did then you would get the same result.

The thing is that neither t2 nor t1 get assigned until both "new/CTOR" finish.


So either use manual(-more code) deletion method or automatic(-careful about scope ) via auto_ptr.

Code Select Expand

auto_ptr<baz> t2(new baz());
auto_ptr<bar> t1(new bar());
foo f = foo( t1.get(), t2.get() );

...but first make sure tha "foo" does not also delete its bar baz pointers.

[redskull]

The thing is that neither t2 nor t1 get assigned until both "new/CTOR" finish.

Hmm, that certainly complicates things.  That means baz() will be fully built, the ctor ran to completion, and yet no pointer ever returned to the creater, and no chance for the dtor to run?

The reason I ask is the situation where 'foo' is a const member; the only way would be an item on the initializer list, which would require the new statements nested inside:

Code Select Expand


class outer
{
    public:
        outer()
            try : foo(new bar(), new baz())
        {}
        catch (...)
        {
            // now what?
        }
       
    private:
        const foo f;
}


I suppose something like this is better, but there's the two extra useless pointers to drag around for the life of the object, plus it's still not exception proof:

Code Select Expand


class outer
{
    public:
        outer()
            try : b1(new bar()),
                  b2(new baz()),
                  foo(b1, b2)
        {}
        catch (...)
        {
            if(b1) delete b1;  // b1/b2  not necessarily 0!
            if(b2) delete b2;
        }
       
    private:
        bar* b1;
        baz* b2;
        const foo f;
}


b1 and b2 would never be initialized to zero, so there's no realiable way to delete them; would it be deleting a valid object, or whatever garbage data was already in it?

Any suggestions?

-r

[drizz]

Now that's a good question :)

Code Select Expand


            try : foo(new bar(), new baz())


This will cause a leak. Use Antariy's "SimplePtr".

Code Select Expand


            try : foo(SimplePtr<bar>(), SimplePtr<baz>())


But now (in case of no exception) "foo.bar_ptr" and "foo.baz_ptr" only exist for this line and you need to make sure that "foo CTOR" makes a copy of them.

[redskull]

Use Antariy's "SimplePtr"

Except that there is no way to specify the arguments to the constructors, should they take them.  Time for an adventure in template programming  :'(

I thank you both for your help

-r

[drizz]

Code Select Expand

foo::foo(auto_ptr<bar> o1, auto_ptr<baz> o2)

Code Select Expand

outer()
        try: f(auto_ptr<bar>(new bar()), auto_ptr<baz>(new baz()))
        {

Code Select Expand


class foo
{
private:
auto_ptr<bar> bar_ptr;
auto_ptr<baz> baz_ptr;

but you must make another throw in outer catch (:dazzled:) to prevent "foo DTOR" if exception was thrown.

[redskull]

Well, after many weeks of working on an "elegant" solution and then throwing it aside, I have pretty much settled on a static function to do the dirty work and resigning myself to having to hard code arguments.  The 'smart pointer' option just won't work; I realized that in the normal case where both bar and baz build successfully, the destructors will *still* delete them after the ctor completes successfully, negating the whole point.  Basically, I have this:

Code Select Expand

class outer
{
    public:
   
        outer() :f1(BuildFoo())     // use static function to build foo
        {

            // f1 is now read only here
           
        }
       
    private:
        const foo* const f1;
       
        static foo const * const BuildFoo()
        {
       
            bar* b1 = new bar();    // if b1 fails, nothing need to be deleted
            baz* b2;
            try
            {
                b2 = new baz();
            }
            catch  (...)
            {
                delete b1;      // if b2 fails after b1, delete just b1
                throw;
            }
           
            try
            {
                return new foo (b1, b2);   
            }
            catch
            {
                delete b1;  // if foo fails, delete both
                delete b2;
                throw;
            }
        }
};

As always, comments are always appreciated.

-r