Use dynamic cast for shared_from_this holder init

Using a dynamic_cast instead of a static_cast is needed to safely cast
from a base to a derived type.  The previous static_pointer_cast isn't
safe, however, when downcasting (and fails to compile when downcasting
with virtual inheritance).

Switching this to always use a dynamic_pointer_cast shouldn't incur any
additional overhead when a static_pointer_cast is safe (i.e. when
upcasting, or self-casting): compilers don't need RTTI checks in those
cases.

Author: jagerman

include/pybind11/pybind11.h

@@ -1119,13 +1119,15 @@ class class_ : public detail::generic_type {
     template <typename T>
     static void init_holder_helper(instance_type *inst, const holder_type * /* unused */, const std::enable_shared_from_this<T> * /* dummy */) {
         try {
-            new (&inst->holder) holder_type(std::static_pointer_cast<typename holder_type::element_type>(inst->value->shared_from_this()));
-            inst->holder_constructed = true;
-        } catch (const std::bad_weak_ptr &) {
-            if (inst->owned) {
-                new (&inst->holder) holder_type(inst->value);
+            auto sh = std::dynamic_pointer_cast<typename holder_type::element_type>(inst->value->shared_from_this());
+            if (sh) {
+                new (&inst->holder) holder_type(std::move(sh));
                 inst->holder_constructed = true;
             }
+        } catch (const std::bad_weak_ptr &) {}
+        if (!inst->holder_constructed && inst->owned) {
+            new (&inst->holder) holder_type(inst->value);
+            inst->holder_constructed = true;
         }
     }
 

tests/test_smart_ptr.cpp

@@ -214,6 +214,12 @@ struct SharedFromThisRef {
     std::shared_ptr<B> shared = std::make_shared<B>();
 };
 
+// Issue #865: shared_from_this doesn't work with virtual inheritance
+struct SharedFromThisVBase : std::enable_shared_from_this<SharedFromThisVBase> {
+    virtual ~SharedFromThisVBase() = default;
+};
+struct SharedFromThisVirt : virtual SharedFromThisVBase {};
+
 template <typename T>
 class CustomUniquePtr {
     std::unique_ptr<T> impl;
@@ -258,6 +264,11 @@ test_initializer smart_ptr_and_references([](py::module &pm) {
         .def("set_ref", [](SharedFromThisRef &, const B &) { return true; })
         .def("set_holder", [](SharedFromThisRef &, std::shared_ptr<B>) { return true; });
 
+    // Issue #865: shared_from_this doesn't work with virtual inheritance
+    static std::shared_ptr<SharedFromThisVirt> sft(new SharedFromThisVirt());
+    py::class_<SharedFromThisVirt, std::shared_ptr<SharedFromThisVirt>>(m, "SharedFromThisVirt")
+        .def_static("get", []() { return sft.get(); });
+
     struct C {
         C() { print_created(this); }
         ~C() { print_destroyed(this); }

tests/test_smart_ptr.py

@@ -166,7 +166,7 @@ def test_shared_ptr_and_references():
 
 
 def test_shared_ptr_from_this_and_references():
-    from pybind11_tests.smart_ptr import SharedFromThisRef, B
+    from pybind11_tests.smart_ptr import SharedFromThisRef, B, SharedFromThisVirt
 
     s = SharedFromThisRef()
     stats = ConstructorStats.get(B)
@@ -202,6 +202,10 @@ def test_shared_ptr_from_this_and_references():
     del ref, bad_wp, copy, holder_ref, holder_copy, s
     assert stats.alive() == 0
 
+    z = SharedFromThisVirt.get()
+    y = SharedFromThisVirt.get()
+    assert y is z
+
 
 def test_move_only_holder():
     from pybind11_tests.smart_ptr import TypeWithMoveOnlyHolder