8000 [3.12] gh-122527: Fix a crash on deallocation of `PyStructSequence` (… · python/cpython@1bfb58c · GitHub
[go: up one dir, main page]
Skip to content

Commit 1bfb58c

Browse files
colesburycolesbury
authored
[3.12] gh-122527: Fix a crash on deallocation of PyStructSequence (GH-122577) (#122626)
The `PyStructSequence` destructor would crash if it was deallocated after its type's dictionary was cleared by the GC, because it couldn't compute the "real size" of the instance. This could occur with relatively straightforward code in the free-threaded build or with a reference cycle involving the type in the default build, due to differing orders in which `tp_clear()` was called. Account for the non-sequence fields in `tp_basicsize` and use that, along with `Py_SIZE()`, to compute the "real" size of a `PyStructSequence` in the dealloc function. This avoids the accesses to the type's dictionary during dealloc, which were unsafe. (cherry picked from commit 4b63cd1)
1 parent 74feab2 commit 1bfb58c

File tree

3 files changed

+39
-6
lines changed

3 files changed

+39
-6
lines changed

‎Lib/test/test_structseq.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import copy
22
import os
33
import pickle
4+
import textwrap
45
import time
56
import unittest
7+
from test.support import script_helper
68

79

810
class StructSeqTest(unittest.TestCase):
@@ -204,6 +206,17 @@ def test_match_args_with_unnamed_fields(self):
204206
self.assertEqual(os.stat_result.n_unnamed_fields, 3)
205207
self.assertEqual(os.stat_result.__match_args__, expected_args)
206208

209+
def test_reference_cycle(self):
210+
10000 # gh-122527: Check that a structseq that's part of a reference cycle
211+
# with its own type doesn't crash. Previously, if the type's dictionary
212+
# was cleared first, the structseq instance would crash in the
213+
# destructor.
214+
script_helper.assert_python_ok("-c", textwrap.dedent(r"""
215+
import time
216+
t = time.gmtime()
217+
type(t).refcyle = t
218+
"""))
219+
207220

208221
if __name__ == "__main__":
209222
unittest.main()

‎Misc/NEWS.d/next/Core_and_Builtins/2024-08-01-19-13-58.gh-issue-122527.eztso6.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix a crash that occurred when a ``PyStructSequence`` was deallocated after
2+
its type's dictionary was cleared by the GC. The type's
3+
:c:member:`~PyTypeObject.tp_basicsize` now accounts for non-sequence fields
4+
that aren't included in the :c:macro:`Py_SIZE` of the sequence.

‎Objects/structseq.c

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,20 @@ get_type_attr_as_size(PyTypeObject *tp, PyObject *name)
4141
get_type_attr_as_size(tp, &_Py_ID(n_sequence_fields))
4242
#define REAL_SIZE_TP(tp) \
4343
get_type_attr_as_size(tp, &_Py_ID(n_fields))
44-
#define REAL_SIZE(op) REAL_SIZE_TP(Py_TYPE(op))
44+
#define REAL_SIZE(op) get_real_size((PyObject *)op)
4545

4646
#define UNNAMED_FIELDS_TP(tp) \
4747
get_type_attr_as_size(tp, &_Py_ID(n_unnamed_fields))
4848
#define UNNAMED_FIELDS(op) UNNAMED_FIELDS_TP(Py_TYPE(op))
4949

50+
static Py_ssize_t
51+
get_real_size(PyObject *op)
52+
{
53+
// Compute the real size from the visible size (i.e., Py_SIZE()) and the
54+
// number of non-sequence fields accounted for in tp_basicsize.
55+
Py_ssize_t hidden = Py_TYPE(op)->tp_basicsize - offsetof(PyStructSequence, ob_item);
56+
return Py_SIZE(op) + hidden / sizeof(PyObject *);
57+
}
5058

5159
PyObject *
5260
PyStructSequence_New(PyTypeObject *type)
@@ -107,6 +115,9 @@ structseq_dealloc(PyStructSequence *obj)
107115
PyObject_GC_UnTrack(obj);
108116

109117
PyTypeObject *tp = Py_TYPE(obj);
118+
// gh-122527: We can't use REAL_SIZE_TP() or any macros that access the
119+
// type's dictionary here, because the dictionary may have already been
120+
// cleared by the garbage collector.
110121
size = REAL_SIZE(obj);
111122
for (i = 0; i < size; ++i) {
112123
Py_XDECREF(obj->ob_item[i]);
@@ -467,10 +478,14 @@ initialize_members(PyStructSequence_Desc *desc,
467478

468479
static void
469480
initialize_static_fields(PyTypeObject *type, PyStructSequence_Desc *desc,
470-
PyMemberDef *tp_members, unsigned long tp_flags)
481+
PyMemberDef *tp_members, Py_ssize_t n_members,
482+
unsigned long tp_flags)
471483
{
472484
type->tp_name = desc->name;
473-
type->tp_basicsize = sizeof(PyStructSequence) - sizeof(PyObject *);
485+
// Account for hidden members in tp_basicsize because they are not
486+
// included in the variable size.
487+
Py_ssize_t n_hidden = n_members - desc->n_in_sequence;
488+
type->tp_basicsize = sizeof(PyStructSequence) + (n_hidden - 1) * sizeof(PyObject *);
474489
type->tp_itemsize = sizeof(PyObject *);
475490
type->tp_dealloc = (destructor)structseq_dealloc;
476491
type->tp_repr = (reprfunc)structseq_repr;
@@ -520,7 +535,7 @@ _PyStructSequence_InitBuiltinWithFlags(PyInterpreterState *interp,
520535
if (members == NULL) {
521536
goto error;
522537
}
523-
initialize_static_fields(type, desc, members, tp_flags);
538+
initialize_static_fields(type, desc, members, n_members, tp_flags);
524539

525540
_Py_SetImmortal(type);
526541
}
@@ -582,7 +597,7 @@ PyStructSequence_InitType2(PyTypeObject *type, PyStructSequence_Desc *desc)
582597
if (members == NULL) {
583598
return -1;
584599
}
585-
initialize_static_fields(type, desc, members, 0);
600+
initialize_static_fields(type, desc, members, n_members, 0);
586601
if (initialize_static_type(type, desc, n_members, n_unnamed_members) < 0) {
587602
PyMem_Free(members);
588603
return -1;
@@ -658,7 +673,8 @@ _PyStructSequence_NewType(PyStructSequence_Desc *desc, unsigned long tp_flags)
658673
/* The name in this PyType_Spec is statically allocated so it is */
659674
/* expected that it'll outlive the PyType_Spec */
660675
spec.name = desc->name;
661-
spec.basicsize = sizeof(PyStructSequence) - sizeof(PyObject *);
676+
Py_ssize_t hidden = n_members - desc->n_in_sequence;
677+
spec.basicsize = (int)(sizeof(PyStructSequence) + (hidden - 1) * sizeof(PyObject *));
662678
spec.itemsize = sizeof(PyObject *);
663679
spec.flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | tp_flags;
664680
spec.slots = slots;

0 commit comments

Comments
 (0)
0