diff --git a/include/pybind11/detail/smart_holder_poc.h b/include/pybind11/detail/smart_holder_poc.h index 5a4ebe08c..99b1fa9b4 100644 --- a/include/pybind11/detail/smart_holder_poc.h +++ b/include/pybind11/detail/smart_holder_poc.h @@ -38,10 +38,8 @@ Details: * If created from an external `shared_ptr`, or a `unique_ptr` with a custom deleter, including life-time management for external objects is infeasible. -* The smart_holder is movable but not copyable, as a consequence of using - unique_ptr for the vptr_deleter_armed_flag_ptr. Note that the bool for - the flag has to live on the heap, for the smart_holder to be movable. - unique_ptr is a great fit for this situation. +* By choice, the smart_holder is movable but not copyable, to keep the design + simple, and to guard against accidental copying overhead. */ #pragma once @@ -60,8 +58,9 @@ namespace memory { template struct guarded_builtin_delete { - bool *flag_ptr; - explicit guarded_builtin_delete(bool *armed_flag_ptr) : flag_ptr{armed_flag_ptr} {} + std::shared_ptr flag_ptr; + explicit guarded_builtin_delete(std::shared_ptr armed_flag_ptr) + : flag_ptr{armed_flag_ptr} {} template ::value, int>::type = 0> void operator()(T *raw_ptr) { @@ -80,8 +79,9 @@ struct guarded_builtin_delete { template struct guarded_custom_deleter { - bool *flag_ptr; - explicit guarded_custom_deleter(bool *armed_flag_ptr) : flag_ptr{armed_flag_ptr} {} + std::shared_ptr flag_ptr; + explicit guarded_custom_deleter(std::shared_ptr armed_flag_ptr) + : flag_ptr{armed_flag_ptr} {} void operator()(T *raw_ptr) { if (*flag_ptr) D()(raw_ptr); @@ -96,13 +96,19 @@ inline bool is_std_default_delete(const std::type_info &rtti_deleter) { struct smart_holder { const std::type_info *rtti_uqp_del; - std::unique_ptr vptr_deleter_armed_flag_ptr; + std::shared_ptr vptr_deleter_armed_flag_ptr; std::shared_ptr vptr; bool vptr_is_using_noop_deleter : 1; bool vptr_is_using_builtin_delete : 1; bool vptr_is_external_shared_ptr : 1; bool is_populated : 1; + // Design choice: smart_holder is movable but not copyable. + smart_holder(smart_holder &&) = default; + smart_holder(const smart_holder &) = delete; + smart_holder &operator=(smart_holder &&) = default; + smart_holder &operator=(const smart_holder &) = delete; + smart_holder() : rtti_uqp_del{nullptr}, vptr_is_using_noop_deleter{false}, vptr_is_using_builtin_delete{false}, vptr_is_external_shared_ptr{false}, is_populated{ @@ -182,7 +188,7 @@ struct smart_holder { template static smart_holder from_raw_ptr_unowned(T *raw_ptr) { smart_holder hld(false); - hld.vptr.reset(raw_ptr, guarded_builtin_delete(hld.vptr_deleter_armed_flag_ptr.get())); + hld.vptr.reset(raw_ptr, guarded_builtin_delete(hld.vptr_deleter_armed_flag_ptr)); hld.vptr_is_using_noop_deleter = true; hld.is_populated = true; return hld; @@ -213,7 +219,7 @@ struct smart_holder { static smart_holder from_raw_ptr_take_ownership(T *raw_ptr) { ensure_pointee_is_destructible("from_raw_ptr_take_ownership"); smart_holder hld(true); - hld.vptr.reset(raw_ptr, guarded_builtin_delete(hld.vptr_deleter_armed_flag_ptr.get())); + hld.vptr.reset(raw_ptr, guarded_builtin_delete(hld.vptr_deleter_armed_flag_ptr)); hld.vptr_is_using_builtin_delete = true; hld.is_populated = true; return hld; @@ -246,10 +252,10 @@ struct smart_holder { hld.vptr_is_using_builtin_delete = is_std_default_delete(*hld.rtti_uqp_del); if (hld.vptr_is_using_builtin_delete) { hld.vptr.reset(unq_ptr.get(), - guarded_builtin_delete(hld.vptr_deleter_armed_flag_ptr.get())); + guarded_builtin_delete(hld.vptr_deleter_armed_flag_ptr)); } else { hld.vptr.reset(unq_ptr.get(), - guarded_custom_deleter(hld.vptr_deleter_armed_flag_ptr.get())); + guarded_custom_deleter(hld.vptr_deleter_armed_flag_ptr)); } unq_ptr.release(); hld.is_populated = true; diff --git a/tests/pure_cpp/smart_holder_poc_test.cpp b/tests/pure_cpp/smart_holder_poc_test.cpp index 3232197f7..970899a05 100644 --- a/tests/pure_cpp/smart_holder_poc_test.cpp +++ b/tests/pure_cpp/smart_holder_poc_test.cpp @@ -302,3 +302,24 @@ TEST_CASE("indestructible_int-from_raw_ptr_take_ownership", "[E]") { REQUIRE_THROWS_WITH(smart_holder::from_raw_ptr_take_ownership(value), "Pointee is not destructible (from_raw_ptr_take_ownership)."); } + +TEST_CASE("from_raw_ptr_take_ownership+as_shared_ptr-outliving_smart_holder", "[S]") { + // Exercises guarded_builtin_delete flag_ptr validity past destruction of smart_holder. + std::shared_ptr longer_living; + { + auto hld = smart_holder::from_raw_ptr_take_ownership(new int(19)); + longer_living = hld.as_shared_ptr(); + } + REQUIRE(*longer_living == 19); +} + +TEST_CASE("from_unique_ptr_with_deleter+as_shared_ptr-outliving_smart_holder", "[S]") { + // Exercises guarded_custom_deleter flag_ptr validity past destruction of smart_holder. + std::shared_ptr longer_living; + { + std::unique_ptr> orig_owner(new int(19)); + auto hld = smart_holder::from_unique_ptr(std::move(orig_owner)); + longer_living = hld.as_shared_ptr(); + } + REQUIRE(*longer_living == 19); +}