Implement reference_internal with a keep_alive

reference_internal requires an `instance` field to track the returned
reference's parent, but that's just a duplication of what
keep_alive<0,1> does, so use a keep alive to do this to eliminate the
duplication.

docs/advanced.rst

@@ -612,17 +612,11 @@ functions. The default policy is :enum:`return_value_policy::automatic`.
 |                                                  | it is no longer used. Warning: undefined behavior will ensue when the C++  |
 |                                                  | side deletes an object that is still referenced and used by Python.        |
 +--------------------------------------------------+----------------------------------------------------------------------------+
-| :enum:`return_value_policy::reference_internal`  | This policy only applies to methods and properties. It references the      |
+| :enum:`return_value_policy::reference_internal`  | Like :enum:`return_value_policy::reference` but additionally applies a     |
-|                                                  | object without taking ownership similar to the above                       |
+|                                                  | :class:`keep_alive<0,1>()` call policy (described next) that keeps the     |
-|                                                  | :enum:`return_value_policy::reference` policy. In contrast to that policy, |
+|                                                  | ``this`` argument of the function or property from being garbage collected |
-|                                                  | the function or property's implicit ``this`` argument (called the *parent*)|
+|                                                  | as long as the return value remains referenced.  See the                   |
-|                                                  | is considered to be the the owner of the return value (the *child*).       |
+|                                                  | :class:`keep_alive` call policy (described next) for details.              |
-|                                                  | pybind11 then couples the lifetime of the parent to the child via a        |
-|                                                  | reference relationship that ensures that the parent cannot be garbage      |
-|                                                  | collected while Python is still using the child. More advanced variations  |
-|                                                  | of this scheme are also possible using combinations of                     |
-|                                                  | :enum:`return_value_policy::reference` and the :class:`keep_alive` call    |
-|                                                  | policy described next.                                                     |
 +--------------------------------------------------+----------------------------------------------------------------------------+
 
 .. warning::

example/issues.cpp

