delete after exception in Constructor?

[Energon]

In C++, if your constructor throws an exception, and you catch that exception, do you then have to delete the memory (if you created with the new operator)? Or does throwing the exception automatically clean things up?

[stavefan]

hmmm...that's a good question...

i don't believe you have to...the newed may not be on the heap until you finish the constructor...i'm just guessing on that one, but give it a shot and see what happens....

good luck,
stavefan

[Stuka]

Darn good question - you know how to test this of course - create a class with a single huge array in it, then set the ctor up so it intentionally throws an exception, and call it in an infinite loop - you'll get a crash really quick if it doesn't clean up - but my guess is that the only items to worry about are those that your ctor creates with new.

[Energon]

Now this is strange. I've looked over this code and can't figure out why it's doing what it's doing:


#include <stdexcept>
#include <iostream>
using namespace std;


class BigPoo
{
public:
BigPoo() throw(runtime_error) : one(0), two(1), three(2), four(3)
{
testPtr = new char[1024];
throw runtime_error("poo");
}

~BigPoo ()
{
delete[] testPtr;
}

private:
int one, two, three, four;
char* testPtr;
};


int main(int argc, char* argv[])
{
BigPoo* poo = NULL;
while(true) {
try {
poo = new BigPoo();
} catch(runtime_error& e) {
delete poo;
continue;
}
}
}



It doesn't crash, but it does eat memory like nothing else. Everything that's being created with new is being explicitly deleted by the code, but up goes the memory until the app is killed.

[furrycat]

Get the destructor to print something to the console. You'll see it isn't called.

Don't ask me why this is. I'm more of a C programmer and I only recently started delving into the arcane workings of C++.

/me reaches for an O'Reilly book...

[furrycat]

Yeah that makes sense: http://groups.google.com/groups?hl=en&lr=&ie=UTF-8&oe=UTF-8&threadm=%238Q3VtOa%24GA.1388%40cppssbbsa06&rnum=2&prev=/groups%3Fq%3Dexception%2Binside%2Bconstructor%26hl %3Den%26lr%3D%26ie%3DUTF-8%26oe%3DUTF-8%26selm%3D%25238Q3VtOa%2524GA.1388%2540cppssbbsa0 6%26rnum%3D2

[Stuka]

Energon: that's what I thought might happen - you're new'd data isn't being deleted due to the exception. What you'd have to do to fix that is make sure that your exception class contained a reference to the object that threw it, and explicitly delete the object, calling its destructor. Say - can you tell how much memory is being eaten every time? I'm curious to see if the ints you declared are being correctly removed or not - though I'd bet they aren't.

[Energon]

Huh. For some reason that just doesn't sit right with me. Not as in somebody's wrong, but that the behavior should be different. If I call delete on an object, it should try with every ounce of its might to destroy it, whether it was fully created or not. I honestly would rather have that app Segfault on the delete than not even call the dtor and chew through memory. In fact, the worst part is that I explicitly call delete and it doesn't do jack... I could understand from a local variable, but a dynamic one? Come on...

I don't personally know of a way to track the memory usage on that close of a level, but if I read the thread furrycat posted correctly then the int memory should be given back properly. It's just the dynamic memory that isn't going back.

Anyway, thanks for helping me figure this one out. I'll have to make a note to always keep a pointer to dynamic memory in my exceptions when I throw them from a ctor.

[Energon]

Wait... this can't be right. I modified my code:


#include <stdexcept>
#include <iostream>
using namespace std;


class BigPoo
{
public:
class BigPooException : public runtime_error
{
public:
BigPooException(const std::string& what, char* poovar) : _what(what), _poovar(poovar) {}
virtual ~BigPooException() throw() { delete[] _poovar; }
virtual const char* what() throw() { return _what.c_str(); }

private:
std::string _what;
char* _poovar;
};

public:
BigPoo() throw(BigPooException) : one(0), two(1), three(2), four(3)
{
testPtr = new char[1024];
if(NULL == testPtr)
cout << "memory error" << endl;
throw BigPooException("poo", testPtr);
}

~BigPoo ()
{
cout << "destructor" << endl;
delete[] testPtr;
}

private:
int one, two, three, four;
char* testPtr;
};


