8000 [3.12] gh-119213: Be More Careful About _PyArg_Parser.kwtuple Across … · python/cpython@0d5fe2c · GitHub
[go: up one dir, main page]
Skip to content

Commit 0d5fe2c

Browse files
ericsnowcurrentlyericsnowcurrently
authored
[3.12] gh-119213: Be More Careful About _PyArg_Parser.kwtuple Across Interpreters (gh-119331) (gh-119425)
_PyArg_Parser holds static global data generated for modules by Argument Clinic. The _PyArg_Parser.kwtuple field is a tuple object, even though it's stored within a static global. In some cases the tuple is statically allocated and thus it's okay that it gets shared by multiple interpreters. However, in other cases the tuple is set lazily, allocated from the heap using the active interprepreter at the point the tuple is needed. This is a problem once that interpreter is destroyed since _PyArg_Parser.kwtuple becomes at dangling pointer, leading to crashes. It isn't a problem if the tuple is allocated under the main interpreter, since its lifetime is bound to the lifetime of the runtime. The solution here is to temporarily switch to the main interpreter. The alternative would be to always statically allocate the tuple. This change also fixes a bug where only the most recent parser was added to the global linked list. (cherry picked from commit 8186500)
1 parent 7eb59cd commit 0d5fe2c

10 files changed

+1032
-869
lines changed

‎Doc/data/python3.12.abi

Lines changed: 893 additions & 868 deletions
Large diffs are not rendered by default.

‎Include/internal/pycore_global_objects_fini_generated.h

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Include/internal/pycore_global_strings.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -675,6 +675,7 @@ struct _Py_global_strings {
675675
STRUCT_FOR_ID(sound)
676676
STRUCT_FOR_ID(source)
677677
STRUCT_FOR_ID(source_traceback)
678+
STRUCT_FOR_ID(spam)
678679
STRUCT_FOR_ID(src)
679680
STRUCT_FOR_ID(src_dir_fd)
680681
STRUCT_FOR_ID(stacklevel)

‎Include/internal/pycore_runtime_init_generated.h

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Include/internal/pycore_unicodeobject_generated.h

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Lib/test/test_capi/test_getargs.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,17 @@
44
import sys
55
from test import support
66
from test.support import import_helper
7+
from test.support import script_helper
78
from test.support import warnings_helper
89
# Skip this test if the _testcapi module isn't available.
910
_testcapi = import_helper.import_module('_testcapi')
1011
from _testcapi import getargs_keywords, getargs_keyword_only
1112

13+
try:
14+
import _testinternalcapi
15+
except ImportError:
16+
_testinternalcapi = NULL
17+
1218
# > How about the following counterproposal. This also changes some of
1319
# > the other format codes to be a little more regular.
1420
# >
@@ -1369,6 +1375,33 @@ def test_nested_tuple(self):
13691375
"argument 1 must be sequence of length 1, not 0"):
13701376
parse(((),), {}, '(' + f + ')', ['a'])
13711377

1378+
@unittest.skipIf(_testinternalcapi is None, 'needs _testinternalcapi')
1379+
def test_gh_119213(self):
1380+
rc, out, err = script_helper.assert_python_ok("-c", """if True:
1381+
from test import support
1382+
script = '''if True:
1383+
import _testinternalcapi
1384+
_testinternalcapi.gh_119213_getargs(spam='eggs')
1385+
'''
1386+
config = dict(
1387+
allow_fork=False,
1388+
allow_exec=False,
1389+
allow_threads=True,
1390+
allow_daemon_threads=False,
1391+
use_main_obmalloc=False,
1392+
gil=2,
1393+
check_multi_interp_extensions=True,
1394+
)
1395+
rc = support.run_in_subinterp_with_config(script, **config)
1396+
assert rc == 0
1397+
1398+
# The crash is different if the interpreter was not destroyed first.
1399+
#interpid = _testinternalcapi.create_interpreter()
1400+
#rc = _testinternalcapi.exec_interpreter(interpid, script)
1401+
#assert rc == 0
1402+
""")
1403+
self.assertEqual(rc, 0)
1404+
13721405

13731406
if __name__ == "__main__":
13741407
unittest.main()

‎Misc/NEWS.d/next/Core and Builtins/2024-05-21-11-27-14.gh-issue-119213.nxjxrt.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Non-builtin modules built with argument clinic were crashing if used in a
2+
subinterpreter before the main interpreter. The objects that were causing
3+
the problem by leaking between interpreters carelessly have been fixed.

‎Modules/_testinternalcapi.c

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1055,6 +1055,24 @@ pending_identify(PyObject *self, PyObject *args)
10551055
}
10561056

10571057

1058+
/*[clinic input]
1059+
gh_119213_getargs
1060+
1061+
spam: object = None
1062+
1063+
Test _PyArg_Parser.kwtuple
1064+
[clinic start generated code]*/
1065+
1066+
static PyObject *
1067+
gh_119213_getargs_impl(PyObject *module, PyObject *spam)
1068+
/*[clinic end generated code: output=d8d9c95d5b446802 input=65ef47511da80fc2]*/
1069+
{
1070+
// It must never have been called in the main interprer
1071+
assert(!_Py_IsMainInterpreter(PyInterpreterState_Get()));
1072+
return Py_NewRef(spam);
1073+
}
1074+
1075+
10581076
static PyMethodDef module_functions[] = {
10591077
{"get_configs", get_configs, METH_NOARGS},
10601078
{"get_recursion_depth", get_recursion_depth, METH_NOARGS},
@@ -1087,6 +1105,7 @@ static PyMethodDef module_functions[] = {
10871105
{"pending_threadfunc", _PyCFunction_CAST(pending_threadfunc),
10881106
METH_VARARGS | METH_KEYWORDS},
10891107
{"pending_identify", pending_identify, METH_VARARGS, NULL},
1108+
GH_119213_GETARGS_METHODDEF
10901109
{NULL, NULL} /* sentinel */
10911110
};
10921111

‎Modules/clinic/_testinternalcapi.c.h

Lines changed: 61 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Python/getargs.c

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include "Python.h"
55
#include "pycore_tuple.h" // _PyTuple_ITEMS()
66
#include "pycore_pylifecycle.h" // _PyArg_Fini
7+
#include "pycore_pystate.h" // _Py_IsMainInterpreter()
78

89
#include <ctype.h>
910
#include <float.h>
@@ -2002,7 +2003,23 @@ _parser_init(struct _PyArg_Parser *parser)
20022003
int owned;
20032004
PyObject *kwtuple = parser->kwtuple;
20042005
if (kwtuple == NULL) {
2006+
/* We may temporarily switch to the main interpreter to avoid
2007+
* creating a tuple that could outlive its owning interpreter. */
2008+
PyThreadState *save_tstate = NULL;
2009+
PyThreadState *temp_tstate = NULL;
2010+
if (!_Py_IsMainInterpreter(PyInterpreterState_Get())) {
2011+
temp_tstate = PyThreadState_New(_PyInterpreterState_Main());
2012+
if (temp_tstate == NULL) {
2013+
return -1;
2014+
}
2015+
save_tstate = PyThreadState_Swap(temp_tstate);
2016+
}
20052017
kwtuple = new_kwtuple(keywords, len, pos);
2018+
if (temp_tstate != NULL) {
2019+
PyThreadState_Clear(temp_tstate);
2020+
(void)PyThreadState_Swap(save_tstate);
2021+
PyThreadState_Delete(temp_tstate);
2022+
}
20062023
if (kwtuple == NULL) {
20072024
return 0;
20082025
}

0 commit comments

Comments
 (0)
0