pybind/pybind11
1
0
Fork 0
mirror of https://github.com/pybind/pybind11.git synced 2026-05-13 09:46:10 +00:00
Code Issues Packages Projects Releases Wiki Activity

Simpler and safe implementation based on new requirement: C++ (outside of the Python object) needs to hold a shared_ptr before shared_from_this is called. This is more similar to the existing implementation on current smart_holder HEAD, but still uses a weak_ptr to always return the same shared_ptr as long as C++ does not let it expire.

Browse Source
This commit is contained in:
Ralf W. Grosse-Kunstleve
2021-06-23 07:00:44 -07:00
committed by Ralf W. Grosse-Kunstleve
parent 3dd89f14df
commit 48eb78ae77
4 changed files with 100 additions and 182 deletions
Download Patch File Download Diff File Expand all files Collapse all files

91
include/pybind11/detail/smart_holder_poc.h
View File

@@ -52,8 +52,8 @@ Details:
#include <typeinfo>
//#include <iostream>
//inline void to_cout(std::string msg) { std::cout << msg << std::endl; }
inline void to_cout(std::string) { }
// inline void to_cout(const std::string &msg) { std::cout << msg << std::endl; }
inline void to_cout(const std::string &) {}
// pybindit = Python Bindings Innovation Track.
// Currently not in pybind11 namespace to signal that this POC does not depend
@@ -66,36 +66,28 @@ inline int shared_from_this_status(...) { return 0; }
template <typename AnyBaseOfT>
#if defined(__cpp_lib_enable_shared_from_this) && (!defined(_MSC_VER) || _MSC_VER >= 1912)
inline int shared_from_this_status(const std::enable_shared_from_this<AnyBaseOfT> *ptr) {
if (ptr->weak_from_this().lock()) return 1;
return -1;
if (ptr->weak_from_this().lock())
return 1;
return -1;
}
#else
inline int shared_from_this_status(const std::enable_shared_from_this<AnyBaseOfT> *) {
return 999;
return 999;
}
#endif
struct smart_holder;
struct guarded_delete {
smart_holder *hld = nullptr;
void (*callback_ptr)(void *) = nullptr;
void *callback_arg = nullptr;
void (*shd_ptr_reset_fptr)(std::shared_ptr<void>&, void *, guarded_delete &&);
void (*del_fptr)(void *);
std::weak_ptr<void> released_ptr; // Trick to keep the smart_holder memory footprint small.
void (*del_ptr)(void *);
bool armed_flag;
guarded_delete(void (*shd_ptr_reset_fptr)(std::shared_ptr<void>&, void *, guarded_delete &&), void (*del_fptr)(void *), bool armed_flag)
: shd_ptr_reset_fptr{shd_ptr_reset_fptr}, del_fptr{del_fptr}, armed_flag{armed_flag} {
to_cout("LOOOK guarded_delete ctor " + std::to_string(__LINE__) + " " + __FILE__);
}
void operator()(void *raw_ptr) const;
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);
}
};
template <typename T>
inline void shd_ptr_reset(std::shared_ptr<void>& shd_ptr, void *raw_ptr, guarded_delete &&gdel) {
shd_ptr.reset(static_cast<T *>(raw_ptr), gdel);
}
template <typename T, typename std::enable_if<std::is_destructible<T>::value, int>::type = 0>
inline void builtin_delete_if_destructible(void *raw_ptr) {
delete static_cast<T *>(raw_ptr);
@@ -111,7 +103,7 @@ inline void builtin_delete_if_destructible(void *) {
template <typename T>
guarded_delete make_guarded_builtin_delete(bool armed_flag) {
return guarded_delete(shd_ptr_reset<T>, builtin_delete_if_destructible<T>, armed_flag);
return guarded_delete(builtin_delete_if_destructible<T>, armed_flag);
}
template <typename T, typename D>
@@ -121,13 +113,7 @@ inline void custom_delete(void *raw_ptr) {
template <typename T, typename D>
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 *) {};
return guarded_delete(custom_delete<T, D>, armed_flag);
};
template <typename T>
@@ -144,7 +130,6 @@ 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.
@@ -156,7 +141,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},
vptr_is_released{false}, pointee_depends_on_holder_owner{false} {}
pointee_depends_on_holder_owner{false} {}
bool has_pointee() const { return vptr != nullptr; }
@@ -231,17 +216,15 @@ struct smart_holder {
}
void reset_vptr_deleter_armed_flag(bool armed_flag) const {
to_cout("LOOOK smart_holder reset_vptr_deleter_armed_flag " + std::to_string(__LINE__) + " " + __FILE__);
auto vptr_del_fptr = std::get_deleter<guarded_delete>(vptr);
if (vptr_del_fptr == nullptr) {
auto vptr_del_ptr = std::get_deleter<guarded_delete>(vptr);
if (vptr_del_ptr == nullptr) {
throw std::runtime_error(
"smart_holder::reset_vptr_deleter_armed_flag() called in an invalid context.");
}
vptr_del_fptr->armed_flag = armed_flag;
vptr_del_ptr->armed_flag = armed_flag;
}
static smart_holder from_raw_ptr_unowned(void *raw_ptr) {
to_cout("LOOOK smart_holder from_raw_ptr_unowned " + std::to_string(__LINE__) + " " + __FILE__);
smart_holder hld;
hld.vptr.reset(raw_ptr, [](void *) {});
hld.vptr_is_using_noop_deleter = true;
@@ -251,7 +234,6 @@ to_cout("LOOOK smart_holder from_raw_ptr_unowned " + std::to_string(__LINE__) +
template <typename T>
T *as_raw_ptr_unowned() const {
to_cout("LOOOK smart_holder as_raw_ptr_unowned " + std::to_string(__LINE__) + " " + __FILE__);
return static_cast<T *>(vptr.get());
}
@@ -273,8 +255,9 @@ to_cout("LOOOK smart_holder as_raw_ptr_unowned " + std::to_string(__LINE__) + "
template <typename T>
static smart_holder from_raw_ptr_take_ownership(T *raw_ptr) {
to_cout("");
to_cout("LOOOK smart_holder from_raw_ptr_take_ownership " + std::to_string(__LINE__) + " " + __FILE__);
to_cout("");
to_cout("LOOOK smart_holder from_raw_ptr_take_ownership " + std::to_string(__LINE__) + " "
+ __FILE__);
ensure_pointee_is_destructible<T>("from_raw_ptr_take_ownership");
smart_holder hld;
hld.vptr.reset(static_cast<void *>(raw_ptr), make_guarded_builtin_delete<T>(true));
@@ -313,7 +296,8 @@ to_cout("LOOOK smart_holder from_raw_ptr_take_ownership " + std::to_string(__LIN
template <typename T>
T *as_raw_ptr_release_ownership(const char *context = "as_raw_ptr_release_ownership") {
to_cout("LOOOK smart_holder as_raw_ptr_release_ownership " + std::to_string(__LINE__) + " " + __FILE__);
to_cout("LOOOK smart_holder as_raw_ptr_release_ownership " + std::to_string(__LINE__) + " "
+ __FILE__);
ensure_can_release_ownership(context);
T *raw_ptr = as_raw_ptr_unowned<T>();
release_ownership();
@@ -322,7 +306,7 @@ to_cout("LOOOK smart_holder as_raw_ptr_release_ownership " + std::to_string(__LI
template <typename T, typename D>
static smart_holder from_unique_ptr(std::unique_ptr<T, D> &&unq_ptr) {
to_cout("LOOOK smart_holder from_unique_ptr " + std::to_string(__LINE__) + " " + __FILE__);
to_cout("LOOOK smart_holder from_unique_ptr " + std::to_string(__LINE__) + " " + __FILE__);
smart_holder hld;
hld.rtti_uqp_del = &typeid(D);
hld.vptr_is_using_builtin_delete = is_std_default_delete<T>(*hld.rtti_uqp_del);
@@ -340,7 +324,7 @@ to_cout("LOOOK smart_holder from_unique_ptr " + std::to_string(__LINE__) + " " +
template <typename T, typename D = std::default_delete<T>>
std::unique_ptr<T, D> as_unique_ptr() {
to_cout("LOOOK smart_holder as_unique_ptr " + std::to_string(__LINE__) + " " + __FILE__);
to_cout("LOOOK smart_holder as_unique_ptr " + std::to_string(__LINE__) + " " + __FILE__);
static const char *context = "as_unique_ptr";
ensure_compatible_rtti_uqp_del<T, D>(context);
ensure_use_count_1(context);
@@ -351,7 +335,7 @@ to_cout("LOOOK smart_holder as_unique_ptr " + std::to_string(__LINE__) + " " + _
template <typename T>
static smart_holder from_shared_ptr(std::shared_ptr<T> shd_ptr) {
to_cout("LOOOK smart_holder from_shared_ptr " + std::to_string(__LINE__) + " " + __FILE__);
to_cout("LOOOK smart_holder from_shared_ptr " + std::to_string(__LINE__) + " " + __FILE__);
smart_holder hld;
hld.vptr = std::static_pointer_cast<void>(shd_ptr);
hld.vptr_is_external_shared_ptr = true;
@@ -361,27 +345,10 @@ to_cout("LOOOK smart_holder from_shared_ptr " + std::to_string(__LINE__) + " " +
template <typename T>
std::shared_ptr<T> as_shared_ptr() const {
to_cout("LOOOK smart_holder as_shared_ptr " + std::to_string(__LINE__) + " " + __FILE__);
to_cout("LOOOK smart_holder as_shared_ptr " + std::to_string(__LINE__) + " " + __FILE__);
return std::static_pointer_cast<T>(vptr);
}
};
inline void guarded_delete::operator()(void *raw_ptr) const {
if (hld) {
to_cout("RECLAIM guarded_delete call hld " + std::to_string(__LINE__) + " " + __FILE__);
assert(armed_flag);
assert(hld->vptr.get() == raw_ptr);
assert(hld->vptr_is_released);
(*shd_ptr_reset_fptr)(hld->vptr, hld->vptr.get(), guarded_delete{shd_ptr_reset_fptr, del_fptr, true});
hld->vptr_is_released = false;
(*callback_ptr)(callback_arg); // Py_DECREF.
} else if (armed_flag) {
to_cout("LOOOK guarded_delete call del " + std::to_string(__LINE__) + " " + __FILE__);
(*del_fptr)(raw_ptr);
} else {
to_cout("LOOOK guarded_delete call noop " + std::to_string(__LINE__) + " " + __FILE__);
}
}
} // namespace memory
} // namespace pybindit

53
include/pybind11/detail/smart_holder_type_casters.h
View File

@@ -370,16 +370,11 @@ struct smart_holder_type_caster_load {
return *raw_ptr;
}
static void dec_ref_void(void *ptr) {
to_cout("LOOOK dec_ref_void Py_REFCNT(" + std::to_string(Py_REFCNT(reinterpret_cast<PyObject *>(ptr))) + ") " + std::to_string(__LINE__) + " " + __FILE__);
gil_scoped_acquire gil;
Py_DECREF(reinterpret_cast<PyObject *>(ptr));
}
struct shared_ptr_dec_ref_deleter {
PyObject *self;
void operator()(void *) {
to_cout("LOOOK shared_ptr_dec_ref_deleter call " + std::to_string(__LINE__) + " " + __FILE__);
to_cout("LOOOK shared_ptr_dec_ref_deleter call " + std::to_string(__LINE__) + " "
+ __FILE__);
gil_scoped_acquire gil;
Py_DECREF(self);
}
@@ -399,41 +394,21 @@ to_cout("LOOOK shared_ptr_dec_ref_deleter call " + std::to_string(__LINE__) + "
if (hld.pointee_depends_on_holder_owner) {
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);
std::shared_ptr<void> non_owning(
hld.vptr.get(),
pybindit::memory::noop_deleter_acting_as_weak_ptr_owner{hld.vptr});
std::shared_ptr<void> released_ptr = vptr_gd_ptr->released_ptr.lock();
if (released_ptr)
return std::shared_ptr<T>(released_ptr, type_raw_ptr);
auto self = reinterpret_cast<PyObject *>(load_impl.loaded_v_h.inst);
// Critical transfer-of-ownership section. This must stay together.
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.
to_cout("FRESHLY released");
return to_be_returned;
std::shared_ptr<T> to_be_released(type_raw_ptr, shared_ptr_dec_ref_deleter{self});
vptr_gd_ptr->released_ptr = to_be_released;
return to_be_released;
}
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) {
to_cout("retrieved released");
return std::shared_ptr<T>(released_vptr, type_raw_ptr);
}
}
pybind11_fail(
"smart_holder_type_casters: loaded_as_shared_ptr failure:"
" fatal internal inconsistency.");
pybind11_fail("smart_holder_type_casters: loaded_as_shared_ptr failure: 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>();
return std::shared_ptr<T>(void_shd_ptr, type_raw_ptr);
}
@@ -512,7 +487,8 @@ private:
// have_holder() must be true or this function will fail.
void throw_if_instance_is_currently_owned_by_shared_ptr() const {
if (holder().vptr_is_released) {
auto vptr_gd_ptr = std::get_deleter<pybindit::memory::guarded_delete>(holder().vptr);
if (vptr_gd_ptr != nullptr && !vptr_gd_ptr->released_ptr.expired()) {
throw value_error("Python instance is currently owned by a std::shared_ptr.");
}
}
@@ -729,7 +705,8 @@ struct smart_holder_type_caster<std::shared_ptr<T>> : smart_holder_type_caster_l
if (handle existing_inst = find_registered_python_instance(src_raw_void_ptr, tinfo)) {
// SMART_HOLDER_WIP: MISSING: Enforcement of consistency with existing smart_holder.
// SMART_HOLDER_WIP: MISSING: keep_alive.
to_cout("LOOOK shtc sh return existing_inst " + std::to_string(__LINE__) + " " + __FILE__);
to_cout("LOOOK shtc sh return existing_inst " + std::to_string(__LINE__) + " "
+ __FILE__);
return existing_inst;
}
@@ -745,7 +722,7 @@ to_cout("LOOOK shtc sh return existing_inst " + std::to_string(__LINE__) + " " +
if (policy == return_value_policy::reference_internal)
keep_alive_impl(inst, parent);
to_cout("LOOOK shtc sh return new inst " + std::to_string(__LINE__) + " " + __FILE__);
to_cout("LOOOK shtc sh return new inst " + std::to_string(__LINE__) + " " + __FILE__);
return inst.release();
}