Add a life support system for type_caster temporaries (+ binary size reduction)

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/eigen.h

@@ -462,6 +462,7 @@ struct type_caster<
             if (!fits || !fits.template stride_compatible<props>())
                 return false;
             copy_or_ref = std::move(copy);
+            loader_life_support::add_patient(copy_or_ref);
         }
 
         ref.reset();

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

tests/test_eigen.cpp

@@ -10,6 +10,7 @@
 #include "pybind11_tests.h"
 #include "constructor_stats.h"
 #include <pybind11/eigen.h>
+#include <pybind11/stl.h>
 #include <Eigen/Cholesky>
 
 using MatrixXdR = Eigen::Matrix<double, Eigen::Dynamic, Eigen::Dynamic, Eigen::RowMajor>;
@@ -298,4 +299,17 @@ test_initializer eigen([](py::module &m) {
         .def(py::init<>())
         .def_readonly("a", &CustomOperatorNew::a)
         .def_readonly("b", &CustomOperatorNew::b);
+
+    // test_eigen_ref_life_support
+    // In case of a failure (the caster's temp array does not live long enough), creating
+    // a new array (np.ones(10)) increases the chances that the temp array will be garbage
+    // collected and/or that its memory will be overridden with different values.
+    m.def("get_elem_direct", [](Eigen::Ref<const Eigen::VectorXd> v) {
+        py::module::import("numpy").attr("ones")(10);
+        return v(5);
+    });
+    m.def("get_elem_indirect", [](std::vector<Eigen::Ref<const Eigen::VectorXd>> v) {
+        py::module::import("numpy").attr("ones")(10);
+        return v[0](5);
+    });
 });

tests/test_eigen.py

@@ -602,6 +602,21 @@ def test_nocopy_wrapper():
             ', flags.c_contiguous' in str(excinfo.value))
 
 
+def test_eigen_ref_life_support():
+    """Ensure the lifetime of temporary arrays created by the `Ref` caster
+
+    The `Ref` caster sometimes creates a copy which needs to stay alive. This needs to
+    happen both for directs casts (just the array) or indirectly (e.g. list of arrays).
+    """
+    from pybind11_tests import get_elem_direct, get_elem_indirect
+
+    a = np.full(shape=10, fill_value=8, dtype=np.int8)
+    assert get_elem_direct(a) == 8
+
+    list_of_a = [a]
+    assert get_elem_indirect(list_of_a) == 8
+
+
 def test_special_matrix_objects():
     from pybind11_tests import incr_diag, symmetric_upper, symmetric_lower