[Cython] [cython-users] C++: how to handle failures of 'new'?

[Dag Sverre Seljebotn]

On 07/03/2012 09:14 AM, Stefan Behnel wrote:
> Robert Bradshaw, 29.06.2012 11:08:
>> On Thu, Jun 28, 2012 at 10:45 PM, Stefan Behnel wrote:
>>> Robert Bradshaw, 28.06.2012 21:46:
>>>> On Thu, Jun 28, 2012 at 11:38 AM, Stefan Behnel wrote:
>>>>> currently, when I write "new CppClass()" in Cython, it generates a straight
>>>>> call to the "new" operator. It doesn't do any error handling. And the
>>>>> current documentation doesn't even mention this case.
>>>>>
>>>>> Is there a "standard" way to handle this? It seems that C++ has different
>>>>> ways to deal with failures here but raises an exception by default. Would
>>>>> you declare the constructor(s) with an "except +MemoryError"? Is there a
>>>>> reason Cython shouldn't be doing this automatically (if nothing else was
>>>>> declared) ?
>>>>
>>>> I think it certainly makes sense to declare the default constructor as
>>>> "except +" (and std::bad_alloc should become MemoryError),
>>>
>>> Right. The code in the constructor can raise other exceptions that must
>>> also be handled properly. An explicit "except +" will handle that.
>>>
>>>
>>>> but whether
>>>> to implicitly annotate declared constructors is less clear, especially
>>>> as there's no way to un-annotate them.
>>>
>>> I agree, but sadly, it's the default behaviour that is wrong. I'm sure we
>>> made lots of users run into this trap already. I fixed the documentation
>>> for now, but the bottom line is that we require users to take care of
>>> proper declarations themselves. Otherwise, the code that we generate is
>>> incorrect, although it's 100% certain that an allocation error can occur,
>>> even if the constructor code doesn't raise any exceptions itself.
>>
>> This is always the case.
>
> Sure, we always rely on correct declarations. However, this is a case where
> we *know* that bad things can happen, even if there is no declaration for
> them. It would be nice to play safe by default.
>
> IMHO, only the second best solution would be to raise a warning when we
> encounter a "new" without an exception declared, so that users are urged
> into writing safe code.
>
>
>>> Apparently, changing the behaviour of the "new" operator requires a special
>>> annotation "std::nothrow", which then returns NULL on allocation failures.
>>> You can pass that from Cython by hacking up a cname, e.g.
>>>
>>>     Rectangle "(std::nothrow) Rectangle" (int w, int h)
>>>
>>> I'm sure there are users out there who figured this out (I mean, I did...)
>>> and use it in their code, so I agree that this isn't easy to handle because
>>> Cython simply wouldn't know what the actual error behaviour is for a given
>>> constructor and how to correctly detect an error.
>>>
>>> This problem applies only to heap allocation in that form. However, stack
>>> allocation and the new exttype field allocation suffer from similar
>>> problems when the default constructor raises an exception. Exttype fields
>>> are a particularly nasty case because the user has no control over the
>>> allocation. A C++ exception in the C++ class constructor would terminate
>>> the exttype constructor unexpectedly and thus leak resources (in the best
>>> case - no idea how CPython reacts if you throw a C++ exception through its
>>> type instantiation code).
>>
>> If the default constructor raises an exception then it should be
>> declared (to not do so is an error on the users part). New raising
>> bad_alloc is a bit of a special case, but doesn't appl to the stack or
>> exttype allocations.
>
> Right. In those two cases, it would definitely be the fault of the user.
>
>
>>> Similarly, a C++ exception in the constructor of a stack allocated object
>>> would then originate from the function entry code and potentially hit the
>>> Python function wrapper etc. Again, potentially leaking resources or worse.
>>>
>>> To me, this sounds like we should do something about it. At least for the
>>> implicit calls to the default constructor, we should generate "except +"
>>> code automatically because there is no other way to handle them safely.
>>
>> If no constructor is declared, it should be "except +" just to be
>> safe
>
> Ok, I'll fix it.
>
>
>> but otherwise I don't see how this is any different than
>> forgetting to declare exceptions on any other function. Unfortunately
>> catching exceptions  (with custom per-object handling) on a set of
>> stack allocated objects seems difficult if not impossible (without
>> resorting to ugly hacks like using placement new everywhere).
>
> I don't know what happens if a C++ exception is not being caught, but I
> guess it would simply crash the application. That's a bit more visible than

Yep.

> just printing a warning when a Python exception is being ignored due to a
> missing declaration. It's really unfortunate that our documentation didn't
> even mention the need for this, because it's not immediately obvious that
> Cython won't handle errors in "new", and testing for memory errors isn't
> quite what people commonly do in their test suites.
>
> Apart from that, I agree, users have to take care to properly declare the
> API they are using.

Is there any time you do NOT want a "catch (...) {}" block? I can't see 
a C++ exception propagating to Python-land doing anything useful ever.

So shouldn't we just make --cplus turn *all* external functions and 
methods (whether C-like or C++-like) into "except +"? (Or keep except+ 
for manual translation, but always have a catch(...)".

Performance overhead is the only reason I can think of to not do this, 
although IIRC C++ catch blocks are only dealt with during stack unwinds 
and doesn't cost anything/much (?) when they're not triggered.

"except -1" should then actually mean both; "except + except -1". So 
it's more a question of just adding catch(...) *everywhere*, than making 
"except +" the default.

Dag