From: Kumar Aditya Date: Tue, 28 Jul 2026 16:54:27 +0000 (+0530) Subject: gh-154821: fix data races on function attributes (#154826) X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=4f3e85640f72ee79d819ea3546868aa6544a6f49;p=thirdparty%2FPython%2Fcpython.git gh-154821: fix data races on function attributes (#154826) --- diff --git a/Lib/test/test_free_threading/test_functions.py b/Lib/test/test_free_threading/test_functions.py new file mode 100644 index 000000000000..65a90a17bd5b --- /dev/null +++ b/Lib/test/test_free_threading/test_functions.py @@ -0,0 +1,98 @@ +import random +import unittest +from unittest import TestCase + +from test.support import threading_helper + +threading_helper.requires_working_threading(module=True) + +NUM_THREADS = 8 +ITERS = 200 + + +def random_string(): + return ''.join(random.choice('0123456789ABCDEF') for _ in range(10)) + + +def template_a(): pass +def template_b(): pass + + +class TestFTFunctionAttributes(TestCase): + + def stress_attribute(self, attr, make_value): + def target(x=1): + return x + + def writer(): + for _ in range(ITERS): + setattr(target, attr, make_value()) + getattr(target, attr) + + threading_helper.run_concurrently(writer, NUM_THREADS) + + def test_name(self): + self.stress_attribute("__name__", random_string) + + def test_qualname(self): + self.stress_attribute("__qualname__", random_string) + + def test_code(self): + codes = (template_a.__code__, template_b.__code__) + self.stress_attribute("__code__", lambda: random.choice(codes)) + + def test_defaults(self): + self.stress_attribute("__defaults__", lambda: (random_string(),)) + + def test_kwdefaults(self): + self.stress_attribute("__kwdefaults__", lambda: {"x": random_string()}) + + def test_annotations(self): + self.stress_attribute("__annotations__", + lambda: {"x": random_string()}) + + def test_annotate(self): + self.stress_attribute("__annotate__", + lambda: (lambda format: {"x": str})) + + def test_type_params(self): + self.stress_attribute("__type_params__", lambda: (random_string(),)) + + def test_annotations_and_annotate(self): + # The __annotations__ and __annotate__ setters clear each other. + def target(): pass + + def set_annotations(): + for _ in range(ITERS): + target.__annotations__ = {"x": random_string()} + target.__annotations__ + + def set_annotate(): + for _ in range(ITERS): + target.__annotate__ = lambda format: {"x": str} + target.__annotate__ + + threading_helper.run_concurrently( + [set_annotations, set_annotate] * (NUM_THREADS // 2)) + + def test_call_while_replacing_defaults(self): + # The eval loop reads __defaults__ and __kwdefaults__ without holding + # a lock while pushing a frame. + def target(x="init", *, y="init"): + return x, y + + def writer(): + for _ in range(ITERS): + target.__defaults__ = (random_string(),) + target.__kwdefaults__ = {"y": random_string()} + + def caller(): + for _ in range(ITERS): + target() + + threading_helper.run_concurrently( + [writer, caller] * (NUM_THREADS // 2)) + + +if __name__ == "__main__": + unittest.main() diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-07-28-10-00-00.gh-issue-133931.fnQzxD.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-07-28-10-00-00.gh-issue-133931.fnQzxD.rst new file mode 100644 index 000000000000..139bdd86b848 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-07-28-10-00-00.gh-issue-133931.fnQzxD.rst @@ -0,0 +1,2 @@ +Fix data races when setting attributes of function objects +on the :term:`free threaded ` build. diff --git a/Objects/funcobject.c b/Objects/funcobject.c index 0fffd36ad462..49a28e8ad667 100644 --- a/Objects/funcobject.c +++ b/Objects/funcobject.c @@ -349,20 +349,6 @@ func_clear_version(PyInterpreterState *interp, PyFunctionObject *func) func->func_version = FUNC_VERSION_CLEARED; } -// Called when any of the critical function attributes are changed -static void -_PyFunction_ClearVersion(PyFunctionObject *func) -{ - if (func->func_version < FUNC_VERSION_FIRST_VALID) { - // Version was never set or has already been cleared. - return; - } - PyInterpreterState *interp = _PyInterpreterState_GET(); - _PyEval_StopTheWorld(interp); - func_clear_version(interp, func); - _PyEval_StartTheWorld(interp); -} - void _PyFunction_ClearCodeByVersion(uint32_t version) { @@ -448,10 +434,15 @@ PyFunction_SetDefaults(PyObject *op, PyObject *defaults) PyErr_SetString(PyExc_SystemError, "non-tuple default args"); return -1; } - handle_func_event(PyFunction_EVENT_MODIFY_DEFAULTS, - (PyFunctionObject *) op, defaults); - _PyFunction_ClearVersion((PyFunctionObject *)op); - Py_XSETREF(((PyFunctionObject *)op)->func_defaults, defaults); + PyFunctionObject *func = (PyFunctionObject *)op; + handle_func_event(PyFunction_EVENT_MODIFY_DEFAULTS, func, defaults); + PyInterpreterState *interp = _PyInterpreterState_GET(); + _PyEval_StopTheWorld(interp); + func_clear_version(interp, func); + PyObject *old_defaults = func->func_defaults; + func->func_defaults = defaults; + _PyEval_StartTheWorld(interp); + Py_XDECREF(old_defaults); return 0; } @@ -459,8 +450,11 @@ void PyFunction_SetVectorcall(PyFunctionObject *func, vectorcallfunc vectorcall) { assert(func != NULL); - _PyFunction_ClearVersion(func); + PyInterpreterState *interp = _PyInterpreterState_GET(); + _PyEval_StopTheWorld(interp); + func_clear_version(interp, func); func->vectorcall = vectorcall; + _PyEval_StartTheWorld(interp); } PyObject * @@ -490,10 +484,15 @@ PyFunction_SetKwDefaults(PyObject *op, PyObject *defaults) "non-dict keyword only default args"); return -1; } - handle_func_event(PyFunction_EVENT_MODIFY_KWDEFAULTS, - (PyFunctionObject *) op, defaults); - _PyFunction_ClearVersion((PyFunctionObject *)op); - Py_XSETREF(((PyFunctionObject *)op)->func_kwdefaults, defaults); + PyFunctionObject *func = (PyFunctionObject *)op; + handle_func_event(PyFunction_EVENT_MODIFY_KWDEFAULTS, func, defaults); + PyInterpreterState *interp = _PyInterpreterState_GET(); + _PyEval_StopTheWorld(interp); + func_clear_version(interp, func); + PyObject *old_kwdefaults = func->func_kwdefaults; + func->func_kwdefaults = defaults; + _PyEval_StartTheWorld(interp); + Py_XDECREF(old_kwdefaults); return 0; } @@ -525,8 +524,14 @@ PyFunction_SetClosure(PyObject *op, PyObject *closure) Py_TYPE(closure)->tp_name); return -1; } - _PyFunction_ClearVersion((PyFunctionObject *)op); - Py_XSETREF(((PyFunctionObject *)op)->func_closure, closure); + PyFunctionObject *func = (PyFunctionObject *)op; + PyInterpreterState *interp = _PyInterpreterState_GET(); + _PyEval_StopTheWorld(interp); + func_clear_version(interp, func); + PyObject *old_closure = func->func_closure; + func->func_closure = closure; + _PyEval_StartTheWorld(interp); + Py_XDECREF(old_closure); return 0; } @@ -605,8 +610,15 @@ PyFunction_SetAnnotations(PyObject *op, PyObject *annotations) return -1; } PyFunctionObject *func = (PyFunctionObject *)op; - Py_XSETREF(func->func_annotations, annotations); - Py_CLEAR(func->func_annotate); + PyInterpreterState *interp = _PyInterpreterState_GET(); + _PyEval_StopTheWorld(interp); + PyObject *old_annotations = func->func_annotations; + func->func_annotations = annotations; + PyObject *old_annotate = func->func_annotate; + func->func_annotate = NULL; + _PyEval_StartTheWorld(interp); + Py_XDECREF(old_annotations); + Py_XDECREF(old_annotate); return 0; } @@ -685,8 +697,13 @@ func_set_code(PyObject *self, PyObject *value, void *Py_UNUSED(ignored)) } handle_func_event(PyFunction_EVENT_MODIFY_CODE, op, value); - _PyFunction_ClearVersion(op); - Py_XSETREF(op->func_code, Py_NewRef(value)); + PyInterpreterState *interp = _PyInterpreterState_GET(); + _PyEval_StopTheWorld(interp); + func_clear_version(interp, op); + PyObject *old_code = op->func_code; + op->func_code = Py_NewRef(value); + _PyEval_StartTheWorld(interp); + Py_XDECREF(old_code); return 0; } @@ -708,7 +725,12 @@ func_set_name(PyObject *self, PyObject *value, void *Py_UNUSED(ignored)) "__name__ must be set to a string object"); return -1; } - Py_XSETREF(op->func_name, Py_NewRef(value)); + PyInterpreterState *interp = _PyInterpreterState_GET(); + _PyEval_StopTheWorld(interp); + PyObject *old_name = op->func_name; + op->func_name = Py_NewRef(value); + _PyEval_StartTheWorld(interp); + Py_XDECREF(old_name); return 0; } @@ -731,7 +753,12 @@ func_set_qualname(PyObject *self, PyObject *value, void *Py_UNUSED(ignored)) return -1; } handle_func_event(PyFunction_EVENT_MODIFY_QUALNAME, (PyFunctionObject *) op, value); - Py_XSETREF(op->func_qualname, Py_NewRef(value)); + PyInterpreterState *interp = _PyInterpreterState_GET(); + _PyEval_StopTheWorld(interp); + PyObject *old_qualname = op->func_qualname; + op->func_qualname = Py_NewRef(value); + _PyEval_StartTheWorld(interp); + Py_XDECREF(old_qualname); return 0; } @@ -772,8 +799,13 @@ func_set_defaults(PyObject *self, PyObject *value, void *Py_UNUSED(ignored)) } handle_func_event(PyFunction_EVENT_MODIFY_DEFAULTS, op, value); - _PyFunction_ClearVersion(op); - Py_XSETREF(op->func_defaults, Py_XNewRef(value)); + PyInterpreterState *interp = _PyInterpreterState_GET(); + _PyEval_StopTheWorld(interp); + func_clear_version(interp, op); + PyObject *old_defaults = op->func_defaults; + op->func_defaults = Py_XNewRef(value); + _PyEval_StartTheWorld(interp); + Py_XDECREF(old_defaults); return 0; } @@ -815,8 +847,13 @@ func_set_kwdefaults(PyObject *self, PyObject *value, void *Py_UNUSED(ignored)) } handle_func_event(PyFunction_EVENT_MODIFY_KWDEFAULTS, op, value); - _PyFunction_ClearVersion(op); - Py_XSETREF(op->func_kwdefaults, Py_XNewRef(value)); + PyInterpreterState *interp = _PyInterpreterState_GET(); + _PyEval_StopTheWorld(interp); + func_clear_version(interp, op); + PyObject *old_kwdefaults = op->func_kwdefaults; + op->func_kwdefaults = Py_XNewRef(value); + _PyEval_StartTheWorld(interp); + Py_XDECREF(old_kwdefaults); return 0; } @@ -854,12 +891,24 @@ function___annotate___set_impl(PyFunctionObject *self, PyObject *value) return -1; } if (Py_IsNone(value)) { - Py_XSETREF(self->func_annotate, value); + PyInterpreterState *interp = _PyInterpreterState_GET(); + _PyEval_StopTheWorld(interp); + PyObject *old_annotate = self->func_annotate; + self->func_annotate = Py_NewRef(value); + _PyEval_StartTheWorld(interp); + Py_XDECREF(old_annotate); return 0; } else if (PyCallable_Check(value)) { - Py_XSETREF(self->func_annotate, Py_XNewRef(value)); - Py_CLEAR(self->func_annotations); + PyInterpreterState *interp = _PyInterpreterState_GET(); + _PyEval_StopTheWorld(interp); + PyObject *old_annotate = self->func_annotate; + self->func_annotate = Py_NewRef(value); + PyObject *old_annotations = self->func_annotations; + self->func_annotations = NULL; + _PyEval_StartTheWorld(interp); + Py_XDECREF(old_annotate); + Py_XDECREF(old_annotations); return 0; } else { @@ -912,8 +961,15 @@ function___annotations___set_impl(PyFunctionObject *self, PyObject *value) "__annotations__ must be set to a dict object"); return -1; } - Py_XSETREF(self->func_annotations, Py_XNewRef(value)); - Py_CLEAR(self->func_annotate); + PyInterpreterState *interp = _PyInterpreterState_GET(); + _PyEval_StopTheWorld(interp); + PyObject *old_annotations = self->func_annotations; + self->func_annotations = Py_XNewRef(value); + PyObject *old_annotate = self->func_annotate; + self->func_annotate = NULL; + _PyEval_StartTheWorld(interp); + Py_XDECREF(old_annotations); + Py_XDECREF(old_annotate); return 0; } @@ -954,7 +1010,12 @@ function___type_params___set_impl(PyFunctionObject *self, PyObject *value) "__type_params__ must be set to a tuple"); return -1; } - Py_XSETREF(self->func_typeparams, Py_NewRef(value)); + PyInterpreterState *interp = _PyInterpreterState_GET(); + _PyEval_StopTheWorld(interp); + PyObject *old_typeparams = self->func_typeparams; + self->func_typeparams = Py_NewRef(value); + _PyEval_StartTheWorld(interp); + Py_XDECREF(old_typeparams); return 0; }