[BUG]: 'wrapper' type casters can crash by raising exceptions through noexcept

[oremanj]

Problem description

nanobind ships with a number of "outer" type casters that are implemented by adding some logic around an "inner" caster, ultimately invoking the inner caster's cast operator in the outer caster's from_python method. Examples include the casters for std::optional<T>, std::map<K, V>, nb::typed<T, Ts...>, etc. Unfortunately, from_python is noexcept, while cast operators can throw. There are three reasons for a cast operator to throw in nanobind's built-in casters:

• tried to initialize T& (where T is a bound nanobind type) with None
• tried to initialize char with a multi-character string
• tried to initialize unique_ptr<T> with a T that was constructed in Python

The first of these is generally avoided by passing cast_flags::disallow_none when casting a bound type to a non-pointer, but the others can still provoke an exception, which will crash the interpreter when it hits the outer caster's noexcept boundary. See the example below.

What to do?

• We could wrap each one of these inner cast-operator calls in a try/catch. I'm guessing that might not appeal, but it definitely solves the problem.
• We could add a member function template to the type caster interface that checks whether conversion to T will throw, and require each 'wrapper' caster to check it before calling the cast operator. (2.0 might be a better time than usual to introduce that?)
• We could change disallow_none into a flag that more generally indicates whether the cast target is a pointer or not; then the char caster could also use it to disallow non-single-char strings if the target isn't a pointer.
• The unique_ptr caster could check for an un-relinquishable object in from_python as well as in the cast operator. Unfortunately, it still needs the cast-op check (which can thus cause this issue) to detect cases where one tries to pass the same object for two unique_ptr parameters (or vector elements, etc) in the same function call, because the second conversion won't fail until we've committed to the first one. I can't immediately think of a way to robustify this, but it might be possible.

Reproducible example code

// C++
#include <nanobind/nanobind.h>
#include <nanobind/stl/vector.h>

NB_MODULE(example, m) {
    m.def("explode", [](std::vector<char>) {});
}

# Python
import example
example.explode(["a", "b", "oops"])

[wjakob]

Hi @oremanj,

I agree that the code here is not as solid as it should be, and that this will require fixes in the future.

This will involve adapting the logic to avoid the possibility of an exception being raised in a noexcept block. I want to keep the casters exception-free and keep the noexcept attribute on the from_python function.

The reasoning is as follows

• Casting failures can frequently happen while trying out overloads. Raising exceptions in C++ expensive in terms of runtime performance (usually considerably higher than an equivalent exception in Python).
• There is also a noticeable code size impact since the C++ compiler must create data structures to facilitate unwinding the stack.
• The casters are components that are usually instantiated many times. I prefer to localize all complexity related casting failures there instead of dumping this complexity on the C++ exception/unwind system.

Therefore, the casters should return false when the conversion fails, instead of raising an exception. In the case of nested casters, the way in which they are combined may need further adaptation. For example by using nb::try_cast instead of nb::cast.

Does that make sense?

[oremanj]

Your reasoning for not wanting to catch exceptions within from_python makes sense to me.

For example by using nb::try_cast instead of nb::cast.

This isn't really better though; nb::try_cast also puts a try/catch around its call to the cast operator (by which I mean caster.operator cast_t<T>()).

The basic problem is that there are certain types of failures that can't be noticed until the cast operator executes (because they depend on the exact casted-to type, etc), and the only way it has to signal those failures is raising an exception. So if we want to avoid catching exceptions in casters (or nb::try_cast for that matter), we're going to need a different way to signal those failures. What do you think of my second bullet - "add a member function template to the type caster interface that checks whether conversion to T will throw, and require each 'wrapper' caster to check it before calling the cast operator"? In practical terms, that might look like a type_caster method

template <typename T>
bool cast_op_will_succeed() const;

which can be defined as static constexpr bool cast_op_will_succeed() { return true; } for almost all casters, and would need to be implemented specially for the fallible-cast ones (the base caster, char, and unique_ptr) and for those that delegate to other casters' cast operators within their own cast operator (pair and tuple). Then in cases where we don't want to catch exceptions, we can check caster.template cast_op_will_succeed<T>() before calling caster.operator cast_t<T>().

[wjakob]

AFAIK the main failure case of try_cast involving an exception occurs when attempting to cast None into Class& for a class binding Class. I think it's an ugly corner case and would be nicer to resolve without exceptions.

The cast_op_will_succeed sounds like a good solution. Are you interested in implementing this as a PR? If so, a super-minor preference would be to give it a shorter name, e.g. can_cast<T>

[oremanj]

Yes, I'll take a shot at implementing this, though likely not for a week or two. Thanks for confirming I wasn't missing something!

[wjakob]

Any news on this? @oremanj ?

[oremanj]

I will hopefully have some time to work on this tomorrow!