From c14c2762f69f2bd36765f793e997751ce63c7b40 Mon Sep 17 00:00:00 2001 From: Wenzel Jakob Date: Fri, 25 Aug 2017 16:02:18 +0200 Subject: [PATCH] Address reference leak issue (fixes #1029) Creating an instance of of a pybind11-bound type caused a reference leak in the associated Python type object, which could prevent these from being collected upon interpreter shutdown. This commit fixes that issue for all types that are defined in a scope (e.g. a module). Unscoped anonymous types (e.g. custom iterator types) always retain a positive reference count to prevent their collection. --- include/pybind11/detail/class.h | 14 +++++++++++++- tests/test_class.py | 21 +++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/include/pybind11/detail/class.h b/include/pybind11/detail/class.h index 40efabcc1..f745992a0 100644 --- a/include/pybind11/detail/class.h +++ b/include/pybind11/detail/class.h @@ -334,7 +334,17 @@ inline void clear_instance(PyObject *self) { /// to destroy the C++ object itself, while the rest is Python bookkeeping. extern "C" inline void pybind11_object_dealloc(PyObject *self) { clear_instance(self); - Py_TYPE(self)->tp_free(self); + + auto type = Py_TYPE(self); + type->tp_free(self); + + // `type->tp_dealloc != pybind11_object_dealloc` means that we're being called + // as part of a derived type's dealloc, in which case we're not allowed to decref + // the type here. For cross-module compatibility, we shouldn't compare directly + // with `pybind11_object_dealloc`, but with the common one stashed in internals. + auto pybind11_object_type = (PyTypeObject *) get_internals().instance_base; + if (type->tp_dealloc == pybind11_object_type->tp_dealloc) + Py_DECREF(type); } /** Create the type which can be used as a common base for all classes. This is @@ -583,6 +593,8 @@ inline PyObject* make_new_python_type(const type_record &rec) { /* Register type with the parent scope */ if (rec.scope) setattr(rec.scope, rec.name, (PyObject *) type); + else + Py_INCREF(type); // Keep it alive forever (reference leak) if (module) // Needed by pydoc setattr((PyObject *) type, "__module__", module); diff --git a/tests/test_class.py b/tests/test_class.py index b2cc27575..ac2a3c0e3 100644 --- a/tests/test_class.py +++ b/tests/test_class.py @@ -202,3 +202,24 @@ def test_brace_initialization(): a = m.BraceInitialization(123, "test") assert a.field1 == 123 assert a.field2 == "test" + + +@pytest.unsupported_on_pypy +def test_class_refcount(): + """Instances must correctly increase/decrease the reference count of their types (#1029)""" + from sys import getrefcount + + class PyDog(m.Dog): + pass + + for cls in m.Dog, PyDog: + refcount_1 = getrefcount(cls) + molly = [cls("Molly") for _ in range(10)] + refcount_2 = getrefcount(cls) + + del molly + pytest.gc_collect() + refcount_3 = getrefcount(cls) + + assert refcount_1 == refcount_3 + assert refcount_2 > refcount_1