8

People have argued pretty strongly against throwing exceptions from destructors. Take this answer as an example. I wonder whether std::uncaught_exception() can be used to portably detect whether we are in the process of unwinding the stack due to some other exception.

I find myself deliberately throwing exceptions in destructors. To mention two possible use cases:

  • Some resource cleanup which involves flushing buffers, so that failure likely signifies truncated output.
  • Destruction of an object holding a std::exception_ptr which might contain an exception encountered in a different thread.

Simply ignoring these exceptional situations feels plain wrong. And chances are that by throwing an exception some exception handler might be able to provide more useful context information than if the destructor itself were writing to std::cerr. Furthermore, throwing exceptions for all failed assertions is an important part of my unit testing approach. An error message followed by an ignored error condition wouldn't work in that case.

So my question is, is it OK to throw exceptions except when another exception is being processed, or is there a reason not to do that?

To put this in code:

Foo::~Foo() {
  bool success = trySomeCleanupOperation();
  if (!success) {
    if (std::uncaught_exception())
      std::cerr << "Error in destructor: " << errorCode << std::endl;
    else
      throw FooOperationFailed("Error in destructor", errorCode);
  }
}

As far as I can tell, this should be safe and in many cases better than not throwing an exception at all. But I'd like to verify that.

Community
  • 1
  • 1
MvG
  • 57,380
  • 22
  • 148
  • 276
  • No, `uncaught_exception` can't really be used for this. – R. Martinho Fernandes Mar 05 '13 at 11:13
  • 2
    `std::uncaught_exception` is not new to C++11. Also, take a look at [this GotW](http://www.gotw.ca/gotw/047.htm). – Xeo Mar 05 '13 at 11:13
  • > It provides a way of knowing whether there is an exception currently active. (Note that this is not the same thing as knowing whether it is safe to throw an exception.) http://www.gotw.ca/gotw/047.htm – R. Martinho Fernandes Mar 05 '13 at 11:14
  • 3
    Both your use cases imply you have larger design issues that should be addressed before you worry about whether throwing an exception from a destructor is a good or bad thing. –  Mar 05 '13 at 11:18
  • 1
    Is there any way you can think of that you could recover from this situation if you catch the exception? – Dariusz Mar 05 '13 at 11:38
  • You're the sort of guy who sticks your head out of train windows because you have a good method for detecting the presence of another train, aren't you? :) If the output being truncated is important to you then flush the buffers in the main thread, not in the destructor. Just stop throwing exceptions in destructors. – TheMathemagician Mar 05 '13 at 12:07

2 Answers2

10

Herb Sutter has written on the subject: http://www.gotw.ca/gotw/047.htm

His conclusion is to never throw from a destructor, always report the error using the mechanism that you would use in the case where you can't throw.

The two reasons are:

  • it doesn't always work. Sometimes uncaught_exception returns true and yet it is safe to throw.
  • it's bad design to have the same error reported in two different ways, both of which the user will have to account for if they want to know about the error.

Note that for any given reusable piece of code, there is no way to know for sure that it will never be called during stack unwinding. Whatever your code does, you can't be certain that some user of it won't want to call it from a destructor with a try/catch in place to handle its exceptions. Therefore, you can't rely on uncaught_exception always returning true if it's safe to throw, except maybe by documenting the function, "must not be called from destructors". If you resorted to that then all callers would also have to document their functions, "must not be called from destructors" and you have an even more annoying restriction.

Aside from anything else, the nothrow guarantee is valuable to users - it helps them write exception-safe code if they know that a particular thing that they do won't throw.

