Leveraging new noop_deleter_acting_as_weak_ptr_owner to retrieve released vptr.

The placeholder `vptr` is never exposed anymore, therefore the externally visible `use_count`s are more intuitive.

Author: rwgk

include/pybind11/detail/smart_holder_poc.h

@@ -123,6 +123,12 @@ guarded_delete make_guarded_custom_deleter(bool armed_flag) {
     return guarded_delete(shd_ptr_reset<T>, custom_delete<T, D>, armed_flag);
 };
 
+// Trick to keep the smart_holder memory footprint small.
+struct noop_deleter_acting_as_weak_ptr_owner {
+    std::weak_ptr<void> passenger;
+    void operator()(void *) {};
+};
+
 template <typename T>
 inline bool is_std_default_delete(const std::type_info &rtti_deleter) {
     return rtti_deleter == typeid(std::default_delete<T>)

include/pybind11/detail/smart_holder_type_casters.h

@@ -394,35 +394,41 @@ to_cout("LOOOK shared_ptr_dec_ref_deleter call " + std::to_string(__LINE__) + "
         auto type_raw_ptr = convert_type(void_raw_ptr);
         if (hld.pointee_depends_on_holder_owner) {
             auto self         = reinterpret_cast<PyObject *>(load_impl.loaded_v_h.inst);
-            auto vptr_del_ptr = std::get_deleter<pybindit::memory::guarded_delete>(hld.vptr);
-            if (vptr_del_ptr != nullptr) {
+            auto vptr_gd_ptr = std::get_deleter<pybindit::memory::guarded_delete>(hld.vptr);
+            if (vptr_gd_ptr != nullptr) {
                 assert(!hld.vptr_is_released);
                 std::shared_ptr<T> to_be_returned(hld.vptr, type_raw_ptr);
-int sftstat = pybindit::memory::shared_from_this_status(type_raw_ptr);
-to_cout("LOOOK loaded_as_shared_ptr return released SFT=" + std::to_string(sftstat) + " #1 " + std::to_string(__LINE__) + " " + __FILE__);
-                std::shared_ptr<void> non_owning(hld.vptr.get(), [](void *) {});
+                std::shared_ptr<void> non_owning(
+                    hld.vptr.get(),
+                    pybindit::memory::noop_deleter_acting_as_weak_ptr_owner{hld.vptr});
                 // Critical transfer-of-ownership section. This must stay together.
-                vptr_del_ptr->hld          = &hld;
-                vptr_del_ptr->callback_ptr = dec_ref_void;
-                vptr_del_ptr->callback_arg = self;
+                vptr_gd_ptr->hld          = &hld;
+                vptr_gd_ptr->callback_ptr = dec_ref_void;
+                vptr_gd_ptr->callback_arg = self;
                 hld.vptr = non_owning;
                 hld.vptr_is_released = true;
                 Py_INCREF(self);
                 // Critical section end.
-sftstat = pybindit::memory::shared_from_this_status(type_raw_ptr);
-to_cout("LOOOK loaded_as_shared_ptr return released SFT=" + std::to_string(sftstat) + " #2 " + std::to_string(__LINE__) + " " + __FILE__);
                 return to_be_returned;
             }
-            // XXX XXX XXX Ensure not shared_from_this.
-            Py_INCREF(self);
-to_cout("LOOOK loaded_as_shared_ptr return shared_ptr_dec_ref_deleter " + std::to_string(__LINE__) + " " + __FILE__);
-            return std::shared_ptr<T>(type_raw_ptr, shared_ptr_dec_ref_deleter{self});
+            auto vptr_ndaawp_ptr = std::get_deleter<
+                pybindit::memory::noop_deleter_acting_as_weak_ptr_owner>(hld.vptr);
+            if (vptr_ndaawp_ptr != nullptr) {
+                assert(hld.vptr_is_released);
+                auto released_vptr = vptr_ndaawp_ptr->passenger.lock();
+                if (released_vptr != nullptr) {
+                    return std::shared_ptr<T>(released_vptr, type_raw_ptr);
+                }
+            }
+            pybind11_fail(
+                "smart_holder_type_casters: loaded_as_shared_ptr failure:"
+                " fatal internal inconsistency.");
         }
         if (hld.vptr_is_using_noop_deleter) {
             throw std::runtime_error("Non-owning holder (loaded_as_shared_ptr).");
         }
+        assert(!hld.vptr_is_released);
         std::shared_ptr<void> void_shd_ptr = hld.template as_shared_ptr<void>();
-to_cout("LOOOK loaded_as_shared_ptr return hld vptr " + std::to_string(__LINE__) + " " + __FILE__);
         return std::shared_ptr<T>(void_shd_ptr, type_raw_ptr);
     }
 

tests/test_class_sh_trampoline_shared_from_this.cpp

@@ -74,8 +74,10 @@ struct SftTrampoline : Sft, py::trampoline_self_life_support {
     using Sft::Sft;
 };
 
-void pass_shared_ptr(const std::shared_ptr<Sft> &obj) {
-    obj->shared_from_this()->history += "_PassSharedPtr";
+long pass_shared_ptr(const std::shared_ptr<Sft> &obj) {
+    auto sft = obj->shared_from_this();
+    sft->history += "_PassSharedPtr";
+    return sft.use_count();
 }
 
 void pass_unique_ptr(const std::unique_ptr<Sft> &) {}

tests/test_class_sh_trampoline_shared_from_this.py

@@ -20,7 +20,8 @@ def test_pass_shared_ptr():
     m.pass_shared_ptr(obj)
     assert obj.history == "PySft_PassSharedPtr"
     assert obj.use_count() in [2, -1]
-    m.pass_shared_ptr(obj)
+    uc = m.pass_shared_ptr(obj)
+    assert uc == 2  # +1 for passed argument, +1 for shared_from_this.
     assert obj.history == "PySft_PassSharedPtr_PassSharedPtr"
     assert obj.use_count() in [2, -1]
 
@@ -32,28 +33,33 @@ def test_pass_shared_ptr_while_stashed():
     stash1.Add(obj)
     assert obj.history == "PySft_Stash1Add"
     assert obj.use_count() in [2, -1]
-    m.pass_shared_ptr(obj)
+    assert stash1.history(0) == "PySft_Stash1Add"
+    assert stash1.use_count(0) == 1  # obj does NOT own the shared_ptr anymore.
+    uc = m.pass_shared_ptr(obj)
+    assert uc == 3  # +1 for passed argument, +1 for shared_from_this.
     assert obj.history == "PySft_Stash1Add_PassSharedPtr"
     assert obj.use_count() in [2, -1]
+    assert stash1.history(0) == "PySft_Stash1Add_PassSharedPtr"
+    assert stash1.use_count(0) == 1
     stash2 = m.SftSharedPtrStash(2)
     stash2.Add(obj)
     assert obj.history == "PySft_Stash1Add_PassSharedPtr_Stash2Add"
-    assert obj.use_count() in [2, -1]
+    assert obj.use_count() in [3, -1]
     assert stash2.history(0) == "PySft_Stash1Add_PassSharedPtr_Stash2Add"
-    assert stash2.use_count(0) == 1  # TODO: this is not great.
+    assert stash2.use_count(0) == 2
     stash2.Add(obj)
     assert obj.history == "PySft_Stash1Add_PassSharedPtr_Stash2Add_Stash2Add"
-    assert obj.use_count() in [2, -1]
-    assert stash1.use_count(0) == 1
+    assert obj.use_count() in [4, -1]
+    assert stash1.use_count(0) == 3
     assert stash1.history(0) == "PySft_Stash1Add_PassSharedPtr_Stash2Add_Stash2Add"
-    assert stash2.use_count(0) == 1
+    assert stash2.use_count(0) == 3
     assert stash2.history(0) == "PySft_Stash1Add_PassSharedPtr_Stash2Add_Stash2Add"
-    assert stash2.use_count(1) == 1
+    assert stash2.use_count(1) == 3
     assert stash2.history(1) == "PySft_Stash1Add_PassSharedPtr_Stash2Add_Stash2Add"
     del obj
-    assert stash2.use_count(0) == 1
+    assert stash2.use_count(0) == 3
     assert stash2.history(0) == "PySft_Stash1Add_PassSharedPtr_Stash2Add_Stash2Add"
-    assert stash2.use_count(1) == 1
+    assert stash2.use_count(1) == 3
     assert stash2.history(1) == "PySft_Stash1Add_PassSharedPtr_Stash2Add_Stash2Add"
     del stash2
     gc.collect()