diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index d95d61f7bb..080d4e06e8 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -778,7 +778,12 @@ class cpp_function : public function { } else { if (overloads->is_constructor && !self_value_and_holder.holder_constructed()) { auto *pi = reinterpret_cast(parent.ptr()); - self_value_and_holder.type->init_instance(pi, nullptr); + try { + self_value_and_holder.type->init_instance(pi, nullptr); + } catch (const std::runtime_error &e) { + PyErr_SetString(PyExc_RuntimeError, e.what()); + return nullptr; + } } return result.ptr(); } @@ -1298,6 +1303,34 @@ class class_ : public detail::generic_type { } private: + /// Initialize holder from pointer when the underlying object is destructible (public destructor) + template + detail::enable_if_t< + !detail::is_instantiation::value || + std::is_destructible::value + , void> + static init_shared_holder_from_pointer(detail::value_and_holder &v_h) { + new (std::addressof(v_h.holder())) holder_type(v_h.value_ptr()); + v_h.set_holder_constructed(); + } + + /// Do *not* initialize holder from pointer when the underlying object is *not* destructible + template + detail::enable_if_t< + detail::is_instantiation::value && + !std::is_destructible::value + , void> + static init_shared_holder_from_pointer(detail::value_and_holder &) { + pybind11_fail("Unable to construct C++ holder" +#if defined(NDEBUG) + " (compile in debug mode for type information)" +#else + ": cannot construct a `" + type_id() + "' holder around a non-destructible `" + + type_id() + "' pointer" +#endif + ); + } + /// Initialize holder object, variant 1: object derives from enable_shared_from_this template static void init_holder(detail::instance *inst, detail::value_and_holder &v_h, @@ -1312,8 +1345,7 @@ class class_ : public detail::generic_type { } catch (const std::bad_weak_ptr &) {} if (!v_h.holder_constructed() && inst->owned) { - new (std::addressof(v_h.holder())) holder_type(v_h.value_ptr()); - v_h.set_holder_constructed(); + init_shared_holder_from_pointer(v_h); } } @@ -1334,8 +1366,7 @@ class class_ : public detail::generic_type { init_holder_from_existing(v_h, holder_ptr, std::is_copy_constructible()); v_h.set_holder_constructed(); } else if (inst->owned || detail::always_construct_holder::value) { - new (std::addressof(v_h.holder())) holder_type(v_h.value_ptr()); - v_h.set_holder_constructed(); + init_shared_holder_from_pointer(v_h); } } diff --git a/tests/test_smart_ptr.cpp b/tests/test_smart_ptr.cpp index 87c9be8c2b..c9289e884d 100644 --- a/tests/test_smart_ptr.cpp +++ b/tests/test_smart_ptr.cpp @@ -212,6 +212,26 @@ TEST_SUBMODULE(smart_ptr, m) { py::class_(m, "MyObject4b") .def(py::init()); + // Object with non-public destructor in a shared_ptr + class MyObject4c { + public: + MyObject4c() { print_created(this); } + protected: + ~MyObject4c() { print_destroyed(this); } + }; + py::class_>(m, "MyObject4c") + .def(py::init()); + + // Object with non-public destructor in a shared_ptr with std::enable_shared_from_this<> + class MyObject4d : public std::enable_shared_from_this { + public: + MyObject4d() { print_created(this); } + protected: + ~MyObject4d() { print_destroyed(this); } + }; + py::class_>(m, "MyObject4d") + .def(py::init()); + // test_large_holder class MyObject5 { // managed by huge_unique_ptr public: diff --git a/tests/test_smart_ptr.py b/tests/test_smart_ptr.py index c6627043bd..5df2e4e0a0 100644 --- a/tests/test_smart_ptr.py +++ b/tests/test_smart_ptr.py @@ -136,6 +136,20 @@ def test_unique_deleter(): assert cstats4b.alive() == 0 # Should be deleted +def test_shared_ptr_non_public_destructor(): + """#1178: Shared Pointers and Protected Destructors""" + with pytest.raises(RuntimeError) as excinfo: + m.MyObject4c() + assert "Unable to construct C++ holder" in str(excinfo.value) + + +def test_shared_ptr_from_this_non_public_destructor(): + """#1178: Shared Pointers and Protected Destructors""" + with pytest.raises(RuntimeError) as excinfo: + m.MyObject4d() + assert "Unable to construct C++ holder" in str(excinfo.value) + + def test_large_holder(): o = m.MyObject5(5) assert o.value == 5