mirror of
https://github.com/python/cpython.git
synced 2026-06-28 11:50:50 +00:00
gh-151218: Replace sys.flags in PyConfig_Set() (#151402)
PyConfig_Set() and sys.set_int_max_str_digits() now replace sys.flags (create a new object), instead of modifying sys.flags in-place. Modifying sys.flags in-place can lead to data races when multiple threads are reading or writing sys.flags in parallel. Use _Py_atomic functions to get and set max_str_digits members. Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
This commit is contained in:
parent
2ce260033b
commit
b16d23fc9f
9 changed files with 111 additions and 23 deletions
|
|
@ -623,6 +623,10 @@ Some options are read from the :mod:`sys` attributes. For example, the option
|
|||
|
||||
.. versionadded:: 3.14
|
||||
|
||||
.. versionchanged:: next
|
||||
The function now replaces :data:`sys.flags` (create a new object),
|
||||
instead of modifying :data:`sys.flags` in-place.
|
||||
|
||||
|
||||
.. _pyconfig_api:
|
||||
|
||||
|
|
|
|||
|
|
@ -356,9 +356,11 @@ def expect_bool_not(value):
|
|||
for value in new_values:
|
||||
expected, expect_flag = expect_func(value)
|
||||
|
||||
old_flags = sys.flags
|
||||
config_set(name, value)
|
||||
self.assertEqual(config_get(name), expected)
|
||||
self.assertEqual(getattr(sys.flags, sys_flag), expect_flag)
|
||||
self.assertIsNot(sys.flags, old_flags)
|
||||
if name == "write_bytecode":
|
||||
self.assertEqual(getattr(sys, "dont_write_bytecode"),
|
||||
expect_flag)
|
||||
|
|
|
|||
28
Lib/test/test_free_threading/test_sys.py
Normal file
28
Lib/test/test_free_threading/test_sys.py
Normal file
|
|
@ -0,0 +1,28 @@
|
|||
import sys
|
||||
import unittest
|
||||
from test.support import threading_helper
|
||||
|
||||
|
||||
class SysModuleTest(unittest.TestCase):
|
||||
def test_int_max_str_digits_thread(self):
|
||||
# gh-151218: Check that it's safe to call get_int_max_str_digits() and
|
||||
# set_int_max_str_digits() in parallel. Previously, this test triggered
|
||||
# warnings in TSan on a free threaded build.
|
||||
|
||||
old_limit = sys.get_int_max_str_digits()
|
||||
self.addCleanup(sys.set_int_max_str_digits, old_limit)
|
||||
|
||||
def worker(worker_id):
|
||||
if not worker_id:
|
||||
for i in range (20_000):
|
||||
sys.get_int_max_str_digits()
|
||||
else:
|
||||
for i in range (20_000):
|
||||
sys.set_int_max_str_digits(4300 + (i & 7))
|
||||
|
||||
workers = [lambda: worker(i) for i in range(5)]
|
||||
threading_helper.run_concurrently(workers)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
|
@ -1349,6 +1349,26 @@ def test_pystats(self):
|
|||
def test_disable_gil_abi(self):
|
||||
self.assertEqual('t' in sys.abiflags, support.Py_GIL_DISABLED)
|
||||
|
||||
def test_int_max_str_digits(self):
|
||||
old_limit = sys.get_int_max_str_digits()
|
||||
self.assertIsInstance(old_limit, int)
|
||||
self.assertGreaterEqual(old_limit, 0)
|
||||
self.addCleanup(sys.set_int_max_str_digits, old_limit)
|
||||
|
||||
sys.set_int_max_str_digits(0)
|
||||
self.assertEqual(sys.get_int_max_str_digits(), 0)
|
||||
|
||||
sys.set_int_max_str_digits(2_048)
|
||||
self.assertEqual(sys.get_int_max_str_digits(), 2_048)
|
||||
|
||||
with self.assertRaises(ValueError):
|
||||
# the minimum is 640 digits
|
||||
sys.set_int_max_str_digits(5)
|
||||
with self.assertRaises(ValueError):
|
||||
sys.set_int_max_str_digits(-2)
|
||||
with self.assertRaises(TypeError):
|
||||
sys.set_int_max_str_digits(2_048.0)
|
||||
|
||||
|
||||
@test.support.cpython_only
|
||||
@test.support.force_not_colorized_test_class
|
||||
|
|
|
|||
|
|
@ -0,0 +1,3 @@
|
|||
:c:func:`PyConfig_Set` and :func:`sys.set_int_max_str_digits` now replace
|
||||
:data:`sys.flags` (create a new object), instead of modifying :data:`sys.flags`
|
||||
in-place. Patch by Victor Stinner.
|
||||
|
|
@ -2125,7 +2125,7 @@ long_to_decimal_string_internal(PyObject *aa,
|
|||
if (size_a >= 10 * _PY_LONG_MAX_STR_DIGITS_THRESHOLD
|
||||
/ (3 * PyLong_SHIFT) + 2) {
|
||||
PyInterpreterState *interp = _PyInterpreterState_GET();
|
||||
int max_str_digits = interp->long_state.max_str_digits;
|
||||
int max_str_digits = _Py_atomic_load_int(&interp->long_state.max_str_digits);
|
||||
if ((max_str_digits > 0) &&
|
||||
(max_str_digits / (3 * PyLong_SHIFT) <= (size_a - 11) / 10)) {
|
||||
PyErr_Format(PyExc_ValueError, _MAX_STR_DIGITS_ERROR_FMT_TO_STR,
|
||||
|
|
@ -2206,7 +2206,7 @@ long_to_decimal_string_internal(PyObject *aa,
|
|||
}
|
||||
if (strlen > _PY_LONG_MAX_STR_DIGITS_THRESHOLD) {
|
||||
PyInterpreterState *interp = _PyInterpreterState_GET();
|
||||
int max_str_digits = interp->long_state.max_str_digits;
|
||||
int max_str_digits = _Py_atomic_load_int(&interp->long_state.max_str_digits);
|
||||
Py_ssize_t strlen_nosign = strlen - negative;
|
||||
if ((max_str_digits > 0) && (strlen_nosign > max_str_digits)) {
|
||||
Py_DECREF(scratch);
|
||||
|
|
@ -3021,7 +3021,7 @@ long_from_string_base(const char **str, int base, PyLongObject **res)
|
|||
* quadratic algorithm. */
|
||||
if (digits > _PY_LONG_MAX_STR_DIGITS_THRESHOLD) {
|
||||
PyInterpreterState *interp = _PyInterpreterState_GET();
|
||||
int max_str_digits = interp->long_state.max_str_digits;
|
||||
int max_str_digits = _Py_atomic_load_int(&interp->long_state.max_str_digits);
|
||||
if ((max_str_digits > 0) && (digits > max_str_digits)) {
|
||||
PyErr_Format(PyExc_ValueError, _MAX_STR_DIGITS_ERROR_FMT_TO_INT,
|
||||
max_str_digits, digits);
|
||||
|
|
|
|||
|
|
@ -4641,7 +4641,8 @@ config_get(const PyConfig *config, const PyConfigSpec *spec,
|
|||
|
||||
if (strcmp(spec->name, "int_max_str_digits") == 0) {
|
||||
PyInterpreterState *interp = _PyInterpreterState_GET();
|
||||
return PyLong_FromLong(interp->long_state.max_str_digits);
|
||||
int maxdigits = _Py_atomic_load_int(&interp->long_state.max_str_digits);
|
||||
return PyLong_FromLong(maxdigits);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -429,7 +429,8 @@ interpreter_update_config(PyThreadState *tstate, int only_update_path_config)
|
|||
}
|
||||
}
|
||||
|
||||
tstate->interp->long_state.max_str_digits = config->int_max_str_digits;
|
||||
_Py_atomic_store_int(&tstate->interp->long_state.max_str_digits,
|
||||
config->int_max_str_digits);
|
||||
|
||||
// Update the sys module for the new configuration
|
||||
if (_PySys_UpdateConfig(tstate) < 0) {
|
||||
|
|
|
|||
|
|
@ -1874,7 +1874,8 @@ sys_get_int_max_str_digits_impl(PyObject *module)
|
|||
/*[clinic end generated code: output=0042f5e8ae0e8631 input=77fb74e987ba7ecb]*/
|
||||
{
|
||||
PyInterpreterState *interp = _PyInterpreterState_GET();
|
||||
return PyLong_FromLong(interp->long_state.max_str_digits);
|
||||
int maxdigits = _Py_atomic_load_int(&interp->long_state.max_str_digits);
|
||||
return PyLong_FromLong(maxdigits);
|
||||
}
|
||||
|
||||
|
||||
|
|
@ -3490,14 +3491,39 @@ sys_set_flag(PyObject *flags, Py_ssize_t pos, PyObject *value)
|
|||
int
|
||||
_PySys_SetFlagObj(Py_ssize_t pos, PyObject *value)
|
||||
{
|
||||
PyObject *flags = PySys_GetAttrString("flags");
|
||||
if (flags == NULL) {
|
||||
return -1;
|
||||
PyObject *new_flags = NULL;
|
||||
PyObject *flags_str = &_Py_ID(flags); // immortal ref
|
||||
|
||||
PyObject *old_flags = PySys_GetAttr(flags_str);
|
||||
if (old_flags == NULL) {
|
||||
goto error;
|
||||
}
|
||||
|
||||
sys_set_flag(flags, pos, value);
|
||||
Py_DECREF(flags);
|
||||
return 0;
|
||||
new_flags = PyStructSequence_New(&FlagsType);
|
||||
if (new_flags == NULL) {
|
||||
goto error;
|
||||
}
|
||||
|
||||
for (Py_ssize_t i = 0; i < (Py_ssize_t)(Py_ARRAY_LENGTH(flags_fields) - 1); i++) {
|
||||
if (i != pos) {
|
||||
PyObject *old_value;
|
||||
old_value = PyStructSequence_GET_ITEM(old_flags, i); // borrowed ref
|
||||
sys_set_flag(new_flags, i, old_value);
|
||||
}
|
||||
else {
|
||||
sys_set_flag(new_flags, pos, value);
|
||||
}
|
||||
}
|
||||
|
||||
int res = _PySys_SetAttr(flags_str, new_flags);
|
||||
Py_DECREF(old_flags);
|
||||
Py_DECREF(new_flags);
|
||||
return res;
|
||||
|
||||
error:
|
||||
Py_XDECREF(old_flags);
|
||||
Py_XDECREF(new_flags);
|
||||
return -1;
|
||||
}
|
||||
|
||||
|
||||
|
|
@ -3521,8 +3547,6 @@ set_flags_from_config(PyInterpreterState *interp, PyObject *flags)
|
|||
const PyPreConfig *preconfig = &interp->runtime->preconfig;
|
||||
const PyConfig *config = _PyInterpreterState_GetConfig(interp);
|
||||
|
||||
// _PySys_UpdateConfig() modifies sys.flags in-place:
|
||||
// Py_XDECREF() is needed in this case.
|
||||
Py_ssize_t pos = 0;
|
||||
#define SetFlagObj(expr) \
|
||||
do { \
|
||||
|
|
@ -4033,7 +4057,7 @@ _PySys_InitCore(PyThreadState *tstate, PyObject *sysdict)
|
|||
/* implementation */
|
||||
SET_SYS("implementation", make_impl_info(version_info));
|
||||
|
||||
// sys.flags: updated in-place later by _PySys_UpdateConfig()
|
||||
// sys.flags: updated later by _PySys_UpdateConfig()
|
||||
ENSURE_INFO_TYPE(FlagsType, flags_desc);
|
||||
SET_SYS("flags", make_flags(tstate->interp));
|
||||
|
||||
|
|
@ -4153,16 +4177,21 @@ _PySys_UpdateConfig(PyThreadState *tstate)
|
|||
#undef COPY_LIST
|
||||
#undef COPY_WSTR
|
||||
|
||||
// sys.flags
|
||||
PyObject *flags = PySys_GetAttrString("flags");
|
||||
if (flags == NULL) {
|
||||
// replace sys.flags
|
||||
PyObject *new_flags = PyStructSequence_New(&FlagsType);
|
||||
if (new_flags == NULL) {
|
||||
return -1;
|
||||
}
|
||||
if (set_flags_from_config(interp, flags) < 0) {
|
||||
Py_DECREF(flags);
|
||||
if (set_flags_from_config(interp, new_flags) < 0) {
|
||||
Py_DECREF(new_flags);
|
||||
return -1;
|
||||
}
|
||||
|
||||
res = _PySys_SetAttr(&_Py_ID(flags), new_flags);
|
||||
Py_DECREF(new_flags);
|
||||
if (res < 0) {
|
||||
return -1;
|
||||
}
|
||||
Py_DECREF(flags);
|
||||
|
||||
SET_SYS("dont_write_bytecode", PyBool_FromLong(!config->write_bytecode));
|
||||
|
||||
|
|
@ -4675,7 +4704,7 @@ _PySys_SetIntMaxStrDigits(int maxdigits)
|
|||
// Set PyInterpreterState.long_state.max_str_digits
|
||||
// and PyInterpreterState.config.int_max_str_digits.
|
||||
PyInterpreterState *interp = _PyInterpreterState_GET();
|
||||
interp->long_state.max_str_digits = maxdigits;
|
||||
interp->config.int_max_str_digits = maxdigits;
|
||||
_Py_atomic_store_int(&interp->long_state.max_str_digits, maxdigits);
|
||||
_Py_atomic_store_int(&interp->config.int_max_str_digits, maxdigits);
|
||||
return 0;
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue