Fix Python object lifetimes associated with shared_ptrs

docs/changelog.rst

@@ -50,6 +50,9 @@ v2.3.0 (Not yet released)
 * ``pybind11/stl.h`` does not convert strings to ``vector<string>`` anymore.
   `#1258 <https://github.com/pybind/pybind11/issues/1258>`_.
 
+* fixed shared_ptrs to Python-derived instances outliving the associated Python object
+  `#1546 <https://github.com/pybind/pybind11/issues/1546>`_.
+
 v2.2.4 (September 11, 2018)
 -----------------------------------------------------
 

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<type> {
     holder_type holder;
 };
 
-/// Specialize for the common std::shared_ptr, so users don't need to
+/// Specialize type_caster for std::shared_ptr<T>.
+/// 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 <typename T>
-class type_caster<std::shared_ptr<T>> : public copyable_holder_caster<T, std::shared_ptr<T>> { };
+class type_caster<std::shared_ptr<T>>
+{
+    PYBIND11_TYPE_CASTER(std::shared_ptr<T>, _(PYBIND11_STRING_NAME));
+
+    // Re-use copyable_holder_caster
+    using BaseCaster = copyable_holder_caster<T, std::shared_ptr<T>>;
+
+    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<object>(src);
+
+        // Construct a shared_ptr to the py::object
+        auto py_obj_ptr = std::shared_ptr<object>{
+            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<T>
+        // * Use this to make an aliased shared_ptr<T> that keeps the py::object alive
+        auto base_ptr = static_cast<std::shared_ptr<T>>(bc);
+        value = std::shared_ptr<T>(py_obj_ptr, base_ptr.get());
+        return true;
+    }
+
+    static handle cast(std::shared_ptr<T> sp,
+                       return_value_policy rvp,
+                       handle h)
+    {
+        return BaseCaster::cast(sp, rvp, h);
+    }
+};
 
 template <typename type, typename holder_type>
 struct move_only_holder_caster {
@@ -1540,6 +1587,9 @@ template <typename base, typename holder> struct is_holder_type :
 template <typename base, typename deleter> struct is_holder_type<base, std::unique_ptr<base, deleter>> :
     std::true_type {};
 
+template <typename T>
+struct is_holder_type<T, std::shared_ptr<T>> : std::true_type {};
+
 template <typename T> struct handle_type_name { static constexpr auto name = _<T>(); };
 template <> struct handle_type_name<bytes> { static constexpr auto name = _(PYBIND11_BYTES_NAME); };
 template <> struct handle_type_name<args> { static constexpr auto name = _("*args"); };

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;

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<SharedPtrBase> ptr)
+        : ptr (std::move (ptr))
+        { }
+
+        void run (int i)
+        {
+            ptr->f (i);
+        }
+
+        std::shared_ptr<SharedPtrBase> ptr;
+    };
+
+    py::class_<SharedPtrBase, Trampoline, std::shared_ptr<SharedPtrBase>> (m, "SharedPtrBase")
+        .def (py::init<> ());
+
+    py::class_<SharedPtrHolder> (m, "SharedPtrHolder")
+        .def (py::init<std::shared_ptr<SharedPtrBase>> ())
+        .def ("run", &SharedPtrHolder::run);
+}

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]