Skip to content

Commit 058f1cf

Browse files
gh-133931: Fix data races when setting attributes of function objects
1 parent c3b7eb0 commit 058f1cf

3 files changed

Lines changed: 202 additions & 41 deletions

File tree

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
import random
2+
import unittest
3+
from unittest import TestCase
4+
5+
from test.support import threading_helper
6+
7+
threading_helper.requires_working_threading(module=True)
8+
9+
NUM_THREADS = 8
10+
ITERS = 200
11+
12+
13+
def random_string():
14+
return ''.join(random.choice('0123456789ABCDEF') for _ in range(10))
15+
16+
17+
def template_a(): pass
18+
def template_b(): pass
19+
20+
21+
class TestFTFunctionAttributes(TestCase):
22+
23+
def stress_attribute(self, attr, make_value):
24+
def target(x=1):
25+
return x
26+
27+
def writer():
28+
for _ in range(ITERS):
29+
setattr(target, attr, make_value())
30+
getattr(target, attr)
31+
32+
threading_helper.run_concurrently(writer, NUM_THREADS)
33+
34+
def test_name(self):
35+
self.stress_attribute("__name__", random_string)
36+
37+
def test_qualname(self):
38+
self.stress_attribute("__qualname__", random_string)
39+
40+
def test_code(self):
41+
codes = (template_a.__code__, template_b.__code__)
42+
self.stress_attribute("__code__", lambda: random.choice(codes))
43+
44+
def test_defaults(self):
45+
self.stress_attribute("__defaults__", lambda: (random_string(),))
46+
47+
def test_kwdefaults(self):
48+
self.stress_attribute("__kwdefaults__", lambda: {"x": random_string()})
49+
50+
def test_annotations(self):
51+
self.stress_attribute("__annotations__",
52+
lambda: {"x": random_string()})
53+
54+
def test_annotate(self):
55+
self.stress_attribute("__annotate__",
56+
lambda: (lambda format: {"x": str}))
57+
58+
def test_type_params(self):
59+
self.stress_attribute("__type_params__", lambda: (random_string(),))
60+
61+
def test_annotations_and_annotate(self):
62+
# The __annotations__ and __annotate__ setters clear each other.
63+
def target(): pass
64+
65+
def set_annotations():
66+
for _ in range(ITERS):
67+
target.__annotations__ = {"x": random_string()}
68+
target.__annotations__
69+
70+
def set_annotate():
71+
for _ in range(ITERS):
72+
target.__annotate__ = lambda format: {"x": str}
73+
target.__annotate__
74+
75+
threading_helper.run_concurrently(
76+
[set_annotations, set_annotate] * (NUM_THREADS // 2))
77+
78+
def test_call_while_replacing_defaults(self):
79+
# The eval loop reads __defaults__ and __kwdefaults__ without holding
80+
# a lock while pushing a frame.
81+
def target(x="init", *, y="init"):
82+
return x, y
83+
84+
def writer():
85+
for _ in range(ITERS):
86+
target.__defaults__ = (random_string(),)
87+
target.__kwdefaults__ = {"y": random_string()}
88+
89+
def caller():
90+
for _ in range(ITERS):
91+
target()
92+
93+
threading_helper.run_concurrently(
94+
[writer, caller] * (NUM_THREADS // 2))
95+
96+
97+
if __name__ == "__main__":
98+
unittest.main()
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix data races when setting attributes of function objects
2+
on the :term:`free threaded <free threading>` build.

Objects/funcobject.c

Lines changed: 102 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -340,20 +340,6 @@ func_clear_version(PyInterpreterState *interp, PyFunctionObject *func)
340340
func->func_version = FUNC_VERSION_CLEARED;
341341
}
342342

343-
// Called when any of the critical function attributes are changed
344-
static void
345-
_PyFunction_ClearVersion(PyFunctionObject *func)
346-
{
347-
if (func->func_version < FUNC_VERSION_FIRST_VALID) {
348-
// Version was never set or has already been cleared.
349-
return;
350-
}
351-
PyInterpreterState *interp = _PyInterpreterState_GET();
352-
_PyEval_StopTheWorld(interp);
353-
func_clear_version(interp, func);
354-
_PyEval_StartTheWorld(interp);
355-
}
356-
357343
void
358344
_PyFunction_ClearCodeByVersion(uint32_t version)
359345
{
@@ -465,19 +451,27 @@ PyFunction_SetDefaults(PyObject *op, PyObject *defaults)
465451
PyErr_SetString(PyExc_SystemError, "non-tuple default args");
466452
return -1;
467453
}
468-
handle_func_event(PyFunction_EVENT_MODIFY_DEFAULTS,
469-
(PyFunctionObject *) op, defaults);
470-
_PyFunction_ClearVersion((PyFunctionObject *)op);
471-
Py_XSETREF(((PyFunctionObject *)op)->func_defaults, defaults);
454+
PyFunctionObject *func = (PyFunctionObject *)op;
455+
handle_func_event(PyFunction_EVENT_MODIFY_DEFAULTS, func, defaults);
456+
PyInterpreterState *interp = _PyInterpreterState_GET();
457+
_PyEval_StopTheWorld(interp);
458+
func_clear_version(interp, func);
459+
PyObject *old_defaults = func->func_defaults;
460+
func->func_defaults = defaults;
461+
_PyEval_StartTheWorld(interp);
462+
Py_XDECREF(old_defaults);
472463
return 0;
473464
}
474465

475466
void
476467
PyFunction_SetVectorcall(PyFunctionObject *func, vectorcallfunc vectorcall)
477468
{
478469
assert(func != NULL);
479-
_PyFunction_ClearVersion(func);
470+
PyInterpreterState *interp = _PyInterpreterState_GET();
471+
_PyEval_StopTheWorld(interp);
472+
func_clear_version(interp, func);
480473
func->vectorcall = vectorcall;
474+
_PyEval_StartTheWorld(interp);
481475
}
482476

483477
PyObject *
@@ -507,10 +501,15 @@ PyFunction_SetKwDefaults(PyObject *op, PyObject *defaults)
507501
"non-dict keyword only default args");
508502
return -1;
509503
}
510-
handle_func_event(PyFunction_EVENT_MODIFY_KWDEFAULTS,
511-
(PyFunctionObject *) op, defaults);
512-
_PyFunction_ClearVersion((PyFunctionObject *)op);
513-
Py_XSETREF(((PyFunctionObject *)op)->func_kwdefaults, defaults);
504+
PyFunctionObject *func = (PyFunctionObject *)op;
505+
handle_func_event(PyFunction_EVENT_MODIFY_KWDEFAULTS, func, defaults);
506+
PyInterpreterState *interp = _PyInterpreterState_GET();
507+
_PyEval_StopTheWorld(interp);
508+
func_clear_version(interp, func);
509+
PyObject *old_kwdefaults = func->func_kwdefaults;
510+
func->func_kwdefaults = defaults;
511+
_PyEval_StartTheWorld(interp);
512+
Py_XDECREF(old_kwdefaults);
514513
return 0;
515514
}
516515

