Fully emulating type_caster_base-related behavior: trying shared_from_this also for unowned pointees.

Author: rwgk

include/pybind11/detail/smart_holder_poc.h

@@ -94,16 +94,6 @@ inline bool is_std_default_delete(const std::type_info &rtti_deleter) {
            || rtti_deleter == typeid(std::default_delete<T const>);
 }
 
-inline void enable_shared_from_this_from_raw_ptr_take_ownership_guard(...) {}
-
-template <typename AnyBaseOfT>
-inline void enable_shared_from_this_from_raw_ptr_take_ownership_guard(
-    const std::enable_shared_from_this<AnyBaseOfT> *) {
-    // This static_assert will always trigger if this template function is instantiated.
-    static_assert(!std::is_base_of<std::enable_shared_from_this<AnyBaseOfT>, AnyBaseOfT>::value,
-                  "Ownership must not be transferred via a raw pointer.");
-}
-
 struct smart_holder {
     const std::type_info *rtti_uqp_del = nullptr;
     std::shared_ptr<bool> vptr_deleter_armed_flag_ptr;
@@ -236,7 +226,6 @@ struct smart_holder {
 
     template <typename T>
     static smart_holder from_raw_ptr_take_ownership(T *raw_ptr) {
-        enable_shared_from_this_from_raw_ptr_take_ownership_guard(raw_ptr);
         ensure_pointee_is_destructible<T>("from_raw_ptr_take_ownership");
         smart_holder hld(true);
         hld.vptr.reset(raw_ptr, guarded_builtin_delete<T>(hld.vptr_deleter_armed_flag_ptr));

include/pybind11/detail/smart_holder_type_casters.h

@@ -269,25 +269,22 @@ struct smart_holder_type_caster_class_hooks : smart_holder_type_caster_base_tag
     using holder_type = pybindit::memory::smart_holder;
 
     template <typename WrappedType>
-    static void from_raw_pointer_take_ownership_or_shared_from_this(
-        holder_type *uninitialized_location, WrappedType *value_ptr, ...) {
-        new (uninitialized_location)
-            holder_type(holder_type::from_raw_ptr_take_ownership(value_ptr));
+    static bool try_initialization_using_shared_from_this(holder_type *, WrappedType *, ...) {
+        return false;
     }
 
     template <typename WrappedType, typename AnyBaseOfWrappedType>
-    static void from_raw_pointer_take_ownership_or_shared_from_this(
+    static bool try_initialization_using_shared_from_this(
         holder_type *uninitialized_location,
-        WrappedType *value_ptr,
+        WrappedType *value_ptr_w_t,
         const std::enable_shared_from_this<AnyBaseOfWrappedType> *) {
-        auto shd_ptr
-            = std::dynamic_pointer_cast<WrappedType>(detail::try_get_shared_from_this(value_ptr));
-        if (shd_ptr) {
-            new (uninitialized_location) holder_type(holder_type::from_shared_ptr(shd_ptr));
-        } else {
-            new (uninitialized_location)
-                holder_type(holder_type::from_shared_ptr(std::shared_ptr<WrappedType>(value_ptr)));
-        }
+        auto shd_ptr = std::dynamic_pointer_cast<WrappedType>(
+            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 <typename WrappedType, typename AliasType>
@@ -301,21 +298,26 @@ 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();
         }
+        auto uninitialized_location = std::addressof(v_h.holder<holder_type>());
+        auto value_ptr_w_t          = v_h.value_ptr<WrappedType>();
         if (holder_void_ptr) {
             // Note: inst->owned ignored.
             auto holder_ptr = static_cast<holder_type *>(holder_void_ptr);
-            new (std::addressof(v_h.holder<holder_type>())) holder_type(std::move(*holder_ptr));
-        } else if (inst->owned) {
-            from_raw_pointer_take_ownership_or_shared_from_this(
-                std::addressof(v_h.holder<holder_type>()),
-                v_h.value_ptr<WrappedType>(),
-                v_h.value_ptr<WrappedType>());
+            new (uninitialized_location) holder_type(std::move(*holder_ptr));
         } else {
-            new (std::addressof(v_h.holder<holder_type>()))
-                holder_type(holder_type::from_raw_ptr_unowned(v_h.value_ptr<WrappedType>()));
+            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));
+                } else {
+                    new (uninitialized_location)
+                        holder_type(holder_type::from_raw_ptr_unowned(value_ptr_w_t));
+                }
+            }
         }
         v_h.holder<holder_type>().pointee_depends_on_holder_owner
-            = dynamic_raw_ptr_cast_if_possible<AliasType>(v_h.value_ptr<WrappedType>()) != nullptr;
+            = dynamic_raw_ptr_cast_if_possible<AliasType>(value_ptr_w_t) != nullptr;
         v_h.set_holder_constructed();
     }
 
@@ -390,6 +392,9 @@ struct smart_holder_type_caster_load {
                 shared_ptr_dec_ref_deleter{
                     handle((PyObject *) load_impl.loaded_v_h.inst).inc_ref()});
         }
+        if (holder().vptr_is_using_noop_deleter) {
+            throw std::runtime_error("Non-owning holder (loaded_as_shared_ptr).");
+        }
         std::shared_ptr<void> void_shd_ptr = holder().template as_shared_ptr<void>();
         return std::shared_ptr<T>(void_shd_ptr, type_raw_ptr);
     }

tests/test_class_sh_shared_from_this.py

@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
-# import pytest
+import pytest
 
 from pybind11_tests import class_sh_shared_from_this as m
 from pybind11_tests import ConstructorStats
@@ -53,10 +53,9 @@ def test_shared_from_this_bad_wp():
     bad_wp = s.bad_wp  # init_holder_helper(holder_ptr=false, owned=false, bad_wp=true)
     assert stats.alive() == 2
     assert s.set_ref(bad_wp)
-    # with pytest.raises(RuntimeError) as excinfo:
-    if 1:  # XXX XXX XXX
+    with pytest.raises(RuntimeError) as excinfo:
         assert s.set_holder(bad_wp)
-    # assert "Unable to cast from non-held to held instance" in str(excinfo.value)
+    assert str(excinfo.value) == "Non-owning holder (loaded_as_shared_ptr)."
     del bad_wp, s
     assert stats.alive() == 0