From 9cca56fbedb69b218b97cefc3822a5e8f21eb5b7 Mon Sep 17 00:00:00 2001 From: Dustin Spicuzza Date: Sun, 17 May 2026 03:51:06 +0000 Subject: [PATCH 1/3] test: reproduce custom holder shared_ptr cast regression --- tests/test_smart_ptr.cpp | 52 ++++++++++++++++++++++++++++++++++++++++ tests/test_smart_ptr.py | 10 ++++++++ 2 files changed, 62 insertions(+) diff --git a/tests/test_smart_ptr.cpp b/tests/test_smart_ptr.cpp index 3617fa3e85..ba1da574a6 100644 --- a/tests/test_smart_ptr.cpp +++ b/tests/test_smart_ptr.cpp @@ -83,6 +83,26 @@ class const_only_shared_ptr { // for demonstration purpose only, this imitates smart pointer with a const-only pointer }; +template +class shared_ptr_as_custom_holder { + std::shared_ptr ptr_; + +public: + using element_type = T; + + shared_ptr_as_custom_holder() = default; + + explicit shared_ptr_as_custom_holder(T *) { + throw std::runtime_error("invalid shared_ptr_as_custom_holder constructor call"); + } + + explicit shared_ptr_as_custom_holder(std::shared_ptr ptr) : ptr_(std::move(ptr)) {} + + T *get() const { return ptr_.get(); } + + operator std::shared_ptr() const { return ptr_; } +}; + // Custom object with builtin reference counting (see 'object.h' for the implementation) class MyObject1 : public Object { public: @@ -239,6 +259,25 @@ struct SharedFromThisRef { std::shared_ptr shared = std::make_shared(); }; +class PrivateDtorWithCustomHolder { +public: + static std::shared_ptr create(int value) { + return {new PrivateDtorWithCustomHolder(value), + [](PrivateDtorWithCustomHolder *ptr) { delete ptr; }}; + } + + int value = 0; + +private: + explicit PrivateDtorWithCustomHolder(int value_) : value(value_) {} + ~PrivateDtorWithCustomHolder() = default; +}; + +std::shared_ptr &private_dtor_with_custom_holder_singleton() { + static auto singleton = PrivateDtorWithCustomHolder::create(17); + return singleton; +} + // Issue #865: shared_from_this doesn't work with virtual inheritance struct SharedFromThisVBase : std::enable_shared_from_this { SharedFromThisVBase() = default; @@ -341,6 +380,7 @@ struct holder_helper> { // Make pybind aware of the ref-counted wrapper type (s): PYBIND11_DECLARE_HOLDER_TYPE(T, ref, true) PYBIND11_DECLARE_HOLDER_TYPE(T, const_only_shared_ptr, true) +PYBIND11_DECLARE_HOLDER_TYPE(T, shared_ptr_as_custom_holder) // The following is not required anymore for std::shared_ptr, but it should compile without error: PYBIND11_DECLARE_HOLDER_TYPE(T, std::shared_ptr) PYBIND11_DECLARE_HOLDER_TYPE(T, huge_unique_ptr) @@ -501,6 +541,18 @@ TEST_SUBMODULE(smart_ptr, m) { .def(py::init([](const std::string &value) { return MyObject6::createObject(value); })) .def_property_readonly("value", &MyObject6::value); + py::class_>( + m, "PrivateDtorWithCustomHolder") + .def_property("value", + [](const PrivateDtorWithCustomHolder &self) { return self.value; }, + [](PrivateDtorWithCustomHolder &self, int value) { self.value = value; }) + .def_static("get_singleton_holder", []() { + return shared_ptr_as_custom_holder( + private_dtor_with_custom_holder_singleton()); + }); + m.def("get_private_dtor_with_custom_holder_shared_ptr", + []() { return private_dtor_with_custom_holder_singleton(); }); + // test_shared_ptr_and_references using A = SharedPtrRef::A; py::class_>(m, "A"); diff --git a/tests/test_smart_ptr.py b/tests/test_smart_ptr.py index 76ebd8cf20..9c34a1b9ed 100644 --- a/tests/test_smart_ptr.py +++ b/tests/test_smart_ptr.py @@ -368,3 +368,13 @@ def test_move_only_holder_caster_shared_ptr_with_smart_holder_support_enabled(): def test_const_only_holder(): o = m.MyObject6("my_data") assert o.value == "my_data" + + +def test_shared_ptr_cast_for_custom_holder_with_private_dtor(): + holder_obj = m.PrivateDtorWithCustomHolder.get_singleton_holder() + shared_ptr_obj = m.get_private_dtor_with_custom_holder_shared_ptr() + + holder_obj.value = 23 + + assert shared_ptr_obj.value == 23 + assert m.get_private_dtor_with_custom_holder_shared_ptr().value == 23 From 9cb95ea19f7be904aba07b1b702bbb3867332197 Mon Sep 17 00:00:00 2001 From: Dustin Spicuzza Date: Sun, 17 May 2026 04:55:48 +0000 Subject: [PATCH 2/3] fix: support shared_ptr casts for compatible custom holders This restores support for custom holders that are constructible from std::shared_ptr without requiring user code changes or an internals ABI bump. The fix keeps the safety check added in #6008 for incompatible custom holders, but adds an internal path for compatible ones: - py::cast(std::shared_ptr) now recognizes bound types using custom holders - it tunnels shared ownership through an internal thread-local payload - non-smart-holder init_instance() consumes that payload and constructs the bound holder when Holder is constructible from std::shared_ptr This preserves the regression fix for unsafe cases while allowing established patterns such as private-destructor types wrapped by shared_ptr-backed custom holders to keep working. --- include/pybind11/cast.h | 8 ++++ include/pybind11/detail/type_caster_base.h | 43 ++++++++++++++++++++++ include/pybind11/pybind11.h | 30 +++++++++++++++ 3 files changed, 81 insertions(+) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index b7a4c2b0ce..bfbce65222 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1026,6 +1026,14 @@ struct copyable_holder_caster< return type_caster_base::cast_holder(srcs, &src); } + if (tinfo != nullptr && tinfo->holder_enum_v == holder_enum_t::custom_holder) { + auto aliasing_owner = std::shared_ptr(src, const_cast(srcs.result.cppobj)); + detail::init_instance_with_shared_ptr shared_ptr_payload(std::move(aliasing_owner)); + detail::init_instance_with_shared_ptr_guard shared_ptr_payload_guard( + &shared_ptr_payload); + return type_caster_base::cast_holder(srcs, &shared_ptr_payload); + } + if (parent) { return type_caster_generic::cast_non_owning( srcs, return_value_policy::reference_internal, parent); diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index 8fbf700e12..f717af4e79 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -101,6 +101,49 @@ class loader_life_support { } }; +struct init_instance_with_shared_ptr { + std::shared_ptr aliasing_owner; + mutable const init_instance_with_shared_ptr *previous = nullptr; + + explicit init_instance_with_shared_ptr(std::shared_ptr aliasing_owner_) + : aliasing_owner(std::move(aliasing_owner_)) {} +}; + +class init_instance_with_shared_ptr_guard { +private: + static const init_instance_with_shared_ptr *&tls_current_payload() { + static thread_local const init_instance_with_shared_ptr *payload_ptr = nullptr; + return payload_ptr; + } + + const init_instance_with_shared_ptr *payload; + +public: + explicit init_instance_with_shared_ptr_guard(const init_instance_with_shared_ptr *payload_) + : payload(payload_) { + auto ¤t = tls_current_payload(); + payload->previous = current; + current = payload; + } + + ~init_instance_with_shared_ptr_guard() { + auto ¤t = tls_current_payload(); + if (current != payload) { + pybind11_fail("init_instance_with_shared_ptr_guard: internal error"); + } + current = payload->previous; + } + + static const init_instance_with_shared_ptr *lookup(const void *holder_ptr) { + for (auto *current = tls_current_payload(); current != nullptr; current = current->previous) { + if (current == holder_ptr) { + return current; + } + } + return nullptr; + } +}; + // Gets the cache entry for the given type, creating it if necessary. The return value is the pair // returned by emplace, i.e. an iterator for the entry and a bool set to `true` if the entry was // just created. diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 9cc45bdbdc..a464a16b78 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -2768,6 +2768,26 @@ class class_ : public detail::generic_type { holder_type(std::move(*const_cast(holder_ptr))); } + template >::value, int> + = 0> + static bool init_holder_from_shared_ptr( + const detail::value_and_holder &v_h, + const detail::init_instance_with_shared_ptr &shared_ptr_payload) { + auto shared_ptr = std::shared_ptr(shared_ptr_payload.aliasing_owner, + v_h.value_ptr()); + new (std::addressof(v_h.holder())) holder_type(std::move(shared_ptr)); + return true; + } + + template >::value, int> + = 0> + static bool init_holder_from_shared_ptr(const detail::value_and_holder &, + const detail::init_instance_with_shared_ptr &) { + return false; + } + /// Initialize holder object, variant 2: try to construct from existing holder object, if /// possible static void init_holder(detail::instance *inst, @@ -2795,6 +2815,16 @@ class class_ : public detail::generic_type { register_instance(inst, v_h.value_ptr(), v_h.type); v_h.set_instance_registered(); } + if (auto *shared_ptr_payload + = detail::init_instance_with_shared_ptr_guard::lookup(holder_ptr)) { + if (init_holder_from_shared_ptr(v_h, *shared_ptr_payload)) { + v_h.set_holder_constructed(); + return; + } + throw cast_error("Unable to convert std::shared_ptr to Python when the bound type " + "does not use std::shared_ptr or py::smart_holder as its holder " + "type"); + } init_holder(inst, v_h, (const holder_type *) holder_ptr, v_h.value_ptr()); } From 8b38ee22a2765c54e4a38a59a6662cbab442047d Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sun, 17 May 2026 05:03:29 +0000 Subject: [PATCH 3/3] style: pre-commit fixes --- include/pybind11/cast.h | 3 ++- include/pybind11/detail/type_caster_base.h | 3 ++- include/pybind11/pybind11.h | 13 ++++++------- tests/test_smart_ptr.cpp | 10 ++++++---- 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index bfbce65222..c6012325b2 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1027,7 +1027,8 @@ struct copyable_holder_caster< } if (tinfo != nullptr && tinfo->holder_enum_v == holder_enum_t::custom_holder) { - auto aliasing_owner = std::shared_ptr(src, const_cast(srcs.result.cppobj)); + auto aliasing_owner + = std::shared_ptr(src, const_cast(srcs.result.cppobj)); detail::init_instance_with_shared_ptr shared_ptr_payload(std::move(aliasing_owner)); detail::init_instance_with_shared_ptr_guard shared_ptr_payload_guard( &shared_ptr_payload); diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index f717af4e79..24d5f4df17 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -135,7 +135,8 @@ class init_instance_with_shared_ptr_guard { } static const init_instance_with_shared_ptr *lookup(const void *holder_ptr) { - for (auto *current = tls_current_payload(); current != nullptr; current = current->previous) { + for (auto *current = tls_current_payload(); current != nullptr; + current = current->previous) { if (current == holder_ptr) { return current; } diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index a464a16b78..a1508d1c84 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -2769,13 +2769,12 @@ class class_ : public detail::generic_type { } template >::value, int> - = 0> - static bool init_holder_from_shared_ptr( - const detail::value_and_holder &v_h, - const detail::init_instance_with_shared_ptr &shared_ptr_payload) { - auto shared_ptr = std::shared_ptr(shared_ptr_payload.aliasing_owner, - v_h.value_ptr()); + detail::enable_if_t>::value, int> = 0> + static bool + init_holder_from_shared_ptr(const detail::value_and_holder &v_h, + const detail::init_instance_with_shared_ptr &shared_ptr_payload) { + auto shared_ptr + = std::shared_ptr(shared_ptr_payload.aliasing_owner, v_h.value_ptr()); new (std::addressof(v_h.holder())) holder_type(std::move(shared_ptr)); return true; } diff --git a/tests/test_smart_ptr.cpp b/tests/test_smart_ptr.cpp index ba1da574a6..68110f77fe 100644 --- a/tests/test_smart_ptr.cpp +++ b/tests/test_smart_ptr.cpp @@ -541,11 +541,13 @@ TEST_SUBMODULE(smart_ptr, m) { .def(py::init([](const std::string &value) { return MyObject6::createObject(value); })) .def_property_readonly("value", &MyObject6::value); - py::class_>( + py::class_>( m, "PrivateDtorWithCustomHolder") - .def_property("value", - [](const PrivateDtorWithCustomHolder &self) { return self.value; }, - [](PrivateDtorWithCustomHolder &self, int value) { self.value = value; }) + .def_property( + "value", + [](const PrivateDtorWithCustomHolder &self) { return self.value; }, + [](PrivateDtorWithCustomHolder &self, int value) { self.value = value; }) .def_static("get_singleton_holder", []() { return shared_ptr_as_custom_holder( private_dtor_with_custom_holder_singleton());