Skip to content

Commit e38beb9

Browse files
authored
[skip ci] Move "Pitfalls with raw pointers and shared ownership" section to a more prominent location. (#5611)
1 parent 1bd1d1c commit e38beb9

File tree

2 files changed

+60
-56
lines changed

2 files changed

+60
-56
lines changed

docs/advanced/smart_ptrs.rst

+2-56
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
.. _py_class_holder:
2+
13
Smart pointers & ``py::class_``
24
###############################
35

@@ -175,59 +177,3 @@ provides ``.get()`` functionality via ``.getPointer()``.
175177
The file :file:`tests/test_smart_ptr.cpp` contains a complete example
176178
that demonstrates how to work with custom reference-counting holder types
177179
in more detail.
178-
179-
180-
Be careful not to accidentally undermine automatic lifetime management
181-
======================================================================
182-
183-
``py::class_``-wrapped objects automatically manage the lifetime of the
184-
wrapped C++ object, in collaboration with the chosen holder type.
185-
When wrapping C++ functions involving raw pointers, care needs to be taken
186-
to not inadvertently transfer ownership, resulting in multiple Python
187-
objects acting as owners, causing heap-use-after-free or double-free errors.
188-
For example:
189-
190-
.. code-block:: cpp
191-
192-
class Child { };
193-
194-
class Parent {
195-
public:
196-
Parent() : child(std::make_shared<Child>()) { }
197-
Child *get_child() { return child.get(); } /* DANGER */
198-
private:
199-
std::shared_ptr<Child> child;
200-
};
201-
202-
PYBIND11_MODULE(example, m) {
203-
py::class_<Child, std::shared_ptr<Child>>(m, "Child");
204-
205-
py::class_<Parent, std::shared_ptr<Parent>>(m, "Parent")
206-
.def(py::init<>())
207-
.def("get_child", &Parent::get_child); /* PROBLEM */
208-
}
209-
210-
The following Python code leads to undefined behavior, likely resulting in
211-
a segmentation fault.
212-
213-
.. code-block:: python
214-
215-
from example import Parent
216-
217-
print(Parent().get_child())
218-
219-
Part of the ``/* PROBLEM */`` here is that pybind11 falls back to using
220-
``return_value_policy::take_ownership`` as the default (see
221-
:ref:`return_value_policies`). The fact that the ``Child`` instance is
222-
already managed by ``std::shared_ptr<Child>`` is lost. Therefore pybind11
223-
will create a second independent ``std::shared_ptr<Child>`` that also
224-
claims ownership of the pointer, eventually leading to heap-use-after-free
225-
or double-free errors.
226-
227-
There are various ways to resolve this issue, either by changing
228-
the ``Child`` or ``Parent`` C++ implementations (e.g. using
229-
``std::enable_shared_from_this<Child>`` as a base class for
230-
``Child``, or adding a member function to ``Parent`` that returns
231-
``std::shared_ptr<Child>``), or if that is not feasible, by using
232-
``return_value_policy::reference_internal``. What is the best approach
233-
depends on the exact situation.

docs/classes.rst

+58
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,64 @@ you can use ``py::detail::overload_cast_impl`` with an additional set of parenth
459459
other using the ``.def(py::init<...>())`` syntax. The existing machinery
460460
for specifying keyword and default arguments also works.
461461

462+
☝️ Pitfalls with raw pointers and shared ownership
463+
==================================================
464+
465+
``py::class_``-wrapped objects automatically manage the lifetime of the
466+
wrapped C++ object, in collaboration with the chosen holder type (see
467+
:ref:`py_class_holder`). When wrapping C++ functions involving raw pointers,
468+
care needs to be taken to not accidentally undermine automatic lifetime
469+
management. For example, ownership is inadvertently transferred here:
470+
471+
.. code-block:: cpp
472+
473+
class Child { };
474+
475+
class Parent {
476+
public:
477+
Parent() : child(std::make_shared<Child>()) { }
478+
Child *get_child() { return child.get(); } /* DANGER */
479+
private:
480+
std::shared_ptr<Child> child;
481+
};
482+
483+
PYBIND11_MODULE(example, m) {
484+
py::class_<Child, std::shared_ptr<Child>>(m, "Child");
485+
486+
py::class_<Parent, std::shared_ptr<Parent>>(m, "Parent")
487+
.def(py::init<>())
488+
.def("get_child", &Parent::get_child); /* PROBLEM */
489+
}
490+
491+
The following Python code leads to undefined behavior, likely resulting in
492+
a segmentation fault.
493+
494+
.. code-block:: python
495+
496+
from example import Parent
497+
498+
print(Parent().get_child())
499+
500+
Part of the ``/* PROBLEM */`` here is that pybind11 falls back to using
501+
``return_value_policy::take_ownership`` as the default (see
502+
:ref:`return_value_policies`). The fact that the ``Child`` instance is
503+
already managed by ``std::shared_ptr<Child>`` is lost. Therefore pybind11
504+
will create a second independent ``std::shared_ptr<Child>`` that also
505+
claims ownership of the pointer, eventually leading to heap-use-after-free
506+
or double-free errors.
507+
508+
There are various ways to resolve this issue, either by changing
509+
the ``Child`` or ``Parent`` C++ implementations (e.g. using
510+
``std::enable_shared_from_this<Child>`` as a base class for
511+
``Child``, or adding a member function to ``Parent`` that returns
512+
``std::shared_ptr<Child>``), or if that is not feasible, by using
513+
``return_value_policy::reference_internal``. What is the best approach
514+
depends on the exact situation.
515+
516+
A highly effective way to stay in the clear — even in pure C++, but
517+
especially when binding C++ code to Python — is to consistently prefer
518+
``std::shared_ptr`` or ``std::unique_ptr`` over passing raw pointers.
519+
462520
.. _native_enum:
463521

464522
Enumerations and internal types

0 commit comments

Comments
 (0)