@@ -542,8 +541,14 @@ PyFunction_SetClosure(PyObject *op, PyObject *closure)
542541
Py_TYPE(closure)->tp_name);
543542
return -1;
544543
}
545-
_PyFunction_ClearVersion((PyFunctionObject *)op);
546-
Py_XSETREF(((PyFunctionObject *)op)->func_closure, closure);
544+
PyFunctionObject *func = (PyFunctionObject *)op;
545+
PyInterpreterState *interp = _PyInterpreterState_GET();
546+
_PyEval_StopTheWorld(interp);
547+
func_clear_version(interp, func);
548+
PyObject *old_closure = func->func_closure;
549+
func->func_closure = closure;
550+
_PyEval_StartTheWorld(interp);
551+
Py_XDECREF(old_closure);
547552
return 0;
548553
}
549554

@@ -621,8 +626,15 @@ PyFunction_SetAnnotations(PyObject *op, PyObject *annotations)
621626
return -1;
622627
}
623628
PyFunctionObject *func = (PyFunctionObject *)op;
624-
Py_XSETREF(func->func_annotations, annotations);
625-
Py_CLEAR(func->func_annotate);
629+
PyInterpreterState *interp = _PyInterpreterState_GET();
630+
_PyEval_StopTheWorld(interp);
631+
PyObject *old_annotations = func->func_annotations;
632+
func->func_annotations = annotations;
633+
PyObject *old_annotate = func->func_annotate;
634+
func->func_annotate = NULL;
635+
_PyEval_StartTheWorld(interp);
636+
Py_XDECREF(old_annotations);
637+
Py_XDECREF(old_annotate);
626638
return 0;
627639
}
628640

