From d4e61bb8abd893f5b9fbcd8b3bf171dc296f011e Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 10 Apr 2025 10:20:50 -0700 Subject: [PATCH] [skip ci] Move "Pitfalls with raw pointers and shared ownership" section to a more prominent location. --- docs/advanced/smart_ptrs.rst | 58 ++---------------------------------- docs/classes.rst | 58 ++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 56 deletions(-) diff --git a/docs/advanced/smart_ptrs.rst b/docs/advanced/smart_ptrs.rst index 599f384db0..aeb98de0f0 100644 --- a/docs/advanced/smart_ptrs.rst +++ b/docs/advanced/smart_ptrs.rst @@ -1,3 +1,5 @@ +.. _py_class_holder: + Smart pointers & ``py::class_`` ############################### @@ -175,59 +177,3 @@ provides ``.get()`` functionality via ``.getPointer()``. The file :file:`tests/test_smart_ptr.cpp` contains a complete example that demonstrates how to work with custom reference-counting holder types in more detail. - - -Be careful not to accidentally undermine automatic lifetime management -====================================================================== - -``py::class_``-wrapped objects automatically manage the lifetime of the -wrapped C++ object, in collaboration with the chosen holder type. -When wrapping C++ functions involving raw pointers, care needs to be taken -to not inadvertently transfer ownership, resulting in multiple Python -objects acting as owners, causing heap-use-after-free or double-free errors. -For example: - -.. code-block:: cpp - - class Child { }; - - class Parent { - public: - Parent() : child(std::make_shared()) { } - Child *get_child() { return child.get(); } /* DANGER */ - private: - std::shared_ptr child; - }; - - PYBIND11_MODULE(example, m) { - py::class_>(m, "Child"); - - py::class_>(m, "Parent") - .def(py::init<>()) - .def("get_child", &Parent::get_child); /* PROBLEM */ - } - -The following Python code leads to undefined behavior, likely resulting in -a segmentation fault. - -.. code-block:: python - - from example import Parent - - print(Parent().get_child()) - -Part of the ``/* PROBLEM */`` here is that pybind11 falls back to using -``return_value_policy::take_ownership`` as the default (see -:ref:`return_value_policies`). The fact that the ``Child`` instance is -already managed by ``std::shared_ptr`` is lost. Therefore pybind11 -will create a second independent ``std::shared_ptr`` that also -claims ownership of the pointer, eventually leading to heap-use-after-free -or double-free errors. - -There are various ways to resolve this issue, either by changing -the ``Child`` or ``Parent`` C++ implementations (e.g. using -``std::enable_shared_from_this`` as a base class for -``Child``, or adding a member function to ``Parent`` that returns -``std::shared_ptr``), or if that is not feasible, by using -``return_value_policy::reference_internal``. What is the best approach -depends on the exact situation. diff --git a/docs/classes.rst b/docs/classes.rst index e68b802ee9..b6923963dc 100644 --- a/docs/classes.rst +++ b/docs/classes.rst @@ -459,6 +459,64 @@ you can use ``py::detail::overload_cast_impl`` with an additional set of parenth other using the ``.def(py::init<...>())`` syntax. The existing machinery for specifying keyword and default arguments also works. +☝️ Pitfalls with raw pointers and shared ownership +================================================== + +``py::class_``-wrapped objects automatically manage the lifetime of the +wrapped C++ object, in collaboration with the chosen holder type (see +:ref:`py_class_holder`). When wrapping C++ functions involving raw pointers, +care needs to be taken to not accidentally undermine automatic lifetime +management. For example, ownership is inadvertently transferred here: + +.. code-block:: cpp + + class Child { }; + + class Parent { + public: + Parent() : child(std::make_shared()) { } + Child *get_child() { return child.get(); } /* DANGER */ + private: + std::shared_ptr child; + }; + + PYBIND11_MODULE(example, m) { + py::class_>(m, "Child"); + + py::class_>(m, "Parent") + .def(py::init<>()) + .def("get_child", &Parent::get_child); /* PROBLEM */ + } + +The following Python code leads to undefined behavior, likely resulting in +a segmentation fault. + +.. code-block:: python + + from example import Parent + + print(Parent().get_child()) + +Part of the ``/* PROBLEM */`` here is that pybind11 falls back to using +``return_value_policy::take_ownership`` as the default (see +:ref:`return_value_policies`). The fact that the ``Child`` instance is +already managed by ``std::shared_ptr`` is lost. Therefore pybind11 +will create a second independent ``std::shared_ptr`` that also +claims ownership of the pointer, eventually leading to heap-use-after-free +or double-free errors. + +There are various ways to resolve this issue, either by changing +the ``Child`` or ``Parent`` C++ implementations (e.g. using +``std::enable_shared_from_this`` as a base class for +``Child``, or adding a member function to ``Parent`` that returns +``std::shared_ptr``), or if that is not feasible, by using +``return_value_policy::reference_internal``. What is the best approach +depends on the exact situation. + +A highly effective way to stay in the clear — even in pure C++, but +especially when binding C++ code to Python — is to consistently prefer +``std::shared_ptr`` or ``std::unique_ptr`` over passing raw pointers. + .. _native_enum: Enumerations and internal types