@@ -161,10 +161,13 @@ void init_issues(py::module &m) {
     // Issue #328: first member in a class can't be used in operators
 #define TRACKERS(CLASS) CLASS() { std::cout << #CLASS "@" << this << " constructor\n"; } \
                         ~CLASS() { std::cout << #CLASS "@" << this << " destructor\n"; }
-    struct NestA { int value = 3; NestA& operator+=(int i) { value += i; return *this; } TRACKERS(NestA) };
+    struct NestABase { int value = -2; TRACKERS(NestABase) };
+    struct NestA : NestABase { int value = 3; NestA& operator+=(int i) { value += i; return *this; } TRACKERS(NestA) };
     struct NestB { NestA a; int value = 4; NestB& operator-=(int i) { value -= i; return *this; } TRACKERS(NestB) };
     struct NestC { NestB b; int value = 5; NestC& operator*=(int i) { value *= i; return *this; } TRACKERS(NestC) };
-    py::class_<NestA>(m2, "NestA").def(py::init<>()).def(py::self += int());
+    py::class_<NestABase>(m2, "NestABase").def(py::init<>()).def_readwrite("value", &NestABase::value);
+    py::class_<NestA>(m2, "NestA").def(py::init<>()).def(py::self += int())
+        .def("as_base", [](NestA &a) -> NestABase& { return (NestABase&) a; }, py::return_value_policy::reference_internal);
     py::class_<NestB>(m2, "NestB").def(py::init<>()).def(py::self -= int()).def_readwrite("a", &NestB::a);
     py::class_<NestC>(m2, "NestC").def(py::init<>()).def(py::self *= int()).def_readwrite("b", &NestC::b);
     m2.def("print_NestA", [](const NestA &a) { std::cout << a.value << std::endl; });

example/issues.py

@@ -95,3 +95,17 @@ print_NestA(c.b.a)
 print_NestB(b)
 print_NestB(c.b)
 print_NestC(c)
+abase = a.as_base()
+print(abase.value)
+a.as_base().value += 44
+print(abase.value)
+print(c.b.a.as_base().value)
+c.b.a.as_base().value += 44
+print(c.b.a.as_base().value)
+del c
+gc.collect()
+del a # Should't delete while abase is still alive
+gc.collect()
+print(abase.value)
+del abase
+gc.collect()

example/issues.ref

@@ -24,9 +24,12 @@ Failed as expected: Incompatible constructor arguments. The following argument t
     1. example.issues.StrIssue(arg0: int)
     2. example.issues.StrIssue()
     Invoked with: no, such, constructor
+NestABase@0x1152940 constructor
 NestA@0x1152940 constructor
+NestABase@0x11f9350 constructor
 NestA@0x11f9350 constructor
 NestB@0x11f9350 constructor
+NestABase@0x112d0d0 constructor
 NestA@0x112d0d0 constructor
 NestB@0x112d0d0 constructor
 NestC@0x112d0d0 constructor
@@ -36,9 +39,17 @@ NestC@0x112d0d0 constructor
 3
 1
 35
+-2
+42
+-2
+42
 NestC@0x112d0d0 destructor
 NestB@0x112d0d0 destructor
 NestA@0x112d0d0 destructor
+NestABase@0x112d0d0 destructor
+42
+NestA@0x1152940 destructor
+NestABase@0x1152940 destructor
 NestB@0x11f9350 destructor
 NestA@0x11f9350 destructor
-NestA@0x1152940 destructor
+NestABase@0x11f9350 destructor

include/pybind11/cast.h

@@ -142,6 +142,9 @@ inline PyThreadState *get_thread_state_unchecked() {
 #endif
 }
 
+// Forward declaration
+inline void keep_alive_impl(handle nurse, handle patient);
+
 class type_caster_generic {
 public:
     PYBIND11_NOINLINE type_caster_generic(const std::type_info &type_info)
@@ -208,7 +211,6 @@ public:
 
         wrapper->value = src;
         wrapper->owned = true;
-        wrapper->parent = nullptr;
 
         if (policy == return_value_policy::automatic)
             policy = return_value_policy::take_ownership;
@@ -229,7 +231,7 @@ public:
             wrapper->owned = false;
         } else if (policy == return_value_policy::reference_internal) {
             wrapper->owned = false;
-            wrapper->parent = parent.inc_ref().ptr();
+            detail::keep_alive_impl(inst, parent);
         }
 
         tinfo->init_holder(inst.ptr(), existing_holder);

include/pybind11/common.h

@@ -245,7 +245,6 @@ inline std::string error_string();
 template <typename type> struct instance_essentials {
     PyObject_HEAD
     type *value;
-    PyObject *parent;
     PyObject *weakrefs;
     bool owned : 1;
     bool constructed : 1;

include/pybind11/pybind11.h

@@ -729,7 +729,6 @@ protected:
         auto tinfo = detail::get_type_info(type);
         self->value = ::operator new(tinfo->type_size);
         self->owned = true;
-        self->parent = nullptr;
         self->constructed = false;
         detail::get_internals().registered_instances.emplace(self->value, (PyObject *) self);
         return (PyObject *) self;
@@ -751,7 +750,6 @@ protected:
             if (!found)
                 pybind11_fail("generic_type::dealloc(): Tried to deallocate unregistered instance!");
 
-            Py_XDECREF(self->parent);
             if (self->weakrefs)
                 PyObject_ClearWeakRefs((PyObject *) self);
         }
@@ -1095,11 +1093,8 @@ template <typename... Args> struct init {
     }
 };
 
-PYBIND11_NOINLINE inline void keep_alive_impl(int Nurse, int Patient, handle args, handle ret) {
+inline void keep_alive_impl(handle nurse, handle patient) {
     /* Clever approach based on weak references taken from Boost.Python */
-    handle nurse  (Nurse   > 0 ? PyTuple_GetItem(args.ptr(), Nurse   - 1) : ret.ptr());
-    handle patient(Patient > 0 ? PyTuple_GetItem(args.ptr(), Patient - 1) : ret.ptr());
-
     if (!nurse || !patient)
         pybind11_fail("Could not activate keep_alive!");
 
@@ -1115,6 +1110,13 @@ PYBIND11_NOINLINE inline void keep_alive_impl(int Nurse, int Patient, handle arg
     (void) wr.release();
 }
 
+PYBIND11_NOINLINE inline void keep_alive_impl(int Nurse, int Patient, handle args, handle ret) {
+    handle nurse  (Nurse   > 0 ? PyTuple_GetItem(args.ptr(), Nurse   - 1) : ret.ptr());
+    handle patient(Patient > 0 ? PyTuple_GetItem(args.ptr(), Patient - 1) : ret.ptr());
+
+    keep_alive_impl(nurse, patient);
+}
+
 template <typename Iterator> struct iterator_state {
     Iterator it, end;
     bool first;