gh-94207: Fix struct module leak (GH-94239) (GH-94265)

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.
(cherry picked from commit 6b865349aa)

Co-authored-by: Mark Dickinson <mdickinson@enthought.com>
This commit is contained in:
Miss Islington (bot) 2022-06-25 07:40:14 -07:00 committed by GitHub
parent 89ba660717
commit 4bc5f9fe8c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 39 additions and 2 deletions

View file

@ -1,10 +1,12 @@
from collections import abc from collections import abc
import array import array
import gc
import math import math
import operator import operator
import unittest import unittest
import struct import struct
import sys import sys
import weakref
from test import support from test import support
from test.support import import_helper from test.support import import_helper
@ -672,6 +674,21 @@ class StructTest(unittest.TestCase):
self.assertIn(b"Exception ignored in:", stderr) self.assertIn(b"Exception ignored in:", stderr)
self.assertIn(b"C.__del__", 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): def test_issue35714(self):
# Embedded null characters should not be allowed in format strings. # Embedded null characters should not be allowed in format strings.
for s in '\0', '2\0i', b'\0': for s in '\0', '2\0i', b'\0':

View file

@ -0,0 +1,2 @@
Made :class:`_struct.Struct` GC-tracked in order to fix a reference leak in
the :mod:`_struct` module.

View file

@ -1496,10 +1496,26 @@ Struct___init___impl(PyStructObject *self, PyObject *format)
return ret; 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 static void
s_dealloc(PyStructObject *s) s_dealloc(PyStructObject *s)
{ {
PyTypeObject *tp = Py_TYPE(s); PyTypeObject *tp = Py_TYPE(s);
PyObject_GC_UnTrack(s);
if (s->weakreflist != NULL) if (s->weakreflist != NULL)
PyObject_ClearWeakRefs((PyObject *)s); PyObject_ClearWeakRefs((PyObject *)s);
if (s->s_codes != NULL) { if (s->s_codes != NULL) {
@ -2078,13 +2094,15 @@ static PyType_Slot PyStructType_slots[] = {
{Py_tp_getattro, PyObject_GenericGetAttr}, {Py_tp_getattro, PyObject_GenericGetAttr},
{Py_tp_setattro, PyObject_GenericSetAttr}, {Py_tp_setattro, PyObject_GenericSetAttr},
{Py_tp_doc, (void*)s__doc__}, {Py_tp_doc, (void*)s__doc__},
{Py_tp_traverse, s_traverse},
{Py_tp_clear, s_clear},
{Py_tp_methods, s_methods}, {Py_tp_methods, s_methods},
{Py_tp_members, s_members}, {Py_tp_members, s_members},
{Py_tp_getset, s_getsetlist}, {Py_tp_getset, s_getsetlist},
{Py_tp_init, Struct___init__}, {Py_tp_init, Struct___init__},
{Py_tp_alloc, PyType_GenericAlloc}, {Py_tp_alloc, PyType_GenericAlloc},
{Py_tp_new, s_new}, {Py_tp_new, s_new},
{Py_tp_free, PyObject_Del}, {Py_tp_free, PyObject_GC_Del},
{0, 0}, {0, 0},
}; };
@ -2092,7 +2110,7 @@ static PyType_Spec PyStructType_spec = {
"_struct.Struct", "_struct.Struct",
sizeof(PyStructObject), sizeof(PyStructObject),
0, 0,
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_BASETYPE,
PyStructType_slots PyStructType_slots
}; };