Skip to content

Commit 010bebb

Browse files
naschemenascheme
authored
gh-159136: Avoid stop-the-world when setting function name attributes (gh-159269)
Setting __name__, __qualname__, __doc__ or __module__ on a function stopped the world in the free-threaded build since gh-154826 and gh-154851, making functools.wraps() orders of magnitude slower with multiple threads. Use the per-object critical section and _PyObject_XSetRefDelayed() instead, as gen_set_name() already does. Use acquire loads in the readers and hold strong references across error reporting, tracing, and eval-frame recording that may execute Python or detach the thread. The __code__, __defaults__, __kwdefaults__ and __closure__ setters still stop the world to clear func_version. Document synchronization requirements for direct field access and the lifetime of borrowed function attributes.
1 parent 8013779 commit 010bebb

13 files changed

Lines changed: 317 additions & 77 deletions

File tree

‎Doc/c-api/function.rst‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@ There are a few functions specific to Python functions.
1414
1515
The C structure used for functions.
1616

17+
In the free-threaded build, direct access to mutable fields requires
18+
synchronization if another thread may modify the function. See
19+
:ref:`freethreading-extensions-howto`.
20+
1721

1822
.. c:var:: PyTypeObject PyFunction_Type
1923
@@ -71,6 +75,11 @@ There are a few functions specific to Python functions.
7175
This is normally a :class:`string <str>` containing the module name,
7276
but can be set to any other object by Python code.
7377
78+
In the free-threaded build, if the attribute may be modified concurrently,
79+
convert the borrowed reference to a strong reference with :c:func:`Py_XNewRef`
80+
immediately, before calling any other Python API. The same applies to
81+
:c:func:`PyFunction_GET_MODULE`.
82+
7483
7584
.. c:function:: PyObject* PyFunction_GetDefaults(PyObject *op)
7685

‎Doc/c-api/reflection.rst‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,12 @@ Reflection
9696
Return the name of *func* if it is a function, class or instance object, else the
9797
name of *func*\s type.
9898
99+
For a Python function or method, the returned pointer refers to the
100+
function's name and must not be retained across operations that may change
101+
that name. In the free-threaded build, copy the string before detaching
102+
the thread state or executing Python code if the name may be modified
103+
concurrently.
104+
99105
100106
.. c:function:: const char* PyEval_GetFuncDesc(PyObject *func)
101107

