diff --git a/include/pybind11/detail/smart_holder_type_casters.h b/include/pybind11/detail/smart_holder_type_casters.h index ba2b5bc040..5e0f791c3c 100644 --- a/include/pybind11/detail/smart_holder_type_casters.h +++ b/include/pybind11/detail/smart_holder_type_casters.h @@ -408,6 +408,18 @@ struct smart_holder_type_caster_class_hooks : smart_holder_type_caster_base_tag } }; +struct shared_ptr_parent_life_support { + PyObject *parent; + explicit shared_ptr_parent_life_support(PyObject *parent) : parent{parent} { + Py_INCREF(parent); + } + // NOLINTNEXTLINE(readability-make-member-function-const) + void operator()(void *) { + gil_scoped_acquire gil; + Py_DECREF(parent); + } +}; + struct shared_ptr_trampoline_self_life_support { PyObject *self; explicit shared_ptr_trampoline_self_life_support(instance *inst) @@ -462,12 +474,23 @@ struct smart_holder_type_caster_load { return *raw_ptr; } - std::shared_ptr loaded_as_shared_ptr() const { + std::shared_ptr make_shared_ptr_with_responsible_parent(handle parent) const { + return std::shared_ptr(loaded_as_raw_ptr_unowned(), + shared_ptr_parent_life_support(parent.ptr())); + } + + std::shared_ptr loaded_as_shared_ptr(handle responsible_parent = nullptr) const { if (load_impl.unowned_void_ptr_from_void_ptr_capsule) { + if (responsible_parent) { + return make_shared_ptr_with_responsible_parent(responsible_parent); + } throw cast_error("Unowned pointer from a void pointer capsule cannot be converted to a" " std::shared_ptr."); } if (load_impl.unowned_void_ptr_from_direct_conversion != nullptr) { + if (responsible_parent) { + return make_shared_ptr_with_responsible_parent(responsible_parent); + } throw cast_error("Unowned pointer from direct conversion cannot be converted to a" " std::shared_ptr."); } @@ -478,6 +501,9 @@ struct smart_holder_type_caster_load { holder_type &hld = holder(); hld.ensure_is_not_disowned("loaded_as_shared_ptr"); if (hld.vptr_is_using_noop_deleter) { + if (responsible_parent) { + return make_shared_ptr_with_responsible_parent(responsible_parent); + } throw std::runtime_error("Non-owning holder (loaded_as_shared_ptr)."); } auto *void_raw_ptr = hld.template as_raw_ptr_unowned(); @@ -579,6 +605,17 @@ struct smart_holder_type_caster_load { return result; } + // This function will succeed even if the `responsible_parent` does not own the + // wrapped C++ object directly. + // It is the responsibility of the caller to ensure that the `responsible_parent` + // has a `keep_alive` relationship with the owner of the wrapped C++ object, or + // that the wrapped C++ object lives for the duration of the process. + static std::shared_ptr shared_ptr_from_python(handle responsible_parent) { + smart_holder_type_caster_load loader; + loader.load(responsible_parent, false); + return loader.loaded_as_shared_ptr(responsible_parent); + } + private: modified_type_caster_generic_load_impl load_impl; diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index eef03cf74f..93de6ec9c3 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1586,7 +1586,8 @@ struct property_cpp_function< template = 0> static cpp_function readonly(PM pm, const handle &hdl) { return cpp_function( - [pm](const std::shared_ptr &c_sp) -> std::shared_ptr { + [pm](handle c_hdl) -> std::shared_ptr { + std::shared_ptr c_sp = detail::type_caster::shared_ptr_from_python(c_hdl); D ptr = (*c_sp).*pm; return std::shared_ptr(c_sp, ptr); }, @@ -1622,8 +1623,8 @@ struct property_cpp_function< template = 0> static cpp_function readonly(PM pm, const handle &hdl) { return cpp_function( - [pm](const std::shared_ptr &c_sp) - -> std::shared_ptr::type> { + [pm](handle c_hdl) -> std::shared_ptr::type> { + std::shared_ptr c_sp = detail::type_caster::shared_ptr_from_python(c_hdl); return std::shared_ptr::type>(c_sp, &(c_sp.get()->*pm)); }, is_method(hdl)); @@ -1632,7 +1633,8 @@ struct property_cpp_function< template = 0> static cpp_function read(PM pm, const handle &hdl) { return cpp_function( - [pm](const std::shared_ptr &c_sp) -> std::shared_ptr { + [pm](handle c_hdl) -> std::shared_ptr { + std::shared_ptr c_sp = detail::type_caster::shared_ptr_from_python(c_hdl); return std::shared_ptr(c_sp, &(c_sp.get()->*pm)); }, is_method(hdl)); @@ -1669,7 +1671,10 @@ struct property_cpp_function< template = 0> static cpp_function read(PM pm, const handle &hdl) { return cpp_function( - [pm](const std::shared_ptr &c_sp) -> D { return D{std::move(c_sp.get()->*pm)}; }, + [pm](handle c_hdl) -> D { + std::shared_ptr c_sp = detail::type_caster::shared_ptr_from_python(c_hdl); + return D{std::move(c_sp.get()->*pm)}; + }, is_method(hdl)); } diff --git a/tests/test_class_sh_property_non_owning.cpp b/tests/test_class_sh_property_non_owning.cpp new file mode 100644 index 0000000000..5be72007be --- /dev/null +++ b/tests/test_class_sh_property_non_owning.cpp @@ -0,0 +1,68 @@ +#include "pybind11/smart_holder.h" +#include "pybind11_tests.h" + +#include +#include + +namespace test_class_sh_property_non_owning { + +struct CoreField { + explicit CoreField(int int_value = -99) : int_value{int_value} {} + int int_value; +}; + +struct DataField { + DataField(int i_value, int i_shared, int i_unique) + : core_fld_value{i_value}, core_fld_shared_ptr{new CoreField{i_shared}}, + core_fld_raw_ptr{core_fld_shared_ptr.get()}, core_fld_unique_ptr{ + new CoreField{i_unique}} {} + CoreField core_fld_value; + std::shared_ptr core_fld_shared_ptr; + CoreField *core_fld_raw_ptr; + std::unique_ptr core_fld_unique_ptr; +}; + +struct DataFieldsHolder { +private: + std::vector vec; + +public: + DataFieldsHolder(std::size_t vec_size) { + for (std::size_t i = 0; i < vec_size; i++) { + int i11 = static_cast(i) * 11; + vec.push_back(DataField(13 + i11, 14 + i11, 15 + i11)); + } + } + + DataField *vec_at(std::size_t index) { + if (index >= vec.size()) { + return nullptr; + } + return &vec[index]; + } +}; + +} // namespace test_class_sh_property_non_owning + +using namespace test_class_sh_property_non_owning; + +PYBIND11_SMART_HOLDER_TYPE_CASTERS(CoreField) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(DataField) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(DataFieldsHolder) + +TEST_SUBMODULE(class_sh_property_non_owning, m) { + py::classh(m, "CoreField").def_readwrite("int_value", &CoreField::int_value); + + py::classh(m, "DataField") + .def_readonly("core_fld_value_ro", &DataField::core_fld_value) + .def_readwrite("core_fld_value_rw", &DataField::core_fld_value) + .def_readonly("core_fld_shared_ptr_ro", &DataField::core_fld_shared_ptr) + .def_readwrite("core_fld_shared_ptr_rw", &DataField::core_fld_shared_ptr) + .def_readonly("core_fld_raw_ptr_ro", &DataField::core_fld_raw_ptr) + .def_readwrite("core_fld_raw_ptr_rw", &DataField::core_fld_raw_ptr) + .def_readwrite("core_fld_unique_ptr_rw", &DataField::core_fld_unique_ptr); + + py::classh(m, "DataFieldsHolder") + .def(py::init()) + .def("vec_at", &DataFieldsHolder::vec_at, py::return_value_policy::reference_internal); +} diff --git a/tests/test_class_sh_property_non_owning.py b/tests/test_class_sh_property_non_owning.py new file mode 100644 index 0000000000..8b6f5f4bd3 --- /dev/null +++ b/tests/test_class_sh_property_non_owning.py @@ -0,0 +1,28 @@ +import pytest + +from pybind11_tests import class_sh_property_non_owning as m + + +@pytest.mark.parametrize("persistent_holder", [True, False]) +@pytest.mark.parametrize( + ("core_fld", "expected"), + [ + ("core_fld_value_ro", (13, 24)), + ("core_fld_value_rw", (13, 24)), + ("core_fld_shared_ptr_ro", (14, 25)), + ("core_fld_shared_ptr_rw", (14, 25)), + ("core_fld_raw_ptr_ro", (14, 25)), + ("core_fld_raw_ptr_rw", (14, 25)), + ("core_fld_unique_ptr_rw", (15, 26)), + ], +) +def test_core_fld_common(core_fld, expected, persistent_holder): + if persistent_holder: + h = m.DataFieldsHolder(2) + for i, exp in enumerate(expected): + c = getattr(h.vec_at(i), core_fld) + assert c.int_value == exp + else: + for i, exp in enumerate(expected): + c = getattr(m.DataFieldsHolder(2).vec_at(i), core_fld) + assert c.int_value == exp