views:

345

answers:

6

I know destructor shouldn't not throw exception.

http://www.parashift.com/c++-faq-lite/dtors.html#faq-11.13

I have the following code :

~a()
{
    cleanup();
}

// I do not expect exception being thrown in this function.
// If exception really happen, I know that it is something not recoverable.
void a::cleaup()
{
    delete p;
}

In my static source code analysis, it complains that I shall call the cleanup function in this way :

~a()
{
    try {
        cleanup();
    }
    catch(...) {
        // What to do? Print out some logging message?
    }
}

// I do not expect exception being thrown in this function.
// If exception really happen, I know that it is something not recoverable.
void a::cleaup()
{
    delete p;
}

I am not sure whether this is a good practice, to place try...catch block in destructor, whenever it calls functions. As :

(1) If cleanup function able to throw exception, I know something bad had happened. I prefer it to be fail-fast. That is, just let the whole system crash, and let the programmer debugs it.

(2) Overhead occur while entering and exiting try...catch block.

(3) The code look cumbersome, with a lot of try...catch block around classes' destructor.

I may miss out some other points, why try...catch block should be in place.

Thanks.

A: 

I never use this, but exception safety purists will say that your cleanup() function may throw without you expecting it. So this pattern is a technique for extra safety.

However, a cleanup function should not throw in the first place. So I would put any exception handling inside cleanup(), not in the destructor itself.

Daniel Daranas
"exception safety purists" should *know* which code can throw exceptions. There is nothing "pure" about blindly putting try/catch everywhere
jalf
+2  A: 

Maybe you can be more precise about your cleanup function. Does the cleanup() function throw an exception intentionally, if only in very rare circumstances? Then you should definitely handle the case in your destructor because, even if rare, exception-throwing is expected behavior of cleanup().

At the very least, your exception handling code could leave your program in a well-defined state that would allow you to end your program gracefully.

Or do you mean cleanup() may throw e.g. an OutOfMemoryException or another runtime exception that you never handle anywhere else? Then I would omit the exception handling in the destructor as well. In such unusual circumstances, your application could probably not run its error handling code anyway. You should do everything possible to make sure even such an exceptions gets reported or logged properly.

Edit:

Now that you have shown the implementation of cleanup, Neil has already answered your question. delete p must not throw, hence a::cleanup will not throw as long as p's destructor does not throw.

Sebastian
+1  A: 

Surely the issue is really in the destructor of p. When, in cleanup(), you say

delete p;

delete itself cannot throw, therefore the exception must comne from p's destructor. It is in that destructor you need to worry about using a try block, not the one for a.

anon
+1  A: 

Fundamentally, it is not safe to throw an exception whilst already handling an exception.

If your destructor is being called whilst the stack is unwinding, and another exception is thrown, then your thread can be terminated immediately on some systems such as Symbian; that your code was planning to catch the exception won't stop that termination - your code won't even be called. Your thread certainly will be terminated if your exception escapes the destructor.

So try {} catch(...) {} in a destructor is not portable and is certainly tricky.

The sound advice is never call code that could throw an exception in cleanup code such as destructors and any function that might be called from a destructor, such as 'cleanup()', 'close()' or 'release_something()'.

The original poster also queries the performance of try-catch. Early in the adoption of C++ exceptions, exception handling code was reasonably expensive; these days, your compiler almost certainly uses zero-cost exceptions, which means that an exception not thrown adds no runtime performance penalty to the code (but of course does slightly increase the binary size of the program).

Will
"If your destructor is being called whilst the stack is unwinding, and another exception is thrown, then your thread will be terminated immediately". Please stop saying this. It is not true. Last time you said it, a whole bunch of people told you it wasn't true, and provided references, and pointed out that your reference did not say what you thought it said. "Thrown from a destructor" != "thrown in a destructor and caught before leaving the destructor".
Steve Jessop
I apologise, I've worked for ages on Symbian and there it definitely kills you if you raise an exception whilst unwinding.
Will
Heh, I've worked on Symbian too. When writing Symbian-style code, I just assume that Symbian will kill you if you raise an exception anywhere, ever. I don't like Leave, but it's what there is.
Steve Jessop
Leave is implemented in terms of proper exceptions these days (for the zero-cost bit, rather than longjmp), but as you say its a bit headaching to mix your own throws, since only an int slot is kept for it, and there's a cleanup stack to worry about
Will
A: 

There's not usually any point in throwing an error due to memory allocation or deletion. It either works or not, and you can spend a ton of work handling silly errors without getting any benefit.

You might want to do this in specific cases but it's nor done much in practice and when there's question you usually want to use debugger or more full instrumentation.

Charles Eli Cheese
+2  A: 

Since delete won't throw, neither will cleanup and as such there is no need to put the call in try-catch.

Since your static analysis tool is probably having a hard time figuring that out, perhaps you could help it out (it's only a guess though) by declaring cleanup as no-throw.

void cleanup() throw();
avakar
+1, although make sure p's destructor really is nothrow. Falling over due to calling `unexpected()` isn't really any better or worse than falling over due to `terminate()` if the exception leaves the ~a when an A is being unwound. But the static analysis tool probably thinks it is...
Steve Jessop