From 2b6b98e28f08a786b781992921896908276ca940 Mon Sep 17 00:00:00 2001
From: Riyaz Haque <haque1@llnl.gov>
Date: Fri, 2 Oct 2020 10:06:04 -0700
Subject: [PATCH] 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 |  2 +-
 tests/test_class.cpp            | 10 ++++++++++
 tests/test_class.py             | 15 +++++++++++++++
 3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/include/pybind11/detail/class.h b/include/pybind11/detail/class.h
index 0f432f25f..ecc6fb5e0 100644
--- a/include/pybind11/detail/class.h
+++ b/include/pybind11/detail/class.h
@@ -258,7 +258,7 @@ inline bool deregister_instance_impl(void *ptr, instance *self) {
     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;
         }
diff --git a/tests/test_class.cpp b/tests/test_class.cpp
index bb9d36442..3ca9d2a94 100644
--- a/tests/test_class.cpp
+++ b/tests/test_class.cpp
@@ -420,6 +420,16 @@ TEST_SUBMODULE(class_, m) {
     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:
diff --git a/tests/test_class.py b/tests/test_class.py
index fb061d1a2..38eb55ff5 100644
--- a/tests/test_class.py
+++ b/tests/test_class.py
@@ -368,3 +368,18 @@ def test_non_final_final():
 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.