summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMark Dickinson <mdickinson@enthought.com>2022-06-25 14:11:58 (GMT)
committerGitHub <noreply@github.com>2022-06-25 14:11:58 (GMT)
commit6b865349aae47b90f9ef0b98f3fe3720c2f05601 (patch)
tree9896212e12df59b30f9302fdeed61895f0600e3e
parent944c7d8a8561d4b637af5c128df1d8d7570ccb46 (diff)
downloadcpython-6b865349aae47b90f9ef0b98f3fe3720c2f05601.zip
cpython-6b865349aae47b90f9ef0b98f3fe3720c2f05601.tar.gz
cpython-6b865349aae47b90f9ef0b98f3fe3720c2f05601.tar.bz2
gh-94207: Fix struct module leak (GH-94239)
Make _struct.Struct a GC type This fixes a memory leak in the _struct module, where as soon as a Struct object is stored in the cache, there's a cycle from the _struct module to the cache to Struct objects to the Struct type back to the module. If _struct.Struct is not gc-tracked, that cycle is never collected. This PR makes _struct.Struct GC-tracked, and adds a regression test.
-rw-r--r--Lib/test/test_struct.py17
-rw-r--r--Misc/NEWS.d/next/Library/2022-06-24-19-23-59.gh-issue-94207.VhS1eS.rst2
-rw-r--r--Modules/_struct.c22
3 files changed, 39 insertions, 2 deletions
diff --git a/Lib/test/test_struct.py b/Lib/test/test_struct.py
index 94873ff..8f14ed3 100644
--- a/Lib/test/test_struct.py
+++ b/Lib/test/test_struct.py
@@ -1,10 +1,12 @@
from collections import abc
import array
+import gc
import math
import operator
import unittest
import struct
import sys
+import weakref
from test import support
from test.support import import_helper
@@ -672,6 +674,21 @@ class StructTest(unittest.TestCase):
self.assertIn(b"Exception ignored in:", stderr)
self.assertIn(b"C.__del__", stderr)
+ def test__struct_reference_cycle_cleaned_up(self):
+ # Regression test for python/cpython#94207.
+
+ # When we create a new struct module, trigger use of its cache,
+ # and then delete it ...
+ _struct_module = import_helper.import_fresh_module("_struct")
+ module_ref = weakref.ref(_struct_module)
+ _struct_module.calcsize("b")
+ del _struct_module
+
+ # Then the module should have been garbage collected.
+ gc.collect()
+ self.assertIsNone(
+ module_ref(), "_struct module was not garbage collected")
+
def test_issue35714(self):
# Embedded null characters should not be allowed in format strings.
for s in '\0', '2\0i', b'\0':
diff --git a/Misc/NEWS.d/next/Library/2022-06-24-19-23-59.gh-issue-94207.VhS1eS.rst b/Misc/NEWS.d/next/Library/2022-06-24-19-23-59.gh-issue-94207.VhS1eS.rst
new file mode 100644
index 0000000..3d38524
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2022-06-24-19-23-59.gh-issue-94207.VhS1eS.rst
@@ -0,0 +1,2 @@
+Made :class:`_struct.Struct` GC-tracked in order to fix a reference leak in
+the :mod:`_struct` module.
diff --git a/Modules/_struct.c b/Modules/_struct.c
index 4daf952..20307ad 100644
--- a/Modules/_struct.c
+++ b/Modules/_struct.c
@@ -1496,10 +1496,26 @@ Struct___init___impl(PyStructObject *self, PyObject *format)
return ret;
}
+static int
+s_clear(PyStructObject *s)
+{
+ Py_CLEAR(s->s_format);
+ return 0;
+}
+
+static int
+s_traverse(PyStructObject *s, visitproc visit, void *arg)
+{
+ Py_VISIT(Py_TYPE(s));
+ Py_VISIT(s->s_format);
+ return 0;
+}
+
static void
s_dealloc(PyStructObject *s)
{
PyTypeObject *tp = Py_TYPE(s);
+ PyObject_GC_UnTrack(s);
if (s->weakreflist != NULL)
PyObject_ClearWeakRefs((PyObject *)s);
if (s->s_codes != NULL) {
@@ -2078,13 +2094,15 @@ static PyType_Slot PyStructType_slots[] = {
{Py_tp_getattro, PyObject_GenericGetAttr},
{Py_tp_setattro, PyObject_GenericSetAttr},
{Py_tp_doc, (void*)s__doc__},
+ {Py_tp_traverse, s_traverse},
+ {Py_tp_clear, s_clear},
{Py_tp_methods, s_methods},
{Py_tp_members, s_members},
{Py_tp_getset, s_getsetlist},
{Py_tp_init, Struct___init__},
{Py_tp_alloc, PyType_GenericAlloc},
{Py_tp_new, s_new},
- {Py_tp_free, PyObject_Del},
+ {Py_tp_free, PyObject_GC_Del},
{0, 0},
};
@@ -2092,7 +2110,7 @@ static PyType_Spec PyStructType_spec = {
"_struct.Struct",
sizeof(PyStructObject),
0,
- Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,
+ Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_BASETYPE,
PyStructType_slots
};