Add a life support system for type_caster temporaries

Author: dean0x7d

include/pybind11/cast.h

@@ -111,6 +111,53 @@ PYBIND11_NOINLINE inline internals &get_internals() {
     return *internals_ptr;
 }
 
+/// A life support system for temporary objects created by `type_caster::load()`.
+/// Adding a patient will keep it alive up until the enclosing function returns.
+class loader_life_support {
+public:
+    /// A new patient frame is created when a function is entered
+    loader_life_support() {
+        get_internals().loader_patient_stack.push_back(nullptr);
+    }
+
+    /// ... and destroyed after it returns
+    ~loader_life_support() {
+        auto &stack = get_internals().loader_patient_stack;
+        if (stack.empty())
+            pybind11_fail("loader_life_support: internal error");
+
+        auto ptr = stack.back();
+        stack.pop_back();
+        Py_CLEAR(ptr);
+
+        // A heuristic to reduce the stack's capacity (e.g. after long recursive calls)
+        if (stack.capacity() > 16 && stack.size() != 0 && stack.capacity() / stack.size() > 2)
+            stack.shrink_to_fit();
+    }
+
+    /// This can only be used inside a pybind11-bound function, either by `argument_loader`
+    /// at argument preparation time or by `py::cast()` at execution time.
+    PYBIND11_NOINLINE static void add_patient(handle h) {
+        auto &stack = get_internals().loader_patient_stack;
+        if (stack.empty())
+            throw cast_error("When called outside a bound function, py::cast() cannot "
+                             "do Python -> C++ conversions which require the creation "
+                             "of temporary values");
+
+        auto &list_ptr = stack.back();
+        if (list_ptr == nullptr) {
+            list_ptr = PyList_New(1);
+            if (!list_ptr)
+                pybind11_fail("loader_life_support: error allocating list");
+            PyList_SET_ITEM(list_ptr, 0, h.inc_ref().ptr());
+        } else {
+            auto result = PyList_Append(list_ptr, h.ptr());
+            if (result == -1)
+                pybind11_fail("loader_life_support: error adding patient");
+        }
+    }
+};
+
 // Gets the cache entry for the given type, creating it if necessary.  The return value is the pair
 // returned by emplace, i.e. an iterator for the entry and a bool set to `true` if the entry was
 // just created.