‎Include/cpython/funcobject.h‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,12 @@ static inline PyObject* PyFunction_GET_GLOBALS(PyObject *func) {
9898
#define PyFunction_GET_GLOBALS(func) PyFunction_GET_GLOBALS(_PyObject_CAST(func))
9999

100100
static inline PyObject* PyFunction_GET_MODULE(PyObject *func) {
101+
#ifdef Py_GIL_DISABLED
102+
return (PyObject *)_Py_atomic_load_ptr_acquire(
103+
&_PyFunction_CAST(func)->func_module);
104+
#else
101105
return _PyFunction_CAST(func)->func_module;
106+
#endif
102107
}
103108
#define PyFunction_GET_MODULE(func) PyFunction_GET_MODULE(_PyObject_CAST(func))
104109

@@ -118,7 +123,12 @@ static inline PyObject* PyFunction_GET_CLOSURE(PyObject *func) {
118123
#define PyFunction_GET_CLOSURE(func) PyFunction_GET_CLOSURE(_PyObject_CAST(func))
119124

120125
static inline PyObject* PyFunction_GET_ANNOTATIONS(PyObject *func) {
126+
#ifdef Py_GIL_DISABLED
127+
return (PyObject *)_Py_atomic_load_ptr_acquire(
128+
&_PyFunction_CAST(func)->func_annotations);
129+
#else
121130
return _PyFunction_CAST(func)->func_annotations;
131+
#endif
122132
}
123133
#define PyFunction_GET_ANNOTATIONS(func) PyFunction_GET_ANNOTATIONS(_PyObject_CAST(func))
124134

‎Lib/test/test_capi/test_function.py‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,12 @@ def some():
4444
with self.assertRaises(SystemError):
4545
_testcapi.function_get_module(None) # not a function
4646

47+
self.assertEqual(_testcapi.function_get_module_macro(some), module)
48+
value = object()
49+
some.__module__ = value
50+
self.assertIs(_testcapi.function_get_module(some), value)
51+
self.assertIs(_testcapi.function_get_module_macro(some), value)
52+
4753
def test_function_get_defaults(self):
4854
# Test PyFunction_GetDefaults()
4955
def some(

‎Lib/test/test_free_threading/test_functions.py‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,123 @@ def set_annotate():
8181
threading_helper.run_concurrently(
8282
[set_annotations, set_annotate] * (NUM_THREADS // 2))
8383

84+
def test_read_while_replacing_name_attrs(self):
85+
# Readers load __name__, __qualname__, __doc__ and __module__
86+
# without holding a lock. A writer thread replacing them must not
87+
# free a value a reader thread is about to incref.
88+
def target():
89+
yield
90+
91+
def writer():
92+
for _ in range(ITERS):
93+
target.__name__ = random_string()
94+
target.__qualname__ = random_string()
95+
target.__doc__ = random_string()
96+
target.__module__ = random_string()
97+
98+
def reader():
99+
for _ in range(ITERS):
100+
target.__name__
101+
target.__qualname__
102+
target.__doc__
103+
target.__module__
104+
# make_gen() copies __name__ and __qualname__.
105+
gen = target()
106+
gen.__name__
107+
gen.__qualname__
108+
gen.close()
109+
110+
threading_helper.run_concurrently(
111+
[writer, reader] * (NUM_THREADS // 2))
112+
113+
def test_repr_while_replacing_qualname(self):
114+
def target():
115+
pass
116+
117+
def writer():
118+
for _ in range(ITERS):
119+
target.__qualname__ = random_string()
120+
121+
def reader():
122+
for _ in range(ITERS):
123+
self.assertRegex(repr(target), r"^<function .* at 0x[0-9a-fA-F]+>$")
124+
125+
threading_helper.run_concurrently(
126+
[writer, reader] * (NUM_THREADS // 2))
127+
128+
def test_c_api_read_while_replacing_module(self):
129+
from test.support import import_helper
130+
_testcapi = import_helper.import_module('_testcapi')
131+
132+
def target():
133+
pass
134+
135+
def writer():
136+
for _ in range(ITERS):
137+
target.__module__ = random_string()
138+
target.__module__ = object()
139+
target.__module__ = None
140+
141+
def reader():
142+
for _ in range(ITERS):
143+
for get_module in (_testcapi.function_get_module,
144+
_testcapi.function_get_module_macro):
145+
value = get_module(target)
146+
self.assertIn(type(value), (str, object, type(None)))
147+
148+
threading_helper.run_concurrently(
149+
[writer, reader] * (NUM_THREADS // 2))
150+
151+
def test_c_api_read_while_replacing_name(self):
152+
from test.support import import_helper
153+
_testlimitedcapi = import_helper.import_module('_testlimitedcapi')
154+
155+
def target():
156+
pass
157+
158+
def writer():
159+
for _ in range(ITERS):
160+
# Non-ASCII names exercise lazy UTF-8 conversion.
161+
target.__name__ = 'name_\u03bb' + random_string()
162+
163+
def reader():
164+
for _ in range(ITERS):
165+
name = _testlimitedcapi.eval_get_func_name(target)
166+
self.assertIsInstance(name, str)
167+
168+
threading_helper.run_concurrently(
169+
[writer, reader] * (NUM_THREADS // 2))
170+
171+
def test_argument_errors_while_replacing_qualname(self):
172+
def target(x, *, y):
173+
pass
174+
175+
def posonly(x, /):
176+
pass
177+
178+
calls = (
179+
lambda: target(),
180+
lambda: target(1),
181+
lambda: target(1, 2),
182+
lambda: target(1, x=2),
183+
lambda: target(1, extra=2),
184+
lambda: posonly(x=1),
185+
)
186+
187+
def writer():
188+
for _ in range(ITERS):
189+
target.__qualname__ = random_string()
190+
posonly.__qualname__ = random_string()
191+
192+
def reader():
193+
for _ in range(ITERS):
194+
for call in calls:
195+
with self.assertRaises(TypeError):
196+
call()
197+
198+
threading_helper.run_concurrently(
199+
[writer, reader] * (NUM_THREADS // 2))
200+
84201
def test_call_while_replacing_defaults(self):
85202
# The eval loop reads __defaults__ and __kwdefaults__ without holding
86203
# a lock while pushing a frame.

‎Lib/test/test_funcattrs.py‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
1+
import gc
12
import textwrap
23
import types
34
import typing
45
import unittest
56
import warnings
7+
import weakref
68
from test import support
79

810

@@ -50,6 +52,35 @@ class FunctionPropertiesTest(FuncAttrsTest):
5052
def test_module(self):
5153
self.assertEqual(self.b.__module__, __name__)
5254

55+
def test_qualname_replaced_during_argument_error(self):
56+
class Qualname(str):
57+
pass
58+
59+
def target(x, /):
60+
pass
61+
62+
target.__qualname__ = Qualname('original_qualname')
63+
ref = weakref.ref(target.__qualname__)
64+
65+
class Keyword(str):
66+
__hash__ = str.__hash__
67+
68+
def __eq__(self, other):
69+
# Replacing and freeing the qualname while comparisons
70+
# execute Python must not leave a dangling pointer.
71+
target.__qualname__ = 'new_qualname'
72+
gc.collect()
73+
return True
74+
75+
with self.assertRaisesRegex(
76+
TypeError,
77+
r"new_qualname\(\) got some positional-only arguments "
78+
r"passed as keyword arguments: 'x'",
79+
):
80+
target(**{Keyword('x'): 1})
81+
gc.collect()
82+
self.assertIsNone(ref())
83+
5384
def test_dir_includes_correct_attrs(self):
5485
self.b.known_attr = 7
5586
self.assertIn('known_attr', dir(self.b),

‎Misc/NEWS.d/next/Core_and_Builtins/2026-10-12-20-00-00.gh-issue-159136.fnAttr.rst‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
Setting ``__name__``, ``__qualname__``, ``__doc__`` or ``__module__`` on a
2+
function no longer stops all threads in the free-threaded build. The setters
3+
now use the per-object lock and defer freeing the old value with QSBR, the
4+
same approach used for generator ``__name__``. This fixes a large slowdown
5+
in multi-threaded code using :func:`functools.wraps` since 3.14.7. Readers
6+
of these attributes now use atomic loads, and argument error reporting reads
7+
``__qualname__`` after keyword comparisons, which may execute Python code and
8+
replace it. Previously this could use a freed object.

‎Modules/_testcapi/function.c‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,13 @@ function_get_module(PyObject *self, PyObject *func)
3838
}
3939

4040

41+
static PyObject *
42+
function_get_module_macro(PyObject *self, PyObject *func)
43+
{
44+
return Py_XNewRef(PyFunction_GET_MODULE(func));
45+
}
46+
47+
4148
static PyObject *
4249
function_get_defaults(PyObject *self, PyObject *func)
4350
{
@@ -134,6 +141,7 @@ static PyMethodDef test_methods[] = {
134141
{"function_get_code", function_get_code, METH_O, NULL},
135142
{"function_get_globals", function_get_globals, METH_O, NULL},
136143
{"function_get_module", function_get_module, METH_O, NULL},
144+
{"function_get_module_macro", function_get_module_macro, METH_O, NULL},
137145
{"function_get_defaults", function_get_defaults, METH_O, NULL},
138146
{"function_set_defaults", function_set_defaults, METH_VARARGS, NULL},
139147
{"function_get_kw_defaults", function_get_kw_defaults, METH_O, NULL},

‎Modules/_testinternalcapi.c‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1182,7 +1182,8 @@ record_eval(PyThreadState *tstate, struct _PyInterpreterFrame *f, int exc)
11821182
assert(module != NULL);
11831183
module_state *state = get_module_state(module);
11841184
Py_DECREF(module);
1185-
int res = PyList_Append(state->record_list, func->func_name);
1185+
int res = PyList_Append(state->record_list,
1186+
FT_ATOMIC_LOAD_PTR_ACQUIRE(func->func_name));
11861187
if (res < 0) {
11871188
return NULL;
11881189
}
@@ -1243,7 +1244,8 @@ record_eval_interp(PyThreadState *tstate, struct _PyInterpreterFrame *f, int exc
12431244
assert(module != NULL);
12441245
module_state *state = get_module_state(module);
12451246
Py_DECREF(module);
1246-
int res = PyList_Append(state->record_list, func->func_name);
1247+
int res = PyList_Append(state->record_list,
1248+
FT_ATOMIC_LOAD_PTR_ACQUIRE(func->func_name));
12471249
if (res < 0) {
12481250
return NULL;
12491251
}

0 commit comments

Comments
 (0)