pybind / pybind/pybind11

[BUG]: `def_property` doesn't seems to keep parent alive when the return object is a `py::array`

Open
#4,236 5 comments 2 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug triage
Dominant language
C++
Stars
18k
Forks
2.3k
Avg merge
5d 17h
Merged PRs (30d)
10

Description

Required prerequisites
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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.