a question of style when catching exceptions
in [C++]
|
Prev: Loki::Function
Next: Right Endian conversion
From: Andrew on 5 Aug 2010 04:54 I am working on enhancements and changes to a C++ system that uses exceptions. A very common way the code catches exceptions is by the base class, std::exception. I used to think there was nothing wrong with this but my experience with this codebase is making me have second thoughts. When I describe the issues, maybe you guys can give your opinions. The code uses some byzantine libraries that can throw for one of three reasons: 1) It really needs to throw, coz there was a runtime error and they do happen from time to time. 2) it throws std::logic_error because it has detected a situation that could only arise as a result of a coding error. 3) Access violation. Microsoft structured exception handling (SEH) turns these into C++ exceptions that have std::exception as the base class. The access violations are distressingly common. They are so common that SEH has been explicitly enabled to allow these errors. I think the reasoning might be that access violation corresponds to std::logic_error. Just guessing. All 3 are dealt with by saying catch(std::exception&). This tends to cause errors of type 2 and 3 to be swallowed. I think it should catch std::runtime_error and let anything else bubble up to the very top where the app will log it then terminate. Is this a good technique/ practise in general? When people create exception classes do they tend to make them inherit from std::runtime_error? I have a horrible feeling that many devs might inherit directly from std::exception. Regards, Andrew Marlow -- [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ]
From: Goran on 5 Aug 2010 19:48 > I am working on enhancements and changes to a C++ system that uses > exceptions. A very common way the code catches exceptions is by the > base class, std::exception. I used to think there was nothing wrong > with this but my experience with this codebase is making me have > second thoughts. When I describe the issues, maybe you guys can give > your opinions. > > The code uses some byzantine libraries that can throw for one of three > reasons: > > 1) It really needs to throw, coz there was a runtime error and they do > happen from time to time. > 2) it throws std::logic_error because it has detected a situation that > could only arise as a result of a coding error. > 3) Access violation. Microsoft structured exception handling (SEH) > turns these into C++ exceptions that have std::exception as the base > class. The access violations are distressingly common. They are so > common that SEH has been explicitly enabled to allow these errors. I > think the reasoning might be that access violation corresponds to > std::logic_error. Just guessing. There is a way to turn SE into C++ exceptions (_set_se_translator), which is probably what happens in these libraries. There is IMO a massive error with this idea: SEs often happen when code is deep into undefined behavior land, and that means it already crashed, it's just that it's not quite dead. Thus, trying to continue after such an error often leads to massive errors later. Sometimes, crashes are equivalent to what logic_error should be used in "standard" C++ (e.g. dereferencing NULL can easily be turned into a logic_error). But that's a small consolation, 'cause error sources are much more numerous and insidious than that. But I digress... What your libs do seem quite common. This: "The access violations are distressingly common." however, should not be. An AV is a crash, and should be fixed. (Strictly speaking, an AV is actually caused by invoking undefined behavior first). That's the end of it for any self-respecting code, really. > All 3 are dealt with by saying catch(std::exception&). catch(const exception_type&) is preferred (note the "const"). > This tends to > cause errors of type 2 and 3 to be swallowed. I think it should catch > std::runtime_error and let anything else bubble up to the very top > where the app will log it then terminate. Is this a good technique/ > practise in general? Quite good IMHO. For case 3, even better thing IMO is to crash, produce a crash dump, and use debugging information to look for the actual cause. This is because a crash dump will be richer in info than any exception one might throw from SE translator function. > When people create exception classes do they tend > to make them inherit from std::runtime_error? I have a horrible > feeling that many devs might inherit directly from std::exception. Why is that so horrible? The problem is not really common base class, the problem is the absence of more selective catch statements. Things really are as you say: runtime_error is recoverable, but logic_error, much less, if at all. SE is IMO, NOT recoverable at all. Ideally, code should just catch(const runtime_error&) (or it's specialization) for all try/catches except top-level ones (top-level try/catch: the one that wraps main() and any thread functions); top level should catch logic_error. That's it, really. See? Exceptions are simple! :-) I don't agree with your idea that you should have exceptions that do not derive from std:exception so that you can e.g. prevent people from catching programming errors (logic_error) in bad places. Sure, you can use something else, but you are one catch(something_else&) away from being in the same situation - again. And since catch-es are rare (see P.P.S), it's not hard to find eventual catch(logic_error)-es, if they re-appear. :-) Goran. P.S. if you often find yourself writing this: try { work } catch(const exception&) { non-throwing_cleanup(); throw } use ScopeGuard of Alexandrescu. Best thing since sliced bread. Heck, this thing alone is worth more than half of boost ;-). P.P.S. my rule 0 of exceptions in C++ is: ___you are not allowed to write try-catch statements___. If you think you need one, first make a failing test case and see where it ends up being caught! -- [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ]
From: Mathias Gaunard on 6 Aug 2010 08:32 On Aug 5, 8:54 pm, Andrew <marlow.and...(a)googlemail.com> wrote: > 2) it throws std::logic_error because it has detected a situation that > could only arise as a result of a coding error. I would suggest using an assert instead. > 3) Access violation. Microsoft structured exception handling (SEH) > turns these into C++ exceptions I believe this is disabled by default in recent versions of MSVC, and should be kept disabled. -- [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ]
From: Nick Hounsome on 6 Aug 2010 19:18 On 5 Aug, 20:54, Andrew <marlow.and...(a)googlemail.com> wrote: > All 3 are dealt with by saying catch(std::exception&). This tends to > cause errors of type 2 and 3 to be swallowed. I think it should catch > std::runtime_error and let anything else bubble up to the very top > where the app will log it then terminate. Is this a good technique/ > practise in general? Better than catch(exception) but you shouldn't really swallow any exception unless you know what it means and you know what to do about it and since you don't know what runtime_error really means (you only know what it doesn't mean) it's hard to see why you would catch it. For example suppose that you are just a lowly function called as part of a higher level transaction. If you swallow the exception the transaction will be committed as successful even though it wasn't. Contrariwise it might just make sense to catch std::exception provided that you are pretty sure that the function called has no side effects (i.e. it is a pure function ) and you can either notify the user or work around it. > When people create exception classes do they tend > to make them inherit from std::runtime_error? I have a horrible > feeling that many devs might inherit directly from std::exception. Many have their own hierarchy based on std::exception. Even Bjarne Stroustrop isn't a fan of the hierarchy defined in the standard. IMHO it would probably be better if there had been something like java's Throwable at the bottom of the hierarchy where you could put stuff like bad_alloc which you really don't want to catch under any circumstances. -- [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ]
From: Kenneth 'Bessarion' Boyd on 11 Aug 2010 16:37
On Aug 7, 5:18 am, Nick Hounsome <nick.houns...(a)gmail.com> wrote: > IMHO it would probably be better if there had been something like > java's Throwable at the bottom of the hierarchy where you could put > stuff like bad_alloc which you really don't want to catch under any > circumstances. Well, any circumstances other than maintaining 100% uptime. std::bad_alloc is the only standard exception that is obviously *reliably* recoverable from. At least, when the target platform's memory manager is honest and the problem domain is loosely coupled enough (this describes my usual problem domains). So it's the one I normally think about catching. [Of course, always verify that NULL return isn't a better option than throwing std::bad_alloc during the first implementation pass.] The entire std::logic_error and std::runtime_error hierarchies look like an assert replacement for applications that want 100% uptime. (Which doesn't describe my usual problem domains, but to the inexperienced would describe a server-type application very well.) -- [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ] |