views:

1267

answers:

7
+17  Q: 

RAII vs. exceptions

The more we use RAII in C++, the more we find ourselves with destructors that do non-trivial deallocation. Now, deallocation (finalization, however you want to call it) can fail, in which case exceptions are really the only way to let anybody upstairs know of our deallocation problem. But then again, throwing-destructors are a bad idea because of the possibility of exceptions being thrown during stack unwinding. std::uncaught_exception() lets you know when that happens, but not much more, so aside from letting you log a message before termination there's not much you can do, unless you're willing to leave your program in an undefined state, where some stuff is deallocated/finalized and some not.

One approach is to have no-throw destructors. But in many cases that just hides a real error. Our destructor might, for example, be closing some RAII-managed DB connections as a result of some exception being thrown, and those DB connections might fail to close. This doesn't necessarily mean we're ok with the program terminating at this point. On the other hand, logging and tracing these errors isn't really a solution for every case; otherwise we would have had no need for exceptions to begin with. With no-throw destructors we also find ourselves having to create "reset()" functions that are supposed to be called before destruction - but that just defeats the whole purpose of RAII.

Another approach is just to let the program terminate, as it's the most predictable thing you can do.

Some people suggest chaining exceptions, so that more than one error can be handled at a time. But I honestly never actually seen that done in C++ and I've no idea how to implement such a thing.

So it's either RAII or exceptions. Isn't it? I'm leaning toward no-throw destructors; mainly because it keeps things simple(r). But I really hope there's a better solution, because, as I said, the more we use RAII, the more we find ourselves using dtors that do non-trivial things.

Appendix

I'm adding links to interesting on-topic articles and discussions I've found:

+17  A: 

You MUST NOT throw an exception out of a destructor.
If an exception is already propagating then the application will terminate.

By terminate I mean stop immediately. Stack unwinding stops. No more destructors are called. All bad stuff. See the discussion here.

http://stackoverflow.com/questions/130117/throwing-exceptions-out-of-a-destructor#130123

I don't follow (as in disagree with) your logic that this causes the destructor to get more complicated.
With the correct usage of smart pointers this actually makes the destructor simpler as everything now becomes automatic. Each class tides up its own little piece of the puzzle. No brain surgery or rocket science here. Another Big win for RAII.

As for the possibility of std::uncaught_exception() I point you at Herb Sutters article about why it does not work

Martin York
Note that it's not always guaranteed to terminate the program: sometimes you get undefined behaviour. For instance if you're part way through destroying an array or collection when the second exception is thrown.
Steve Jessop
I never said the possibility of dense faults causes dtors to be more complicated. RAII causes that. The more you use dtors to tear-down stuff, the more likely you are to encounter errors during tear-down.
Assaf Lavie
I do agree with Assaf. You put more (cleanup) stuff in dtors so it will be executed when exceptions are thrown, but the degenerate case of this is a function with only ctors and (implicit) dtors. So a lot of stuff in dtors -> is opportunity for a lot of exceptions.
QBziZ
Sorry 'QBziZ' I am unable to follow your logic. Why don't you post an answer with the appropriate detail that explains your position so it can be voted up or down appropriately.
Martin York
@QBziZ I agree with Martin: What you are saying is quite similar to "the more code we add in a program, the more risks we're taking something will fail". Like any cleanup funct, the destructor must TRY to clean. But unlike other functs, it can't throw because a destructor is NOT like others functs.
paercebal
A: 

You can tell whether there is currently an exception in flight (e.g. we are between the throw and catch block performing stack unwinding, perhaps copying exception objects, or similar) by checking

bool std::uncaught_exception()

If it returns true, throwing at this point will terminate the program, If not, it's safe to throw (or at least as safe as it ever is). This is discussed in Section 15.2 and 15.5.3 of ISO 14882 (C++ standard).

This doesn't answer the question of what to do when you hit an error while cleaning up an exception, but there really aren't any good answers to that. But it does let you distinguish between normal exit and exceptional exit if you wait to do something different (like log&ignore it) in the latter case, rather than simply panicing.

puetzk
NOTE: see Martin's post above... this can return true even in a try{..} if an exception unwind is active.
Aaron
A: 

If someone wants to know whether the DB close succeeded (and committed all transactions and so on), then I don't think it's unreasonable for them to have to call reset or whatever. It doesn't defeat RAII, since they're still protected from resource leaks, which is the point of RAII. They just don't know the result of the operation.

If they want to know the result of the operation, I suppose they could write their own RAII wrapping yours, and in its destructor call reset(), record the result somewhere, and then destroy your object. If reset() throws then obviously they'd need another try/catch, but I think that's valid. I'd look it up before doing it myself, mind.

