From 055310d60f34db5b2907500ef502691ebc9619d5 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sat, 14 Jun 2025 08:52:30 -0700 Subject: [PATCH 01/14] Revert PR #5700 production code change (pybind11/detail/struct_smart_holder.h). ``` git checkout b19489145b2c7a117138632d624809dfb3b380bb~1 include/pybind11/detail/struct_smart_holder.h ``` --- include/pybind11/detail/struct_smart_holder.h | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/include/pybind11/detail/struct_smart_holder.h b/include/pybind11/detail/struct_smart_holder.h index 9b2da87837..758182119c 100644 --- a/include/pybind11/detail/struct_smart_holder.h +++ b/include/pybind11/detail/struct_smart_holder.h @@ -58,19 +58,6 @@ High-level aspects: #include #include -// IMPORTANT: This code block must stay BELOW the #include above. -// This is only required on some builds with libc++ (one of three implementations -// in -// https://github.com/llvm/llvm-project/blob/a9b64bb3180dab6d28bf800a641f9a9ad54d2c0c/libcxx/include/typeinfo#L271-L276 -// require it) -#if !defined(PYBIND11_EXPORT_GUARDED_DELETE) -# if defined(_LIBCPP_VERSION) && !defined(WIN32) && !defined(_WIN32) -# define PYBIND11_EXPORT_GUARDED_DELETE __attribute__((visibility("default"))) -# else -# define PYBIND11_EXPORT_GUARDED_DELETE -# endif -#endif - PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) PYBIND11_NAMESPACE_BEGIN(memory) @@ -91,7 +78,7 @@ static constexpr bool type_has_shared_from_this(const void *) { return false; } -struct PYBIND11_EXPORT_GUARDED_DELETE guarded_delete { +struct guarded_delete { std::weak_ptr released_ptr; // Trick to keep the smart_holder memory footprint small. std::function del_fun; // Rare case. void (*del_ptr)(void *); // Common case. From 14f60a0fab4363b0ea5fee45c6d1894ffc558c8e Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sat, 14 Jun 2025 10:41:15 -0700 Subject: [PATCH 02/14] Introduce `get_internals().get_memory_guarded_delete()` --- include/pybind11/detail/internals.h | 15 ++++++++++++--- include/pybind11/detail/struct_smart_holder.h | 18 ++++++++---------- include/pybind11/detail/type_caster_base.h | 17 ++++++++++------- tests/pure_cpp/smart_holder_poc.h | 4 ++-- tests/pure_cpp/smart_holder_poc_test.cpp | 12 +++++++----- 5 files changed, 39 insertions(+), 27 deletions(-) diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index 972682e5eb..79cf9b5d5a 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -10,6 +10,7 @@ #pragma once #include +#include #include #include @@ -17,6 +18,7 @@ #include #include +#include #include #include @@ -35,11 +37,11 @@ /// further ABI-incompatible changes may be made before the ABI is officially /// changed to the new version. #ifndef PYBIND11_INTERNALS_VERSION -# define PYBIND11_INTERNALS_VERSION 10 +# define PYBIND11_INTERNALS_VERSION 11 #endif -#if PYBIND11_INTERNALS_VERSION < 10 -# error "PYBIND11_INTERNALS_VERSION 10 is the minimum for all platforms for pybind11v3." +#if PYBIND11_INTERNALS_VERSION < 11 +# error "PYBIND11_INTERNALS_VERSION 11 is the minimum for all platforms for pybind11v3." #endif PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) @@ -254,6 +256,10 @@ struct internals { type_map native_enum_type_map; + using get_memory_guarded_delete_fn + = memory::guarded_delete *(*) (const std::shared_ptr &); + get_memory_guarded_delete_fn get_memory_guarded_delete = nullptr; + internals() : static_property_type(make_static_property_type()), default_metaclass(make_default_metaclass()) { @@ -272,6 +278,9 @@ struct internals { instance_shards.reset(new instance_map_shard[num_shards]); instance_shards_mask = num_shards - 1; #endif + get_memory_guarded_delete = [](const std::shared_ptr &ptr) { + return std::get_deleter(ptr); + }; } internals(const internals &other) = delete; internals(internals &&other) = delete; diff --git a/include/pybind11/detail/struct_smart_holder.h b/include/pybind11/detail/struct_smart_holder.h index 758182119c..b99161b391 100644 --- a/include/pybind11/detail/struct_smart_holder.h +++ b/include/pybind11/detail/struct_smart_holder.h @@ -231,8 +231,7 @@ struct smart_holder { } } - void reset_vptr_deleter_armed_flag(bool armed_flag) const { - auto *vptr_del_ptr = std::get_deleter(vptr); + void reset_vptr_deleter_armed_flag(guarded_delete *vptr_del_ptr, bool armed_flag) const { if (vptr_del_ptr == nullptr) { throw std::runtime_error( "smart_holder::reset_vptr_deleter_armed_flag() called in an invalid context."); @@ -242,8 +241,7 @@ struct smart_holder { // Caller is responsible for precondition: ensure_compatible_rtti_uqp_del() must succeed. template - std::unique_ptr extract_deleter(const char *context) const { - const auto *gd = std::get_deleter(vptr); + std::unique_ptr extract_deleter(const char *context, const guarded_delete *gd) const { if (gd && gd->use_del_fun) { const auto &custom_deleter_ptr = gd->del_fun.template target>(); if (custom_deleter_ptr == nullptr) { @@ -288,15 +286,15 @@ struct smart_holder { // Caller is responsible for ensuring the complex preconditions // (see `smart_holder_type_caster_support::load_helper`). - void disown() { - reset_vptr_deleter_armed_flag(false); + void disown(guarded_delete *vptr_del_ptr) { + reset_vptr_deleter_armed_flag(vptr_del_ptr, false); is_disowned = true; } // Caller is responsible for ensuring the complex preconditions // (see `smart_holder_type_caster_support::load_helper`). - void reclaim_disowned() { - reset_vptr_deleter_armed_flag(true); + void reclaim_disowned(guarded_delete *vptr_del_ptr) { + reset_vptr_deleter_armed_flag(vptr_del_ptr, true); is_disowned = false; } @@ -312,8 +310,8 @@ struct smart_holder { // Caller is responsible for ensuring the complex preconditions // (see `smart_holder_type_caster_support::load_helper`). - void release_ownership() { - reset_vptr_deleter_armed_flag(false); + void release_ownership(guarded_delete *vptr_del_ptr) { + reset_vptr_deleter_armed_flag(vptr_del_ptr, false); release_disowned(); } diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index aa8f2cf7e7..65300618d7 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -532,7 +532,7 @@ struct value_and_holder_helper { // 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); + auto *vptr_gd_ptr = get_internals().get_memory_guarded_delete(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."); } @@ -576,7 +576,7 @@ handle smart_holder_from_unique_ptr(std::unique_ptr &&src, } // Critical transfer-of-ownership section. This must stay together. self_life_support->deactivate_life_support(); - holder.reclaim_disowned(); + holder.reclaim_disowned(get_internals().get_memory_guarded_delete(holder.vptr)); (void) src.release(); // Critical section end. return existing_inst; @@ -763,7 +763,7 @@ struct load_helper : value_and_holder_helper { } auto *type_raw_ptr = static_cast(void_raw_ptr); if (python_instance_is_alias && !force_potentially_slicing_shared_ptr) { - auto *vptr_gd_ptr = std::get_deleter(hld.vptr); + auto *vptr_gd_ptr = get_internals().get_memory_guarded_delete(holder().vptr); if (vptr_gd_ptr != nullptr) { std::shared_ptr released_ptr = vptr_gd_ptr->released_ptr.lock(); if (released_ptr) { @@ -818,13 +818,14 @@ struct load_helper : value_and_holder_helper { // This is enforced indirectly by a static_assert in the class_ implementation: assert(!python_instance_is_alias || self_life_support); - std::unique_ptr extracted_deleter = holder().template extract_deleter(context); + std::unique_ptr extracted_deleter = holder().template extract_deleter( + context, get_internals().get_memory_guarded_delete(holder().vptr)); // Critical transfer-of-ownership section. This must stay together. if (self_life_support != nullptr) { - holder().disown(); + holder().disown(get_internals().get_memory_guarded_delete(holder().vptr)); } else { - holder().release_ownership(); + holder().release_ownership(get_internals().get_memory_guarded_delete(holder().vptr)); } auto result = unique_with_deleter(raw_type_ptr, std::move(extracted_deleter)); if (self_life_support != nullptr) { @@ -849,7 +850,9 @@ struct load_helper : value_and_holder_helper { } holder().template ensure_compatible_rtti_uqp_del(context); return unique_with_deleter( - raw_type_ptr, std::move(holder().template extract_deleter(context))); + raw_type_ptr, + std::move(holder().template extract_deleter( + context, get_internals().get_memory_guarded_delete(holder().vptr)))); } }; diff --git a/tests/pure_cpp/smart_holder_poc.h b/tests/pure_cpp/smart_holder_poc.h index 37160f1e64..3a9d20ded0 100644 --- a/tests/pure_cpp/smart_holder_poc.h +++ b/tests/pure_cpp/smart_holder_poc.h @@ -36,7 +36,7 @@ T *as_raw_ptr_release_ownership(smart_holder &hld, const char *context = "as_raw_ptr_release_ownership") { hld.ensure_can_release_ownership(context); T *raw_ptr = hld.as_raw_ptr_unowned(); - hld.release_ownership(); + hld.release_ownership(std::get_deleter(hld.vptr)); return raw_ptr; } @@ -46,7 +46,7 @@ std::unique_ptr as_unique_ptr(smart_holder &hld) { hld.ensure_compatible_rtti_uqp_del(context); hld.ensure_use_count_1(context); T *raw_ptr = hld.as_raw_ptr_unowned(); - hld.release_ownership(); + hld.release_ownership(std::get_deleter(hld.vptr)); // KNOWN DEFECT (see PR #4850): Does not copy the deleter. return std::unique_ptr(raw_ptr); } diff --git a/tests/pure_cpp/smart_holder_poc_test.cpp b/tests/pure_cpp/smart_holder_poc_test.cpp index 55018e65b2..f7baa36190 100644 --- a/tests/pure_cpp/smart_holder_poc_test.cpp +++ b/tests/pure_cpp/smart_holder_poc_test.cpp @@ -14,6 +14,7 @@ #define CATCH_CONFIG_MAIN #include "catch.hpp" +using pybind11::memory::guarded_delete; using pybind11::memory::smart_holder; namespace poc = pybind11::memory::smart_holder_poc; @@ -160,11 +161,12 @@ TEST_CASE("from_raw_ptr_take_ownership+as_shared_ptr", "[S]") { TEST_CASE("from_raw_ptr_take_ownership+disown+reclaim_disowned", "[S]") { auto hld = smart_holder::from_raw_ptr_take_ownership(new int(19)); std::unique_ptr new_owner(hld.as_raw_ptr_unowned()); - hld.disown(); + hld.disown(std::get_deleter(hld.vptr)); REQUIRE(poc::as_lvalue_ref(hld) == 19); REQUIRE(*new_owner == 19); - hld.reclaim_disowned(); // Manually veriified: without this, clang++ -fsanitize=address reports - // "detected memory leaks". + // Manually verified: without this, clang++ -fsanitize=address reports + // "detected memory leaks". + hld.reclaim_disowned(std::get_deleter(hld.vptr)); // NOLINTNEXTLINE(bugprone-unused-return-value) (void) new_owner.release(); // Manually verified: without this, clang++ -fsanitize=address // reports "attempting double-free". @@ -175,7 +177,7 @@ TEST_CASE("from_raw_ptr_take_ownership+disown+reclaim_disowned", "[S]") { TEST_CASE("from_raw_ptr_take_ownership+disown+release_disowned", "[S]") { auto hld = smart_holder::from_raw_ptr_take_ownership(new int(19)); std::unique_ptr new_owner(hld.as_raw_ptr_unowned()); - hld.disown(); + hld.disown(std::get_deleter(hld.vptr)); REQUIRE(poc::as_lvalue_ref(hld) == 19); REQUIRE(*new_owner == 19); hld.release_disowned(); @@ -187,7 +189,7 @@ TEST_CASE("from_raw_ptr_take_ownership+disown+ensure_is_not_disowned", "[E]") { auto hld = smart_holder::from_raw_ptr_take_ownership(new int(19)); hld.ensure_is_not_disowned(context); // Does not throw. std::unique_ptr new_owner(hld.as_raw_ptr_unowned()); - hld.disown(); + hld.disown(std::get_deleter(hld.vptr)); REQUIRE_THROWS_WITH(hld.ensure_is_not_disowned(context), "Holder was disowned already (test_case)."); } From fb836cb464a17f0efa6fb5fceaae00a3618819af Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sun, 15 Jun 2025 13:20:31 -0700 Subject: [PATCH 03/14] [skip ci] Only pass around `memory::get_guarded_delete` function pointer. --- include/pybind11/detail/internals.h | 8 +----- include/pybind11/detail/struct_smart_holder.h | 25 +++++++++++++------ include/pybind11/detail/type_caster_base.h | 15 ++++++----- tests/pure_cpp/smart_holder_poc.h | 4 +-- tests/pure_cpp/smart_holder_poc_test.cpp | 8 +++--- 5 files changed, 31 insertions(+), 29 deletions(-) diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index 79cf9b5d5a..a6d2702648 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -18,7 +18,6 @@ #include #include -#include #include #include @@ -256,9 +255,7 @@ struct internals { type_map native_enum_type_map; - using get_memory_guarded_delete_fn - = memory::guarded_delete *(*) (const std::shared_ptr &); - get_memory_guarded_delete_fn get_memory_guarded_delete = nullptr; + memory::get_guarded_delete_fn get_memory_guarded_delete = memory::get_guarded_delete; internals() : static_property_type(make_static_property_type()), @@ -278,9 +275,6 @@ struct internals { instance_shards.reset(new instance_map_shard[num_shards]); instance_shards_mask = num_shards - 1; #endif - get_memory_guarded_delete = [](const std::shared_ptr &ptr) { - return std::get_deleter(ptr); - }; } internals(const internals &other) = delete; internals(internals &&other) = delete; diff --git a/include/pybind11/detail/struct_smart_holder.h b/include/pybind11/detail/struct_smart_holder.h index b99161b391..b7b6f11fa9 100644 --- a/include/pybind11/detail/struct_smart_holder.h +++ b/include/pybind11/detail/struct_smart_holder.h @@ -100,6 +100,12 @@ struct guarded_delete { } }; +inline guarded_delete *get_guarded_delete(const std::shared_ptr &ptr) { + return std::get_deleter(ptr); +} + +using get_guarded_delete_fn = guarded_delete *(*) (const std::shared_ptr &); + template ::value, int>::type = 0> inline void builtin_delete_if_destructible(void *raw_ptr) { std::default_delete{}(static_cast(raw_ptr)); @@ -231,7 +237,8 @@ struct smart_holder { } } - void reset_vptr_deleter_armed_flag(guarded_delete *vptr_del_ptr, bool armed_flag) const { + void reset_vptr_deleter_armed_flag(const get_guarded_delete_fn ggd_fn, bool armed_flag) const { + auto *vptr_del_ptr = ggd_fn(vptr); if (vptr_del_ptr == nullptr) { throw std::runtime_error( "smart_holder::reset_vptr_deleter_armed_flag() called in an invalid context."); @@ -241,7 +248,9 @@ struct smart_holder { // Caller is responsible for precondition: ensure_compatible_rtti_uqp_del() must succeed. template - std::unique_ptr extract_deleter(const char *context, const guarded_delete *gd) const { + std::unique_ptr extract_deleter(const char *context, + const get_guarded_delete_fn ggd_fn) const { + auto *gd = ggd_fn(vptr); if (gd && gd->use_del_fun) { const auto &custom_deleter_ptr = gd->del_fun.template target>(); if (custom_deleter_ptr == nullptr) { @@ -286,15 +295,15 @@ struct smart_holder { // Caller is responsible for ensuring the complex preconditions // (see `smart_holder_type_caster_support::load_helper`). - void disown(guarded_delete *vptr_del_ptr) { - reset_vptr_deleter_armed_flag(vptr_del_ptr, false); + void disown(const get_guarded_delete_fn ggd_fn) { + reset_vptr_deleter_armed_flag(ggd_fn, false); is_disowned = true; } // Caller is responsible for ensuring the complex preconditions // (see `smart_holder_type_caster_support::load_helper`). - void reclaim_disowned(guarded_delete *vptr_del_ptr) { - reset_vptr_deleter_armed_flag(vptr_del_ptr, true); + void reclaim_disowned(const get_guarded_delete_fn ggd_fn) { + reset_vptr_deleter_armed_flag(ggd_fn, true); is_disowned = false; } @@ -310,8 +319,8 @@ struct smart_holder { // Caller is responsible for ensuring the complex preconditions // (see `smart_holder_type_caster_support::load_helper`). - void release_ownership(guarded_delete *vptr_del_ptr) { - reset_vptr_deleter_armed_flag(vptr_del_ptr, false); + void release_ownership(const get_guarded_delete_fn ggd_fn) { + reset_vptr_deleter_armed_flag(ggd_fn, false); release_disowned(); } diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index 65300618d7..ba44ccc9d7 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -576,7 +576,7 @@ handle smart_holder_from_unique_ptr(std::unique_ptr &&src, } // Critical transfer-of-ownership section. This must stay together. self_life_support->deactivate_life_support(); - holder.reclaim_disowned(get_internals().get_memory_guarded_delete(holder.vptr)); + holder.reclaim_disowned(get_internals().get_memory_guarded_delete); (void) src.release(); // Critical section end. return existing_inst; @@ -819,13 +819,13 @@ struct load_helper : value_and_holder_helper { assert(!python_instance_is_alias || self_life_support); std::unique_ptr extracted_deleter = holder().template extract_deleter( - context, get_internals().get_memory_guarded_delete(holder().vptr)); + context, get_internals().get_memory_guarded_delete); // Critical transfer-of-ownership section. This must stay together. if (self_life_support != nullptr) { - holder().disown(get_internals().get_memory_guarded_delete(holder().vptr)); + holder().disown(get_internals().get_memory_guarded_delete); } else { - holder().release_ownership(get_internals().get_memory_guarded_delete(holder().vptr)); + holder().release_ownership(get_internals().get_memory_guarded_delete); } auto result = unique_with_deleter(raw_type_ptr, std::move(extracted_deleter)); if (self_life_support != nullptr) { @@ -849,10 +849,9 @@ struct load_helper : value_and_holder_helper { return unique_with_deleter(nullptr, std::unique_ptr()); } holder().template ensure_compatible_rtti_uqp_del(context); - return unique_with_deleter( - raw_type_ptr, - std::move(holder().template extract_deleter( - context, get_internals().get_memory_guarded_delete(holder().vptr)))); + return unique_with_deleter(raw_type_ptr, + std::move(holder().template extract_deleter( + context, get_internals().get_memory_guarded_delete))); } }; diff --git a/tests/pure_cpp/smart_holder_poc.h b/tests/pure_cpp/smart_holder_poc.h index 3a9d20ded0..9a456edce6 100644 --- a/tests/pure_cpp/smart_holder_poc.h +++ b/tests/pure_cpp/smart_holder_poc.h @@ -36,7 +36,7 @@ T *as_raw_ptr_release_ownership(smart_holder &hld, const char *context = "as_raw_ptr_release_ownership") { hld.ensure_can_release_ownership(context); T *raw_ptr = hld.as_raw_ptr_unowned(); - hld.release_ownership(std::get_deleter(hld.vptr)); + hld.release_ownership(get_guarded_delete); return raw_ptr; } @@ -46,7 +46,7 @@ std::unique_ptr as_unique_ptr(smart_holder &hld) { hld.ensure_compatible_rtti_uqp_del(context); hld.ensure_use_count_1(context); T *raw_ptr = hld.as_raw_ptr_unowned(); - hld.release_ownership(std::get_deleter(hld.vptr)); + hld.release_ownership(get_guarded_delete); // KNOWN DEFECT (see PR #4850): Does not copy the deleter. return std::unique_ptr(raw_ptr); } diff --git a/tests/pure_cpp/smart_holder_poc_test.cpp b/tests/pure_cpp/smart_holder_poc_test.cpp index f7baa36190..09720432cf 100644 --- a/tests/pure_cpp/smart_holder_poc_test.cpp +++ b/tests/pure_cpp/smart_holder_poc_test.cpp @@ -161,12 +161,12 @@ TEST_CASE("from_raw_ptr_take_ownership+as_shared_ptr", "[S]") { TEST_CASE("from_raw_ptr_take_ownership+disown+reclaim_disowned", "[S]") { auto hld = smart_holder::from_raw_ptr_take_ownership(new int(19)); std::unique_ptr new_owner(hld.as_raw_ptr_unowned()); - hld.disown(std::get_deleter(hld.vptr)); + hld.disown(pybind11::memory::get_guarded_delete); REQUIRE(poc::as_lvalue_ref(hld) == 19); REQUIRE(*new_owner == 19); // Manually verified: without this, clang++ -fsanitize=address reports // "detected memory leaks". - hld.reclaim_disowned(std::get_deleter(hld.vptr)); + hld.reclaim_disowned(pybind11::memory::get_guarded_delete); // NOLINTNEXTLINE(bugprone-unused-return-value) (void) new_owner.release(); // Manually verified: without this, clang++ -fsanitize=address // reports "attempting double-free". @@ -177,7 +177,7 @@ TEST_CASE("from_raw_ptr_take_ownership+disown+reclaim_disowned", "[S]") { TEST_CASE("from_raw_ptr_take_ownership+disown+release_disowned", "[S]") { auto hld = smart_holder::from_raw_ptr_take_ownership(new int(19)); std::unique_ptr new_owner(hld.as_raw_ptr_unowned()); - hld.disown(std::get_deleter(hld.vptr)); + hld.disown(pybind11::memory::get_guarded_delete); REQUIRE(poc::as_lvalue_ref(hld) == 19); REQUIRE(*new_owner == 19); hld.release_disowned(); @@ -189,7 +189,7 @@ TEST_CASE("from_raw_ptr_take_ownership+disown+ensure_is_not_disowned", "[E]") { auto hld = smart_holder::from_raw_ptr_take_ownership(new int(19)); hld.ensure_is_not_disowned(context); // Does not throw. std::unique_ptr new_owner(hld.as_raw_ptr_unowned()); - hld.disown(std::get_deleter(hld.vptr)); + hld.disown(pybind11::memory::get_guarded_delete); REQUIRE_THROWS_WITH(hld.ensure_is_not_disowned(context), "Holder was disowned already (test_case)."); } From 5093a03ec49822b43d610936fddef6990cbed451 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sun, 15 Jun 2025 13:33:13 -0700 Subject: [PATCH 04/14] [skip ci] Change a variable name for internal consistency. Add 3 x NOTE: PYBIND11_INTERNALS_VERSION needs to be bumped if changes are made to this struct. --- include/pybind11/detail/struct_smart_holder.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/include/pybind11/detail/struct_smart_holder.h b/include/pybind11/detail/struct_smart_holder.h index b7b6f11fa9..dd197499cd 100644 --- a/include/pybind11/detail/struct_smart_holder.h +++ b/include/pybind11/detail/struct_smart_holder.h @@ -79,6 +79,7 @@ static constexpr bool type_has_shared_from_this(const void *) { } struct guarded_delete { + // NOTE: PYBIND11_INTERNALS_VERSION needs to be bumped if changes are made to this struct. std::weak_ptr released_ptr; // Trick to keep the smart_holder memory footprint small. std::function del_fun; // Rare case. void (*del_ptr)(void *); // Common case. @@ -126,6 +127,7 @@ guarded_delete make_guarded_builtin_delete(bool armed_flag) { template struct custom_deleter { + // NOTE: PYBIND11_INTERNALS_VERSION needs to be bumped if changes are made to this struct. D deleter; explicit custom_deleter(D &&deleter) : deleter{std::forward(deleter)} {} void operator()(void *raw_ptr) { deleter(static_cast(raw_ptr)); } @@ -144,6 +146,7 @@ inline bool is_std_default_delete(const std::type_info &rtti_deleter) { } struct smart_holder { + // NOTE: PYBIND11_INTERNALS_VERSION needs to be bumped if changes are made to this struct. const std::type_info *rtti_uqp_del = nullptr; std::shared_ptr vptr; bool vptr_is_using_noop_deleter : 1; @@ -238,12 +241,12 @@ struct smart_holder { } void reset_vptr_deleter_armed_flag(const get_guarded_delete_fn ggd_fn, bool armed_flag) const { - auto *vptr_del_ptr = ggd_fn(vptr); - if (vptr_del_ptr == nullptr) { + auto *gd = ggd_fn(vptr); + if (gd == nullptr) { throw std::runtime_error( "smart_holder::reset_vptr_deleter_armed_flag() called in an invalid context."); } - vptr_del_ptr->armed_flag = armed_flag; + gd->armed_flag = armed_flag; } // Caller is responsible for precondition: ensure_compatible_rtti_uqp_del() must succeed. From 60877346b7fb927bb54a372ac202c9ac4dc71c55 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sun, 15 Jun 2025 14:03:12 -0700 Subject: [PATCH 05/14] Add comment: get_internals().get_memory_guarded_delete does not need with_internals() --- include/pybind11/detail/internals.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index a6d2702648..e7174bcc85 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -255,6 +255,8 @@ struct internals { type_map native_enum_type_map; + // Note: get_internals().get_memory_guarded_delete does + // not need to be wrapped in with_internals(). memory::get_guarded_delete_fn get_memory_guarded_delete = memory::get_guarded_delete; internals() From 91bca10de2bd319154adcbf50e781865b1c7c700 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sun, 15 Jun 2025 21:58:35 -0700 Subject: [PATCH 06/14] Traverse all DSOs to find memory::guarded_delete with matching RTTI. --- include/pybind11/detail/internals.h | 18 +++++++++++++++--- include/pybind11/detail/type_caster_base.h | 16 ++++++++-------- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index e7174bcc85..e2ed7888c3 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -255,9 +255,7 @@ struct internals { type_map native_enum_type_map; - // Note: get_internals().get_memory_guarded_delete does - // not need to be wrapped in with_internals(). - memory::get_guarded_delete_fn get_memory_guarded_delete = memory::get_guarded_delete; + std::vector memory_guarded_delete_fns; // See PRs #5728, #5700. internals() : static_property_type(make_static_property_type()), @@ -267,6 +265,7 @@ struct internals { istate = cur_tstate->interp; registered_exception_translators.push_front(&translate_exception); + memory_guarded_delete_fns.push_back(memory::get_guarded_delete); #ifdef Py_GIL_DISABLED // Scale proportional to the number of cores. 2x is a heuristic to reduce contention. auto num_shards @@ -676,6 +675,19 @@ inline auto with_exception_translators(const F &cb) local_internals.registered_exception_translators); } +// Traverse all DSOs to find memory::guarded_delete with matching RTTI. +inline memory::guarded_delete *find_memory_guarded_delete(const std::shared_ptr &ptr) { + return with_internals([&](detail::internals &internals) -> memory::guarded_delete * { + for (auto fn : internals.memory_guarded_delete_fns) { + memory::guarded_delete *gd = fn(ptr); + if (gd) { + return gd; + } + } + return nullptr; + }); +} + inline std::uint64_t mix64(std::uint64_t z) { // David Stafford's variant 13 of the MurmurHash3 finalizer popularized // by the SplitMix PRNG. diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index ba44ccc9d7..cd9ec8dbda 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -532,7 +532,7 @@ struct value_and_holder_helper { // 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 = get_internals().get_memory_guarded_delete(holder().vptr); + auto *vptr_gd_ptr = find_memory_guarded_delete(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."); } @@ -576,7 +576,7 @@ handle smart_holder_from_unique_ptr(std::unique_ptr &&src, } // Critical transfer-of-ownership section. This must stay together. self_life_support->deactivate_life_support(); - holder.reclaim_disowned(get_internals().get_memory_guarded_delete); + holder.reclaim_disowned(find_memory_guarded_delete); (void) src.release(); // Critical section end. return existing_inst; @@ -763,7 +763,7 @@ struct load_helper : value_and_holder_helper { } auto *type_raw_ptr = static_cast(void_raw_ptr); if (python_instance_is_alias && !force_potentially_slicing_shared_ptr) { - auto *vptr_gd_ptr = get_internals().get_memory_guarded_delete(holder().vptr); + auto *vptr_gd_ptr = find_memory_guarded_delete(holder().vptr); if (vptr_gd_ptr != nullptr) { std::shared_ptr released_ptr = vptr_gd_ptr->released_ptr.lock(); if (released_ptr) { @@ -818,14 +818,14 @@ struct load_helper : value_and_holder_helper { // This is enforced indirectly by a static_assert in the class_ implementation: assert(!python_instance_is_alias || self_life_support); - std::unique_ptr extracted_deleter = holder().template extract_deleter( - context, get_internals().get_memory_guarded_delete); + std::unique_ptr extracted_deleter + = holder().template extract_deleter(context, find_memory_guarded_delete); // Critical transfer-of-ownership section. This must stay together. if (self_life_support != nullptr) { - holder().disown(get_internals().get_memory_guarded_delete); + holder().disown(find_memory_guarded_delete); } else { - holder().release_ownership(get_internals().get_memory_guarded_delete); + holder().release_ownership(find_memory_guarded_delete); } auto result = unique_with_deleter(raw_type_ptr, std::move(extracted_deleter)); if (self_life_support != nullptr) { @@ -851,7 +851,7 @@ struct load_helper : value_and_holder_helper { holder().template ensure_compatible_rtti_uqp_del(context); return unique_with_deleter(raw_type_ptr, std::move(holder().template extract_deleter( - context, get_internals().get_memory_guarded_delete))); + context, find_memory_guarded_delete))); } }; From 4e6c0dd5c0b0ecd654bafaa34d4e269234d6df6f Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sun, 15 Jun 2025 23:32:30 -0700 Subject: [PATCH 07/14] Add nullptr check to dynamic_cast overload. Suggested by ChatGPT for these reasons: * Prevents runtime RTTI lookups on nullptr. * Helps avoid undefined behavior if users pass in nulls from failed casts or optional paths. * Ensures consistent return value semantics and no accidental access to vtable. --- include/pybind11/detail/dynamic_raw_ptr_cast_if_possible.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/detail/dynamic_raw_ptr_cast_if_possible.h b/include/pybind11/detail/dynamic_raw_ptr_cast_if_possible.h index 7c00fe98c1..dddf28f0fb 100644 --- a/include/pybind11/detail/dynamic_raw_ptr_cast_if_possible.h +++ b/include/pybind11/detail/dynamic_raw_ptr_cast_if_possible.h @@ -32,7 +32,7 @@ template ::value, int> = 0> To *dynamic_raw_ptr_cast_if_possible(From *ptr) { - return dynamic_cast(ptr); + return ptr ? dynamic_cast(ptr) : nullptr; } PYBIND11_NAMESPACE_END(detail) From 45b9eacad61ec0107d3b7dea514562dc65396b75 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 16 Jun 2025 11:34:45 -0700 Subject: [PATCH 08/14] Improve smart_holder unique_ptr deleter compatibility checks across DSOs: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Replace RTTI-based detection of std::default_delete with a constexpr check to avoid RTTI reliance * Add type_info_equal_across_dso_boundaries() fallback using type_info::name() for RTTI equality across macOS DSOs * Rename related flags and functions for clarity (e.g., builtin → std_default) * Improves ABI robustness and clarity of ownership checks in smart_holder --- include/pybind11/detail/struct_smart_holder.h | 50 +++++++++++-------- include/pybind11/detail/type_caster_base.h | 4 +- tests/pure_cpp/smart_holder_poc.h | 2 +- 3 files changed, 33 insertions(+), 23 deletions(-) diff --git a/include/pybind11/detail/struct_smart_holder.h b/include/pybind11/detail/struct_smart_holder.h index dd197499cd..9aa23867e6 100644 --- a/include/pybind11/detail/struct_smart_holder.h +++ b/include/pybind11/detail/struct_smart_holder.h @@ -50,6 +50,7 @@ High-level aspects: #include "pybind11_namespace_macros.h" +#include #include #include #include @@ -139,10 +140,17 @@ guarded_delete make_guarded_custom_deleter(D &&uqp_del, bool armed_flag) { std::function(custom_deleter(std::forward(uqp_del))), armed_flag); } -template -inline bool is_std_default_delete(const std::type_info &rtti_deleter) { - return rtti_deleter == typeid(std::default_delete) - || rtti_deleter == typeid(std::default_delete); +template +constexpr bool uqp_del_is_std_default_delete() { + return std::is_same>::value + || std::is_same>::value; +} + +inline bool type_info_equal_across_dso_boundaries(const std::type_info &a, + const std::type_info &b) { + // RTTI pointer comparison may fail across DSOs (e.g., macOS libc++). + // Fallback to name comparison, which is generally safe and ABI-stable enough for our use. + return a == b || std::strcmp(a.name(), b.name()) == 0; } struct smart_holder { @@ -150,7 +158,7 @@ struct smart_holder { const std::type_info *rtti_uqp_del = nullptr; std::shared_ptr vptr; bool vptr_is_using_noop_deleter : 1; - bool vptr_is_using_builtin_delete : 1; + bool vptr_is_using_std_default_delete : 1; bool vptr_is_external_shared_ptr : 1; bool is_populated : 1; bool is_disowned : 1; @@ -162,7 +170,7 @@ struct smart_holder { smart_holder &operator=(const smart_holder &) = delete; smart_holder() - : vptr_is_using_noop_deleter{false}, vptr_is_using_builtin_delete{false}, + : vptr_is_using_noop_deleter{false}, vptr_is_using_std_default_delete{false}, vptr_is_external_shared_ptr{false}, is_populated{false}, is_disowned{false} {} bool has_pointee() const { return vptr != nullptr; } @@ -187,7 +195,7 @@ struct smart_holder { } } - void ensure_vptr_is_using_builtin_delete(const char *context) const { + void ensure_vptr_is_using_std_default_delete(const char *context) const { if (vptr_is_external_shared_ptr) { throw std::invalid_argument(std::string("Cannot disown external shared_ptr (") + context + ")."); @@ -196,24 +204,26 @@ struct smart_holder { throw std::invalid_argument(std::string("Cannot disown non-owning holder (") + context + ")."); } - if (!vptr_is_using_builtin_delete) { + if (!vptr_is_using_std_default_delete) { throw std::invalid_argument(std::string("Cannot disown custom deleter (") + context + ")."); } } template - void ensure_compatible_rtti_uqp_del(const char *context) const { - const std::type_info *rtti_requested = &typeid(D); + void ensure_compatible_uqp_del(const char *context) const { if (!rtti_uqp_del) { - if (!is_std_default_delete(*rtti_requested)) { + if (!uqp_del_is_std_default_delete()) { throw std::invalid_argument(std::string("Missing unique_ptr deleter (") + context + ")."); } - ensure_vptr_is_using_builtin_delete(context); - } else if (!(*rtti_requested == *rtti_uqp_del) - && !(vptr_is_using_builtin_delete - && is_std_default_delete(*rtti_requested))) { + ensure_vptr_is_using_std_default_delete(context); + return; + } + if (uqp_del_is_std_default_delete() && vptr_is_using_std_default_delete) { + return; + } + if (!type_info_equal_across_dso_boundaries(typeid(D), *rtti_uqp_del)) { throw std::invalid_argument(std::string("Incompatible unique_ptr deleter (") + context + ")."); } @@ -249,7 +259,7 @@ struct smart_holder { gd->armed_flag = armed_flag; } - // Caller is responsible for precondition: ensure_compatible_rtti_uqp_del() must succeed. + // Caller is responsible for precondition: ensure_compatible_uqp_del() must succeed. template std::unique_ptr extract_deleter(const char *context, const get_guarded_delete_fn ggd_fn) const { @@ -291,7 +301,7 @@ struct smart_holder { } else { hld.vptr.reset(raw_ptr, std::move(gd)); } - hld.vptr_is_using_builtin_delete = true; + hld.vptr_is_using_std_default_delete = true; hld.is_populated = true; return hld; } @@ -316,7 +326,7 @@ struct smart_holder { void ensure_can_release_ownership(const char *context = "ensure_can_release_ownership") const { ensure_is_not_disowned(context); - ensure_vptr_is_using_builtin_delete(context); + ensure_vptr_is_using_std_default_delete(context); ensure_use_count_1(context); } @@ -332,9 +342,9 @@ struct smart_holder { void *void_ptr = nullptr) { smart_holder hld; hld.rtti_uqp_del = &typeid(D); - hld.vptr_is_using_builtin_delete = is_std_default_delete(*hld.rtti_uqp_del); + hld.vptr_is_using_std_default_delete = uqp_del_is_std_default_delete(); guarded_delete gd{nullptr, false}; - if (hld.vptr_is_using_builtin_delete) { + if (hld.vptr_is_using_std_default_delete) { gd = make_guarded_builtin_delete(true); } else { gd = make_guarded_custom_deleter(std::move(unq_ptr.get_deleter()), true); diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index cd9ec8dbda..4debb7d6bd 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -808,7 +808,7 @@ struct load_helper : value_and_holder_helper { throw_if_uninitialized_or_disowned_holder(typeid(T)); throw_if_instance_is_currently_owned_by_shared_ptr(); holder().ensure_is_not_disowned(context); - holder().template ensure_compatible_rtti_uqp_del(context); + holder().template ensure_compatible_uqp_del(context); holder().ensure_use_count_1(context); T *raw_type_ptr = static_cast(raw_void_ptr); @@ -848,7 +848,7 @@ struct load_helper : value_and_holder_helper { if (!have_holder()) { return unique_with_deleter(nullptr, std::unique_ptr()); } - holder().template ensure_compatible_rtti_uqp_del(context); + holder().template ensure_compatible_uqp_del(context); return unique_with_deleter(raw_type_ptr, std::move(holder().template extract_deleter( context, find_memory_guarded_delete))); diff --git a/tests/pure_cpp/smart_holder_poc.h b/tests/pure_cpp/smart_holder_poc.h index 9a456edce6..038cddc7ab 100644 --- a/tests/pure_cpp/smart_holder_poc.h +++ b/tests/pure_cpp/smart_holder_poc.h @@ -43,7 +43,7 @@ T *as_raw_ptr_release_ownership(smart_holder &hld, template > std::unique_ptr as_unique_ptr(smart_holder &hld) { static const char *context = "as_unique_ptr"; - hld.ensure_compatible_rtti_uqp_del(context); + hld.ensure_compatible_uqp_del(context); hld.ensure_use_count_1(context); T *raw_ptr = hld.as_raw_ptr_unowned(); hld.release_ownership(get_guarded_delete); From ad7230ea1c70cea1578dfd08a3de31c479a36d62 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 16 Jun 2025 11:42:31 -0700 Subject: [PATCH 09/14] =?UTF-8?q?Trivial=20renaming=20for=20internal=20con?= =?UTF-8?q?sistency:=20builtin=5Fdelete=20=E2=86=92=20std=5Fdefault=5Fdele?= =?UTF-8?q?te?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- include/pybind11/detail/struct_smart_holder.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/include/pybind11/detail/struct_smart_holder.h b/include/pybind11/detail/struct_smart_holder.h index 9aa23867e6..5b65b4a9b2 100644 --- a/include/pybind11/detail/struct_smart_holder.h +++ b/include/pybind11/detail/struct_smart_holder.h @@ -109,12 +109,12 @@ inline guarded_delete *get_guarded_delete(const std::shared_ptr &ptr) { using get_guarded_delete_fn = guarded_delete *(*) (const std::shared_ptr &); template ::value, int>::type = 0> -inline void builtin_delete_if_destructible(void *raw_ptr) { +inline void std_default_delete_if_destructible(void *raw_ptr) { std::default_delete{}(static_cast(raw_ptr)); } template ::value, int>::type = 0> -inline void builtin_delete_if_destructible(void *) { +inline void std_default_delete_if_destructible(void *) { // This noop operator is needed to avoid a compilation error (for `delete raw_ptr;`), but // throwing an exception from a destructor will std::terminate the process. Therefore the // runtime check for lifetime-management correctness is implemented elsewhere (in @@ -122,8 +122,8 @@ inline void builtin_delete_if_destructible(void *) { } template -guarded_delete make_guarded_builtin_delete(bool armed_flag) { - return guarded_delete(builtin_delete_if_destructible, armed_flag); +guarded_delete make_guarded_std_default_delete(bool armed_flag) { + return guarded_delete(std_default_delete_if_destructible, armed_flag); } template @@ -295,7 +295,7 @@ struct smart_holder { 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; - auto gd = make_guarded_builtin_delete(true); + auto gd = make_guarded_std_default_delete(true); if (void_cast_raw_ptr) { hld.vptr.reset(static_cast(raw_ptr), std::move(gd)); } else { @@ -345,7 +345,7 @@ struct smart_holder { hld.vptr_is_using_std_default_delete = uqp_del_is_std_default_delete(); guarded_delete gd{nullptr, false}; if (hld.vptr_is_using_std_default_delete) { - gd = make_guarded_builtin_delete(true); + gd = make_guarded_std_default_delete(true); } else { gd = make_guarded_custom_deleter(std::move(unq_ptr.get_deleter()), true); } From f3e1d6a217dddf8f9a532f6c3cb25849c68eeb67 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 16 Jun 2025 14:40:05 -0700 Subject: [PATCH 10/14] Add get_trampoline_self_life_support to detail::type_info (passes local testing). --- include/pybind11/attr.h | 5 +++++ include/pybind11/cast.h | 2 +- include/pybind11/detail/internals.h | 2 ++ include/pybind11/detail/type_caster_base.h | 13 ++++++++----- include/pybind11/pybind11.h | 8 ++++++++ include/pybind11/trampoline_self_life_support.h | 5 +++++ 6 files changed, 29 insertions(+), 6 deletions(-) diff --git a/include/pybind11/attr.h b/include/pybind11/attr.h index 786133afe1..20bc124f91 100644 --- a/include/pybind11/attr.h +++ b/include/pybind11/attr.h @@ -12,6 +12,7 @@ #include "detail/common.h" #include "cast.h" +#include "trampoline_self_life_support.h" #include @@ -312,6 +313,10 @@ struct type_record { /// Function pointer to class_<..>::dealloc void (*dealloc)(detail::value_and_holder &) = nullptr; + /// Function pointer for casting alias class (aka trampoline) pointer to + /// trampoline_self_life_support pointer. + get_trampoline_self_life_support_fn get_trampoline_self_life_support = nullptr; + /// List of base classes of the newly created type list bases; diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 7a6edf25b7..3f58a12647 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1240,7 +1240,7 @@ struct move_only_holder_caster< explicit operator std::unique_ptr() { if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) { - return sh_load_helper.template load_as_unique_ptr(value); + return sh_load_helper.template load_as_unique_ptr(typeinfo, value); } pybind11_fail("Expected to be UNREACHABLE: " __FILE__ ":" PYBIND11_TOSTRING(__LINE__)); } diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index e2ed7888c3..3e7ae35bc2 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -13,6 +13,7 @@ #include #include #include +#include #include "common.h" @@ -312,6 +313,7 @@ struct type_info { void *(*operator_new)(size_t); void (*init_instance)(instance *, const void *); void (*dealloc)(value_and_holder &v_h); + get_trampoline_self_life_support_fn get_trampoline_self_life_support = nullptr; std::vector implicit_conversions; std::vector> implicit_casts; std::vector *direct_conversions; diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index 4debb7d6bd..a704459f4b 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -564,8 +564,9 @@ handle smart_holder_from_unique_ptr(std::unique_ptr &&src, assert(st.second != nullptr); const detail::type_info *tinfo = st.second; if (handle existing_inst = find_registered_python_instance(src_raw_void_ptr, tinfo)) { - auto *self_life_support - = dynamic_raw_ptr_cast_if_possible(src.get()); + auto *self_life_support = tinfo->get_trampoline_self_life_support + ? tinfo->get_trampoline_self_life_support(src.get()) + : nullptr; if (self_life_support != nullptr) { value_and_holder &v_h = self_life_support->v_h; if (v_h.inst != nullptr && v_h.vh != nullptr) { @@ -800,7 +801,8 @@ struct load_helper : value_and_holder_helper { } template - std::unique_ptr load_as_unique_ptr(void *raw_void_ptr, + std::unique_ptr load_as_unique_ptr(const type_info *tinfo, + void *raw_void_ptr, const char *context = "load_as_unique_ptr") { if (!have_holder()) { return unique_with_deleter(nullptr, std::unique_ptr()); @@ -813,8 +815,9 @@ struct load_helper : value_and_holder_helper { T *raw_type_ptr = static_cast(raw_void_ptr); - auto *self_life_support - = dynamic_raw_ptr_cast_if_possible(raw_type_ptr); + auto *self_life_support = tinfo->get_trampoline_self_life_support + ? tinfo->get_trampoline_self_life_support(raw_type_ptr) + : nullptr; // This is enforced indirectly by a static_assert in the class_ implementation: assert(!python_instance_is_alias || self_life_support); diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index e747e274d1..8431f1cfcb 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1571,6 +1571,7 @@ class generic_type : public object { tinfo->holder_size_in_ptrs = size_in_ptrs(rec.holder_size); tinfo->init_instance = rec.init_instance; tinfo->dealloc = rec.dealloc; + tinfo->get_trampoline_self_life_support = rec.get_trampoline_self_life_support; tinfo->simple_type = true; tinfo->simple_ancestors = true; tinfo->module_local = rec.module_local; @@ -2066,6 +2067,13 @@ class class_ : public detail::generic_type { record.dealloc = dealloc_without_manipulating_gil; } + if (std::is_base_of::value) { + record.get_trampoline_self_life_support = [](void *type_ptr) { + return dynamic_raw_ptr_cast_if_possible( + static_cast(type_ptr)); + }; + } + generic_type::initialize(record); if (has_alias) { diff --git a/include/pybind11/trampoline_self_life_support.h b/include/pybind11/trampoline_self_life_support.h index 484045bb17..cbfec7f974 100644 --- a/include/pybind11/trampoline_self_life_support.h +++ b/include/pybind11/trampoline_self_life_support.h @@ -19,6 +19,7 @@ PYBIND11_NAMESPACE_END(detail) // https://github.com/google/clif/blob/07f95d7e69dca2fcf7022978a55ef3acff506c19/clif/python/runtime.cc#L37 // URL provided here mainly to give proper credit. struct trampoline_self_life_support { + // NOTE: PYBIND11_INTERNALS_VERSION needs to be bumped if changes are made to this struct. detail::value_and_holder v_h; trampoline_self_life_support() = default; @@ -57,4 +58,8 @@ struct trampoline_self_life_support { trampoline_self_life_support &operator=(trampoline_self_life_support &&) = delete; }; +PYBIND11_NAMESPACE_BEGIN(detail) +using get_trampoline_self_life_support_fn = trampoline_self_life_support *(*) (void *); +PYBIND11_NAMESPACE_END(detail) + PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) From fb9708b78d7fa469d22af231ee9ae1ca6286ad4b Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 16 Jun 2025 15:17:31 -0700 Subject: [PATCH 11/14] Polish previous commit slightly. --- include/pybind11/attr.h | 6 ++++-- include/pybind11/detail/type_caster_base.h | 8 ++------ include/pybind11/pybind11.h | 3 +++ 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/include/pybind11/attr.h b/include/pybind11/attr.h index 20bc124f91..f8fd0cf32c 100644 --- a/include/pybind11/attr.h +++ b/include/pybind11/attr.h @@ -314,8 +314,10 @@ struct type_record { void (*dealloc)(detail::value_and_holder &) = nullptr; /// Function pointer for casting alias class (aka trampoline) pointer to - /// trampoline_self_life_support pointer. - get_trampoline_self_life_support_fn get_trampoline_self_life_support = nullptr; + /// trampoline_self_life_support pointer. Sidesteps dynamic_cast RTTI issues + /// on platforms like macOS. + get_trampoline_self_life_support_fn get_trampoline_self_life_support + = [](void *) -> trampoline_self_life_support * { return nullptr; }; /// List of base classes of the newly created type list bases; diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index a704459f4b..32011ba8ce 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -564,9 +564,7 @@ handle smart_holder_from_unique_ptr(std::unique_ptr &&src, assert(st.second != nullptr); const detail::type_info *tinfo = st.second; if (handle existing_inst = find_registered_python_instance(src_raw_void_ptr, tinfo)) { - auto *self_life_support = tinfo->get_trampoline_self_life_support - ? tinfo->get_trampoline_self_life_support(src.get()) - : nullptr; + auto *self_life_support = tinfo->get_trampoline_self_life_support(src.get()); if (self_life_support != nullptr) { value_and_holder &v_h = self_life_support->v_h; if (v_h.inst != nullptr && v_h.vh != nullptr) { @@ -815,9 +813,7 @@ struct load_helper : value_and_holder_helper { T *raw_type_ptr = static_cast(raw_void_ptr); - auto *self_life_support = tinfo->get_trampoline_self_life_support - ? tinfo->get_trampoline_self_life_support(raw_type_ptr) - : nullptr; + auto *self_life_support = tinfo->get_trampoline_self_life_support(raw_type_ptr); // This is enforced indirectly by a static_assert in the class_ implementation: assert(!python_instance_is_alias || self_life_support); diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 8431f1cfcb..06be7f1d4f 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -2068,6 +2068,9 @@ class class_ : public detail::generic_type { } if (std::is_base_of::value) { + // Store a cross-DSO-safe getter. + // This lambda is defined in the same DSO that instantiates + // class_, but it can be called safely from any other DSO. record.get_trampoline_self_life_support = [](void *type_ptr) { return dynamic_raw_ptr_cast_if_possible( static_cast(type_ptr)); From f838bcf4794bfe10296289855fb3e2ff3a911f58 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 16 Jun 2025 15:52:15 -0700 Subject: [PATCH 12/14] [skip ci] Store memory::get_guarded_delete in `detail::type_info` instead of `detail::internals` (no searching across DSOs required). --- include/pybind11/cast.h | 12 ++++++---- include/pybind11/detail/internals.h | 20 ++++------------ include/pybind11/detail/type_caster_base.h | 27 ++++++++++++---------- 3 files changed, 26 insertions(+), 33 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 3f58a12647..60dfea5b6f 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -980,7 +980,7 @@ struct copyable_holder_caster< explicit operator std::shared_ptr &() { if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) { - shared_ptr_storage = sh_load_helper.load_as_shared_ptr(value); + shared_ptr_storage = sh_load_helper.load_as_shared_ptr(typeinfo, value); } return shared_ptr_storage; } @@ -989,7 +989,8 @@ struct copyable_holder_caster< if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) { // Reusing shared_ptr code to minimize code complexity. shared_ptr_storage - = sh_load_helper.load_as_shared_ptr(value, + = sh_load_helper.load_as_shared_ptr(typeinfo, + value, /*responsible_parent=*/nullptr, /*force_potentially_slicing_shared_ptr=*/true); } @@ -1019,7 +1020,8 @@ struct copyable_holder_caster< copyable_holder_caster loader; loader.load(responsible_parent, /*convert=*/false); assert(loader.typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder); - return loader.sh_load_helper.load_as_shared_ptr(loader.value, responsible_parent); + return loader.sh_load_helper.load_as_shared_ptr( + loader.typeinfo, loader.value, responsible_parent); } protected: @@ -1248,12 +1250,12 @@ struct move_only_holder_caster< explicit operator const std::unique_ptr &() { if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) { // Get shared_ptr to ensure that the Python object is not disowned elsewhere. - shared_ptr_storage = sh_load_helper.load_as_shared_ptr(value); + shared_ptr_storage = sh_load_helper.load_as_shared_ptr(typeinfo, value); // Build a temporary unique_ptr that is meant to never expire. unique_ptr_storage = std::shared_ptr>( new std::unique_ptr{ sh_load_helper.template load_as_const_unique_ptr( - shared_ptr_storage.get())}, + typeinfo, shared_ptr_storage.get())}, [](std::unique_ptr *ptr) { if (!ptr) { pybind11_fail("FATAL: `const std::unique_ptr &` was disowned " diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index 3e7ae35bc2..db29936a62 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -256,8 +256,6 @@ struct internals { type_map native_enum_type_map; - std::vector memory_guarded_delete_fns; // See PRs #5728, #5700. - internals() : static_property_type(make_static_property_type()), default_metaclass(make_default_metaclass()) { @@ -266,7 +264,6 @@ struct internals { istate = cur_tstate->interp; registered_exception_translators.push_front(&translate_exception); - memory_guarded_delete_fns.push_back(memory::get_guarded_delete); #ifdef Py_GIL_DISABLED // Scale proportional to the number of cores. 2x is a heuristic to reduce contention. auto num_shards @@ -313,7 +310,11 @@ struct type_info { void *(*operator_new)(size_t); void (*init_instance)(instance *, const void *); void (*dealloc)(value_and_holder &v_h); + + // Cross-DSO-safe function pointers (to sidestep cross-DSO RTTI issues): + memory::get_guarded_delete_fn get_memory_guarded_delete = memory::get_guarded_delete; get_trampoline_self_life_support_fn get_trampoline_self_life_support = nullptr; + std::vector implicit_conversions; std::vector> implicit_casts; std::vector *direct_conversions; @@ -677,19 +678,6 @@ inline auto with_exception_translators(const F &cb) local_internals.registered_exception_translators); } -// Traverse all DSOs to find memory::guarded_delete with matching RTTI. -inline memory::guarded_delete *find_memory_guarded_delete(const std::shared_ptr &ptr) { - return with_internals([&](detail::internals &internals) -> memory::guarded_delete * { - for (auto fn : internals.memory_guarded_delete_fns) { - memory::guarded_delete *gd = fn(ptr); - if (gd) { - return gd; - } - } - return nullptr; - }); -} - inline std::uint64_t mix64(std::uint64_t z) { // David Stafford's variant 13 of the MurmurHash3 finalizer popularized // by the SplitMix PRNG. diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index 32011ba8ce..1b23c5c681 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -531,8 +531,8 @@ struct value_and_holder_helper { } // 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 = find_memory_guarded_delete(holder().vptr); + void throw_if_instance_is_currently_owned_by_shared_ptr(const type_info *tinfo) const { + auto *vptr_gd_ptr = tinfo->get_memory_guarded_delete(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."); } @@ -575,7 +575,7 @@ handle smart_holder_from_unique_ptr(std::unique_ptr &&src, } // Critical transfer-of-ownership section. This must stay together. self_life_support->deactivate_life_support(); - holder.reclaim_disowned(find_memory_guarded_delete); + holder.reclaim_disowned(tinfo->get_memory_guarded_delete); (void) src.release(); // Critical section end. return existing_inst; @@ -741,7 +741,8 @@ struct load_helper : value_and_holder_helper { return std::shared_ptr(raw_ptr, shared_ptr_parent_life_support(parent.ptr())); } - std::shared_ptr load_as_shared_ptr(void *void_raw_ptr, + std::shared_ptr load_as_shared_ptr(const type_info *tinfo, + void *void_raw_ptr, handle responsible_parent = nullptr, // to support py::potentially_slicing_weak_ptr // with minimal added code complexity: @@ -762,7 +763,7 @@ struct load_helper : value_and_holder_helper { } auto *type_raw_ptr = static_cast(void_raw_ptr); if (python_instance_is_alias && !force_potentially_slicing_shared_ptr) { - auto *vptr_gd_ptr = find_memory_guarded_delete(holder().vptr); + auto *vptr_gd_ptr = tinfo->get_memory_guarded_delete(holder().vptr); if (vptr_gd_ptr != nullptr) { std::shared_ptr released_ptr = vptr_gd_ptr->released_ptr.lock(); if (released_ptr) { @@ -806,7 +807,7 @@ struct load_helper : value_and_holder_helper { return unique_with_deleter(nullptr, std::unique_ptr()); } throw_if_uninitialized_or_disowned_holder(typeid(T)); - throw_if_instance_is_currently_owned_by_shared_ptr(); + throw_if_instance_is_currently_owned_by_shared_ptr(tinfo); holder().ensure_is_not_disowned(context); holder().template ensure_compatible_uqp_del(context); holder().ensure_use_count_1(context); @@ -818,13 +819,13 @@ struct load_helper : value_and_holder_helper { assert(!python_instance_is_alias || self_life_support); std::unique_ptr extracted_deleter - = holder().template extract_deleter(context, find_memory_guarded_delete); + = holder().template extract_deleter(context, tinfo->get_memory_guarded_delete); // Critical transfer-of-ownership section. This must stay together. if (self_life_support != nullptr) { - holder().disown(find_memory_guarded_delete); + holder().disown(tinfo->get_memory_guarded_delete); } else { - holder().release_ownership(find_memory_guarded_delete); + holder().release_ownership(tinfo->get_memory_guarded_delete); } auto result = unique_with_deleter(raw_type_ptr, std::move(extracted_deleter)); if (self_life_support != nullptr) { @@ -842,15 +843,17 @@ struct load_helper : value_and_holder_helper { // This assumes load_as_shared_ptr succeeded(), and the returned shared_ptr is still alive. // The returned unique_ptr is meant to never expire (the behavior is undefined otherwise). template - std::unique_ptr - load_as_const_unique_ptr(T *raw_type_ptr, const char *context = "load_as_const_unique_ptr") { + std::unique_ptr load_as_const_unique_ptr(const type_info *tinfo, + T *raw_type_ptr, + const char *context + = "load_as_const_unique_ptr") { if (!have_holder()) { return unique_with_deleter(nullptr, std::unique_ptr()); } holder().template ensure_compatible_uqp_del(context); return unique_with_deleter(raw_type_ptr, std::move(holder().template extract_deleter( - context, find_memory_guarded_delete))); + context, tinfo->get_memory_guarded_delete))); } }; From 616f5b8014713602cacef9c3054c1b82738ef514 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 16 Jun 2025 15:59:28 -0700 Subject: [PATCH 13/14] Revert change suggested by ChatGPT. After double-checking, ChatGPT agrees this isn't needed. --- include/pybind11/detail/dynamic_raw_ptr_cast_if_possible.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/detail/dynamic_raw_ptr_cast_if_possible.h b/include/pybind11/detail/dynamic_raw_ptr_cast_if_possible.h index dddf28f0fb..7c00fe98c1 100644 --- a/include/pybind11/detail/dynamic_raw_ptr_cast_if_possible.h +++ b/include/pybind11/detail/dynamic_raw_ptr_cast_if_possible.h @@ -32,7 +32,7 @@ template ::value, int> = 0> To *dynamic_raw_ptr_cast_if_possible(From *ptr) { - return ptr ? dynamic_cast(ptr) : nullptr; + return dynamic_cast(ptr); } PYBIND11_NAMESPACE_END(detail) From e943c2a8795a42252c026f8596fc313d38a00843 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 16 Jun 2025 16:07:05 -0700 Subject: [PATCH 14/14] Minor polishing. --- include/pybind11/attr.h | 4 ++-- include/pybind11/detail/internals.h | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/include/pybind11/attr.h b/include/pybind11/attr.h index f8fd0cf32c..9b631fa48d 100644 --- a/include/pybind11/attr.h +++ b/include/pybind11/attr.h @@ -314,8 +314,8 @@ struct type_record { void (*dealloc)(detail::value_and_holder &) = nullptr; /// Function pointer for casting alias class (aka trampoline) pointer to - /// trampoline_self_life_support pointer. Sidesteps dynamic_cast RTTI issues - /// on platforms like macOS. + /// trampoline_self_life_support pointer. Sidesteps cross-DSO RTTI issues + /// on platforms like macOS (see PR #5728 for details). get_trampoline_self_life_support_fn get_trampoline_self_life_support = [](void *) -> trampoline_self_life_support * { return nullptr; }; diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index db29936a62..414ab897e7 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -10,12 +10,12 @@ #pragma once #include -#include #include #include #include #include "common.h" +#include "struct_smart_holder.h" #include #include @@ -311,7 +311,8 @@ struct type_info { void (*init_instance)(instance *, const void *); void (*dealloc)(value_and_holder &v_h); - // Cross-DSO-safe function pointers (to sidestep cross-DSO RTTI issues): + // Cross-DSO-safe function pointers, to sidestep cross-DSO RTTI issues + // on platforms like macOS (see PR #5728 for details): memory::get_guarded_delete_fn get_memory_guarded_delete = memory::get_guarded_delete; get_trampoline_self_life_support_fn get_trampoline_self_life_support = nullptr;