[BUG]: `def_property` doesn't seems to keep parent alive when the return object is a `py::array`
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 18k
- Forks
- 2.3k
- Avg merge
- 5d 17h
- Merged PRs (30d)
- 10
Description
Required prerequisites
- Make sure you've read the documentation. Your issue may be addressed there.
- Search the issue tracker and Discussions to verify that this hasn't already been reported. +1 or comment there if it has.
- Consider asking first in the Gitter chat room or in a Discussion.
Problem description
I posted this as a discussion before: https://github.com/pybind/pybind11/discussions/4200, but got no answer for a while, so repost it here.
I have a use case that have some internal C++ vectors and want to return to python side as numpy array. To avoid copy, we only return a view on a C++ vector. It works OK in most of times, but I'm now hitting below corner case:
#include <vector>
#include <pybind11/pybind11.h>
#include <pybind11/numpy.h>
struct Wrapper1 {
Wrapper1(const std::vector<double> &values): values{values} {};
std::vector<double> values;
};
struct Wrapper2 {
Wrapper2(const std::vector<double> &values): values{values} {};
std::shared_ptr<Wrapper1> wrapper1() const { return std::make_shared<Wrapper1>(values); };
std::vector<double> values;
};
namespace py = pybind11;
using namespace pybind11::literals;
PYBIND11_MODULE(test_return_numpy_array_no_copy_helper, m) {
py::class_<Wrapper1, std::shared_ptr<Wrapper1>>(m, "Wrapper1")
.def(py::init<const std::vector<double> &>(), "values"_a)
.def_property_readonly("values", [](const Wrapper1& self) {
// https://github.com/pybind/pybind11/issues/1042#issuecomment-325941022
auto array = pybind11::array(self.values.size(), self.values.data(),
// the base of the numpy array, as long as it is set to a python object, it won't do copy
pybind11::handle{Py_None});
// https://github.com/pybind/pybind11/issues/481, make numpy array read-only
reinterpret_cast<pybind11::detail::PyArray_Proxy*>(array.ptr())->flags &=
~pybind11::detail::npy_api::NPY_ARRAY_WRITEABLE_;
return array;
});
py::class_<Wrapper2, std::shared_ptr<Wrapper2>>(m, "Wrapper2")
.def(py::init<const std::vector<double> &>(), "values"_a)
.def_property_readonly("wrapper1", &Wrapper2::wrapper1);
}
Wrapper2 create a new instance of std::shared_ptr<Wrapper1> which contains a C++ std::vector values. The values is then return to python as a numpy array view.
From the docs: https://pybind11.readthedocs.io/en/stable/advanced/functions.html, def_property_readonly will use return_value_policy::reference_internal and thus keep the parent object alive. So I expect the values returned will keep its parent, i.e. the new instance of std::shared_ptr<Wrapper1> alive and the underlying memory will be valid as long as values is alive in Python.
However, below test case shows it is not the case
def test_return_np_array_with_no_copy():
vec = [0.0, 0.5, 1.3, 2.2]
wrapper2 = Wrapper2(vec)
values = wrapper2.wrapper1.values
# in some cases, you will see values be something like
# array([1.08885983e-309, 5.00000000e-001, 1.30000000e+000, 2.20000000e+000])).
# It means the underlying memory is destroyed
assert np.all(values == vec), f"{values}"
I also try to manually to use keep_alive<0, 1> in the values binding of Wrapper1, but that doesn't seem to help.
So is the keep_alive system not supposed to work with an existing python object, e.g. py::array? If so, what should I do to make above test case work?
Any thoughts would be appreciated. Thanks.
Reproducible example code
No response
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the reproducible C++ bindings for Wrapper1, Wrapper2, def_property_readonly, and the Python test test_return_np_array_with_no_copy. Trace how reference_internal and keep_alive interact with the returned py::array and its backing vector. Done means the test reliably preserves the array data after the temporary Wrapper1 would otherwise be destroyed.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp, python
- Domain
- backend-api-design
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100