From 780750e5ea476b27bec804e6bf8a9321fa380e83 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Thu, 8 Oct 2020 23:01:14 -0700 Subject: [PATCH 1/8] Track all heap types --- Objects/typeobject.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 3bb2c338fe0b53..4474dca057f844 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -2815,11 +2815,9 @@ type_new(PyTypeObject *metatype, PyObject *args, PyObject *kwds) } type->tp_dealloc = subtype_dealloc; - /* Enable GC unless this class is not adding new instance variables and - the base class did not use GC. */ - if ((base->tp_flags & Py_TPFLAGS_HAVE_GC) || - type->tp_basicsize > base->tp_basicsize) - type->tp_flags |= Py_TPFLAGS_HAVE_GC; + // All heap types need GC, since we can create a reference cycle by storing + // an instance on one of its parents: + type->tp_flags |= Py_TPFLAGS_HAVE_GC; /* Always override allocation strategy to use regular heap */ type->tp_alloc = PyType_GenericAlloc; From 26d8109b6d3bd3cf8a105c0dd232f45b94e35962 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Thu, 8 Oct 2020 23:01:22 -0700 Subject: [PATCH 2/8] Fix failing tests --- Lib/test/test_gc.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_gc.py b/Lib/test/test_gc.py index 1b096efdbcf5fb..ba667370159063 100644 --- a/Lib/test/test_gc.py +++ b/Lib/test/test_gc.py @@ -582,9 +582,9 @@ class UserIntSlots(int): self.assertTrue(gc.is_tracked(UserInt())) self.assertTrue(gc.is_tracked([])) self.assertTrue(gc.is_tracked(set())) - self.assertFalse(gc.is_tracked(UserClassSlots())) - self.assertFalse(gc.is_tracked(UserFloatSlots())) - self.assertFalse(gc.is_tracked(UserIntSlots())) + self.assertTrue(gc.is_tracked(UserClassSlots())) + self.assertTrue(gc.is_tracked(UserFloatSlots())) + self.assertTrue(gc.is_tracked(UserIntSlots())) def test_is_finalized(self): # Objects not tracked by the always gc return false From 1d6f62f37a11d581936b564947b49476a1d302a5 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Tue, 13 Oct 2020 13:33:58 -0700 Subject: [PATCH 3/8] Clean up type flagging. --- Objects/typeobject.c | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 4474dca057f844..36c7662e081a40 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -2612,10 +2612,10 @@ type_new(PyTypeObject *metatype, PyObject *args, PyObject *kwds) slots = NULL; /* Initialize tp_flags */ + // All heap types need GC, since we can create a reference cycle by storing + // an instance on one of its parents: type->tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HEAPTYPE | - Py_TPFLAGS_BASETYPE; - if (base->tp_flags & Py_TPFLAGS_HAVE_GC) - type->tp_flags |= Py_TPFLAGS_HAVE_GC; + Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC; /* Initialize essential fields */ type->tp_as_async = &et->as_async; @@ -2815,19 +2815,11 @@ type_new(PyTypeObject *metatype, PyObject *args, PyObject *kwds) } type->tp_dealloc = subtype_dealloc; - // All heap types need GC, since we can create a reference cycle by storing - // an instance on one of its parents: - type->tp_flags |= Py_TPFLAGS_HAVE_GC; - /* Always override allocation strategy to use regular heap */ type->tp_alloc = PyType_GenericAlloc; - if (type->tp_flags & Py_TPFLAGS_HAVE_GC) { - type->tp_free = PyObject_GC_Del; - type->tp_traverse = subtype_traverse; - type->tp_clear = subtype_clear; - } - else - type->tp_free = PyObject_Del; + type->tp_free = PyObject_GC_Del; + type->tp_traverse = subtype_traverse; + type->tp_clear = subtype_clear; /* store type in class' cell if one is supplied */ cell = _PyDict_GetItemIdWithError(dict, &PyId___classcell__); From ef2d84d7f5b7bf49a50f93b7138c55b13ea4ce59 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 14 Oct 2020 16:14:09 -0700 Subject: [PATCH 4/8] Add without_gc decorator to _testcapi. --- Modules/_testcapimodule.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 8c7544fa90e280..81ca44b2dc439f 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -3888,6 +3888,23 @@ with_tp_del(PyObject *self, PyObject *args) return obj; } +static PyObject * +without_gc(PyObject *Py_UNUSED(self), PyObject *obj) +{ + PyTypeObject *tp = (PyTypeObject*)obj; + if (!PyType_Check(obj) || !PyType_HasFeature(tp, Py_TPFLAGS_HEAPTYPE)) { + return PyErr_Format(PyExc_TypeError, "heap type expected, got %R", obj); + } + if (PyType_IS_GC(tp)) { + // Don't try this at home, kids: + tp->tp_flags -= Py_TPFLAGS_HAVE_GC; + tp->tp_free = PyObject_Del; + } + assert(!PyType_IS_GC(tp)); + Py_INCREF(obj); + return (PyObject*)obj; +} + static PyMethodDef ml; static PyObject * @@ -5805,6 +5822,7 @@ static PyMethodDef TestMethods[] = { {"meth_fastcall", (PyCFunction)(void(*)(void))meth_fastcall, METH_FASTCALL}, {"meth_fastcall_keywords", (PyCFunction)(void(*)(void))meth_fastcall_keywords, METH_FASTCALL|METH_KEYWORDS}, {"pynumber_tobase", pynumber_tobase, METH_VARARGS}, + {"without_gc", without_gc, METH_O}, {NULL, NULL} /* sentinel */ }; From 65ca804d18194d5ae56a1e4212904284a357d2c0 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 14 Oct 2020 16:14:36 -0700 Subject: [PATCH 5/8] Fix GC tracking in test_finalization. --- Lib/test/test_finalization.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/Lib/test/test_finalization.py b/Lib/test/test_finalization.py index 35d7913e5b89b5..359604d8189143 100644 --- a/Lib/test/test_finalization.py +++ b/Lib/test/test_finalization.py @@ -16,6 +16,15 @@ def __new__(cls, *args, **kwargs): raise TypeError('requires _testcapi.with_tp_del') return C +try: + from _testcapi import without_gc +except ImportError: + def without_gc(cls): + class C: + def __new__(cls, *args, **kwargs): + raise TypeError('requires _testcapi.without_gc') + return C + from test import support @@ -94,9 +103,11 @@ def check_sanity(self): assert self.id_ == id(self) +@without_gc class NonGC(NonGCSimpleBase): __slots__ = () +@without_gc class NonGCResurrector(NonGCSimpleBase): __slots__ = () @@ -178,6 +189,7 @@ def test_simple_resurrect(self): self.assert_survivors([]) self.assertIs(wr(), None) + @support.cpython_only def test_non_gc(self): with SimpleBase.test(): s = NonGC() @@ -191,6 +203,7 @@ def test_non_gc(self): self.assert_del_calls(ids) self.assert_survivors([]) + @support.cpython_only def test_non_gc_resurrect(self): with SimpleBase.test(): s = NonGCResurrector() From f8cc619c8ebfe9b2d27277bda6bedd11d4549b85 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 14 Oct 2020 16:19:57 -0700 Subject: [PATCH 6/8] Add NEWS entry. --- .../Core and Builtins/2020-10-14-16-19-43.bpo-41984.SEtKMr.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2020-10-14-16-19-43.bpo-41984.SEtKMr.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2020-10-14-16-19-43.bpo-41984.SEtKMr.rst b/Misc/NEWS.d/next/Core and Builtins/2020-10-14-16-19-43.bpo-41984.SEtKMr.rst new file mode 100644 index 00000000000000..e70d5dc2b8ddec --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2020-10-14-16-19-43.bpo-41984.SEtKMr.rst @@ -0,0 +1,2 @@ +The garbage collector now tracks all user-defined classes. Patch by Brandt +Bucher. From 20fb9567de8d692c6f99969a1976a223d7390f9c Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 14 Oct 2020 18:13:13 -0700 Subject: [PATCH 7/8] Fix non-CPython finalization tests. --- Lib/test/test_finalization.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_finalization.py b/Lib/test/test_finalization.py index 359604d8189143..1d134430909d84 100644 --- a/Lib/test/test_finalization.py +++ b/Lib/test/test_finalization.py @@ -120,8 +120,14 @@ def side_effect(self): class Simple(SimpleBase): pass -class SimpleResurrector(NonGCResurrector, SimpleBase): - pass +# Can't inherit from NonGCResurrector, in case importing without_gc fails. +class SimpleResurrector(SimpleBase): + + def side_effect(self): + """ + Resurrect self by storing self in a class-wide list. + """ + self.survivors.append(self) class TestBase: From 8f3d1b780e73d2dba31ff148c6c664a2b440f9cb Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Wed, 14 Oct 2020 18:13:25 -0700 Subject: [PATCH 8/8] Minor cleanup. --- Modules/_testcapimodule.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 81ca44b2dc439f..28d2c124d51775 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -3899,10 +3899,12 @@ without_gc(PyObject *Py_UNUSED(self), PyObject *obj) // Don't try this at home, kids: tp->tp_flags -= Py_TPFLAGS_HAVE_GC; tp->tp_free = PyObject_Del; + tp->tp_traverse = NULL; + tp->tp_clear = NULL; } assert(!PyType_IS_GC(tp)); Py_INCREF(obj); - return (PyObject*)obj; + return obj; } static PyMethodDef ml;