Skip to content

[3.8] bpo-41984: GC track all user classes (GH-22701) #22703

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 21 additions & 2 deletions Lib/test/test_finalization.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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__ = ()

Expand All @@ -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:
Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand Down
6 changes: 3 additions & 3 deletions Lib/test/test_gc.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
The garbage collector now tracks all user-defined classes. Patch by Brandt
Bucher.
20 changes: 20 additions & 0 deletions Modules/_testcapimodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 *
Expand Down Expand Up @@ -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 */
};

Expand Down
22 changes: 6 additions & 16 deletions Objects/typeobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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__);
Expand Down