int main(int argc, char* argv[])
{
BigPoo* poo = NULL;
while(true) {
try {
poo = new BigPoo();
} catch(BigPoo::BigPooException e) {
// delete poo;
continue;
}
}
}


And it segfaults when it deletes the memory in the exception. I also modified it to pass a pointer to the object instead of just that memory, and it segfaulted in the destructor. So either where did I go wrong or where am I losing that memory?

EDIT - Nevermind, deleting that memory in the exception constructor rather than the destructor works perfectly.

[Silent Bob]

I don't mean to hijack your thread but I have a similar problem with new/delete.

this is the code:
struct Face
{
//these have to be dynamically allocated because there is no
//guarantee as to how many vertices are in a face
unsigned int* v;
unsigned int* vt;
unsigned int* vn;
unsigned int count;
Face(unsigned int amount)
{
v = new unsigned int[amount];
vt = new unsigned int[amount];
vn = new unsigned int[amount];
//ensure that the arrays have been allocated
if(v == NULL || vn == NULL || vt == NULL)
{
cerr << "Out of memory" << endl;
exit(EXIT_FAILURE);
}
//set the count of vertices to the correct amount
count = amount;
};
~Face()
{
if(v != NULL)
delete [] v;
if(vt != NULL)
delete [] vt;
if(vn != NULL)
delete [] vn;
};
}

What happens is that it segfaults as soon as it comes to destructing a Face.
Maybe there I have made some really basic mistake, the kind that it takes another person to notice, but I don't think that I have.

Anybody got any ideas?

<edit>Damn formatting</edit>

[furrycat]

You are checking if v, vt or vn are null and deleting them if not.

If a Face is default or copy-constructed, these variables will "have" the value of whatever happened to be in the area of memory that was allocated for them. Therefore the null test will indicate that they should be deleted whereas in fact you did not allocate your int array.

[Silent Bob]

I thought it could be something like that which is why I only have the one constructor which allocates the arrays.

It's very odd. I think I'll try compiling on win32 and see if it behaves the same way.

[Energon]

The behavior is exactly what should happen, so if it doesn't behave that way in win32, that's just one more reason to not write code for it...

Just define a default constructor that initializes the values to NULL and you're set...

[Silent Bob]

I've got me a default constructor that sets all the pointers to NULL (and I am setting them to NULL in the other constructor too, for good measure).

However I STILL get segfaults whenever a Face is destructed.

As I understand it the whole memory allocation procedure goes a bit like this:

1) use new to create stuff on the heap and assign to a pointer.

2) use the heap memory through the pointer

3) use delete to free up the heap memory.

Maybe I'm being really dumb but I can't for the life of me see what I have done wrong.

Your help is much appreciated

[Energon]

I'm just gonna go out on a limb and say try not declaring it as a struct, but rather a class and see what happens.

[Silent Bob]

Thanks for the help.

I found out what I was doing wrong (eventually:))

I am copying Faces into an std::vector<> and hadn't provided a copy constructor. So when the vector was copying the values it just copied the values of the pointers not the arrays. Then when the temporary Faces fell out of scope and the destructor was called it was trying to free memory that was referenced by the copy of the Face stored in the vector.

I added a copy-constructor to deal with the arrays properly and it works now!

[furrycat]

Don't believe the hype. Anyone who tells you I said that five posts up is lying.

[Silent Bob]

Originally posted by furrycat
Don't believe the hype. Anyone who tells you I said that five posts up is lying.

Sorry :) I didn't quite get what you had said.

What confused me was that a 'default' copy constructor was being used when the Face was copied into the vector. I was under the impression that once you had provided a constructor then the only way to initialise an instance was to use that constructor (ie the default one can't be used any more). I guess I was wrong.