Skip to content

bpo-44553 : Implement GC methods for types.Union #26993

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

Merged
merged 3 commits into from
Jul 3, 2021
Merged
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
20 changes: 19 additions & 1 deletion Lib/test/test_types.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
# Python test set -- part 6, built-in types

from test.support import run_with_locale
from test.support import run_with_locale, cpython_only
import collections.abc
from collections import namedtuple
import gc
import inspect
import pickle
import locale
Expand Down Expand Up @@ -756,6 +757,23 @@ def __module__(self):
with self.assertRaises(ZeroDivisionError):
str | TypeVar()

@cpython_only
def test_or_type_operator_reference_cycle(self):
if not hasattr(sys, 'gettotalrefcount'):
self.skipTest('Cannot get total reference count.')
gc.collect()
before = sys.gettotalrefcount()
for _ in range(30):
T = typing.TypeVar('T')
U = int | list[T]
T.blah = U
del T
del U
gc.collect()
leeway = 15
self.assertLessEqual(sys.gettotalrefcount() - before, leeway,
msg='Check for union reference leak.')

def test_ellipsis_type(self):
self.assertIsInstance(Ellipsis, types.EllipsisType)

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Implement GC methods for ``types.Union`` to break reference cycles and
prevent memory leaks.
21 changes: 17 additions & 4 deletions Objects/unionobject.c
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// types.Union -- used to represent e.g. Union[int, str], int | str
#include "Python.h"
#include "pycore_object.h" // _PyObject_GC_TRACK/UNTRACK
#include "pycore_unionobject.h"
#include "structmember.h"

Expand All @@ -14,10 +15,20 @@ unionobject_dealloc(PyObject *self)
{
unionobject *alias = (unionobject *)self;

_PyObject_GC_UNTRACK(self);

Py_XDECREF(alias->args);
Py_TYPE(self)->tp_free(self);
}

static int
union_traverse(PyObject *self, visitproc visit, void *arg)
{
unionobject *alias = (unionobject *)self;
Py_VISIT(alias->args);
return 0;
}

static Py_hash_t
union_hash(PyObject *self)
{
Expand Down Expand Up @@ -437,8 +448,9 @@ PyTypeObject _Py_UnionType = {
.tp_basicsize = sizeof(unionobject),
.tp_dealloc = unionobject_dealloc,
.tp_alloc = PyType_GenericAlloc,
.tp_free = PyObject_Del,
.tp_flags = Py_TPFLAGS_DEFAULT,
.tp_free = PyObject_GC_Del,
.tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC,
.tp_traverse = union_traverse,
.tp_hash = union_hash,
.tp_getattro = PyObject_GenericGetAttr,
.tp_members = union_members,
Expand Down Expand Up @@ -472,15 +484,16 @@ _Py_Union(PyObject *args)
}
}

result = PyObject_New(unionobject, &_Py_UnionType);
result = PyObject_GC_New(unionobject, &_Py_UnionType);
if (result == NULL) {
return NULL;
}

result->args = dedup_and_flatten_args(args);
if (result->args == NULL) {
Py_DECREF(result);
PyObject_GC_Del(result);
Copy link
Member

@pablogsal pablogsal Jul 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is incorrect, this should still be Py_DECREF, PyObject_GC_Del cannot be called like that because it overrides a bunch of cleanups like the call to _Py_ForgetReference.

return NULL;
}
_PyObject_GC_TRACK(result);
return (PyObject*)result;
}