diff --git a/include/pybind11/detail/init.h b/include/pybind11/detail/init.h index 0aea1f52ca..faf6e14a52 100644 --- a/include/pybind11/detail/init.h +++ b/include/pybind11/detail/init.h @@ -182,8 +182,13 @@ void construct(value_and_holder &v_h, std::unique_ptr, D> &&unq_ptr, if (Class::has_alias && need_alias && !is_alias(ptr)) throw type_error("pybind11::init(): construction failed: returned std::unique_ptr pointee " "is not an alias instance"); - auto smhldr - = type_caster>::template smart_holder_from_unique_ptr(std::move(unq_ptr)); + // Here and below: if the new object is a trampoline, the shared_from_this mechanism needs + // to be prevented from accessing the smart_holder vptr, because it does not keep the + // trampoline Python object alive. For types that don't inherit from enable_shared_from_this + // it does not matter if void_cast_raw_ptr is true or false, therefore it's not necessary + // to also inspect the type. + auto smhldr = type_caster>::template smart_holder_from_unique_ptr( + std::move(unq_ptr), /*void_cast_raw_ptr*/ Class::has_alias && is_alias(ptr)); v_h.value_ptr() = ptr; v_h.type->init_instance(v_h.inst, &smhldr); } @@ -197,8 +202,8 @@ void construct(value_and_holder &v_h, bool /*need_alias*/) { auto *ptr = unq_ptr.get(); no_nullptr(ptr); - auto smhldr - = type_caster>::template smart_holder_from_unique_ptr(std::move(unq_ptr)); + auto smhldr = type_caster>::template smart_holder_from_unique_ptr( + std::move(unq_ptr), /*void_cast_raw_ptr*/ true); v_h.value_ptr() = ptr; v_h.type->init_instance(v_h.inst, &smhldr); } diff --git a/include/pybind11/detail/smart_holder_poc.h b/include/pybind11/detail/smart_holder_poc.h index 36c873dce6..953852e27b 100644 --- a/include/pybind11/detail/smart_holder_poc.h +++ b/include/pybind11/detail/smart_holder_poc.h @@ -40,6 +40,10 @@ High-level aspects: * By choice, the smart_holder is movable but not copyable, to keep the design simple, and to guard against accidental copying overhead. + +* The `void_cast_raw_ptr` option is needed to make the `smart_holder` `vptr` + member invisible to the `shared_from_this` mechanism, in case the lifetime + of a `PyObject` is tied to the pointee. */ #pragma once @@ -56,7 +60,15 @@ High-level aspects: namespace pybindit { namespace memory { +static constexpr bool type_has_shared_from_this(...) { return false; } + +template +static constexpr bool type_has_shared_from_this(const std::enable_shared_from_this *) { + return true; +} + struct guarded_delete { + std::weak_ptr released_ptr; // Trick to keep the smart_holder memory footprint small. void (*del_ptr)(void *); bool armed_flag; guarded_delete(void (*del_ptr)(void *), bool armed_flag) @@ -69,7 +81,7 @@ struct guarded_delete { template ::value, int>::type = 0> inline void builtin_delete_if_destructible(void *raw_ptr) { - delete (T *) raw_ptr; + delete static_cast(raw_ptr); } template ::value, int>::type = 0> @@ -87,7 +99,7 @@ guarded_delete make_guarded_builtin_delete(bool armed_flag) { template inline void custom_delete(void *raw_ptr) { - D()((T *) raw_ptr); + D()(static_cast(raw_ptr)); } template @@ -203,8 +215,7 @@ struct smart_holder { vptr_del_ptr->armed_flag = armed_flag; } - template - static smart_holder from_raw_ptr_unowned(T *raw_ptr) { + static smart_holder from_raw_ptr_unowned(void *raw_ptr) { smart_holder hld; hld.vptr.reset(raw_ptr, [](void *) {}); hld.vptr_is_using_noop_deleter = true; @@ -234,10 +245,14 @@ struct smart_holder { } template - static smart_holder from_raw_ptr_take_ownership(T *raw_ptr) { + static smart_holder from_raw_ptr_take_ownership(T *raw_ptr, bool void_cast_raw_ptr = false) { ensure_pointee_is_destructible("from_raw_ptr_take_ownership"); smart_holder hld; - hld.vptr.reset(raw_ptr, make_guarded_builtin_delete(true)); + auto gd = make_guarded_builtin_delete(true); + if (void_cast_raw_ptr) + hld.vptr.reset(static_cast(raw_ptr), std::move(gd)); + else + hld.vptr.reset(raw_ptr, std::move(gd)); hld.vptr_is_using_builtin_delete = true; hld.is_populated = true; return hld; @@ -280,15 +295,20 @@ struct smart_holder { } template - static smart_holder from_unique_ptr(std::unique_ptr &&unq_ptr) { + static smart_holder from_unique_ptr(std::unique_ptr &&unq_ptr, + bool void_cast_raw_ptr = false) { smart_holder hld; hld.rtti_uqp_del = &typeid(D); hld.vptr_is_using_builtin_delete = is_std_default_delete(*hld.rtti_uqp_del); - if (hld.vptr_is_using_builtin_delete) { - hld.vptr.reset(unq_ptr.get(), make_guarded_builtin_delete(true)); - } else { - hld.vptr.reset(unq_ptr.get(), make_guarded_custom_deleter(true)); - } + guarded_delete gd{nullptr, false}; + if (hld.vptr_is_using_builtin_delete) + gd = make_guarded_builtin_delete(true); + else + gd = make_guarded_custom_deleter(true); + if (void_cast_raw_ptr) + hld.vptr.reset(static_cast(unq_ptr.get()), std::move(gd)); + else + hld.vptr.reset(unq_ptr.get(), std::move(gd)); unq_ptr.release(); hld.is_populated = true; return hld; diff --git a/include/pybind11/detail/smart_holder_type_casters.h b/include/pybind11/detail/smart_holder_type_casters.h index b61d0a9999..797c152cea 100644 --- a/include/pybind11/detail/smart_holder_type_casters.h +++ b/include/pybind11/detail/smart_holder_type_casters.h @@ -270,6 +270,32 @@ struct smart_holder_type_caster_class_hooks : smart_holder_type_caster_base_tag return &modified_type_caster_generic_load_impl::local_load; } + using holder_type = pybindit::memory::smart_holder; + + template + static bool try_initialization_using_shared_from_this(holder_type *, WrappedType *, ...) { + return false; + } + + // Adopting existing approach used by type_caster_base, although it leads to somewhat fuzzy + // ownership semantics: if we deteced via shared_from_this that a shared_ptr exists already, it + // is reused, irrespective of the return_value_policy in effect. + // "SomeBaseOfWrappedType" is needed because std::enable_shared_from_this is not necessarily a + // direct base of WrappedType. + template + static bool try_initialization_using_shared_from_this( + holder_type *uninitialized_location, + WrappedType *value_ptr_w_t, + const std::enable_shared_from_this *) { + auto shd_ptr = std::dynamic_pointer_cast( + detail::try_get_shared_from_this(value_ptr_w_t)); + if (!shd_ptr) + return false; + // Note: inst->owned ignored. + new (uninitialized_location) holder_type(holder_type::from_shared_ptr(shd_ptr)); + return true; + } + template static void init_instance_for_type(detail::instance *inst, const void *holder_const_void_ptr) { // Need for const_cast is a consequence of the type_info::init_instance type: @@ -281,26 +307,34 @@ struct smart_holder_type_caster_class_hooks : smart_holder_type_caster_base_tag register_instance(inst, v_h.value_ptr(), v_h.type); v_h.set_instance_registered(); } - using holder_type = pybindit::memory::smart_holder; + auto uninitialized_location = std::addressof(v_h.holder()); + auto value_ptr_w_t = v_h.value_ptr(); + bool pointee_depends_on_holder_owner + = dynamic_raw_ptr_cast_if_possible(value_ptr_w_t) != nullptr; if (holder_void_ptr) { // Note: inst->owned ignored. auto holder_ptr = static_cast(holder_void_ptr); - new (std::addressof(v_h.holder())) holder_type(std::move(*holder_ptr)); - } else if (inst->owned) { - new (std::addressof(v_h.holder())) holder_type( - holder_type::from_raw_ptr_take_ownership(v_h.value_ptr())); - } else { - new (std::addressof(v_h.holder())) - holder_type(holder_type::from_raw_ptr_unowned(v_h.value_ptr())); + new (uninitialized_location) holder_type(std::move(*holder_ptr)); + } else if (!try_initialization_using_shared_from_this( + uninitialized_location, value_ptr_w_t, value_ptr_w_t)) { + if (inst->owned) { + new (uninitialized_location) holder_type(holder_type::from_raw_ptr_take_ownership( + value_ptr_w_t, /*void_cast_raw_ptr*/ pointee_depends_on_holder_owner)); + } else { + new (uninitialized_location) + holder_type(holder_type::from_raw_ptr_unowned(value_ptr_w_t)); + } } v_h.holder().pointee_depends_on_holder_owner - = dynamic_raw_ptr_cast_if_possible(v_h.value_ptr()) != nullptr; + = pointee_depends_on_holder_owner; v_h.set_holder_constructed(); } template - static smart_holder smart_holder_from_unique_ptr(std::unique_ptr &&unq_ptr) { - return pybindit::memory::smart_holder::from_unique_ptr(std::move(unq_ptr)); + static smart_holder smart_holder_from_unique_ptr(std::unique_ptr &&unq_ptr, + bool void_cast_raw_ptr) { + return pybindit::memory::smart_holder::from_unique_ptr(std::move(unq_ptr), + void_cast_raw_ptr); } template @@ -309,6 +343,18 @@ struct smart_holder_type_caster_class_hooks : smart_holder_type_caster_base_tag } }; +struct shared_ptr_trampoline_self_life_support { + PyObject *self; + explicit shared_ptr_trampoline_self_life_support(instance *inst) + : self{reinterpret_cast(inst)} { + Py_INCREF(self); + } + void operator()(void *) { + gil_scoped_acquire gil; + Py_DECREF(self); + } +}; + template struct smart_holder_type_caster_load { using holder_type = pybindit::memory::smart_holder; @@ -342,16 +388,6 @@ struct smart_holder_type_caster_load { return *raw_ptr; } - struct shared_ptr_dec_ref_deleter { - // Note: deleter destructor fails on MSVC 2015 and GCC 4.8, so we manually call dec_ref - // here instead. - handle ref; - void operator()(void *) { - gil_scoped_acquire gil; - ref.dec_ref(); - } - }; - std::shared_ptr loaded_as_shared_ptr() const { if (load_impl.unowned_void_ptr_from_direct_conversion != nullptr) throw cast_error("Unowned pointer from direct conversion cannot be converted to a" @@ -359,17 +395,49 @@ struct smart_holder_type_caster_load { if (!have_holder()) return nullptr; throw_if_uninitialized_or_disowned_holder(); - holder().ensure_is_not_disowned("loaded_as_shared_ptr"); - auto void_raw_ptr = holder().template as_raw_ptr_unowned(); + holder_type &hld = holder(); + hld.ensure_is_not_disowned("loaded_as_shared_ptr"); + if (hld.vptr_is_using_noop_deleter) { + throw std::runtime_error("Non-owning holder (loaded_as_shared_ptr)."); + } + auto void_raw_ptr = hld.template as_raw_ptr_unowned(); auto type_raw_ptr = convert_type(void_raw_ptr); - if (holder().pointee_depends_on_holder_owner) { - // Tie lifetime of trampoline Python part to C++ part (PR #2839). - return std::shared_ptr( - type_raw_ptr, - shared_ptr_dec_ref_deleter{ - handle((PyObject *) load_impl.loaded_v_h.inst).inc_ref()}); + if (hld.pointee_depends_on_holder_owner) { + auto vptr_gd_ptr = std::get_deleter(hld.vptr); + if (vptr_gd_ptr != nullptr) { + std::shared_ptr released_ptr = vptr_gd_ptr->released_ptr.lock(); + if (released_ptr) + return std::shared_ptr(released_ptr, type_raw_ptr); + std::shared_ptr to_be_released( + type_raw_ptr, + shared_ptr_trampoline_self_life_support(load_impl.loaded_v_h.inst)); + vptr_gd_ptr->released_ptr = to_be_released; + return to_be_released; + } + auto sptsls_ptr = std::get_deleter(hld.vptr); + if (sptsls_ptr != nullptr) { + // This code is reachable only if there are multiple registered_instances for the + // same pointee. + if (reinterpret_cast(load_impl.loaded_v_h.inst) == sptsls_ptr->self) { + pybind11_fail("smart_holder_type_casters loaded_as_shared_ptr failure: " + "load_impl.loaded_v_h.inst == sptsls_ptr->self"); + } + } + if (sptsls_ptr != nullptr + || !pybindit::memory::type_has_shared_from_this(type_raw_ptr)) { + return std::shared_ptr( + type_raw_ptr, + shared_ptr_trampoline_self_life_support(load_impl.loaded_v_h.inst)); + } + if (hld.vptr_is_external_shared_ptr) { + pybind11_fail("smart_holder_type_casters loaded_as_shared_ptr failure: not " + "implemented: trampoline-self-life-support for external shared_ptr " + "to type inheriting from std::enable_shared_from_this."); + } + pybind11_fail("smart_holder_type_casters: loaded_as_shared_ptr failure: internal " + "inconsistency."); } - std::shared_ptr void_shd_ptr = holder().template as_shared_ptr(); + std::shared_ptr void_shd_ptr = hld.template as_shared_ptr(); return std::shared_ptr(void_shd_ptr, type_raw_ptr); } @@ -381,6 +449,7 @@ struct smart_holder_type_caster_load { if (!have_holder()) return nullptr; throw_if_uninitialized_or_disowned_holder(); + throw_if_instance_is_currently_owned_by_shared_ptr(); holder().ensure_is_not_disowned(context); holder().template ensure_compatible_rtti_uqp_del(context); holder().ensure_use_count_1(context); @@ -444,6 +513,14 @@ struct smart_holder_type_caster_load { } } + // have_holder() must be true or this function will fail. + void throw_if_instance_is_currently_owned_by_shared_ptr() const { + auto vptr_gd_ptr = std::get_deleter(holder().vptr); + if (vptr_gd_ptr != nullptr && !vptr_gd_ptr->released_ptr.expired()) { + throw value_error("Python instance is currently owned by a std::shared_ptr."); + } + } + T *convert_type(void *void_ptr) const { if (void_ptr != nullptr && load_impl.loaded_v_h_cpptype != nullptr && !load_impl.reinterpret_cast_deemed_ok && load_impl.implicit_cast != nullptr) { @@ -748,7 +825,8 @@ struct smart_holder_type_caster> : smart_holder_type_caste void *&valueptr = values_and_holders(inst_raw_ptr).begin()->value_ptr(); valueptr = src_raw_void_ptr; - auto smhldr = pybindit::memory::smart_holder::from_unique_ptr(std::move(src)); + auto smhldr = pybindit::memory::smart_holder::from_unique_ptr(std::move(src), + /*void_cast_raw_ptr*/ false); tinfo->init_instance(inst_raw_ptr, static_cast(&smhldr)); if (policy == return_value_policy::reference_internal) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 0c850ac9ee..f7be2075b8 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -109,6 +109,7 @@ set(PYBIND11_TEST_FILES test_class_sh_shared_ptr_copy_move.cpp test_class_sh_trampoline_basic.cpp test_class_sh_trampoline_self_life_support.cpp + test_class_sh_trampoline_shared_from_this.cpp test_class_sh_trampoline_shared_ptr_cpp_arg.cpp test_class_sh_trampoline_unique_ptr.cpp test_class_sh_unique_ptr_member.cpp diff --git a/tests/test_class_sh_trampoline_shared_from_this.cpp b/tests/test_class_sh_trampoline_shared_from_this.cpp new file mode 100644 index 0000000000..d6b0e8eed0 --- /dev/null +++ b/tests/test_class_sh_trampoline_shared_from_this.cpp @@ -0,0 +1,129 @@ +// Copyright (c) 2021 The Pybind Development Team. +// All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +#include "pybind11/smart_holder.h" +#include "pybind11_tests.h" + +#include +#include + +namespace { + +struct Sft : std::enable_shared_from_this { + std::string history; + explicit Sft(const std::string &history_seed) : history{history_seed} {} + virtual ~Sft() = default; + +#if defined(__clang__) + // "Group of 4" begin. + // This group is not meant to be used, but will leave a trace in the + // history in case something goes wrong. + // However, compilers other than clang have a variety of issues. It is not + // worth the trouble covering all platforms. + Sft(const Sft &other) { history = other.history + "_CpCtor"; } + + Sft(Sft &&other) noexcept { history = other.history + "_MvCtor"; } + + Sft &operator=(const Sft &other) { + history = other.history + "_OpEqLv"; + return *this; + } + + Sft &operator=(Sft &&other) noexcept { + history = other.history + "_OpEqRv"; + return *this; + } + // "Group of 4" end. +#endif +}; + +struct SftSharedPtrStash { + int ser_no; + std::vector> stash; + explicit SftSharedPtrStash(int ser_no) : ser_no{ser_no} {} + void Clear() { stash.clear(); } + void Add(const std::shared_ptr &obj) { + if (!obj->history.empty()) { + obj->history += "_Stash" + std::to_string(ser_no) + "Add"; + } + stash.push_back(obj); + } + void AddSharedFromThis(Sft *obj) { + auto sft = obj->shared_from_this(); + if (!sft->history.empty()) { + sft->history += "_Stash" + std::to_string(ser_no) + "AddSharedFromThis"; + } + stash.push_back(sft); + } + std::string history(unsigned i) { + if (i < stash.size()) + return stash[i]->history; + return "OutOfRange"; + } + long use_count(unsigned i) { + if (i < stash.size()) + return stash[i].use_count(); + return -1; + } +}; + +struct SftTrampoline : Sft, py::trampoline_self_life_support { + using Sft::Sft; +}; + +long use_count(const std::shared_ptr &obj) { return obj.use_count(); } + +long pass_shared_ptr(const std::shared_ptr &obj) { + auto sft = obj->shared_from_this(); + if (!sft->history.empty()) { + sft->history += "_PassSharedPtr"; + } + return sft.use_count(); +} + +void pass_unique_ptr(const std::unique_ptr &) {} + +Sft *make_pure_cpp_sft_raw_ptr(const std::string &history_seed) { return new Sft{history_seed}; } + +std::unique_ptr make_pure_cpp_sft_unq_ptr(const std::string &history_seed) { + return std::unique_ptr(new Sft{history_seed}); +} + +std::shared_ptr make_pure_cpp_sft_shd_ptr(const std::string &history_seed) { + return std::make_shared(history_seed); +} + +std::shared_ptr pass_through_shd_ptr(const std::shared_ptr &obj) { return obj; } + +} // namespace + +PYBIND11_SMART_HOLDER_TYPE_CASTERS(Sft) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(SftSharedPtrStash) + +TEST_SUBMODULE(class_sh_trampoline_shared_from_this, m) { + py::classh(m, "Sft") + .def(py::init()) + .def(py::init([](const std::string &history, int) { + return std::make_shared(history); + })) + .def_readonly("history", &Sft::history) + // This leads to multiple entries in registered_instances: + .def(py::init([](const std::shared_ptr &existing) { return existing; })); + + py::classh(m, "SftSharedPtrStash") + .def(py::init()) + .def("Clear", &SftSharedPtrStash::Clear) + .def("Add", &SftSharedPtrStash::Add) + .def("AddSharedFromThis", &SftSharedPtrStash::AddSharedFromThis) + .def("history", &SftSharedPtrStash::history) + .def("use_count", &SftSharedPtrStash::use_count); + + m.def("use_count", use_count); + m.def("pass_shared_ptr", pass_shared_ptr); + m.def("pass_unique_ptr", pass_unique_ptr); + m.def("make_pure_cpp_sft_raw_ptr", make_pure_cpp_sft_raw_ptr); + m.def("make_pure_cpp_sft_unq_ptr", make_pure_cpp_sft_unq_ptr); + m.def("make_pure_cpp_sft_shd_ptr", make_pure_cpp_sft_shd_ptr); + m.def("pass_through_shd_ptr", pass_through_shd_ptr); +} diff --git a/tests/test_class_sh_trampoline_shared_from_this.py b/tests/test_class_sh_trampoline_shared_from_this.py new file mode 100644 index 0000000000..b4a62275b9 --- /dev/null +++ b/tests/test_class_sh_trampoline_shared_from_this.py @@ -0,0 +1,230 @@ +# -*- coding: utf-8 -*- +import pytest + +import env # noqa: F401 + +import weakref + +import pybind11_tests.class_sh_trampoline_shared_from_this as m + + +class PySft(m.Sft): + pass + + +def test_release_and_shared_from_this(): + # Exercises the most direct path from building a shared_from_this-visible + # shared_ptr to calling shared_from_this. + obj = PySft("PySft") + assert obj.history == "PySft" + assert m.use_count(obj) == 1 + assert m.pass_shared_ptr(obj) == 2 + assert obj.history == "PySft_PassSharedPtr" + assert m.use_count(obj) == 1 + assert m.pass_shared_ptr(obj) == 2 + assert obj.history == "PySft_PassSharedPtr_PassSharedPtr" + assert m.use_count(obj) == 1 + + +def test_release_and_shared_from_this_leak(): + obj = PySft("") + while True: + m.pass_shared_ptr(obj) + assert obj.history == "" + assert m.use_count(obj) == 1 + break # Comment out for manual leak checking (use `top` command). + + +def test_release_and_stash(): + # Exercises correct functioning of guarded_delete weak_ptr. + obj = PySft("PySft") + stash1 = m.SftSharedPtrStash(1) + stash1.Add(obj) + exp_hist = "PySft_Stash1Add" + assert obj.history == exp_hist + assert m.use_count(obj) == 2 + assert stash1.history(0) == exp_hist + assert stash1.use_count(0) == 1 + assert m.pass_shared_ptr(obj) == 3 + exp_hist += "_PassSharedPtr" + assert obj.history == exp_hist + assert m.use_count(obj) == 2 + assert stash1.history(0) == exp_hist + assert stash1.use_count(0) == 1 + stash2 = m.SftSharedPtrStash(2) + stash2.Add(obj) + exp_hist += "_Stash2Add" + assert obj.history == exp_hist + assert m.use_count(obj) == 3 + assert stash2.history(0) == exp_hist + assert stash2.use_count(0) == 2 + stash2.Add(obj) + exp_hist += "_Stash2Add" + assert obj.history == exp_hist + assert m.use_count(obj) == 4 + assert stash1.history(0) == exp_hist + assert stash1.use_count(0) == 3 + assert stash2.history(0) == exp_hist + assert stash2.use_count(0) == 3 + assert stash2.history(1) == exp_hist + assert stash2.use_count(1) == 3 + del obj + assert stash2.history(0) == exp_hist + assert stash2.use_count(0) == 3 + assert stash2.history(1) == exp_hist + assert stash2.use_count(1) == 3 + stash2.Clear() + assert stash1.history(0) == exp_hist + assert stash1.use_count(0) == 1 + + +def test_release_and_stash_leak(): + obj = PySft("") + while True: + stash1 = m.SftSharedPtrStash(1) + stash1.Add(obj) + assert obj.history == "" + assert m.use_count(obj) == 2 + assert stash1.use_count(0) == 1 + stash1.Add(obj) + assert obj.history == "" + assert m.use_count(obj) == 3 + assert stash1.use_count(0) == 2 + assert stash1.use_count(1) == 2 + break # Comment out for manual leak checking (use `top` command). + + +def test_release_and_stash_via_shared_from_this(): + # Exercises that the smart_holder vptr is invisible to the shared_from_this mechnism. + obj = PySft("PySft") + stash1 = m.SftSharedPtrStash(1) + with pytest.raises(RuntimeError) as exc_info: + stash1.AddSharedFromThis(obj) + assert str(exc_info.value) == "bad_weak_ptr" + stash1.Add(obj) + assert obj.history == "PySft_Stash1Add" + assert stash1.use_count(0) == 1 + stash1.AddSharedFromThis(obj) + assert obj.history == "PySft_Stash1Add_Stash1AddSharedFromThis" + assert stash1.use_count(0) == 2 + assert stash1.use_count(1) == 2 + + +def test_release_and_stash_via_shared_from_this_leak(): + obj = PySft("") + while True: + stash1 = m.SftSharedPtrStash(1) + with pytest.raises(RuntimeError) as exc_info: + stash1.AddSharedFromThis(obj) + assert str(exc_info.value) == "bad_weak_ptr" + stash1.Add(obj) + assert obj.history == "" + assert stash1.use_count(0) == 1 + stash1.AddSharedFromThis(obj) + assert obj.history == "" + assert stash1.use_count(0) == 2 + assert stash1.use_count(1) == 2 + break # Comment out for manual leak checking (use `top` command). + + +def test_pass_released_shared_ptr_as_unique_ptr(): + # Exercises that returning a unique_ptr fails while a shared_from_this + # visible shared_ptr exists. + obj = PySft("PySft") + stash1 = m.SftSharedPtrStash(1) + stash1.Add(obj) # Releases shared_ptr to C++. + with pytest.raises(ValueError) as exc_info: + m.pass_unique_ptr(obj) + assert str(exc_info.value) == ( + "Python instance is currently owned by a std::shared_ptr." + ) + + +@pytest.mark.parametrize( + "make_f", + [ + m.make_pure_cpp_sft_raw_ptr, + m.make_pure_cpp_sft_unq_ptr, + m.make_pure_cpp_sft_shd_ptr, + ], +) +def test_pure_cpp_sft_raw_ptr(make_f): + # Exercises void_cast_raw_ptr logic for different situations. + obj = make_f("PureCppSft") + assert m.pass_shared_ptr(obj) == 3 + assert obj.history == "PureCppSft_PassSharedPtr" + obj = make_f("PureCppSft") + stash1 = m.SftSharedPtrStash(1) + stash1.AddSharedFromThis(obj) + assert obj.history == "PureCppSft_Stash1AddSharedFromThis" + + +def test_multiple_registered_instances_for_same_pointee(): + obj0 = PySft("PySft") + obj0.attachment_in_dict = "Obj0" + assert m.pass_through_shd_ptr(obj0) is obj0 + while True: + obj = m.Sft(obj0) + assert obj is not obj0 + obj_pt = m.pass_through_shd_ptr(obj) + # Unpredictable! Because registered_instances is as std::unordered_multimap. + assert obj_pt is obj0 or obj_pt is obj + # Multiple registered_instances for the same pointee can lead to unpredictable results: + if obj_pt is obj0: + assert obj_pt.attachment_in_dict == "Obj0" + else: + assert not hasattr(obj_pt, "attachment_in_dict") + assert obj0.history == "PySft" + break # Comment out for manual leak checking (use `top` command). + + +def test_multiple_registered_instances_for_same_pointee_leak(): + obj0 = PySft("") + while True: + stash1 = m.SftSharedPtrStash(1) + stash1.Add(m.Sft(obj0)) + assert stash1.use_count(0) == 1 + stash1.Add(m.Sft(obj0)) + assert stash1.use_count(0) == 1 + assert stash1.use_count(1) == 1 + assert obj0.history == "" + break # Comment out for manual leak checking (use `top` command). + + +def test_multiple_registered_instances_for_same_pointee_recursive(): + while True: + obj0 = PySft("PySft") + if not env.PYPY: + obj0_wr = weakref.ref(obj0) + obj = obj0 + # This loop creates a chain of instances linked by shared_ptrs. + for _ in range(10): + obj_next = m.Sft(obj) + assert obj_next is not obj + obj = obj_next + del obj_next + assert obj.history == "PySft" + del obj0 + if not env.PYPY: + assert obj0_wr() is not None + del obj # This releases the chain recursively. + if not env.PYPY: + assert obj0_wr() is None + break # Comment out for manual leak checking (use `top` command). + + +def test_std_make_shared_factory(): + class PySftMakeShared(m.Sft): + def __init__(self, history): + super(PySftMakeShared, self).__init__(history, 0) + + obj = PySftMakeShared("PySftMakeShared") + assert obj.history == "PySftMakeShared" + with pytest.raises(RuntimeError) as exc_info: + m.pass_through_shd_ptr(obj) + assert ( + str(exc_info.value) + == "smart_holder_type_casters loaded_as_shared_ptr failure: not implemented:" + " trampoline-self-life-support for external shared_ptr to type inheriting" + " from std::enable_shared_from_this." + ) diff --git a/tests/test_class_sh_trampoline_shared_ptr_cpp_arg.cpp b/tests/test_class_sh_trampoline_shared_ptr_cpp_arg.cpp index 41dace8e23..866667a699 100644 --- a/tests/test_class_sh_trampoline_shared_ptr_cpp_arg.cpp +++ b/tests/test_class_sh_trampoline_shared_ptr_cpp_arg.cpp @@ -26,7 +26,10 @@ struct SpBase { virtual ~SpBase() = default; }; +std::shared_ptr pass_through_shd_ptr(const std::shared_ptr &obj) { return obj; } + struct PySpBase : SpBase { + using SpBase::SpBase; bool is_base_used() override { PYBIND11_OVERRIDE(bool, SpBase, is_base_used); } }; @@ -61,9 +64,12 @@ TEST_SUBMODULE(class_sh_trampoline_shared_ptr_cpp_arg, m) { py::classh(m, "SpBase") .def(py::init<>()) + .def(py::init([](int) { return std::make_shared(); })) .def("is_base_used", &SpBase::is_base_used) .def("has_python_instance", &SpBase::has_python_instance); + m.def("pass_through_shd_ptr", pass_through_shd_ptr); + py::classh(m, "SpBaseTester") .def(py::init<>()) .def("get_object", &SpBaseTester::get_object) diff --git a/tests/test_class_sh_trampoline_shared_ptr_cpp_arg.py b/tests/test_class_sh_trampoline_shared_ptr_cpp_arg.py index 09e72172b6..ec72800f34 100644 --- a/tests/test_class_sh_trampoline_shared_ptr_cpp_arg.py +++ b/tests/test_class_sh_trampoline_shared_ptr_cpp_arg.py @@ -130,4 +130,15 @@ def test_infinite(): tester = m.SpBaseTester() while True: tester.set_object(m.SpBase()) - return # Comment out for manual leak checking (use `top` command). + break # Comment out for manual leak checking (use `top` command). + + +def test_std_make_shared_factory(): + class PyChild(m.SpBase): + def __init__(self): + super(PyChild, self).__init__(0) + + obj = PyChild() + while True: + assert m.pass_through_shd_ptr(obj) is obj + break # Comment out for manual leak checking (use `top` command).