diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index f6f9e02897..a9694b2c23 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1547,6 +1547,14 @@ struct holder_retriever> { }; static auto get_derivative_holder(const value_and_holder &v_h) -> std::shared_ptr { + // If there's no trampoline class, nothing special needed + if (!v_h.inst->has_alias) { + return v_h.template holder>(); + } + + // If there's a trampoline class, ensure the python side of the object doesn't + // die until the C++ portion also dies + // // The shared_ptr is always given to C++ code, so construct a new shared_ptr // that is given a custom deleter. The custom deleter increments the python // reference count to bind the python instance lifetime with the lifetime diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 5560198a05..5c23da0fe1 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -475,6 +475,8 @@ struct instance { bool simple_instance_registered : 1; /// If true, get_internals().patients has an entry for this object bool has_patients : 1; + /// If true, created with an associated alias class (set via `init_instance`) + bool has_alias : 1; /// Initializes all of the above type/values/holders data (but not the instance values themselves) void allocate_layout(); diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 648300eef2..e3fda81c04 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1282,7 +1282,11 @@ class class_ : public detail::generic_type { record.type_size = sizeof(conditional_t); record.type_align = alignof(conditional_t&); record.holder_size = sizeof(holder_type); - record.init_instance = init_instance; + if (has_alias) { + record.init_instance = init_alias_instance; + } else { + record.init_instance = init_instance; + } record.dealloc = dealloc; record.default_holder = detail::is_instantiation::value; @@ -1555,6 +1559,12 @@ class class_ : public detail::generic_type { init_holder(inst, v_h, (const holder_type *) holder_ptr, v_h.value_ptr()); } + /// Sets the `has_alias` flag in the instance and calls init_instance + static void init_alias_instance(detail::instance *inst, const void *holder_ptr) { + inst->has_alias = true; + init_instance(inst, holder_ptr); + } + /// Deallocates an instance; via holder, if constructed; otherwise via operator delete. static void dealloc(detail::value_and_holder &v_h) { // We could be deallocating because we are cleaning up after a Python exception. diff --git a/tests/test_smart_ptr.cpp b/tests/test_smart_ptr.cpp index 1d9e2cba3e..fb2992ffb5 100644 --- a/tests/test_smart_ptr.cpp +++ b/tests/test_smart_ptr.cpp @@ -430,4 +430,19 @@ TEST_SUBMODULE(smart_ptr, m) { .def("set_object", &SpBaseTester::set_object) .def("is_base_used", &SpBaseTester::is_base_used) .def_readwrite("obj", &SpBaseTester::m_obj); + + // For testing that a C++ class without an alias does not retain the python + // portion of the object + struct SpGoAway {}; + + struct SpGoAwayTester { + std::shared_ptr m_obj; + }; + + py::class_>(m, "SpGoAway") + .def(py::init<>()); + + py::class_(m, "SpGoAwayTester") + .def(py::init<>()) + .def_readwrite("obj", &SpGoAwayTester::m_obj); } diff --git a/tests/test_smart_ptr.py b/tests/test_smart_ptr.py index fa2a3fc170..d57b9e74f8 100644 --- a/tests/test_smart_ptr.py +++ b/tests/test_smart_ptr.py @@ -378,3 +378,23 @@ def test_shared_ptr_arg_identity(): tester.set_object(None) pytest.gc_collect() assert objref() is None + + +def test_shared_ptr_goaway(): + import weakref + + tester = m.SpGoAwayTester() + + obj = m.SpGoAway() + objref = weakref.ref(obj) + + assert tester.obj is None + + tester.obj = obj + del obj + pytest.gc_collect() + + # python reference is no longer around + assert objref() is None + # C++ reference is still around + assert tester.obj is not None