Adding a few comments after review by @laramiel.

This commit is contained in:
Ralf W. Grosse-Kunstleve 2021-06-24 15:42:56 -07:00 committed by Ralf W. Grosse-Kunstleve
parent 4a08160a1f
commit 64716cc14c
2 changed files with 14 additions and 2 deletions

View File

@ -182,8 +182,13 @@ void construct(value_and_holder &v_h, std::unique_ptr<Cpp<Class>, D> &&unq_ptr,
if (Class::has_alias && need_alias && !is_alias<Class>(ptr)) if (Class::has_alias && need_alias && !is_alias<Class>(ptr))
throw type_error("pybind11::init(): construction failed: returned std::unique_ptr pointee " throw type_error("pybind11::init(): construction failed: returned std::unique_ptr pointee "
"is not an alias instance"); "is not an alias instance");
// Here and below: if the new object is a trampoline, the shared_from_this mechanism needs
// to be prevented from accessing the smart_holder vptr, because it does not keep the
// trampoline Python object alive. For types that don't inherit from enable_shared_from_this
// it does not matter if void_cast_raw_ptr is true or false, therefore it's not necessary
// to also inspect the type.
auto smhldr = type_caster<Cpp<Class>>::template smart_holder_from_unique_ptr( auto smhldr = type_caster<Cpp<Class>>::template smart_holder_from_unique_ptr(
std::move(unq_ptr), Class::has_alias && is_alias<Class>(ptr)); std::move(unq_ptr), /*void_cast_raw_ptr*/ Class::has_alias && is_alias<Class>(ptr));
v_h.value_ptr() = ptr; v_h.value_ptr() = ptr;
v_h.type->init_instance(v_h.inst, &smhldr); v_h.type->init_instance(v_h.inst, &smhldr);
} }
@ -198,7 +203,7 @@ void construct(value_and_holder &v_h,
auto *ptr = unq_ptr.get(); auto *ptr = unq_ptr.get();
no_nullptr(ptr); no_nullptr(ptr);
auto smhldr = type_caster<Alias<Class>>::template smart_holder_from_unique_ptr( auto smhldr = type_caster<Alias<Class>>::template smart_holder_from_unique_ptr(
std::move(unq_ptr), true); std::move(unq_ptr), /*void_cast_raw_ptr*/ true);
v_h.value_ptr() = ptr; v_h.value_ptr() = ptr;
v_h.type->init_instance(v_h.inst, &smhldr); v_h.type->init_instance(v_h.inst, &smhldr);
} }

View File

@ -9,6 +9,8 @@ class PySft(m.Sft):
def test_release_and_shared_from_this(): def test_release_and_shared_from_this():
# Exercises the most direct path from building a shared_from_this-visible
# shared_ptr to calling shared_from_this.
obj = PySft("PySft") obj = PySft("PySft")
assert obj.history == "PySft" assert obj.history == "PySft"
assert m.use_count(obj) == 1 assert m.use_count(obj) == 1
@ -30,6 +32,7 @@ def test_release_and_shared_from_this_leak():
def test_release_and_stash(): def test_release_and_stash():
# Exercises correct functioning of guarded_delete weak_ptr.
obj = PySft("PySft") obj = PySft("PySft")
stash1 = m.SftSharedPtrStash(1) stash1 = m.SftSharedPtrStash(1)
stash1.Add(obj) stash1.Add(obj)
@ -88,6 +91,7 @@ def test_release_and_stash_leak():
def test_release_and_stash_via_shared_from_this(): def test_release_and_stash_via_shared_from_this():
# Exercises that the smart_holder vptr is invisible to the shared_from_this mechnism.
obj = PySft("PySft") obj = PySft("PySft")
stash1 = m.SftSharedPtrStash(1) stash1 = m.SftSharedPtrStash(1)
with pytest.raises(RuntimeError) as exc_info: with pytest.raises(RuntimeError) as exc_info:
@ -120,6 +124,8 @@ def test_release_and_stash_via_shared_from_this_leak():
def test_pass_released_shared_ptr_as_unique_ptr(): def test_pass_released_shared_ptr_as_unique_ptr():
# Exercises that returning a unique_ptr fails while a shared_from_this
# visible shared_ptr exists.
obj = PySft("PySft") obj = PySft("PySft")
stash1 = m.SftSharedPtrStash(1) stash1 = m.SftSharedPtrStash(1)
stash1.Add(obj) # Releases shared_ptr to C++. stash1.Add(obj) # Releases shared_ptr to C++.
@ -139,6 +145,7 @@ def test_pass_released_shared_ptr_as_unique_ptr():
], ],
) )
def test_pure_cpp_sft_raw_ptr(make_f): def test_pure_cpp_sft_raw_ptr(make_f):
# Exercises void_cast_raw_ptr logic for different situations.
obj = make_f("PureCppSft") obj = make_f("PureCppSft")
assert m.pass_shared_ptr(obj) == 3 assert m.pass_shared_ptr(obj) == 3
assert obj.history == "PureCppSft_PassSharedPtr" assert obj.history == "PureCppSft_PassSharedPtr"