News:

MASM32 SDK Description, downloads and other helpful links
MASM32.com New Forum Link
masmforum WebSite

Will this leak?

Started by redskull, February 23, 2011, 08:52:26 PM

Previous topic - Next topic

redskull

For the C++ experts:

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

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

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

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
Strange women, lying in ponds, distributing swords, is no basis for a system of government

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.

Quote from: redskull on February 23, 2011, 08:52:26 PMAccording to the spec, bar() will be deallocated, but what about baz()?
What spec?
The truth cannot be learned ... it can only be recognized.

redskull

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


foo::foo()
{
     throw 1;
}


that the statment


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,


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:


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:



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:


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
Strange women, lying in ponds, distributing swords, is no basis for a system of government

drizz

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

Quote from: redskull on February 25, 2011, 12:30:44 AM
my next guess at an exception safe snippit would be something hacky like this, but I would anticipate scoping issues:


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:
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{}.
The truth cannot be learned ... it can only be recognized.

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

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

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

#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:


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:


baz CTOR   
bar CTOR
baz DTOR
Caught!


However, this modification:


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:


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...
Strange women, lying in ponds, distributing swords, is no basis for a system of government

drizz

Quote from: redskull on February 25, 2011, 01:41:06 PMSo 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.
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.
The truth cannot be learned ... it can only be recognized.

redskull

Quote from: drizz on February 25, 2011, 05:06:22 PM
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:


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:


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
Strange women, lying in ponds, distributing swords, is no basis for a system of government

drizz

Now that's a good question :)


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


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



            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.


The truth cannot be learned ... it can only be recognized.

redskull

Quote from: drizz on February 25, 2011, 08:15:33 PM
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
Strange women, lying in ponds, distributing swords, is no basis for a system of government

drizz

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


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.

The truth cannot be learned ... it can only be recognized.

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:


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
Strange women, lying in ponds, distributing swords, is no basis for a system of government