One way out is to give your class a member function close that calls trySomeCleanupOperation and throws if it fails. Then the destructor calls trySomeCleanupOperation and logs or suppresses the error, but doesn't throw. Then users can call close if they want to know whether their operation succeeds or not, and just let the destructor handle it if they don't care (including the case where the destructor is called as part of stack unwinding, because an exception was thrown before getting to the user's call to close). "Aha!", you say, "but that defeats the purpose of RAII because the user has to remember to call close!". Yes, a bit, but what's in question isn't whether RAII can do everything you want. It can't. What's in question is whether it consistently does less than you'd like (you'd like it to throw an exception if trySomeCleanupOperator fails), or does less surprisingly when used during stack unwinding.

Furthermore, throwing exceptions for all failed assertions is an important part of my unit testing approach

That's probably a mistake - your unit testing framework should be capable of treating a terminate() as a test failure. Suppose that an assert fails during stack unwinding - surely you want to record that, but you can't do so by throwing an exception, so you've painted yourself into a corner. If your assertions terminate then you can detect them as terminations.

Unfortunately if you terminate then you can't run the rest of the tests (at least, not in that process). But if an assertion fails then generally speaking your program is in an unknown and potentially unsafe state. So once you've failed an assertion you can't rely on doing anything else in that process anyway. You could consider designing your test framework to use more than one process, or just accept that a sufficiently severe test failure will prevent the rest of the tests running. Externally to the test framework, you can consider that your test run has three possible outcomes "all passed, something failed, tests crashed". If the test run fails to complete then you don't treat it as a pass.

Steve Jessop
  • 273,490
  • 39
  • 460
  • 699
  • " your unit testing framework should be capable of treating a terminate() as a test failure" -- I wonder how you'd do that... `set_terminate_handler` or something? – Xeo Mar 05 '13 at 11:31
  • @Xeo that would rely on UB. – R. Martinho Fernandes Mar 05 '13 at 11:32
  • @Xeo: that's one way (although you can't guarantee that anything you do in your terminate handler, like printing an error message, will actually succeed). I would also ensure that if the program terminates then the message "all tests passed" is not printed ;-) – Steve Jessop Mar 05 '13 at 11:32
  • Thanks for the article reference. Your answer appears to be somewhat inconsistent about what to do in case of an dtor failure. In one case you suggest a `close` method and a dtor which ignores all errors. In the unit test situation you talk about `terminate`. My unit tests will treat `terminate` as a failed test, but terminating an application just because some buffer couldn't be flushed might be too severe in many cases. And wouldn't be any better than the `terminate` caused by two concurrent exceptions, right? Catching error messages to `cerr` from dtors would be much work. – MvG Mar 05 '13 at 12:11
  • @MvG: `terminate()` is for assertion failures. Other errors would be handled by throwing an exception (in `close()`) or by logging/ignoring (in the dtor). – Steve Jessop Mar 05 '13 at 12:16
3

This is what standard says about dtors and exceptions:

15.2

(...)

The process of calling destructors for automatic objects constructed on the path from a try block to the point where an exception is thrown is called “stack unwinding.” If a destructor called during stack unwinding exits with an exception, std::terminate is called (15.5.1). [ Note: So destructors should generally catch exceptions and not let them propagate out of the destructor. —end note ]

Since you asked very ambiguous question, the answer depends:

  • Can you throw an exception in dtor such that application won't crash? Yes, you can (in terms: it will compile and sometimes it will run correctly).
  • Should you throw an exception in dtor? No, you should not, because it may (and usually it will) cause you problems.

I'd say, that a need for throwing an exception from dtor is a sign of bad design. It seems, that you're doing something in your destructor, that should be done elsewhere.

Spook
  • 25,318
  • 18
  • 90
  • 167
  • I'd rather think of situations, when you use a class, which author also thought, that throwing exception from dtor is safe, but he made sure, that only one dtor on the way will do so (no uncaught_exception checking on the way). Your dtor throws exception, his does, bang you're dead. I still say, that the simple need of throwing exception from dtor is a bad design, you should probably extract the part of it to another method. Maybe provide some code, which proves, that you *really need* dtors throwing exceptions. – Spook Mar 05 '13 at 11:40
  • Another proof of bad design - in erroneous situation you either should throw an exception or not. You sometimes throw one and sometimes not, the code is inconsistent. Additionally, your exceptions will fire in weird places - for example, at the moment, when valid method ended. It makes debugging a lot harder. – Spook Mar 05 '13 at 11:42
  • My rationale is that I'm *always* throwing an exception in an erroneous situation. In some cases it's an exception already under way, in other cases it's a new one. I realize that this depends in large parts on the calling code, and whether it catches and processes some kinds of exceptions different from others. But lacking any fine-grained `try`/`catch` blocks around the scopes in question, some rather coarse-grained global `catch` would be the place where any erroneous exception eventually ends up, and where I'd report *one* such exception, preferrably the first one encountered. – MvG Mar 05 '13 at 12:16
  • It clearly seems then, that code which causes erroneous situation should be extracted from the destructor to some outside `deinit()` or `dispose()` method. Actually, I thought of situation, when exceptions could be safely thrown from destructors and still I won't do so. That's a bad design. This object is just being destroyed and is supposed to seize to exist, not to throw an error. Error in destructor probably should be guarded by an assertion, not an exception. – Spook Mar 05 '13 at 12:20