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.
This commit is contained in:
Ralf W. Grosse-Kunstleve 2021-06-21 18:09:14 -07:00 committed by Ralf W. Grosse-Kunstleve
parent 0e4b38e34d
commit bfd4c4893e
4 changed files with 47 additions and 27 deletions

View File

@ -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>)

View File

@ -398,35 +398,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);
}

View File

@ -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> &) {}

View File

@ -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()