[Python-checkins] bpo-41984: GC track all user classes (GH-22701/GH-22707)

Thu Oct 15 11:51:56 EDT 2020

https://github.com/python/cpython/commit/aeb66c1abbf4ec214e2e80eb972546996d1a1571
commit: aeb66c1abbf4ec214e2e80eb972546996d1a1571
branch: 3.8
author: Miss Skeleton (bot) <31488909+miss-islington at users.noreply.github.com>
committer: GitHub <noreply at github.com>
date: 2020年10月15日T08:51:48-07:00
summary:
bpo-41984: GC track all user classes (GH-22701/GH-22707)
(cherry picked from commit c13b847a6f913b72eeb71651ff626390b738d973)
files:
A Misc/NEWS.d/next/Core and Builtins/2020-10-14-16-19-43.bpo-41984.SEtKMr.rst
M Lib/test/test_finalization.py
M Lib/test/test_gc.py
M Modules/_testcapimodule.c
M Objects/typeobject.c

diff --git a/Lib/test/test_finalization.py b/Lib/test/test_finalization.py
index 35d7913e5b89b..1d134430909d8 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)
 
 
+ at without_gc
 class NonGC(NonGCSimpleBase):
 __slots__ = ()
 
+ at without_gc
 class NonGCResurrector(NonGCSimpleBase):
 __slots__ = ()
 
@@ -109,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:
@@ -178,6 +195,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 +209,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()
diff --git a/Lib/test/test_gc.py b/Lib/test/test_gc.py
index f52db1eab169c..58d461334b955 100644
--- a/Lib/test/test_gc.py
+++ b/Lib/test/test_gc.py
@@ -576,9 +576,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_bug1055820b(self):
 # Corresponds to temp2b.py in the bug report.
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 0000000000000..e70d5dc2b8dde
--- /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.
diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
index af28af50c0650..5089f593da4d6 100644
--- a/Modules/_testcapimodule.c
+++ b/Modules/_testcapimodule.c
@@ -3511,6 +3511,25 @@ 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;
+ tp->tp_traverse = NULL;
+ tp->tp_clear = NULL;
+ }
+ assert(!PyType_IS_GC(tp));
+ Py_INCREF(obj);
+ return obj;
+}
+
 static PyMethodDef ml;
 
 static PyObject *
@@ -5325,6 +5344,7 @@ static PyMethodDef TestMethods[] = {
 #endif
 {"write_unraisable_exc", test_write_unraisable_exc, METH_VARARGS},
 {"pynumber_tobase", pynumber_tobase, METH_VARARGS},
+ {"without_gc", without_gc, METH_O},
 {NULL, NULL} /* sentinel */
 };
 
diff --git a/Objects/typeobject.c b/Objects/typeobject.c
index 0acde714a87b1..21b1360b4bbca 100644
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -2577,10 +2577,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;
@@ -2777,21 +2777,11 @@ 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;
-
 /* 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__);
