6

I was recently tasked with hunting down a memory leak in a part of our code. The leak ended up being in the destructor for a particular object...and I found something really strange. A former coworker wrote this:

File::~File()
try
{
    Clear();
}
catch (...)
{
    Log("caught exception");
}

The file class inherits from some base classes. My first question is: is this strictly legal C++? It compiles in Visual Studio 2008, but I showed it to a few friends / coworkers and they were fairly horrified that it worked.

It doesn't actually work as intended, though: the base class that this object inherits from has a destructor that is now never called (as opposed to if you just wrapped the destructor in a regular method block, having the try / catch as part of that method).

Can anyone take a stab at explaining why this is allowed, and why the base class destructor was not called? The destructor here was not throwing.

phyllis diller
  • 795
  • 2
  • 9
  • 21

2 Answers2

10

This is a function try block and it's completely legal.

See, for example, here.

The only time that you can do something in a function try block that you can't do in a normal try block in a function is catch exceptions thrown by expression in a constructor initializer list (and even then you end up having to throw something), but that doesn't apply here.

This GOTW #66 is particularly interesting, although it concentrates more on constructors. It contains this "moral":

Since destructors should never emit an exception, destructor function-try-blocks have no practical use at all.

Just to add clarification, the code as written will cause any exception caught to be rethrown due to ISO/IEC 14882:2003 15.3 [except.handle] / 16:

The exception being handled is rethrown if control reaches the end of a handler of the function-try-block of a constructor or destructor. [...]

However it is legal to have a parameterless return in the handler of a function try block for a destructor - it is only forbidden in a function try block for a constructor - and this will supress the rethrow of the exception. So either of these alternatives would prevent the exception from leaving the destructor.

File::~File()
try
{
    Clear();
}
catch (...)
{
    Log("caught exception");
    return;
}

File::~File()
{
    try
    {
        Clear();
    }
    catch (...)
    {
        Log("caught exception");
    }
}
Community
  • 1
  • 1
CB Bailey
  • 755,051
  • 104
  • 632
  • 656
  • 1
    @Phyllis: In short, no. Destructors should never, ever throw exceptions, so you should never, ever have anything to catch. – Dennis Zickefoose May 10 '11 at 15:58
  • @phyllisdiller: What @DennisZickefoose said. Don't use them. – CB Bailey May 10 '11 at 16:00
  • Actually it appears that the entire purpose of it here is to prevent `~File` from throwing when it calls `Clear()`, so I think this is a valid use. – Cory Nelson May 10 '11 at 16:06
  • @CoryNelson: But it won't prevent that. The exception will be caught and rethrown. "The exception being handled is rethrown if control reaches the end of a handler of the function-try-block of a constructor or destructor." – CB Bailey May 10 '11 at 16:09
  • Gotcha. My plan was to just slap the Clear call in a regular try / catch, which seems to be what GOTW is suggesting: Moral #4: Always clean up unmanaged resource acquisition in local try-block handlers within the constructor or destructor body, never in constructor or destructor function-try-block handlers. – phyllis diller May 10 '11 at 16:23
  • @RedX -- functions called by destructors *are* allowed to throw. But they must be caught in a regular try block, not a function try block. – Robᵩ May 10 '11 at 16:40
  • Even destructors are _allowed_ to throw in C++, it's just (almost?) never a good idea. – CB Bailey May 10 '11 at 16:42
  • @Charles: I'm pretty sure it's never a good idea, but if you had a bad situation to begin with, where the thing called by the destructor, that throws, needs to be handled higher up, then you'd have no choice but for the dtor to throw. And if it's called and throws as part of stack unwinding from another exception, then `terminate` is about the best thing that can happen, and that's what's defined to happen. Better would be to fix the thing that can throw and needs catching, but I suspect cases *could* occur where that's not within the power of this particular class. – Steve Jessop May 10 '11 at 18:48
  • Then again, the definition of "needs to be handled higher up" would be, "some specific code must execute to heal the problem before the program can continue, that's in a catch clause higher up, and that's why we can't just swallow the exception, *but* in the event that there isn't already an exception in flight when the dtor is called, it's OK for further destructors to run before that healing occurs". Which could only happen as the result of some design catastrophe. – Steve Jessop May 10 '11 at 18:50
4

To answer the second part, "why the base class destructor was not called?", 12.4/6:

After executing the body of the destructor and destroying any automatic objects allocated within the body, a destructor for class X calls the destructors for X’s direct members, the destructors for X’s direct base classes... A return statement (6.6.3) in a destructor might not directly return to the caller; before transferring control to the caller, the destructors for the members and bases are called.

This doesn't say that the member and base destructors are called if the destructor throws. However, 15.2/2 says:

An object that is partially constructed or partially destroyed will have destructors executed for all of its fully constructed subobjects,

I think this should be true whether the object is "partially destroyed" because of an exception thrown from the body of the destructor, or because of an exception thrown from the the function try block of the destructor. I'm pretty sure that "after the body of the destructor" is supposed to mean also after a function try block.

Apparently Microsoft disagrees, though, and because of the function try block it hasn't generated "the body of the destructor", and hasn't done the things that happen after executing "the body of the destructor".

That doesn't sound right to me. GCC 4.3.4 does execute the base class destructor, whether the derived class dtor function try block throws or not. In the case where it throws, the base is destructed before the catch clause is executed.

Steve Jessop
  • 273,490
  • 39
  • 460
  • 699