Skip to content

Commit 09f17c8

Browse files
committed
gh-151218: fix data race in sys_set_flag for free-threading
Concurrent calls to sys.set_int_max_str_digits() in free-threaded builds could double-free the same sys.flags tuple item because sys_set_flag() updated the slot without synchronization. Protect sys.flags updates with a mutex in free-threaded builds and hold the same lock across the flag and interpreter int_max_str_digits state updates so sys.get_int_max_str_digits() stays consistent with sys.flags.
1 parent ce916dc commit 09f17c8

3 files changed

Lines changed: 104 additions & 18 deletions

File tree

Lib/test/test_sys.py

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -887,6 +887,59 @@ def test_sys_flags_name_only_attributes(self):
887887
self.assertIsInstance(sys.flags.context_aware_warnings, int|type(None))
888888
self.assertIsInstance(sys.flags.lazy_imports, int|type(None))
889889

890+
@unittest.skipUnless(support.Py_GIL_DISABLED,
891+
"test is only useful if the GIL is disabled")
892+
@threading_helper.reap_threads
893+
@threading_helper.requires_working_threading()
894+
def test_set_int_max_str_digits_concurrent(self):
895+
# Regression test for gh-151218: concurrent
896+
# sys.set_int_max_str_digits() in free-threaded builds previously
897+
# double-freed the old sys.flags tuple item. With the fix in place
898+
# the loop must not crash and must only ever observe valid values
899+
# that some worker wrote.
900+
import threading
901+
902+
original = sys.get_int_max_str_digits()
903+
self.addCleanup(sys.set_int_max_str_digits, original)
904+
905+
values = (4300, 5000, 0)
906+
allowed = set(values) | {original}
907+
908+
start = threading.Barrier(4)
909+
done = threading.Event()
910+
errors = []
911+
912+
def worker():
913+
try:
914+
start.wait(timeout=support.SHORT_TIMEOUT)
915+
iterations = 0
916+
while iterations < 200 and not done.is_set():
917+
for value in values:
918+
sys.set_int_max_str_digits(value)
919+
observed = sys.get_int_max_str_digits()
920+
if observed not in allowed:
921+
errors.append(('getter', observed))
922+
observed_flag = sys.flags.int_max_str_digits
923+
if observed_flag not in allowed:
924+
errors.append(('flag', observed_flag))
925+
iterations += 1
926+
except BaseException as exc:
927+
errors.append(exc)
928+
929+
threads = [threading.Thread(target=worker) for _ in range(4)]
930+
try:
931+
for thread in threads:
932+
thread.start()
933+
finally:
934+
done.set()
935+
for thread in threads:
936+
thread.join()
937+
938+
self.assertEqual(errors, [])
939+
sys.set_int_max_str_digits(original)
940+
self.assertEqual(sys.get_int_max_str_digits(), original)
941+
self.assertEqual(sys.flags.int_max_str_digits, original)
942+
890943
def assert_raise_on_new_sys_type(self, sys_attr):
891944
# Users are intentionally prevented from creating new instances of
892945
# sys.flags, sys.version_info, and sys.getwindowsversion.
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix a data race in :func:`sys.set_int_max_str_digits` when updating
2+
:data:`sys.flags` in the free-threaded build.

Python/sysmodule.c

Lines changed: 49 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ Data members:
2222
#include "pycore_import.h" // _PyImport_SetDLOpenFlags()
2323
#include "pycore_initconfig.h" // _PyStatus_EXCEPTION()
2424
#include "pycore_interpframe.h" // _PyFrame_GetFirstComplete()
25+
#ifdef Py_GIL_DISABLED
26+
# include "pycore_lock.h" // PyMutex_Lock()
27+
#endif
2528
#include "pycore_long.h" // _PY_LONG_MAX_STR_DIGITS_THRESHOLD
2629
#include "pycore_modsupport.h" // _PyModule_CreateInitialized()
2730
#include "pycore_namespace.h" // _PyNamespace_New()
@@ -3476,13 +3479,31 @@ static PyStructSequence_Desc flags_desc = {
34763479
// https://github.com/python/cpython/issues/122575#issuecomment-2416497086
34773480
};
34783481

