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. (cherry picked from commit 6b86534) Co-authored-by: Mark Dickinson <mdickinson@enthought.com>
python · mdickinson · Jun 25, 2022 · Jun 25, 2022 · Jun 25, 2022 · c892a5bf5672599d1f6f6fc71a3b432203244f4c
commit c892a5bf5672599d1f6f6fc71a3b432203244f4c
diff --git 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 @@ def __del__(self):
         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
@@ -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
@@ -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,21 +2094,23 @@ 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},
 };
 
 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
 };
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		Made :class:`_struct.Struct` GC-tracked in order to fix a reference leak in
		the :mod:`_struct` module.