Adding unit test: test_multiple_registered_instances_for_same_pointee

pybind · Jun 28, 2021 · 5918b2e · 5918b2e
1 parent 0b30a4a
commit 5918b2e
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 2 deletions.
diff --git a/include/pybind11/detail/smart_holder_type_casters.h b/include/pybind11/detail/smart_holder_type_casters.h
@@ -411,7 +411,9 @@ struct smart_holder_type_caster_load {
                 return to_be_released;
             }
             if (std::get_deleter<shared_ptr_dec_ref_deleter>(hld.vptr) != nullptr) {
-                // SMART_HOLDER_WIP: unit test coverage.
+                // This code is reachable only if there are multiple registered_instances for the
+                // same pointee.
+                // SMART_HOLDER_WIP: keep weak_ref?
                 std::shared_ptr<void> void_shd_ptr = hld.template as_shared_ptr<void>();
                 return std::shared_ptr<T>(void_shd_ptr, type_raw_ptr);
             }

diff --git a/tests/test_class_sh_trampoline_shared_from_this.cpp b/tests/test_class_sh_trampoline_shared_from_this.cpp
@@ -94,6 +94,8 @@ std::shared_ptr<Sft> make_pure_cpp_sft_shd_ptr(const std::string &history_seed)
     return std::make_shared<Sft>(history_seed);
 }
 
+std::shared_ptr<Sft> pass_through_shd_ptr(const std::shared_ptr<Sft> &obj) { return obj; }
+
 } // namespace
 
 PYBIND11_SMART_HOLDER_TYPE_CASTERS(Sft)
@@ -102,7 +104,9 @@ PYBIND11_SMART_HOLDER_TYPE_CASTERS(SftSharedPtrStash)
 TEST_SUBMODULE(class_sh_trampoline_shared_from_this, m) {
     py::classh<Sft, SftTrampoline>(m, "Sft")
         .def(py::init<std::string>())
-        .def_readonly("history", &Sft::history);
+        .def_readonly("history", &Sft::history)
+        // This leads to multiple entries in registered_instances:
+        .def(py::init([](const std::shared_ptr<Sft> &existing) { return existing; }));
 
     py::classh<SftSharedPtrStash>(m, "SftSharedPtrStash")
         .def(py::init<int>())
@@ -118,4 +122,5 @@ TEST_SUBMODULE(class_sh_trampoline_shared_from_this, m) {
     m.def("make_pure_cpp_sft_raw_ptr", make_pure_cpp_sft_raw_ptr);
     m.def("make_pure_cpp_sft_unq_ptr", make_pure_cpp_sft_unq_ptr);
     m.def("make_pure_cpp_sft_shd_ptr", make_pure_cpp_sft_shd_ptr);
+    m.def("pass_through_shd_ptr", pass_through_shd_ptr);
 }
diff --git a/tests/test_class_sh_trampoline_shared_from_this.py b/tests/test_class_sh_trampoline_shared_from_this.py
@@ -153,3 +153,21 @@ def test_pure_cpp_sft_raw_ptr(make_f):
     stash1 = m.SftSharedPtrStash(1)
     stash1.AddSharedFromThis(obj)
     assert obj.history == "PureCppSft_Stash1AddSharedFromThis"
+
+
+def test_multiple_registered_instances_for_same_pointee():
+    obj0 = PySft("PySft")
+    obj0.attachment_in_dict = "Obj0"
+    assert m.pass_through_shd_ptr(obj0) is obj0
+    while True:
+        obj = m.Sft(obj0)
+        assert obj is not obj0
+        obj_pt = m.pass_through_shd_ptr(obj)
+        # Unpredictable! Because registered_instances is as std::unordered_multimap.
+        assert obj_pt is obj0 or obj_pt is obj
+        # Multiple registered_instances for the same pointee can lead to unpredictable results:
+        if obj_pt is obj0:
+            assert obj_pt.attachment_in_dict == "Obj0"
+        else:
+            assert not hasattr(obj_pt, "attachment_in_dict")
+        break  # Comment out for manual leak checking (use `top` command).