@@ -701,8 +713,13 @@ func_set_code(PyObject *self, PyObject *value, void *Py_UNUSED(ignored))
701713
}
702714

703715
handle_func_event(PyFunction_EVENT_MODIFY_CODE, op, value);
704-
_PyFunction_ClearVersion(op);
705-
Py_XSETREF(op->func_code, Py_NewRef(value));
716+
PyInterpreterState *interp = _PyInterpreterState_GET();
717+
_PyEval_StopTheWorld(interp);
718+
func_clear_version(interp, op);
719+
PyObject *old_code = op->func_code;
720+
op->func_code = Py_NewRef(value);
721+
_PyEval_StartTheWorld(interp);
722+
Py_XDECREF(old_code);
706723
return 0;
707724
}
708725

@@ -724,7 +741,12 @@ func_set_name(PyObject *self, PyObject *value, void *Py_UNUSED(ignored))
724741
"__name__ must be set to a string object");
725742
return -1;
726743
}
727-
Py_XSETREF(op->func_name, Py_NewRef(value));
744+
PyInterpreterState *interp = _PyInterpreterState_GET();
745+
_PyEval_StopTheWorld(interp);
746+
PyObject *old_name = op->func_name;
747+
op->func_name = Py_NewRef(value);
748+
_PyEval_StartTheWorld(interp);
749+
Py_XDECREF(old_name);
728750
return 0;
729751
}
730752

@@ -746,7 +768,12 @@ func_set_qualname(PyObject *self, PyObject *value, void *Py_UNUSED(ignored))
746768
"__qualname__ must be set to a string object");
747769
return -1;
748770
}
749-
Py_XSETREF(op->func_qualname, Py_NewRef(value));
771+
PyInterpreterState *interp = _PyInterpreterState_GET();
772+
_PyEval_StopTheWorld(interp);
773+
PyObject *old_qualname = op->func_qualname;
774+
op->func_qualname = Py_NewRef(value);
775+
_PyEval_StartTheWorld(interp);
776+
Py_XDECREF(old_qualname);
750777
return 0;
751778
}
752779

@@ -787,8 +814,13 @@ func_set_defaults(PyObject *self, PyObject *value, void *Py_UNUSED(ignored))
787814
}
788815

789816
handle_func_event(PyFunction_EVENT_MODIFY_DEFAULTS, op, value);
790-
_PyFunction_ClearVersion(op);
791-
Py_XSETREF(op->func_defaults, Py_XNewRef(value));
817+
PyInterpreterState *interp = _PyInterpreterState_GET();
818+
_PyEval_StopTheWorld(interp);
819+
func_clear_version(interp, op);
820+
PyObject *old_defaults = op->func_defaults;
821+
op->func_defaults = Py_XNewRef(value);
822+
_PyEval_StartTheWorld(interp);
823+
Py_XDECREF(old_defaults);
792824
return 0;
793825
}
794826

