Skip to content

Commit ff6e973

Browse files
miss-islingtonvstinnerpicnixz
authored
[3.15] gh-151218: Replace sys.flags in PyConfig_Set() (GH-151402) (#151552)
gh-151218: Replace sys.flags in PyConfig_Set() (GH-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. (cherry picked from commit b16d23f) Co-authored-by: Victor Stinner <vstinner@python.org> Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
1 parent 19bf6a3 commit ff6e973

9 files changed

Lines changed: 111 additions & 23 deletions

File tree

Doc/c-api/init_config.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -623,6 +623,10 @@ Some options are read from the :mod:`sys` attributes. For example, the option
623623
624624
.. versionadded:: 3.14
625625
626+
.. versionchanged:: next
627+
The function now replaces :data:`sys.flags` (create a new object),
628+
instead of modifying :data:`sys.flags` in-place.
629+
626630
627631
.. _pyconfig_api:
628632

Lib/test/test_capi/test_config.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,9 +356,11 @@ def expect_bool_not(value):
356356
for value in new_values:
357357
expected, expect_flag = expect_func(value)
358358

359+
old_flags = sys.flags
359360
config_set(name, value)
360361
self.assertEqual(config_get(name), expected)
361362
self.assertEqual(getattr(sys.flags, sys_flag), expect_flag)
363+
self.assertIsNot(sys.flags, old_flags)
362364
if name == "write_bytecode":
363365
self.assertEqual(getattr(sys, "dont_write_bytecode"),
364366
expect_flag)
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import sys
2+
import unittest
3+
from test.support import threading_helper
4+
5+
6+
class SysModuleTest(unittest.TestCase):
7+
def test_int_max_str_digits_thread(self):
8+
# gh-151218: Check that it's safe to call get_int_max_str_digits() and
9+
# set_int_max_str_digits() in parallel. Previously, this test triggered
10+
# warnings in TSan on a free threaded build.
11+
12+
old_limit = sys.get_int_max_str_digits()
13+
self.addCleanup(sys.set_int_max_str_digits, old_limit)
14+
15+
def worker(worker_id):
16+
if not worker_id:
17+
for i in range (20_000):
18+
sys.get_int_max_str_digits()
19+
else:
20+
for i in range (20_000):
21+
sys.set_int_max_str_digits(4300 + (i & 7))
22+
23+
workers = [lambda: worker(i) for i in range(5)]
24+
threading_helper.run_concurrently(workers)
25+
26+
27+
if __name__ == "__main__":
28+
unittest.main()

Lib/test/test_sys.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1359,6 +1359,26 @@ def test_pystats(self):
13591359
def test_disable_gil_abi(self):
13601360
self.assertEqual('t' in sys.abiflags, support.Py_GIL_DISABLED)
13611361

1362+
def test_int_max_str_digits(self):
1363+
old_limit = sys.get_int_max_str_digits()
1364+
self.assertIsInstance(old_limit, int)
1365+
self.assertGreaterEqual(old_limit, 0)
1366+
self.addCleanup(sys.set_int_max_str_digits, old_limit)
1367+
1368+
sys.set_int_max_str_digits(0)
1369+
self.assertEqual(sys.get_int_max_str_digits(), 0)
1370+
1371+
sys.set_int_max_str_digits(2_048)
1372+
self.assertEqual(sys.get_int_max_str_digits(), 2_048)
1373+
1374+
with self.assertRaises(ValueError):
1375+
# the minimum is 640 digits
1376+
sys.set_int_max_str_digits(5)
1377+
with self.assertRaises(ValueError):
1378+
sys.set_int_max_str_digits(-2)
1379+
with self.assertRaises(TypeError):
1380+
sys.set_int_max_str_digits(2_048.0)
1381+
13621382

13631383
@test.support.cpython_only
13641384
@test.support.force_not_colorized_test_class
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
:c:func:`PyConfig_Set` and :func:`sys.set_int_max_str_digits` now replace
2+
:data:`sys.flags` (create a new object), instead of modifying :data:`sys.flags`
3+
in-place. Patch by Victor Stinner.

Objects/longobject.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2125,7 +2125,7 @@ long_to_decimal_string_internal(PyObject *aa,
21252125
if (size_a >= 10 * _PY_LONG_MAX_STR_DIGITS_THRESHOLD
21262126
/ (3 * PyLong_SHIFT) + 2) {
21272127
PyInterpreterState *interp = _PyInterpreterState_GET();
2128-
int max_str_digits = interp->long_state.max_str_digits;
2128+
int max_str_digits = _Py_atomic_load_int(&interp->long_state.max_str_digits);
21292129
if ((max_str_digits > 0) &&
21302130
(max_str_digits / (3 * PyLong_SHIFT) <= (size_a - 11) / 10)) {
21312131
PyErr_Format(PyExc_ValueError, _MAX_STR_DIGITS_ERROR_FMT_TO_STR,
@@ -2206,7 +2206,7 @@ long_to_decimal_string_internal(PyObject *aa,
22062206
}
22072207
if (strlen > _PY_LONG_MAX_STR_DIGITS_THRESHOLD) {
22082208
PyInterpreterState *interp = _PyInterpreterState_GET();
2209-
int max_str_digits = interp->long_state.max_str_digits;
2209+
int max_str_digits = _Py_atomic_load_int(&interp->long_state.max_str_digits);
22102210
Py_ssize_t strlen_nosign = strlen - negative;
22112211
if ((max_str_digits > 0) && (strlen_nosign > max_str_digits)) {
22122212
Py_DECREF(scratch);
@@ -3021,7 +3021,7 @@ long_from_string_base(const char **str, int base, PyLongObject **res)
30213021
* quadratic algorithm. */
30223022
if (digits > _PY_LONG_MAX_STR_DIGITS_THRESHOLD) {
30233023
PyInterpreterState *interp = _PyInterpreterState_GET();
3024-
int max_str_digits = interp->long_state.max_str_digits;
3024+
int max_str_digits = _Py_atomic_load_int(&interp->long_state.max_str_digits);
30253025
if ((max_str_digits > 0) && (digits > max_str_digits)) {
30263026
PyErr_Format(PyExc_ValueError, _MAX_STR_DIGITS_ERROR_FMT_TO_INT,
30273027
max_str_digits, digits);

Python/initconfig.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4596,7 +4596,8 @@ config_get(const PyConfig *config, const PyConfigSpec *spec,
45964596

45974597
if (strcmp(spec->name, "int_max_str_digits") == 0) {
45984598
PyInterpreterState *interp = _PyInterpreterState_GET();
4599-
return PyLong_FromLong(interp->long_state.max_str_digits);
4599+
int maxdigits = _Py_atomic_load_int(&interp->long_state.max_str_digits);
4600+
return PyLong_FromLong(maxdigits);
46004601
}
46014602
}
46024603

Python/pylifecycle.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -428,7 +428,8 @@ interpreter_update_config(PyThreadState *tstate, int only_update_path_config)
428428
}
429429
}
430430

431-
tstate->interp->long_state.max_str_digits = config->int_max_str_digits;
431+
_Py_atomic_store_int(&tstate->interp->long_state.max_str_digits,
432+
config->int_max_str_digits);
432433

433434
// Update the sys module for the new configuration
434435
if (_PySys_UpdateConfig(tstate) < 0) {

Python/sysmodule.c

Lines changed: 47 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1904,7 +1904,8 @@ sys_get_int_max_str_digits_impl(PyObject *module)
19041904
/*[clinic end generated code: output=0042f5e8ae0e8631 input=77fb74e987ba7ecb]*/
19051905
{
19061906
PyInterpreterState *interp = _PyInterpreterState_GET();
1907-
return PyLong_FromLong(interp->long_state.max_str_digits);
1907+
int maxdigits = _Py_atomic_load_int(&interp->long_state.max_str_digits);
1908+
return PyLong_FromLong(maxdigits);
19081909
}
19091910

19101911

@@ -3528,14 +3529,39 @@ sys_set_flag(PyObject *flags, Py_ssize_t pos, PyObject *value)
35283529
int
35293530
_PySys_SetFlagObj(Py_ssize_t pos, PyObject *value)
35303531
{
3531-
PyObject *flags = PySys_GetAttrString("flags");
3532-
if (flags == NULL) {
3533-
return -1;
3532+
PyObject *new_flags = NULL;
3533+
PyObject *flags_str = &_Py_ID(flags); // immortal ref
3534+
3535+
PyObject *old_flags = PySys_GetAttr(flags_str);
3536+
if (old_flags == NULL) {
3537+
goto error;
35343538
}
35353539

3536-
sys_set_flag(flags, pos, value);
3537-
Py_DECREF(flags);
3538-
return 0;
3540+
new_flags = PyStructSequence_New(&FlagsType);
3541+
if (new_flags == NULL) {
3542+
goto error;
3543+
}
3544+
3545+
for (Py_ssize_t i = 0; i < (Py_ssize_t)(Py_ARRAY_LENGTH(flags_fields) - 1); i++) {
3546+
if (i != pos) {
3547+
PyObject *old_value;
3548+
old_value = PyStructSequence_GET_ITEM(old_flags, i); // borrowed ref
3549+
sys_set_flag(new_flags, i, old_value);
3550+
}
3551+
else {
3552+
sys_set_flag(new_flags, pos, value);
3553+
}
3554+
}
3555+
3556+
int res = _PySys_SetAttr(flags_str, new_flags);
3557+
Py_DECREF(old_flags);
3558+
Py_DECREF(new_flags);
3559+
return res;
3560+
3561+
error:
3562+
Py_XDECREF(old_flags);
3563+
Py_XDECREF(new_flags);
3564+
return -1;
35393565
}
35403566

35413567

@@ -3559,8 +3585,6 @@ set_flags_from_config(PyInterpreterState *interp, PyObject *flags)
35593585
const PyPreConfig *preconfig = &interp->runtime->preconfig;
35603586
const PyConfig *config = _PyInterpreterState_GetConfig(interp);
35613587

3562-
// _PySys_UpdateConfig() modifies sys.flags in-place:
3563-
// Py_XDECREF() is needed in this case.
35643588
Py_ssize_t pos = 0;
35653589
#define SetFlagObj(expr) \
35663590
do { \
@@ -4071,7 +4095,7 @@ _PySys_InitCore(PyThreadState *tstate, PyObject *sysdict)
40714095
/* implementation */
40724096
SET_SYS("implementation", make_impl_info(version_info));
40734097

4074-
// sys.flags: updated in-place later by _PySys_UpdateConfig()
4098+
// sys.flags: updated later by _PySys_UpdateConfig()
40754099
ENSURE_INFO_TYPE(FlagsType, flags_desc);
40764100
SET_SYS("flags", make_flags(tstate->interp));
40774101

@@ -4191,16 +4215,21 @@ _PySys_UpdateConfig(PyThreadState *tstate)
41914215
#undef COPY_LIST
41924216
#undef COPY_WSTR
41934217

4194-
// sys.flags
4195-
PyObject *flags = PySys_GetAttrString("flags");
4196-
if (flags == NULL) {
4218+
// replace sys.flags
4219+
PyObject *new_flags = PyStructSequence_New(&FlagsType);
4220+
if (new_flags == NULL) {
41974221
return -1;
41984222
}
4199-
if (set_flags_from_config(interp, flags) < 0) {
4200-
Py_DECREF(flags);
4223+
if (set_flags_from_config(interp, new_flags) < 0) {
4224+
Py_DECREF(new_flags);
4225+
return -1;
4226+
}
4227+
4228+
res = _PySys_SetAttr(&_Py_ID(flags), new_flags);
4229+
Py_DECREF(new_flags);
4230+
if (res < 0) {
42014231
return -1;
42024232
}
4203-
Py_DECREF(flags);
42044233

42054234
SET_SYS("dont_write_bytecode", PyBool_FromLong(!config->write_bytecode));
42064235

@@ -4713,7 +4742,7 @@ _PySys_SetIntMaxStrDigits(int maxdigits)
47134742
// Set PyInterpreterState.long_state.max_str_digits
47144743
// and PyInterpreterState.config.int_max_str_digits.
47154744
PyInterpreterState *interp = _PyInterpreterState_GET();
4716-
interp->long_state.max_str_digits = maxdigits;
4717-
interp->config.int_max_str_digits = maxdigits;
4745+
_Py_atomic_store_int(&interp->long_state.max_str_digits, maxdigits);
4746+
_Py_atomic_store_int(&interp->config.int_max_str_digits, maxdigits);
47184747
return 0;
47194748
}

0 commit comments

Comments
 (0)