Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow nonconstructible holders #2067

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 36 additions & 5 deletions include/pybind11/pybind11.h
Original file line number Diff line number Diff line change
Expand Up @@ -778,7 +778,12 @@ class cpp_function : public function {
} else {
if (overloads->is_constructor && !self_value_and_holder.holder_constructed()) {
auto *pi = reinterpret_cast<instance *>(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();
}
Expand Down Expand Up @@ -1298,6 +1303,34 @@ class class_ : public detail::generic_type {
}

private:
/// Initialize holder from pointer when the underlying object is destructible (public destructor)
template <typename T = type>
detail::enable_if_t<
!detail::is_instantiation<std::shared_ptr, holder_type>::value ||
std::is_destructible<T>::value
, void>
static init_shared_holder_from_pointer(detail::value_and_holder &v_h) {
new (std::addressof(v_h.holder<holder_type>())) holder_type(v_h.value_ptr<type>());
v_h.set_holder_constructed();
}

/// Do *not* initialize holder from pointer when the underlying object is *not* destructible
template <typename T = type>
detail::enable_if_t<
detail::is_instantiation<std::shared_ptr, holder_type>::value &&
!std::is_destructible<T>::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_type>() + "' holder around a non-destructible `" +
type_id<type>() + "' pointer"
#endif
);
}

/// Initialize holder object, variant 1: object derives from enable_shared_from_this
template <typename T>
static void init_holder(detail::instance *inst, detail::value_and_holder &v_h,
Expand All @@ -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>())) holder_type(v_h.value_ptr<type>());
v_h.set_holder_constructed();
init_shared_holder_from_pointer(v_h);
}
}

Expand All @@ -1334,8 +1366,7 @@ class class_ : public detail::generic_type {
init_holder_from_existing(v_h, holder_ptr, std::is_copy_constructible<holder_type>());
v_h.set_holder_constructed();
} else if (inst->owned || detail::always_construct_holder<holder_type>::value) {
new (std::addressof(v_h.holder<holder_type>())) holder_type(v_h.value_ptr<type>());
v_h.set_holder_constructed();
init_shared_holder_from_pointer(v_h);
}
}

Expand Down
20 changes: 20 additions & 0 deletions tests/test_smart_ptr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,26 @@ TEST_SUBMODULE(smart_ptr, m) {
py::class_<MyObject4b, MyObject4a>(m, "MyObject4b")
.def(py::init<int>());

// Object with non-public destructor in a shared_ptr
class MyObject4c {
public:
MyObject4c() { print_created(this); }
protected:
~MyObject4c() { print_destroyed(this); }
};
py::class_<MyObject4c, std::shared_ptr<MyObject4c>>(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<MyObject4d> {
public:
MyObject4d() { print_created(this); }
protected:
~MyObject4d() { print_destroyed(this); }
};
py::class_<MyObject4d, std::shared_ptr<MyObject4d>>(m, "MyObject4d")
.def(py::init());

// test_large_holder
class MyObject5 { // managed by huge_unique_ptr
public:
Expand Down
14 changes: 14 additions & 0 deletions tests/test_smart_ptr.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down