Note that with good design of the resource itself, clients will never be required to perform an action which might fail. It makes no sense - if it's not essential then why is it required, and if it is essential then how come it can fail? Work out what reset() does, move the essential things into the destructor, and leave the things that can fail in reset(). Then RAII will work cleanly for resource management, which is what it is for, and clients that want to know the result of the unreliable operations can store the result when they are performed.

RAII isn't there to remind clients that they might want to know the results of their database transaction. It's there to remind them to clean up the database resource.

Steve Jessop
That's only if you use RAII for simple resource deallocation. If you disallow closing DB connections through RAII, then you withhold a very useful idiom. What's the difference between deallocation and finalization, anyway? It's just more stuff that needs to be called after your bit of code is done.
Assaf Lavie
@Assaf - an RAII object can have a 'close' member that performs cleanup and reports failure by exception, and a destructor that is no-throw and reports no errors. these two features can co-exist, and usually do.
Aaron
@Aaron - as far as I'm concerned that's broken. It's a workaround for Assaf's problem, but basically it's broken. RAII should not have an explicit cleanup method that needs to be called just because it could throw exceptions. That's hidden semantics. Bjarne should fix this Not ISO/ANSI, Bjarne :)
QBziZ
It's not the use of RAII that's broken. You're saying that for this resource, something which may fail (close) is required to be performed. How can something be essential if it might fail? It's the resource API which is broken, and is preventing RAII from working as you think it should.
Steve Jessop
@onebyone.livejournal.com - If we could predict exceptions, we wouldn't really need them. I don't see the relation between being essential and not throwing exceptions.
QBziZ
You can't predict whether an exception will actually occur, but you can (and should) predict and document which functions may throw exceptions. Check out the "nothrow guarantee" in (for example) Effective C++.
Steve Jessop
And the relation is that if it has to happen, but it might fail, then you face the problem that sometimes it won't happen even though it has to. This is a bug.
Steve Jessop
OK API is broken. Suppose you want to make your app behave gracefully faced with these API's. Each time you have to implement an add-hoc -i.o.generic- solution, by handling this in the RAII dtor.RAII is broken in that regard. Why not let the exception propagate? And suppress when already unwinding?
QBziZ
If you suppress when unwinding, then I understand you end up sometimes suppress when not unwinding (see the Herb Sutter article someone linked). So you may as well always suppress for consistency. Since your RAII is lexically scoped, it is very easy for clients to call close() in non-throwing cases.
Steve Jessop
Yes. I do understand all this. And this is exactly why I think it is broken : the suppression should work automatically. And also, if you have to call close(), RAII loses some of its magic.
QBziZ
Things like type and const correctness are very well supported by the compiler, exception correctness is a second class citizen and too much burden is put on the developer. So, to give some back, dtors should be able to throw exceptions, but not cause havock if they do it while unwinding.
QBziZ
Your "should" seems be "C++ should be different", which is a fair point, but isn't going to happen. Even as things stand, clients never *have* to call close, only to get the result. Destructors can't return a value either, and that never harmed anyone.
Steve Jessop
I agree though that C++ missed some tricks (such as checked exceptions). Having destructors magically handle exceptions might be nice, but IMO the current situation isn't onerous. I still think RAII means "clean up my resources", and you're trying to make it mean "finish what I started".
Steve Jessop
>> Your "should" seems be "C++ should be different"Indeed, that's what I meant by being broken.
QBziZ
It's true that dtors cannot return a value but that's where the beauty of return values come into play, they're much harder to ignore than exceptions. If someone ignores a return value, he is explicitly doing things wrong. So a close() with return value has added semantics visible in the interface.
QBziZ
>> I still think RAII means "clean up my resources", and you're trying to make it mean "finish what I started"I'm trying to make it mean : automatic cleanup with minimal hidden semantics, minimal surprises and minimal overhead.
QBziZ
I don't think that (for example) completing a database transaction is "cleanup". I guess that's where we differ. I also don't think it's ever useful for "cleanup" to fail. But the argument has been around since (at least) flush/close in ANSI, so I guess it'll stay for the forseeable future.
Steve Jessop
Actually I agree with you on the DB transaction argument. Yet I would like to throw exceptions- or get them thrown at me -in dtors without worrying that any prior unwinding will behave weird. And if that ex is thrown by a DB, then you may have a bug, but one that would at least surface nicely.
QBziZ
@QBziZ: You should consider what could happen if multiple exceptions were thrown by multiple failing destructors, and how you could handle them if the compiler let you. Anyway, is there any programming language that handle this problem? Has this problem a real solution (real as in "real life")?
paercebal
+5  A: 

From the original question:

Now, deallocation (finalization, however you want to call it) can fail, in which case exceptions are really the only way to let anybody upstairs know of our deallocation problem

Failure to cleanup a resource either indicates:

  1. Programmer error, in which case, you should log the failure, followed by notifying the user or terminating the application, depending on application scenario. For example, freeing an allocation that has already been freed.

  2. Allocator bug or design flaw. Consult the documentation. Chances are the error is probably there to help diagnose programmer errors. See item 1 above.

  3. Otherwise unrecoverable adverse condition that can be continued.

