diff --git a/include/pybind11/detail/smart_holder_poc.h b/include/pybind11/detail/smart_holder_poc.h index 36c873dce..5b25558a9 100644 --- a/include/pybind11/detail/smart_holder_poc.h +++ b/include/pybind11/detail/smart_holder_poc.h @@ -44,6 +44,7 @@ Details: #pragma once +#include #include #include #include @@ -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 ::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 diff --git a/include/pybind11/detail/smart_holder_type_casters.h b/include/pybind11/detail/smart_holder_type_casters.h index 366474d2e..2e3f215b0 100644 --- a/include/pybind11/detail/smart_holder_type_casters.h +++ b/include/pybind11/detail/smart_holder_type_casters.h @@ -18,6 +18,7 @@ #include "type_caster_base.h" #include "typeid.h" +#include #include #include #include @@ -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(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(); + holder_type &hld = holder(); + hld.ensure_is_not_disowned("loaded_as_shared_ptr"); + auto void_raw_ptr = hld.template as_raw_ptr_unowned(); 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( - 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(load_impl.loaded_v_h.inst); + auto vptr_del_ptr = std::get_deleter(hld.vptr); + if (vptr_del_ptr != nullptr) { + assert(!hld.vptr_is_released); + std::shared_ptr 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(released, type_raw_ptr); + } + // XXX XXX XXX Ensure not shared_from_this. + Py_INCREF(self); + return std::shared_ptr(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_shd_ptr = holder().template as_shared_ptr(); + std::shared_ptr void_shd_ptr = hld.template as_shared_ptr(); return std::shared_ptr(void_shd_ptr, type_raw_ptr); }