diff --git a/docs/advanced.rst b/docs/advanced.rst index 4fd1d41e..6f57d0be 100644 --- a/docs/advanced.rst +++ b/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 | -| | object without taking ownership similar to the above | -| | :enum:`return_value_policy::reference` policy. In contrast to that policy, | -| | the function or property's implicit ``this`` argument (called the *parent*)| -| | is considered to be the the owner of the return value (the *child*). | -| | 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. | +| :enum:`return_value_policy::reference_internal` | Like :enum:`return_value_policy::reference` but additionally applies a | +| | :class:`keep_alive<0,1>()` call policy (described next) that keeps the | +| | ``this`` argument of the function or property from being garbage collected | +| | as long as the return value remains referenced. See the | +| | :class:`keep_alive` call policy (described next) for details. | +--------------------------------------------------+----------------------------------------------------------------------------+ .. warning:: diff --git a/example/issues.cpp b/example/issues.cpp index 66934ee0..032b6ddf 100644 --- a/example/issues.cpp +++ b/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_(m2, "NestA").def(py::init<>()).def(py::self += int()); + py::class_(m2, "NestABase").def(py::init<>()).def_readwrite("value", &NestABase::value); + py::class_(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_(m2, "NestB").def(py::init<>()).def(py::self -= int()).def_readwrite("a", &NestB::a); py::class_(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; }); diff --git a/example/issues.py b/example/issues.py index 12b46b7f..facdafa8 100644 --- a/example/issues.py +++ b/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() diff --git a/example/issues.ref b/example/issues.ref index f25ab298..8a05c5dc 100644 --- a/example/issues.ref +++ b/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 diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index ad8f2759..76906719 100644 --- a/include/pybind11/cast.h +++ b/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); diff --git a/include/pybind11/common.h b/include/pybind11/common.h index c0d24825..6a081161 100644 --- a/include/pybind11/common.h +++ b/include/pybind11/common.h @@ -245,7 +245,6 @@ inline std::string error_string(); template struct instance_essentials { PyObject_HEAD type *value; - PyObject *parent; PyObject *weakrefs; bool owned : 1; bool constructed : 1; diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 632f955e..ebd6f6f5 100644 --- a/include/pybind11/pybind11.h +++ b/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 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 struct iterator_state { Iterator it, end; bool first;