Sign in to follow this  
beebs1

Exceptions in constructor

Recommended Posts

beebs1    398
Hiya, Quick question - can anyone see a problem with calling the destructor from a class' constructor, when an exception is thrown? Something like this:
Test::Test()
{
   try {

      /* ... */

      throw std::exception( "Uh-Oh..." );

   } catch( const std::exception& ) {

      this->~Test();   // assuming the destructor will never throw.
      throw;
   }
}
It looks fine to me, but I'd appreciate a second opinion if possible :) Cheers, James

Share this post


Link to post
Share on other sites
SiCrane    11839
Well, that will probably double delete all your member variables. That would be a bad thing.

Share this post


Link to post
Share on other sites
beebs1    398
Thanks! Good job I asked :)

I'll just go with a common Release() method which the ctor/dtor can both call then.

Thanks again.

Share this post


Link to post
Share on other sites
visitor    643
As long as the constructor hasn't returned, the object is not considered ready and it would be an error to call the destructor. Besides, the only place where it is valid to call destructor manually is objects created with placement new (and C++0x unions?)

The ideal solution is that there is no manual memory management to be done, for example dynamically allocated member arrays are handled by std::vector, dynamically allocated member objects are handled by smart pointers etc. Then you'd just throw the exception and the clean-up is done by the destructors of the (successfully created) members.

Share this post


Link to post
Share on other sites
Zahlman    1682
Quote:
Original post by beebs1
Hiya,

Quick question - can anyone see a problem with calling the destructor from a class' constructor, when an exception is thrown?


Yes; the destructor is *already called for you automatically*. That's why you would get a double-deletion, and why calling a common Release method is just hiding the problem (you would still get a double-cleanup that might still do something Bad(TM)).

You see, the try and catch parts of a try-catch block each create a scope for variables - just like every other {} pair. So, eventually something or someone catches the exception, and the way that happens is with a try-catch. That means that the try part of that try-catch ends, which calls destructors for everything local to the try block - and everything in any nested scopes has already had its destructors called. And the constructor call can't be earlier the call stack, either, for the simple reason that the exception has been caught.

Quote:
It looks fine to me, but I'd appreciate a second opinion if possible :)


Manually calling a destructor should never "look fine" to a beginner. It is only used to balance a call to placement new, which is a technique a beginner won't have to deal with directly (even experts will, generally, almost never have to).

Share this post


Link to post
Share on other sites
SiCrane    11839
Quote:
Original post by Zahlman
Yes; the destructor is *already called for you automatically*.

No, just the destructors of the member variables.

Share this post


Link to post
Share on other sites
visitor    643
If the constructor throws an error, the constructed instance was never finished and its destructor will be never called. However, what will get double-deleted if you call the destructor manually like that are base classes and members (once when exiting constructor due to the exception, and once by the explicit destructor call).

With the destructor call commented out, everything happens exactly as it should: the base class and the first, successfully created Test instance are destructed (the latter by the std::auto_ptr).

If you call the destructor explicitly, hell breaks loose and Base and the first Test member will be destructed twice.


#include <iostream>
#include <stdexcept>

class Test
{
int id;
public:
Test(int i): id(i)
{
if (id == 666) {
throw std::runtime_error("Evil");
}
else {
std::cout << "Test(" << i << ")\n";
}
}
~Test()
{
std::cout << "~Test(" << id << ")\n";
}
};

struct Base
{
Base() { std::cout << "Base()\n"; }
~Base() { std::cout << "~Base()\n"; }
};

struct Derived: Base
{
std::auto_ptr<Test> a, b;
Derived()
{
try {
a.reset(new Test(1));
b.reset(new Test(666));
}
catch (...) {
std::cout << "Caught ...\n";
//this->~Derived();
throw;
}
}
~Derived() { std::cout << "~Derived\n"; }
};

int main()
{
try {
Derived d;
}
catch (std::exception& e) {
std::cout << e.what() << '\n';
}
}




Also note that due to the use of smart pointers (well, std::auto_ptr has very limited usage - Derived should probably be non-copiable to avoid issues with ownership transfer) there's nothing to do in the catch(...) block (if we didn't want to print the message). And there wouldn't be any need for the try-catch-rethrow scenario at all. (Exceptions don't go well hand-in-hand with unmanaged resources, though.)

Share this post


Link to post
Share on other sites
Zahlman    1682
Quote:
Original post by SiCrane
Quote:
Original post by Zahlman
Yes; the destructor is *already called for you automatically*.

No, just the destructors of the member variables.


Er, yes. Oversimplification :)

The destructors are called for everything that needs a destructor called; if an object throws an exception in its constructor, then it "officially never existed", so its own destructor is not called. If your class does any kind of dynamic allocation (especially in the initialization list), this will result in leaking the memory (or other resource) if you aren't careful. In particular, you can't properly dynamically allocate more than one thing in the initialization list ever, because if the attempt at the second allocation throws, there is no way to set up code to clean up the first allocation.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this