Only keep python portion around if there's an alias class

Author: virtuald

include/pybind11/cast.h

@@ -1547,6 +1547,14 @@ struct holder_retriever<std::shared_ptr<T>> {
     };
 
     static auto get_derivative_holder(const value_and_holder &v_h) -> std::shared_ptr<T> {
+        // If there's no trampoline class, nothing special needed
+        if (!v_h.inst->has_alias) {
+            return v_h.template holder<std::shared_ptr<T>>();
+        }
+
+        // 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

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();

include/pybind11/pybind11.h

@@ -1282,7 +1282,11 @@ class class_ : public detail::generic_type {
         record.type_size = sizeof(conditional_t<has_alias, type_alias, type>);
         record.type_align = alignof(conditional_t<has_alias, type_alias, type>&);
         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<std::unique_ptr, holder_type>::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<type>());
     }
 
+    /// 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.

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<SpGoAway> m_obj;
+    };
+
+    py::class_<SpGoAway, std::shared_ptr<SpGoAway>>(m, "SpGoAway")
+        .def(py::init<>());
+
+    py::class_<SpGoAwayTester>(m, "SpGoAwayTester")
+        .def(py::init<>())
+        .def_readwrite("obj", &SpGoAwayTester::m_obj);
 }

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