3482+
#ifdef Py_GIL_DISABLED
3483+
static PyMutex sys_flags_mutex;
3484+
#endif
3485+
34793486
static void
3480-
sys_set_flag(PyObject *flags, Py_ssize_t pos, PyObject *value)
3487+
sys_set_flag_unlocked(PyObject *flags, Py_ssize_t pos, PyObject *value,
3488+
PyObject **p_old_value)
34813489
{
34823490
assert(pos >= 0 && pos < (Py_ssize_t)(Py_ARRAY_LENGTH(flags_fields) - 1));
34833491

3484-
PyObject *old_value = PyStructSequence_GET_ITEM(flags, pos);
3492+
*p_old_value = PyStructSequence_GET_ITEM(flags, pos);
34853493
PyStructSequence_SET_ITEM(flags, pos, Py_NewRef(value));
3494+
}
3495+
3496+
static void
3497+
sys_set_flag(PyObject *flags, Py_ssize_t pos, PyObject *value)
3498+
{
3499+
PyObject *old_value;
3500+
#ifdef Py_GIL_DISABLED
3501+
PyMutex_Lock(&sys_flags_mutex);
3502+
#endif
3503+
sys_set_flag_unlocked(flags, pos, value, &old_value);
3504+
#ifdef Py_GIL_DISABLED
3505+
PyMutex_Unlock(&sys_flags_mutex);
3506+
#endif
34863507
Py_XDECREF(old_value);
34873508
}
34883509

@@ -3501,20 +3522,6 @@ _PySys_SetFlagObj(Py_ssize_t pos, PyObject *value)
35013522
}
35023523

35033524

3504-
static int
3505-
_PySys_SetFlagInt(Py_ssize_t pos, int value)
3506-
{
3507-
PyObject *obj = PyLong_FromLong(value);
3508-
if (obj == NULL) {
3509-
return -1;
3510-
}
3511-
3512-
int res = _PySys_SetFlagObj(pos, obj);
3513-
Py_DECREF(obj);
3514-
return res;
3515-
}
3516-
3517-
35183525
static int
35193526
set_flags_from_config(PyInterpreterState *interp, PyObject *flags)
35203527
{
@@ -4666,16 +4673,40 @@ _PySys_SetIntMaxStrDigits(int maxdigits)
46664673
return -1;
46674674
}
46684675

4669-
// Set sys.flags.int_max_str_digits
46704676
const Py_ssize_t pos = SYS_FLAGS_INT_MAX_STR_DIGITS;
4671-
if (_PySys_SetFlagInt(pos, maxdigits) < 0) {
4677+
PyObject *obj = PyLong_FromLong(maxdigits);
4678+
if (obj == NULL) {
4679+
return -1;
4680+
}
4681+
4682+
#ifdef Py_GIL_DISABLED
4683+
PyMutex_Lock(&sys_flags_mutex);
4684+
#endif
4685+
4686+
PyObject *flags = PySys_GetAttrString("flags");
4687+
if (flags == NULL) {
4688+
Py_DECREF(obj);
4689+
#ifdef Py_GIL_DISABLED
4690+
PyMutex_Unlock(&sys_flags_mutex);
4691+
#endif
46724692
return -1;
46734693
}
46744694

4695+
PyObject *old_value;
4696+
sys_set_flag_unlocked(flags, pos, obj, &old_value);
4697+
Py_DECREF(flags);
4698+
46754699
// Set PyInterpreterState.long_state.max_str_digits
46764700
// and PyInterpreterState.config.int_max_str_digits.
46774701
PyInterpreterState *interp = _PyInterpreterState_GET();
46784702
interp->long_state.max_str_digits = maxdigits;
46794703
interp->config.int_max_str_digits = maxdigits;
4704+
4705+
#ifdef Py_GIL_DISABLED
4706+
PyMutex_Unlock(&sys_flags_mutex);
4707+
#endif
4708+
4709+
Py_DECREF(obj);
4710+
Py_XDECREF(old_value);
46804711
return 0;
46814712
}

0 commit comments

Comments
 (0)