From 30f6c3b36e580e49419f8775e2b22bea48b0929b Mon Sep 17 00:00:00 2001 From: Dean Moldovan Date: Mon, 26 Jun 2017 23:20:39 +0200 Subject: [PATCH] Fix indirect loading of Eigen::Ref Put the caster's temporary array on life support to ensure correct lifetime when it's being used as a subcaster. --- include/pybind11/eigen.h | 1 + tests/test_eigen.cpp | 14 ++++++++++++++ tests/test_eigen.py | 15 +++++++++++++++ 3 files changed, 30 insertions(+) diff --git a/include/pybind11/eigen.h b/include/pybind11/eigen.h index 53d4dab08..fc0705167 100644 --- a/include/pybind11/eigen.h +++ b/include/pybind11/eigen.h @@ -462,6 +462,7 @@ public: if (!fits || !fits.template stride_compatible()) return false; copy_or_ref = std::move(copy); + loader_life_support::add_patient(copy_or_ref); } ref.reset(); diff --git a/tests/test_eigen.cpp b/tests/test_eigen.cpp index 52358ab6f..413fed343 100644 --- a/tests/test_eigen.cpp +++ b/tests/test_eigen.cpp @@ -10,6 +10,7 @@ #include "pybind11_tests.h" #include "constructor_stats.h" #include +#include #include using MatrixXdR = Eigen::Matrix; @@ -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 v) { + py::module::import("numpy").attr("ones")(10); + return v(5); + }); + m.def("get_elem_indirect", [](std::vector> v) { + py::module::import("numpy").attr("ones")(10); + return v[0](5); + }); }); diff --git a/tests/test_eigen.py b/tests/test_eigen.py index f9ad3a5fc..c9fe69f91 100644 --- a/tests/test_eigen.py +++ b/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