For example, the C++ free store has a no-fail operator delete. Other APIs (such as Win32) provide error codes, but will only fail due to programmer error or hardware fault, with errors indicating conditions like heap corruption, or double free, etc.

As for unrecoverable adverse conditions, take the DB connection. If closing the connection failed because the connection was dropped -- cool, you're done. Don't throw! A dropped connection (should) result in a closed connection, so there's no need to do anything else. If anything, log a trace message to help diagnose usage issues. Example:

class DBCon{
public:
  DBCon() { 
    handle = fooOpenDBConnection();
  }
  ~DBCon() {
    int err = fooCloseDBConnection();
    if(err){
      if(err == E_fooConnectionDropped){
        // do nothing.  must have timed out
      } else if(fooIsCriticalError(err)){
        // critical errors aren't recoverable.  log, save 
        //  restart information, and die
        std::clog << "critical DB error: " << err << "\n";
        save_recovery_information();
        std::terminate();
      } else {
        // log, in case we need to gather this info in the future,
        //  but continue normally.
        std::clog << "non-critical DB error: " << err << "\n";
      }
    }
    // done!
  }
};

None of these conditions justify attempting a second kind of unwind. Either the program can continue normally (including exception unwind, if unwind is in progress), or it dies here and now.

Edit-Add

If you really want to be able to keep some sort of link to those DB connections that can't close -- perhaps they failed to close due to intermittent conditions, and you'd like to retry later -- then you can always defer cleanup:

vector<DBHandle> to_be_closed_later;  // startup reserves space

DBCon::~DBCon(){
  int err = fooCloseDBConnection();
  if(err){
    ..
    else if( fooIsRetryableError(err) ){
      try{
        to_be_closed.push_back(handle);
      } catch (const bad_alloc&){
        std::clog << "could not close connection, err " << err << "\n"
      }
    }
  }
}

Very not pretty, but it might get the job done for you.

Aaron
Your style assumes your object knows what is best for the application by using terminate(). This is hardly ever (read never) the case, it is the surrounding code (ie the controlling code) that has the context to decide what to do with errors.
Martin York
You seem to be suggesting that unless it's a bug in code or design, it's recoverable, as in your DB example; but there are examples in which a non-recoverable error may be encountered during RAII unwinding.
Assaf Lavie
Unrecoverable for your object, but your object should not stop the application. You must cede that responsibility to a part of the application that has more context. e.g. Other resources may still need cleaning up properly. Unilateral termination is not acceptable in the majority of situations.
Martin York
@Assaf - exception handling doesn't support this sort of recovery. Consider the case of two DB objects being unwound, and both 'fail' to cleanup. If an application needs to know about such failures, it should invoke a 'close()' member on the object before destruction.
Aaron
@Martin - agreed, termination isn't the only answer, nor is it the typical answer. 'do nothing' or 'redesign' are the common answers. Give the object a throwing 'close()' member if you need to be informed of the failure. dtors should clean up and be silent.
Aaron
+2  A: 

One thing I would ask is, ignoring the question of termination and so on, what do you think an appropriate response is if your program can't close its DB connection, either due to normal destruction or exceptional destruction.

You seem to rule out "merely logging" and are disinclined to terminate, so what do you think is the best thing to do?

I think if we had an answer to that question then we would have a better idea of how to proceed.

No strategy seems particularly obvious to me; apart from anything else, I don't really know what it means for closing a database connection to throw. What is the state of the connection if close() throws? Is it closed, still open, or indeterminate? And if it's indeterminate, is there any way for the program to revert to a known state?

A destructor failing means that there was no way to undo the creation of an object; the only way to return the program to a known (safe) state is to tear down the entire process and start over.

DrPizza
+1  A: 

What are the reasons why your destruction might fail? Why not look to handling those before actually destructing?

For example, closing a database connection may be because:

  • Transaction in progress. (Check std::uncaught_exception() - if true, rollback, else commit - these are the most likely desired actions unless you have a policy that says otherwise, before actually closing the connection.)
  • Connection is dropped. (Detect and ignore. The server will rollback automatically.)
  • Other DB error. (Log it so we can investigate and possibly handle appropriately in the future. Which may be to detect and ignore. In the meantime, try rollback and disconnect again and ignore all errors.)

If I understand RAII properly (which I might not), the whole point is its scope. So it's not like you WANT transactions lasting longer than the object anyway. It seems reasonable to me, then, that you want to ensure closure as best you can. RAII doesn't make this unique - even without objects at all (say in C), you still would try to catch all error conditions and deal with them as best as you can (which is sometimes to ignore them). All RAII does is force you to put all that code in a single place, no matter how many functions are using that resource type.

Tanktalus
+1  A: 
paercebal