From 6364b732e9e30cbd79b795e9290bfa3360aa32a1 Mon Sep 17 00:00:00 2001 From: Yannick Jadoul Date: Fri, 16 Oct 2020 22:38:51 +0200 Subject: [PATCH] fix: test_factory_constructors.py failure triggered by test_register_duplicate_class (#2564) * Demonstrate test_factory_constructors.py failure without functional changes from #2335 * Revert "Demonstrate test_factory_constructors.py failure without functional changes from #2335" This reverts commit ca33a8021fc2a3617c3356b188796528f4594419. * Fix test crash where registered Python type gets garbage collected * Clean up some more internal structures when class objects go out of scope * Reduce length of std::erase_if-in-C++20 comment * Clean up code for cleaning up type internals * Move cleaning up of type info in internals to tp_dealloc on pybind11_metaclass --- include/pybind11/detail/class.h | 40 +++++++++++++++++++++++++++++++++ tests/test_class.py | 1 - 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/include/pybind11/detail/class.h b/include/pybind11/detail/class.h index 54a857388..569c5415d 100644 --- a/include/pybind11/detail/class.h +++ b/include/pybind11/detail/class.h @@ -193,6 +193,44 @@ extern "C" inline PyObject *pybind11_meta_call(PyObject *type, PyObject *args, P return self; } +/// Cleanup the type-info for a pybind11-registered type. +extern "C" inline void pybind11_meta_dealloc(PyObject *obj) { + auto *type = (PyTypeObject *) obj; + auto &internals = get_internals(); + + // A pybind11-registered type will: + // 1) be found in internals.registered_types_py + // 2) have exactly one associated `detail::type_info` + auto found_type = internals.registered_types_py.find(type); + if (found_type != internals.registered_types_py.end() && + found_type->second.size() == 1 && + found_type->second[0]->type == type) { + + auto *tinfo = found_type->second[0]; + auto tindex = std::type_index(*tinfo->cpptype); + internals.direct_conversions.erase(tindex); + + if (tinfo->module_local) + registered_local_types_cpp().erase(tindex); + else + internals.registered_types_cpp.erase(tindex); + internals.registered_types_py.erase(tinfo->type); + + // Actually just `std::erase_if`, but that's only available in C++20 + auto &cache = internals.inactive_override_cache; + for (auto it = cache.begin(), last = cache.end(); it != last; ) { + if (it->first == (PyObject *) tinfo->type) + it = cache.erase(it); + else + ++it; + } + + delete tinfo; + } + + PyType_Type.tp_dealloc(obj); +} + /** This metaclass is assigned by default to all pybind11 types and is required in order for static properties to function correctly. Users may override this using `py::metaclass`. Return value: New reference. */ @@ -225,6 +263,8 @@ inline PyTypeObject* make_default_metaclass() { type->tp_getattro = pybind11_meta_getattro; #endif + type->tp_dealloc = pybind11_meta_dealloc; + if (PyType_Ready(type) < 0) pybind11_fail("make_default_metaclass(): failure in PyType_Ready()!"); diff --git a/tests/test_class.py b/tests/test_class.py index c34ee8a12..bdcced964 100644 --- a/tests/test_class.py +++ b/tests/test_class.py @@ -434,7 +434,6 @@ def test_base_and_derived_nested_scope(): assert m.DerivedWithNested.Nested.get_name() == "DerivedWithNested::Nested" -@pytest.mark.skip("See https://github.com/pybind/pybind11/pull/2564") def test_register_duplicate_class(): import types