GeezerVison is showing github.com — CMultiplyNested closure crash: Python 3.13 regression · Issue #121863 · python/cpython
CMultiplyNested closure crash: Python 3.13 regression #121863DescriptionTest extracted from Cython tests/run/methodmangling_T5.py:
class CMultiplyNested:
def f1(self):
__arg = 1
class D:
def g(self, __arg):
return __arg
return D().g(_CMultiplyNested__arg=2)
def f2(self):
__arg = 1
class D:
def g(self, __arg):
return __arg
return D().g
if __name__ == "__main__":
import faulthandler; faulthandler.enable()
inst = CMultiplyNested()
name = '_CMultiplyNested__arg'
try:
inst.f1()
except TypeError:
print("TypeError")
closure = inst.f2()
closure(_CMultiplyNested__arg=2)Output:
$ ./python bug.py
TypeError
Fatal Python error: Segmentation fault
Current thread 0x00007fa713149740 (most recent call first):
File "/home/vstinner/python/3.13/bug.py", line 27 in <module>
Erreur de segmentation (core dumped)
Linked PRs
Activityda-woods commented on Jul 16, 2024 Just checking - the code here crashes when run with Python and you don't need to compile with Cython to get the bug? vstinner commented on Jul 16, 2024 Regression introduced by commit 9769b7a.
cc @encukou
commit 9769b7ae064a0546a98cbcbec2561dbaba20cd23 (HEAD)
Author: Petr Viktorin <[email protected]>
Date: Mon Jun 24 20:24:19 2024 +0200
[3.13] gh-113993: Allow interned strings to be mortal, and fix related issues (GH-120520) (GH-120945)
vstinner commented on Jul 16, 2024
the code here crashes when run with Python and you don't need to compile with Cython to get the bug?
The reproducer doesn't need Cython at all. Only these few lines of Python code without any import. Reacted by da-woods and Petr Viktorin vstinner commented on Jul 16, 2024 The bug can be worked around by reverting codeobject.c changes:
diff --git a/Objects/codeobject.c b/Objects/codeobject.c
index 7b1244a8d5f..55b512b3f1a 100644
--- a/Objects/codeobject.c
+++ b/Objects/codeobject.c
@@ -137,7 +137,6 @@ static PyObject *intern_one_constant(PyObject *op);
static int
intern_strings(PyObject *tuple)
{
- PyInterpreterState *interp = _PyInterpreterState_GET();
Py_ssize_t i;
for (i = PyTuple_GET_SIZE(tuple); --i >= 0; ) {
@@ -147,7 +146,7 @@ intern_strings(PyObject *tuple)
"non-string found in code slot");
return -1;
}
- _PyUnicode_InternMortal(interp, &_PyTuple_ITEMS(tuple)[i]);
+ PyUnicode_InternInPlace(&_PyTuple_ITEMS(tuple)[i]);
}
return 0;
}
@@ -158,13 +157,12 @@ intern_strings(PyObject *tuple)
static int
intern_constants(PyObject *tuple, int *modified)
{
- PyInterpreterState *interp = _PyInterpreterState_GET();
for (Py_ssize_t i = PyTuple_GET_SIZE(tuple); --i >= 0; ) {
PyObject *v = PyTuple_GET_ITEM(tuple, i);
if (PyUnicode_CheckExact(v)) {
if (should_intern_string(v)) {
PyObject *w = v;
- _PyUnicode_InternMortal(interp, &v);
+ PyUnicode_InternInPlace(&v);
if (w != v) {
PyTuple_SET_ITEM(tuple, i, v);
if (modified) {
@@ -460,13 +458,12 @@ init_code(PyCodeObject *co, struct _PyCodeConstructor *con)
con->stacksize = 1;
}
- PyInterpreterState *interp = _PyInterpreterState_GET();
co->co_filename = Py_NewRef(con->filename);
co->co_name = Py_NewRef(con->name);
co->co_qualname = Py_NewRef(con->qualname);
- _PyUnicode_InternMortal(interp, &co->co_filename);
- _PyUnicode_InternMortal(interp, &co->co_name);
- _PyUnicode_InternMortal(interp, &co->co_qualname);
+ PyUnicode_InternInPlace(&co->co_filename);
+ PyUnicode_InternInPlace(&co->co_name);
+ PyUnicode_InternInPlace(&co->co_qualname);
co->co_flags = con->flags;
co->co_firstlineno = con->firstlineno;
@@ -492,6 +489,7 @@ init_code(PyCodeObject *co, struct _PyCodeConstructor *con)
co->co_framesize = nlocalsplus + con->stacksize + FRAME_SPECIALS_SIZE;
co->co_ncellvars = ncellvars;
co->co_nfreevars = nfreevars;
+ PyInterpreterState *interp = _PyInterpreterState_GET();
#ifdef Py_GIL_DISABLED
PyMutex_Lock(&interp->func_state.mutex);
#endifencukou commented on Jul 17, 2024 Thanks for the bisect!
Immortalizing in intern_strings (a.k.a. the first part of the diff) fixes this for me. That is used for names and localsplusnames; it should be OK to immortalize those (and accept that the memory for these strings will not be reclaimed if the code object is GCd). See discussion in #113993.
PR: #121903 markshannon commented on Jul 17, 2024 intern_strings(PyObject *tuple) looks suspect to me, not because of the interning, but because it modifies a tuple.
That is risky as other code might (rightly) assume that tuples are immutable.
Have you tried having it return a new tuple?
encukou commented on Jul 17, 2024 I'll investigate that. I'd still like to merge the quick partial revert for 3.13.0b4 today. encukou commented on Jul 18, 2024 intern_strings can indeed modify existing tuples. It's not a 3.13 regression, so I filed a new issue: #121954
The regression is fixed, to the best of my knowledge.
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|