diff --git a/docs/changelog.rst b/docs/changelog.rst index 606be413aa..18963ea7a0 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -50,6 +50,9 @@ v2.3.0 (Not yet released) * ``pybind11/stl.h`` does not convert strings to ``vector`` anymore. `#1258 `_. +* fixed shared_ptrs to Python-derived instances outliving the associated Python object + `#1546 `_. + v2.2.4 (September 11, 2018) ----------------------------------------------------- diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 5529768b38..e976129c06 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -10,6 +10,7 @@ #pragma once +#include "pybind11.h" #include "pytypes.h" #include "detail/typeid.h" #include "detail/descr.h" @@ -1496,9 +1497,55 @@ struct copyable_holder_caster : public type_caster_base { holder_type holder; }; -/// Specialize for the common std::shared_ptr, so users don't need to +/// Specialize type_caster for std::shared_ptr. +/// This is the same as copyable_holder_caster, except that when casting to C++ +/// we keep the Python object alive through the shared_ptr as e.g. virtual +/// functions and derived state might be defined there. template -class type_caster> : public copyable_holder_caster> { }; +class type_caster> +{ + PYBIND11_TYPE_CASTER(std::shared_ptr, _(PYBIND11_STRING_NAME)); + + // Re-use copyable_holder_caster + using BaseCaster = copyable_holder_caster>; + + bool load(pybind11::handle src, bool b) + { + BaseCaster bc; + bool success = bc.load(src, b); + if (!success) { + return false; + } + + // Get src as a py::object + auto py_obj = reinterpret_borrow(src); + + // Construct a shared_ptr to the py::object + auto py_obj_ptr = std::shared_ptr{ + new object{py_obj}, + [](auto py_object_ptr) { + // It's possible that when the shared_ptr dies we won't have the + // gil (if the last holder is in a non-Python thread), so we + // make sure to acquire it in the deleter. + gil_scoped_acquire gil; + delete py_object_ptr; + } + }; + + // * Use BaseCaster to get it as the shared_ptr + // * Use this to make an aliased shared_ptr that keeps the py::object alive + auto base_ptr = static_cast>(bc); + value = std::shared_ptr(py_obj_ptr, base_ptr.get()); + return true; + } + + static handle cast(std::shared_ptr sp, + return_value_policy rvp, + handle h) + { + return BaseCaster::cast(sp, rvp, h); + } +}; template struct move_only_holder_caster { @@ -1540,6 +1587,9 @@ template struct is_holder_type : template struct is_holder_type> : std::true_type {}; +template +struct is_holder_type> : std::true_type {}; + template struct handle_type_name { static constexpr auto name = _(); }; template <> struct handle_type_name { static constexpr auto name = _(PYBIND11_BYTES_NAME); }; template <> struct handle_type_name { static constexpr auto name = _("*args"); }; diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index fa5ed7cbd4..fe88fcc4c9 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -20,6 +20,8 @@ NAMESPACE_BEGIN(PYBIND11_NAMESPACE) class handle; class object; class str; class iterator; struct arg; struct arg_v; +class gil_scoped_acquire; +class gil_scoped_release; NAMESPACE_BEGIN(detail) class args_proxy; diff --git a/tests/test_virtual_functions.cpp b/tests/test_virtual_functions.cpp index c9a561c09f..8788319a34 100644 --- a/tests/test_virtual_functions.cpp +++ b/tests/test_virtual_functions.cpp @@ -475,4 +475,46 @@ void initialize_inherited_virtuals(py::module &m) { // Fix issue #1454 (crash when acquiring/releasing GIL on another thread in Python 2.7) m.def("test_gil", &test_gil); m.def("test_gil_from_thread", &test_gil_from_thread); -}; + + // Fix issue #1546 (Python object not kept alive by shared_ptr) + struct SharedPtrBase + { + virtual void f (int i) const = 0; + virtual ~SharedPtrBase () { }; + }; + + struct Trampoline : SharedPtrBase + { + void f (int i) const override + { + PYBIND11_OVERLOAD_PURE_NAME ( + void, + SharedPtrBase, + "f", + impl, + /* args */ + i); + } + }; + + struct SharedPtrHolder + { + SharedPtrHolder (std::shared_ptr ptr) + : ptr (std::move (ptr)) + { } + + void run (int i) + { + ptr->f (i); + } + + std::shared_ptr ptr; + }; + + py::class_> (m, "SharedPtrBase") + .def (py::init<> ()); + + py::class_ (m, "SharedPtrHolder") + .def (py::init> ()) + .def ("run", &SharedPtrHolder::run); +} diff --git a/tests/test_virtual_functions.py b/tests/test_virtual_functions.py index 5ce9abd355..eff2a05c6f 100644 --- a/tests/test_virtual_functions.py +++ b/tests/test_virtual_functions.py @@ -375,3 +375,22 @@ def test_issue_1454(): # Fix issue #1454 (crash when acquiring/releasing GIL on another thread in Python 2.7) m.test_gil() m.test_gil_from_thread() + + +def test_issue_1546(): + # Fix issue #1546 (Python object not kept alive by shared_ptr) + somelist = [] + + class Derived(m.SharedPtrBase): + def __init__(self): + super(Derived, self).__init__() + + def f(self, value): + somelist.append(value) + print("Right one", 42) + + holder = m.SharedPtrHolder(Derived()) + # At this point, our 'Derived' instance has gone out of scope in Python but + # is being kept alive by a shared_ptr inside 'holder'. + holder.run(42) + assert somelist == [42]