[exception] current_exception mishandling standard exceptions
It seems that current_exception mishandles standard exceptions. I noted two problems: 1) Some standard exceptions are not handled at all. This includes std::runtime_error and its derived (std::overflow_error, std::range_error, std::underflow_error), std::domain_error (handled by general std::logic_error), std::length_error (handled by general std::logic_error) and std::ios_base::failure (handled by general std::exception). Why is it so? 2) Non-standard exceptions derived from standard exceptions are sliced by current_exception. It detects those types by throwing and caching and thus will catch also derived types (like null_pointer : public std::invalid_argument). But during construction of internal structures the standard exception type is used and thus copy construction slices the original exception object. If typeid was used instead of throwing/catching (or dynamic_cast) to detect the type current_exception could at least detect that is will slice the original object and use a different type so it would not pretend everything is OK. Adam Badura
Adam Badura:
2)
Non-standard exceptions derived from standard exceptions are sliced by current_exception. It detects those types by throwing and caching and thus will catch also derived types (like null_pointer : public std::invalid_argument). But during construction of internal structures the standard exception type is used and thus copy construction slices the original exception object. If typeid was used instead of throwing/catching (or dynamic_cast) to detect the type current_exception could at least detect that is will slice the original object and use a different type so it would not pretend everything is OK.
What would you gain from that? If you catch(invalid_argument) you want null_pointer to be caught; if current_exception translated it to something else, it wouldn't be. current_exception can store something derived from invalid_argument, but what are the benefits of doing so? Either way, catching an invalid_argument works, catching a null_pointer doesn't.
On Wed, May 13, 2009 at 10:18 AM, Adam Badura <abadura@o2.pl> wrote:
It seems that current_exception mishandles standard exceptions. I noted two problems:
1)
Some standard exceptions are not handled at all. This includes std::runtime_error and its derived (std::overflow_error, std::range_error, std::underflow_error), std::domain_error (handled by general std::logic_error), std::length_error (handled by general std::logic_error) and std::ios_base::failure (handled by general std::exception). Why is it so?
Dunno. :) I've added them now.
2)
Non-standard exceptions derived from standard exceptions are sliced by current_exception. It detects those types by throwing and caching and thus will catch also derived types (like null_pointer : public std::invalid_argument). But during construction of internal structures the standard exception type is used and thus copy construction slices the original exception object. If typeid was used instead of throwing/catching (or dynamic_cast) to detect the type current_exception could at least detect that is will slice the original object and use a different type so it would not pretend everything is OK.
The explicit handling of std exceptions in boost::current_exception is a fallback, mostly to deal with exceptions emitted by the standard library itself. For user-defined types (such as null_pointer) boost::enable_current_exception should be used at throw time. Note: - This slicing of standard exceptions -- while not ideal -- is still better than getting an exception_ptr that refers to boost::unknown_exception. - The caller of boost::current_exception should reasonably deal with the possibility of getting exception_ptr that refers to a type the program did not actually throw, such as boost::unknown_exception or (unlikely) std::bad_alloc. - Even if you get a boost::unknown_exception, its boost::exception subobject will have all the data contained in the boost::exception subojbect of the original exception object (assuming of course that the original exception object is of type that derives from boost::exception.) - Finally, with trunk revision 52981 whenever current_exception fails to properly clone the exception object, it attempts to store the type_info of the original exception object as error_info. See http://svn.boost.org/svn/boost/trunk/libs/exception/doc/current_exception.ht... for the updated documentation. Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
The explicit handling of std exceptions in boost::current_exception is a fallback, mostly to deal with exceptions emitted by the standard library itself. For user-defined types (such as null_pointer) boost::enable_current_exception should be used at throw time.
Note:
- This slicing of standard exceptions -- while not ideal -- is still better than getting an exception_ptr that refers to boost::unknown_exception.
What about providing an interface: class ClonableException { public: virtual ~ClonableException() {} virtual ClonableException* clone() const = 0; }; The cloning code would query for this interface first and use it (the exception class made by enable_current_exception would inherit from this interface as well) to make a copy. This way user would be able to clone exactly his exceptions. Note that this would also solved problem in enable_current_exception with virtual base classes that are not default constructible - mentioned in earlier thread. And IMO it does not seem complicated to introduce/explain/document/use. Obviously new standard will render it useless however it is not known how long we will await that standard (and its support in arbitrary compilers) while this change seems to be possibly with next Boost version. Adam Badura
On Wed, May 13, 2009 at 11:58 PM, Adam Badura <abadura@o2.pl> wrote:
The explicit handling of std exceptions in boost::current_exception is a fallback, mostly to deal with exceptions emitted by the standard library itself. For user-defined types (such as null_pointer) boost::enable_current_exception should be used at throw time.
Note:
- This slicing of standard exceptions -- while not ideal -- is still better than getting an exception_ptr that refers to boost::unknown_exception.
What about providing an interface:
class ClonableException { public: virtual ~ClonableException() {}
virtual ClonableException* clone() const = 0; };
The library already contains a similar undocumented type.
The cloning code would query for this interface first and use it (the exception class made by enable_current_exception would inherit from this interface as well) to make a copy. This way user would be able to clone exactly his exceptions.
That's how it works currently. The only difference is that instead of documenting the interface that needs to be implemented for current_exception to work, the burden of implementing it is shifted to the enable_current_exception function.
Note that this would also solved problem in enable_current_exception with virtual base classes that are not default constructible - mentioned in earlier thread.
It would, but I don't see a reason to use exception types without default constructors with Boost Exception. So, in my mind this problem is only theoretical. Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
Note that this would also solved problem in enable_current_exception with virtual base classes that are not default constructible - mentioned in earlier thread.
It would, but I don't see a reason to use exception types without default constructors with Boost Exception. So, in my mind this problem is only theoretical.
What about integrating with existing exception hierarchies? (I know the example I gave previously of my project is not such hierarchy... :)). But on the other hand existing hierarchies likely do not use virtual inheritance as you have already mentioned somewhere before if I am not mistaken. Still there are reasons. In cases someone wants extra care he/she cannot use Boost.Exception data injection because this always adds risk of double failure - lack of memory and std::bad_alloc (or even something unpredictable as if I am not mistaken Standard puts low requirements on implementations on what, when and where can be thrown and only recommends standard exceptions) - which would mask the original failure. In case of my example of storing DWORD with GetLastError result in a modern PC machine chances of lack of memory are extremely low. However when talking about embedded systems or exceptions with more (maybe even lots) of data this becomes important. So to sum up IMHO if the implementation already uses this mechanism and all the required work is to make it public and document it then I would do it even if I didn't see practical use at the moment (I feel a bit mathematician and I do recall theories which also seemed useless at the moment of creation but then became important... :)). And in this case it seems (at least IMO) there are justified and possible if not likely use cases (the ones I mentioned earlier in the post). But you are not me and I do not feel entitled to modify Boost.Exception on my own so the final choice is yours. Since it seems I am the first and only one requesting this then maybe after all it is not important enough. (And yes. I know I can modify my own Boost to make this change and use it but I don't think it is a good idea especially that I am not the only person working on the project and we do constantly update Boost version instead of sticking to one particular.) Adam Badura
On Thu, May 14, 2009 at 1:16 PM, Adam Badura <abadura@o2.pl> wrote:
Still there are reasons. In cases someone wants extra care he/she cannot use Boost.Exception data injection because this always adds risk of double failure - lack of memory and std::bad_alloc
This isn't double failure -- your program correctly attempts to do something (throw an exception, in this case) and fails, which results in bad_alloc exception. There is no difference between this situation and a bad_alloc you could get if you attempt to copy a string (for example.) Also, some compilers allocate exceptions on the heap. I don't see 15.1 specifying behavior in case this allocation fails, so clearly an attempt to throw an exception may fail. In this case, whether you get a std::bad_alloc or abort() won't make much difference, either way your program isn't going to throw the exception you wanted to throw (any expert care to comment on this issue?)
So to sum up IMHO if the implementation already uses this mechanism and all the required work is to make it public and document it then I would do it even if I didn't see practical use at the moment
Maybe I wasn't clear: I am not against making this change in Boost Exception, I am against doing it without understanding the use cases that require the change. Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode
participants (3)
-
Adam Badura -
Emil Dotchevski -
Peter Dimov