commit 2b6b98e28f08a786b781992921896908276ca940
author Riyaz Haque <haque1@llnl.gov> Fri Oct 02 10:06:04 2020 -0700
committer GitHub <noreply@github.com> Fri Oct 02 19:06:04 2020 +0200
tree 04558ae1d9fcd5db4e667a4c214760d0bbe283b1
parent b9d00273eefd4c07f7c9e27fbcf04fba590b798f

Bugfix/Check actual value when deregistering pybind11 instance (#2252)

* Add tests demonstrating the problem with deregistering pybind11 instances

* Fix deregistering of different pybind11 instance from internals

Co-authored-by: Yannick Jadoul <yannick.jadoul@belgacom.net>
Co-authored-by: Blistic <wots_wot@hotmail.com>

include/pybind11/detail/class.h

diff --git a/include/pybind11/detail/class.h b/include/pybind11/detail/class.h
index 0f432f2..ecc6fb5 100644
--- a/include/pybind11/detail/class.h
+++ b/include/pybind11/detail/class.h
@@ -258,7 +258,7 @@
     auto &registered_instances = get_internals().registered_instances;
     auto range = registered_instances.equal_range(ptr);
     for (auto it = range.first; it != range.second; ++it) {
-        if (Py_TYPE(self) == Py_TYPE(it->second)) {
+        if (self == it->second) {
             registered_instances.erase(it);
             return true;
         }

tests/test_class.cpp

diff --git a/tests/test_class.cpp b/tests/test_class.cpp
index bb9d364..3ca9d2a 100644
--- a/tests/test_class.cpp
+++ b/tests/test_class.cpp
@@ -420,6 +420,16 @@
     py::class_<PyPrintDestructor>(m, "PyPrintDestructor")
         .def(py::init<>())
         .def("throw_something", &PyPrintDestructor::throw_something);
+
+    struct SamePointer {};
+    static SamePointer samePointer;
+    py::class_<SamePointer, std::unique_ptr<SamePointer, py::nodelete>>(m, "SamePointer")
+        .def(py::init([]() { return &samePointer; }))
+        .def("__del__", [](SamePointer&) { py::print("__del__ called"); });
+
+    struct Empty {};
+    py::class_<Empty>(m, "Empty")
+        .def(py::init<>());
 }
 
 template <int N> class BreaksBase { public:

tests/test_class.py

diff --git a/tests/test_class.py b/tests/test_class.py
index fb061d1..38eb55f 100644
--- a/tests/test_class.py
+++ b/tests/test_class.py
@@ -368,3 +368,18 @@
 def test_exception_rvalue_abort():
     with pytest.raises(RuntimeError):
         m.PyPrintDestructor().throw_something()
+
+
+# https://github.com/pybind/pybind11/issues/1568
+def test_multiple_instances_with_same_pointer(capture):
+    n = 100
+    instances = [m.SamePointer() for _ in range(n)]
+    for i in range(n):
+        # We need to reuse the same allocated memory for with a different type,
+        # to ensure the bug in `deregister_instance_impl` is detected. Otherwise
+        # `Py_TYPE(self) == Py_TYPE(it->second)` will still succeed, even though
+        # the `instance` is already deleted.
+        instances[i] = m.Empty()
+    # No assert: if this does not trigger the error
+    #   pybind11_fail("pybind11_object_dealloc(): Tried to deallocate unregistered instance!");
+    # and just completes without crashing, we're good.