[skip ci] Only pass around memory::get_guarded_delete function pointer.

Author: rwgk

include/pybind11/detail/internals.h

@@ -18,7 +18,6 @@
 
 #include <atomic>
 #include <exception>
-#include <memory>
 #include <mutex>
 #include <thread>
 
@@ -256,9 +255,7 @@ struct internals {
 
     type_map<PyObject *> native_enum_type_map;
 
-    using get_memory_guarded_delete_fn
-        = memory::guarded_delete *(*) (const std::shared_ptr<void> &);
-    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<void> &ptr) {
-            return std::get_deleter<memory::guarded_delete>(ptr);
-        };
     }
     internals(const internals &other) = delete;
     internals(internals &&other) = delete;

include/pybind11/detail/struct_smart_holder.h

@@ -100,6 +100,12 @@ struct guarded_delete {
     }
 };
 
+inline guarded_delete *get_guarded_delete(const std::shared_ptr<void> &ptr) {
+    return std::get_deleter<guarded_delete>(ptr);
+}
+
+using get_guarded_delete_fn = guarded_delete *(*) (const std::shared_ptr<void> &);
+
 template <typename T, typename std::enable_if<std::is_destructible<T>::value, int>::type = 0>
 inline void builtin_delete_if_destructible(void *raw_ptr) {
     std::default_delete<T>{}(static_cast<T *>(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<T, D>() must succeed.
     template <typename T, typename D>
-    std::unique_ptr<D> extract_deleter(const char *context, const guarded_delete *gd) const {
+    std::unique_ptr<D> 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<custom_deleter<T, D>>();
             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();
     }
 

include/pybind11/detail/type_caster_base.h

@@ -576,7 +576,7 @@ handle smart_holder_from_unique_ptr(std::unique_ptr<T, D> &&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<D> extracted_deleter = holder().template extract_deleter<T, D>(
-            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<T, D>(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<T, D>(nullptr, std::unique_ptr<D>());
         }
         holder().template ensure_compatible_rtti_uqp_del<T, D>(context);
-        return unique_with_deleter<T, D>(
-            raw_type_ptr,
-            std::move(holder().template extract_deleter<T, D>(
-                context, get_internals().get_memory_guarded_delete(holder().vptr))));
+        return unique_with_deleter<T, D>(raw_type_ptr,
+                                         std::move(holder().template extract_deleter<T, D>(
+                                             context, get_internals().get_memory_guarded_delete)));
     }
 };
 

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<T>();
-    hld.release_ownership(std::get_deleter<guarded_delete>(hld.vptr));
+    hld.release_ownership(get_guarded_delete);
     return raw_ptr;
 }
 
@@ -46,7 +46,7 @@ std::unique_ptr<T, D> as_unique_ptr(smart_holder &hld) {
     hld.ensure_compatible_rtti_uqp_del<T, D>(context);
     hld.ensure_use_count_1(context);
     T *raw_ptr = hld.as_raw_ptr_unowned<T>();
-    hld.release_ownership(std::get_deleter<guarded_delete>(hld.vptr));
+    hld.release_ownership(get_guarded_delete);
     // KNOWN DEFECT (see PR #4850): Does not copy the deleter.
     return std::unique_ptr<T, D>(raw_ptr);
 }

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<int> new_owner(hld.as_raw_ptr_unowned<int>());
-    hld.disown(std::get_deleter<guarded_delete>(hld.vptr));
+    hld.disown(pybind11::memory::get_guarded_delete);
     REQUIRE(poc::as_lvalue_ref<int>(hld) == 19);
     REQUIRE(*new_owner == 19);
     // Manually verified: without this, clang++ -fsanitize=address reports
     // "detected memory leaks".
-    hld.reclaim_disowned(std::get_deleter<guarded_delete>(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<int> new_owner(hld.as_raw_ptr_unowned<int>());
-    hld.disown(std::get_deleter<guarded_delete>(hld.vptr));
+    hld.disown(pybind11::memory::get_guarded_delete);
     REQUIRE(poc::as_lvalue_ref<int>(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<int> new_owner(hld.as_raw_ptr_unowned<int>());
-    hld.disown(std::get_deleter<guarded_delete>(hld.vptr));
+    hld.disown(pybind11::memory::get_guarded_delete);
     REQUIRE_THROWS_WITH(hld.ensure_is_not_disowned(context),
                         "Holder was disowned already (test_case).");
 }