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

[Stefan Behnel]

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
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.

Stefan