[cpyrt] Unregister proxies whose C++ object was replaced through T**

src/cpyrt/MemoryRegulator.cxx

@@ -8,6 +8,7 @@ using namespace cppjit;
 #include "ProxyWrappers.h"
 
 // Standard
+#include <algorithm>
 #include <assert.h>
 #include <iostream>
 #include <string.h>
@@ -199,10 +200,8 @@ bool cpyrt::MemoryRegulator::UnregisterPyObject(CPPInstance* pyobj,
 
   interop::TCppObject_t cppobj =
       pyobj->IsSmart() ? pyobj->GetObjectRaw() : pyobj->GetObject();
-  if (!cppobj)
-    return false;
 
-  if (unregisterHook) {
+  if (cppobj && unregisterHook) {
     auto res = unregisterHook(cppobj, ((CPPClass*)pyclass)->fCppType);
     if (!res.second)
       return res.first;
@@ -212,9 +211,26 @@ bool cpyrt::MemoryRegulator::UnregisterPyObject(CPPInstance* pyobj,
   if (!cppobjs)
     return false;
 
-  // erase if tracked
-  if (cppobjs->erase(cppobj)) {
-    pyobj->fFlags &= ~CPPInstance::kIsRegulated;
+  // The proxy is normally tracked under the address it holds now. That
+  // address can have changed since registration, though: a function taking
+  // the object as T** (or void**) may delete it and write a new object into
+  // the proxy, and cross-inheritance can null the held pointer. Only erase an
+  // entry that belongs to this proxy, because another proxy may be tracked
+  // under the new address by now, and otherwise search the map, because an
+  // entry left behind under the old address would dangle once this proxy is
+  // gone: a later C++ object at that address would be matched with the dead
+  // proxy, and re-registering the address writes into its freed memory.
+  auto ppo = cppobj ? cppobjs->find(cppobj) : cppobjs->end();
+  if (ppo == cppobjs->end() || ppo->second != (PyObject*)pyobj) {
+    ppo = std::find_if(cppobjs->begin(), cppobjs->end(),
+                       [pyobj](const CppToPyMap_t::value_type& entry) {
+                         return entry.second == (PyObject*)pyobj;
+                       });
+  }
+
+  pyobj->fFlags &= ~CPPInstance::kIsRegulated;
+  if (ppo != cppobjs->end()) {
+    cppobjs->erase(ppo);
     return true;
   }
 

test/test_regression.py

@@ -1727,3 +1727,47 @@ def test56_enum_arg_typedefed_underlying_type(self):
 
         assert ns.pick(ns.S.A) == 1
         assert ns.pick(ns.S.B) == 5
+
+    def test57_regulator_follows_replaced_object(self):
+        """A proxy whose object a T** argument replaced unregisters cleanly"""
+
+        import gc
+
+        import cppjit
+
+        cppjit.cppdef("""\
+        namespace RegulatorReplaced {
+            struct Pod { int m_int; };
+            // allocate the replacement first, so its address always differs
+            void replace(Pod** pp) {
+                Pod* fresh = new Pod(**pp);
+                delete *pp;
+                *pp = fresh;
+            }
+        }""")
+
+        ns = cppjit.gbl.RegulatorReplaced
+
+        p = ns.Pod()
+        old = cppjit.addressof(p)
+        ns.replace(p)
+        new = cppjit.addressof(p)
+        assert new != old
+
+        # a second proxy for the object at the new address
+        q = cppjit.bind_object(new, ns.Pod)
+        assert q is not p
+
+        # p must not take q's entry along when it dies (the C++ object stays
+        # alive for q)
+        p.__python_owns__ = False
+        del p
+        gc.collect()
+        assert cppjit.bind_object(new, ns.Pod) is q
+
+        # nor leave its own entry behind under the old address, where a later
+        # object at that address would be matched with the dead proxy (binding
+        # does not dereference the address)
+        r = cppjit.bind_object(old, ns.Pod)
+        assert cppjit.addressof(r) == old
+        assert r is not q