@@ -643,9 +690,11 @@ class type_caster_generic {
         // Perform an implicit conversion
         if (convert) {
             for (auto &converter : typeinfo->implicit_conversions) {
-                temp = reinterpret_steal<object>(converter(src.ptr(), typeinfo->type));
-                if (load_impl<ThisT>(temp, false))
+                auto temp = reinterpret_steal<object>(converter(src.ptr(), typeinfo->type));
+                if (load_impl<ThisT>(temp, false)) {
+                    loader_life_support::add_patient(temp);
                     return true;
+                }
             }
             if (this_.try_direct_conversions(src))
                 return true;
@@ -674,7 +723,6 @@ class type_caster_generic {
 
     const type_info *typeinfo = nullptr;
     void *value = nullptr;
-    object temp;
 };
 
 /**
@@ -1064,7 +1112,7 @@ template <typename StringType, bool IsView = false> struct string_caster {
 
         // If we're loading a string_view we need to keep the encoded Python object alive:
         if (IsView)
-            view_into = std::move(utfNbytes);
+            loader_life_support::add_patient(utfNbytes);
 
         return true;
     }
@@ -1080,8 +1128,6 @@ template <typename StringType, bool IsView = false> struct string_caster {
     PYBIND11_TYPE_CASTER(StringType, _(PYBIND11_STRING_NAME));
 
 private:
-    object view_into;
-
     static handle decode_utfN(const char *buffer, ssize_t nbytes) {
 #if !defined(PYPY_VERSION)
         return
@@ -1336,7 +1382,6 @@ struct copyable_holder_caster : public type_caster_base<type> {
     using base::cast;
     using base::typeinfo;
     using base::value;
-    using base::temp;
 
     bool load(handle src, bool convert) {
         return base::template load_impl<copyable_holder_caster<type, holder_type>>(src, convert);

include/pybind11/common.h

@@ -474,6 +474,7 @@ struct internals {
     std::unordered_map<const PyObject *, std::vector<PyObject *>> patients;
     std::forward_list<void (*) (std::exception_ptr)> registered_exception_translators;
     std::unordered_map<std::string, void *> shared_data; // Custom data to be shared across extensions
+    std::vector<PyObject *> loader_patient_stack; // Used by `loader_life_support`
     PyTypeObject *static_property_type;
     PyTypeObject *default_metaclass;
     PyObject *instance_base;

include/pybind11/pybind11.h

@@ -573,6 +573,7 @@ class cpp_function : public function {
 
                 // 6. Call the function.
                 try {
+                    loader_life_support guard{};
                     result = func.impl(call);
                 } catch (reference_cast_error &) {
                     result = PYBIND11_TRY_NEXT_OVERLOAD;
@@ -601,6 +602,7 @@ class cpp_function : public function {
                 // The no-conversion pass finished without success, try again with conversion allowed
                 for (auto &call : second_pass) {
                     try {
+                        loader_life_support guard{};
                         result = call.func.impl(call);
                     } catch (reference_cast_error &) {
                         result = PYBIND11_TRY_NEXT_OVERLOAD;

tests/test_class.cpp

@@ -150,6 +150,40 @@ TEST_SUBMODULE(class_, m) {
     py::class_<MyDerived, MyBase>(m, "MyDerived")
         .def_static("make", &MyDerived::make)
         .def_static("make2", &MyDerived::make);
+
+    // test_implicit_conversion_life_support
+    struct ConvertibleFromUserType {
+        int i;
+
+        ConvertibleFromUserType(UserType u) : i(u.value()) { }
+    };
+
+    py::class_<ConvertibleFromUserType>(m, "AcceptsUserType")
+        .def(py::init<UserType>());
+    py::implicitly_convertible<UserType, ConvertibleFromUserType>();
+
+    m.def("implicitly_convert_argument", [](const ConvertibleFromUserType &r) { return r.i; });
+    m.def("implicitly_convert_variable", [](py::object o) {
+        // `o` is `UserType` and `r` is a reference to a temporary created by implicit
+        // conversion. This is valid when called inside a bound function because the temp
+        // object is attached to the same life support system as the arguments.
+        const auto &r = o.cast<const ConvertibleFromUserType &>();
+        return r.i;
+    });
+    m.add_object("implicitly_convert_variable_fail", [&] {
+        auto f = [](PyObject *, PyObject *args) -> PyObject * {
+            auto o = py::reinterpret_borrow<py::tuple>(args)[0];
+            try { // It should fail here because there is no life support.
+                o.cast<const ConvertibleFromUserType &>();
+            } catch (const py::cast_error &e) {
+                return py::str(e.what()).release().ptr();
+            }
+            return py::str().release().ptr();
+        };
+
+        auto def = new PyMethodDef{"f", f, METH_VARARGS, nullptr};
+        return py::reinterpret_steal<py::object>(PyCFunction_NewEx(def, nullptr, m.ptr()));
+    }());
 }
 
 template <int N> class BreaksBase {};

tests/test_class.py

@@ -119,3 +119,11 @@ def test_override_static():
     assert isinstance(b, m.MyBase)
     assert isinstance(d1, m.MyDerived)
     assert isinstance(d2, m.MyDerived)
+
+
+def test_implicit_conversion_life_support():
+    """Ensure the lifetime of temporary objects created for implicit conversions"""
+    assert m.implicitly_convert_argument(UserType(5)) == 5
+    assert m.implicitly_convert_variable(UserType(5)) == 5
+
+    assert "outside a bound function" in m.implicitly_convert_variable_fail(UserType(5))