@@ -830,8 +862,13 @@ func_set_kwdefaults(PyObject *self, PyObject *value, void *Py_UNUSED(ignored))
830862
}
831863

832864
handle_func_event(PyFunction_EVENT_MODIFY_KWDEFAULTS, op, value);
833-
_PyFunction_ClearVersion(op);
834-
Py_XSETREF(op->func_kwdefaults, Py_XNewRef(value));
865+
PyInterpreterState *interp = _PyInterpreterState_GET();
866+
_PyEval_StopTheWorld(interp);
867+
func_clear_version(interp, op);
868+
PyObject *old_kwdefaults = op->func_kwdefaults;
869+
op->func_kwdefaults = Py_XNewRef(value);
870+
_PyEval_StartTheWorld(interp);
871+
Py_XDECREF(old_kwdefaults);
835872
return 0;
836873
}
837874

@@ -869,12 +906,24 @@ function___annotate___set_impl(PyFunctionObject *self, PyObject *value)
869906
return -1;
870907
}
871908
if (Py_IsNone(value)) {
872-
Py_XSETREF(self->func_annotate, value);
909+
PyInterpreterState *interp = _PyInterpreterState_GET();
910+
_PyEval_StopTheWorld(interp);
911+
PyObject *old_annotate = self->func_annotate;
912+
self->func_annotate = Py_NewRef(value);
913+
_PyEval_StartTheWorld(interp);
914+
Py_XDECREF(old_annotate);
873915
return 0;
874916
}
875917
else if (PyCallable_Check(value)) {
876-
Py_XSETREF(self->func_annotate, Py_XNewRef(value));
877-
Py_CLEAR(self->func_annotations);
918+
PyInterpreterState *interp = _PyInterpreterState_GET();
919+
_PyEval_StopTheWorld(interp);
920+
PyObject *old_annotate = self->func_annotate;
921+
self->func_annotate = Py_NewRef(value);
922+
PyObject *old_annotations = self->func_annotations;
923+
self->func_annotations = NULL;
924+
_PyEval_StartTheWorld(interp);
925+
Py_XDECREF(old_annotate);
926+
Py_XDECREF(old_annotations);
878927
return 0;
879928
}
880929
else {
@@ -927,8 +976,15 @@ function___annotations___set_impl(PyFunctionObject *self, PyObject *value)
927976
"__annotations__ must be set to a dict object");
928977
return -1;
929978
}
930-
Py_XSETREF(self->func_annotations, Py_XNewRef(value));
931-
Py_CLEAR(self->func_annotate);
979+
PyInterpreterState *interp = _PyInterpreterState_GET();
980+
_PyEval_StopTheWorld(interp);
981+
PyObject *old_annotations = self->func_annotations;
982+
self->func_annotations = Py_XNewRef(value);
983+
PyObject *old_annotate = self->func_annotate;
984+
self->func_annotate = NULL;
985+
_PyEval_StartTheWorld(interp);
986+
Py_XDECREF(old_annotations);
987+
Py_XDECREF(old_annotate);
932988
return 0;
933989
}
934990

@@ -969,7 +1025,12 @@ function___type_params___set_impl(PyFunctionObject *self, PyObject *value)
9691025
"__type_params__ must be set to a tuple");
9701026
return -1;
9711027
}
972-
Py_XSETREF(self->func_typeparams, Py_NewRef(value));
1028+
PyInterpreterState *interp = _PyInterpreterState_GET();
1029+
_PyEval_StopTheWorld(interp);
1030+
PyObject *old_typeparams = self->func_typeparams;
1031+
self->func_typeparams = Py_NewRef(value);
1032+
_PyEval_StartTheWorld(interp);
1033+
Py_XDECREF(old_typeparams);
9731034
return 0;
9741035
}
9751036

0 commit comments

Comments
 (0)