From 50914824e786d909fd1974a25ff6eca671efb254 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Tue, 1 Sep 2026 23:24:33 -0400 Subject: [PATCH] fix(smart_holder): keep void-cast semantics in from_unique_ptr Fixes item 4 of #6159. --- include/pybind11/detail/init.h | 4 ++-- include/pybind11/detail/struct_smart_holder.h | 18 ++++++++++++--- ...t_class_sh_trampoline_shared_from_this.cpp | 4 ++++ ...st_class_sh_trampoline_shared_from_this.py | 22 +++++++++++++++++++ 4 files changed, 43 insertions(+), 5 deletions(-) diff --git a/include/pybind11/detail/init.h b/include/pybind11/detail/init.h index 5f9e925c3f..cce3eb5607 100644 --- a/include/pybind11/detail/init.h +++ b/include/pybind11/detail/init.h @@ -206,8 +206,8 @@ void construct(value_and_holder &v_h, Alias &&result, bool) { template smart_holder init_smart_holder_from_unique_ptr(std::unique_ptr &&unq_ptr, bool void_cast_raw_ptr) { - void *void_ptr = void_cast_raw_ptr ? static_cast(unq_ptr.get()) : nullptr; - return smart_holder::from_unique_ptr(std::move(unq_ptr), void_ptr); + return smart_holder::from_unique_ptr( + std::move(unq_ptr), /*mi_subobject_ptr*/ nullptr, void_cast_raw_ptr); } template static smart_holder from_unique_ptr(std::unique_ptr &&unq_ptr, - void *mi_subobject_ptr = nullptr) { + void *mi_subobject_ptr = nullptr, + bool void_cast_raw_ptr = false) { smart_holder hld; hld.rtti_uqp_del = &typeid(D); hld.vptr_is_using_std_default_delete = uqp_del_is_std_default_delete(); @@ -344,7 +345,18 @@ struct smart_holder { ? make_guarded_std_default_delete(true) : make_guarded_custom_deleter(std::move(unq_ptr.get_deleter()), true); // Critical: construct owner with pointer we intend to delete - std::shared_ptr owner(unq_ptr.get(), std::move(gd)); + std::shared_ptr owner; + if (void_cast_raw_ptr) { + // A `shared_ptr` would connect the `std::enable_shared_from_this` machinery + // to this control block. That must be avoided if the lifetime of a `PyObject` is + // tied to the pointee (see the `void_cast_raw_ptr` comment near the top of this + // file). Passing a `void *` keeps the control block invisible to + // `shared_from_this()`. + owner = std::shared_ptr(static_cast(unq_ptr.get()), std::move(gd)); + } else { + owner + = std::static_pointer_cast(std::shared_ptr(unq_ptr.get(), std::move(gd))); + } // Relinquish ownership only after successful construction of owner (void) unq_ptr.release(); @@ -366,7 +378,7 @@ struct smart_holder { if (mi_subobject_ptr) { hld.vptr = std::shared_ptr(owner, mi_subobject_ptr); } else { - hld.vptr = std::static_pointer_cast(owner); + hld.vptr = std::move(owner); } hld.is_populated = true; diff --git a/tests/test_class_sh_trampoline_shared_from_this.cpp b/tests/test_class_sh_trampoline_shared_from_this.cpp index dc6bf1c72a..c973685ec8 100644 --- a/tests/test_class_sh_trampoline_shared_from_this.cpp +++ b/tests/test_class_sh_trampoline_shared_from_this.cpp @@ -114,6 +114,10 @@ TEST_SUBMODULE(class_sh_trampoline_shared_from_this, m) { .def(py::init([](const std::string &history, int) { return std::make_shared(history); })) + // The second argument is only used to make this overload unambiguous. + .def(py::init([](const std::string &history, const std::string &) { + return std::unique_ptr(new SftTrampoline(history)); + })) .def_readonly("history", &Sft::history) // This leads to multiple entries in registered_instances: .def(py::init([](const std::shared_ptr &existing) { return existing; })); diff --git a/tests/test_class_sh_trampoline_shared_from_this.py b/tests/test_class_sh_trampoline_shared_from_this.py index c59d0d1dbb..6872fe50b3 100644 --- a/tests/test_class_sh_trampoline_shared_from_this.py +++ b/tests/test_class_sh_trampoline_shared_from_this.py @@ -162,6 +162,28 @@ def test_pure_cpp_sft_raw_ptr(make_f): assert obj.history == "PureCppSft_Stash1AddSharedFromThis" +def test_unique_ptr_factory_and_stash_via_shared_from_this(): + # Exercises that the smart_holder vptr stays invisible to the shared_from_this + # mechanism, also for a trampoline made by a unique_ptr factory. + class PySftUniquePtr(m.Sft): + def __init__(self, history): + super().__init__(history, "unique_ptr") + + obj = PySftUniquePtr("PySftUniquePtr") + assert obj.history == "PySftUniquePtr" + 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 == "PySftUniquePtr_Stash1Add" + assert stash1.use_count(0) == 1 + stash1.AddSharedFromThis(obj) + assert obj.history == "PySftUniquePtr_Stash1Add_Stash1AddSharedFromThis" + assert stash1.use_count(0) == 2 + assert stash1.use_count(1) == 2 + + def test_multiple_registered_instances_for_same_pointee(): obj0 = PySft("PySft") obj0.attachment_in_dict = "Obj0"