First attempt to make shared_from_this and trampolines play nicely. Passes all pybind11 and PyCLIF unit tests ASAN clean, but not the open_spiel iterated_prisoners_dilemma_test.

include/pybind11/detail/smart_holder_poc.h

@@ -44,6 +44,7 @@ Details:
 
 #pragma once
 
+#include <cassert>
 #include <memory>
 #include <stdexcept>
 #include <string>
@@ -56,15 +57,17 @@ Details:
 namespace pybindit {
 namespace memory {
 
+struct smart_holder;
+
 struct guarded_delete {
+    smart_holder *hld            = nullptr;
+    void (*callback_ptr)(void *) = nullptr;
+    void *callback_arg           = nullptr;
     void (*del_ptr)(void *);
     bool armed_flag;
     guarded_delete(void (*del_ptr)(void *), bool armed_flag)
         : del_ptr{del_ptr}, armed_flag{armed_flag} {}
-    void operator()(void *raw_ptr) const {
-        if (armed_flag)
-            (*del_ptr)(raw_ptr);
-    }
+    void operator()(void *raw_ptr) const;
 };
 
 template <typename T, typename std::enable_if<std::is_destructible<T>::value, int>::type = 0>
@@ -109,6 +112,7 @@ struct smart_holder {
     bool vptr_is_external_shared_ptr : 1;
     bool is_populated : 1;
     bool is_disowned : 1;
+    bool vptr_is_released : 1;
     bool pointee_depends_on_holder_owner : 1; // SMART_HOLDER_WIP: See PR #2839.
 
     // Design choice: smart_holder is movable but not copyable.
@@ -120,7 +124,7 @@ struct smart_holder {
     smart_holder()
         : vptr_is_using_noop_deleter{false}, vptr_is_using_builtin_delete{false},
           vptr_is_external_shared_ptr{false}, is_populated{false}, is_disowned{false},
-          pointee_depends_on_holder_owner{false} {}
+          vptr_is_released{false}, pointee_depends_on_holder_owner{false} {}
 
     bool has_pointee() const { return vptr != nullptr; }
 
@@ -319,5 +323,16 @@ struct smart_holder {
     }
 };
 
+inline void guarded_delete::operator()(void *raw_ptr) const {
+    if (hld) {
+        assert(armed_flag);
+        hld->vptr.reset(hld->vptr.get(), guarded_delete{del_ptr, true});
+        hld->vptr_is_released = false;
+        (*callback_ptr)(callback_arg); // Py_DECREF.
+    } else if (armed_flag) {
+        (*del_ptr)(raw_ptr);
+    }
+}
+
 } // namespace memory
 } // namespace pybindit

include/pybind11/detail/smart_holder_type_casters.h

@@ -18,6 +18,7 @@
 #include "type_caster_base.h"
 #include "typeid.h"
 
+#include <cassert>
 #include <cstddef>
 #include <memory>
 #include <new>
@@ -369,13 +370,16 @@ struct smart_holder_type_caster_load {
         return *raw_ptr;
     }
 
+    static void dec_ref_void(void *ptr) {
+        gil_scoped_acquire gil;
+        Py_DECREF(reinterpret_cast<PyObject *>(ptr));
+    }
+
     struct shared_ptr_dec_ref_deleter {
-        // Note: deleter destructor fails on MSVC 2015 and GCC 4.8, so we manually call dec_ref
-        // here instead.
-        handle ref;
+        PyObject *self;
         void operator()(void *) {
             gil_scoped_acquire gil;
-            ref.dec_ref();
+            Py_DECREF(self);
         }
     };
 
@@ -386,20 +390,34 @@ struct smart_holder_type_caster_load {
         if (!have_holder())
             return nullptr;
         throw_if_uninitialized_or_disowned_holder();
-        holder().ensure_is_not_disowned("loaded_as_shared_ptr");
-        auto void_raw_ptr = holder().template as_raw_ptr_unowned<void>();
+        holder_type &hld = holder();
+        hld.ensure_is_not_disowned("loaded_as_shared_ptr");
+        auto void_raw_ptr = hld.template as_raw_ptr_unowned<void>();
         auto type_raw_ptr = convert_type(void_raw_ptr);
-        if (holder().pointee_depends_on_holder_owner) {
-            // Tie lifetime of trampoline Python part to C++ part (PR #2839).
-            return std::shared_ptr<T>(
-                type_raw_ptr,
-                shared_ptr_dec_ref_deleter{
-                    handle((PyObject *) load_impl.loaded_v_h.inst).inc_ref()});
+        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) {
+                assert(!hld.vptr_is_released);
+                std::shared_ptr<void> released(hld.vptr.get(), [](void *) {});
+                // 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;
+                hld.vptr.swap(released);
+                hld.vptr_is_released = true;
+                Py_INCREF(self);
+                // Critical section end.
+                return std::shared_ptr<T>(released, type_raw_ptr);
+            }
+            // XXX XXX XXX Ensure not shared_from_this.
+            Py_INCREF(self);
+            return std::shared_ptr<T>(type_raw_ptr, shared_ptr_dec_ref_deleter{self});
         }
-        if (holder().vptr_is_using_noop_deleter) {
+        if (hld.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>();
+        std::shared_ptr<void> void_shd_ptr = hld.template as_shared_ptr<void>();
         return std::shared_ptr<T>(void_shd_ptr, type_raw_ptr);
     }