From b8ac438386363d5b50e0be52157034be9cd2c2bb Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Fri, 19 May 2017 13:34:55 -0400 Subject: [PATCH] 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. --- include/pybind11/pybind11.h | 12 +++++++----- tests/test_smart_ptr.cpp | 11 +++++++++++ tests/test_smart_ptr.py | 6 +++++- 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 098d7e0cf..4decfd1da 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1119,13 +1119,15 @@ private: template static void init_holder_helper(instance_type *inst, const holder_type * /* unused */, const std::enable_shared_from_this * /* dummy */) { try { - new (&inst->holder) holder_type(std::static_pointer_cast(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(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; } } diff --git a/tests/test_smart_ptr.cpp b/tests/test_smart_ptr.cpp index 83c1c018a..14b49d554 100644 --- a/tests/test_smart_ptr.cpp +++ b/tests/test_smart_ptr.cpp @@ -214,6 +214,12 @@ struct SharedFromThisRef { std::shared_ptr shared = std::make_shared(); }; +// Issue #865: shared_from_this doesn't work with virtual inheritance +struct SharedFromThisVBase : std::enable_shared_from_this { + virtual ~SharedFromThisVBase() = default; +}; +struct SharedFromThisVirt : virtual SharedFromThisVBase {}; + template class CustomUniquePtr { std::unique_ptr 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) { return true; }); + // Issue #865: shared_from_this doesn't work with virtual inheritance + static std::shared_ptr sft(new SharedFromThisVirt()); + py::class_>(m, "SharedFromThisVirt") + .def_static("get", []() { return sft.get(); }); + struct C { C() { print_created(this); } ~C() { print_destroyed(this); } diff --git a/tests/test_smart_ptr.py b/tests/test_smart_ptr.py index b5af3bd38..4b1e20ea4 100644 --- a/tests/test_smart_ptr.py +++ b/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