Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions Doc/c-api/function.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ There are a few functions specific to Python functions.

The C structure used for functions.

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


.. c:var:: PyTypeObject PyFunction_Type

Expand Down Expand Up @@ -71,6 +75,11 @@ There are a few functions specific to Python functions.
This is normally a :class:`string <str>` containing the module name,
but can be set to any other object by Python code.

In the free-threaded build, if the attribute may be modified concurrently,
convert the borrowed reference to a strong reference with :c:func:`Py_XNewRef`
immediately, before calling any other Python API. The same applies to
:c:func:`PyFunction_GET_MODULE`.


.. c:function:: PyObject* PyFunction_GetDefaults(PyObject *op)

Expand Down
6 changes: 6 additions & 0 deletions Doc/c-api/reflection.rst
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,12 @@ Reflection
Return the name of *func* if it is a function, class or instance object, else the
name of *func*\s type.

For a Python function or method, the returned pointer refers to the
function's name and must not be retained across operations that may change
that name. In the free-threaded build, copy the string before detaching
the thread state or executing Python code if the name may be modified
concurrently.


.. c:function:: const char* PyEval_GetFuncDesc(PyObject *func)

Expand Down
5 changes: 5 additions & 0 deletions Include/cpython/funcobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,12 @@ static inline PyObject* PyFunction_GET_GLOBALS(PyObject *func) {
#define PyFunction_GET_GLOBALS(func) PyFunction_GET_GLOBALS(_PyObject_CAST(func))

static inline PyObject* PyFunction_GET_MODULE(PyObject *func) {
#ifdef Py_GIL_DISABLED
return (PyObject *)_Py_atomic_load_ptr_acquire(
&_PyFunction_CAST(func)->func_module);
#else
return _PyFunction_CAST(func)->func_module;
#endif
Comment on lines +101 to +106

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would something like this work? It's a little shorter and should have the same effect. (Guessing there's a macro-expansion related reason you didn't use FT_ATOMIC_LOAD_PTR_ACQUIRE here because it's used liberally elsewhere in the PR.)

Suggested change
#ifdef Py_GIL_DISABLED
return (PyObject *)_Py_atomic_load_ptr_acquire(
&_PyFunction_CAST(func)->func_module);
#else
return _PyFunction_CAST(func)->func_module;
#endif
PyFunctionObject *fn = _PyFunction_CAST(func);
return FT_ATOMIC_LOAD_PTR_ACQUIRE(fn->func_module);

}
#define PyFunction_GET_MODULE(func) PyFunction_GET_MODULE(_PyObject_CAST(func))

Expand Down
6 changes: 6 additions & 0 deletions Lib/test/test_capi/test_function.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ def some():
with self.assertRaises(SystemError):
_testcapi.function_get_module(None) # not a function

self.assertEqual(_testcapi.function_get_module_macro(some), module)
value = object()
some.__module__ = value
self.assertIs(_testcapi.function_get_module(some), value)
self.assertIs(_testcapi.function_get_module_macro(some), value)

def test_function_get_defaults(self):
# Test PyFunction_GetDefaults()
def some(
Expand Down
117 changes: 117 additions & 0 deletions Lib/test/test_free_threading/test_functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,123 @@ def set_annotate():
threading_helper.run_concurrently(
[set_annotations, set_annotate] * (NUM_THREADS // 2))

def test_read_while_replacing_name_attrs(self):
# Readers load __name__, __qualname__, __doc__ and __module__
# without holding a lock. A writer thread replacing them must not
# free a value a reader thread is about to incref.
def target():
yield

def writer():
for _ in range(ITERS):
target.__name__ = random_string()
target.__qualname__ = random_string()
target.__doc__ = random_string()
target.__module__ = random_string()

def reader():
for _ in range(ITERS):
target.__name__
target.__qualname__
target.__doc__
target.__module__
# make_gen() copies __name__ and __qualname__.
gen = target()
gen.__name__
gen.__qualname__
gen.close()

threading_helper.run_concurrently(
[writer, reader] * (NUM_THREADS // 2))

def test_repr_while_replacing_qualname(self):
def target():
pass

def writer():
for _ in range(ITERS):
target.__qualname__ = random_string()

def reader():
for _ in range(ITERS):
self.assertRegex(repr(target), r"^<function .* at 0x[0-9a-fA-F]+>$")

threading_helper.run_concurrently(
[writer, reader] * (NUM_THREADS // 2))

def test_c_api_read_while_replacing_module(self):
from test.support import import_helper
_testcapi = import_helper.import_module('_testcapi')

def target():
pass

def writer():
for _ in range(ITERS):
target.__module__ = random_string()
target.__module__ = object()
target.__module__ = None

def reader():
for _ in range(ITERS):
for get_module in (_testcapi.function_get_module,
_testcapi.function_get_module_macro):
value = get_module(target)
self.assertIn(type(value), (str, object, type(None)))

threading_helper.run_concurrently(
[writer, reader] * (NUM_THREADS // 2))

def test_c_api_read_while_replacing_name(self):
from test.support import import_helper
_testlimitedcapi = import_helper.import_module('_testlimitedcapi')

def target():
pass

def writer():
for _ in range(ITERS):
# Non-ASCII names exercise lazy UTF-8 conversion.
target.__name__ = 'name_\u03bb' + random_string()

def reader():
for _ in range(ITERS):
name = _testlimitedcapi.eval_get_func_name(target)
self.assertIsInstance(name, str)

threading_helper.run_concurrently(
[writer, reader] * (NUM_THREADS // 2))

def test_argument_errors_while_replacing_qualname(self):
def target(x, *, y):
pass

def posonly(x, /):
pass

calls = (
lambda: target(),
lambda: target(1),
lambda: target(1, 2),
lambda: target(1, x=2),
lambda: target(1, extra=2),
lambda: posonly(x=1),
)

def writer():
for _ in range(ITERS):
target.__qualname__ = random_string()
posonly.__qualname__ = random_string()

def reader():
for _ in range(ITERS):
for call in calls:
with self.assertRaises(TypeError):
call()

threading_helper.run_concurrently(
[writer, reader] * (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.
Expand Down
31 changes: 31 additions & 0 deletions Lib/test/test_funcattrs.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import gc
import textwrap
import types
import typing
import unittest
import warnings
import weakref
from test import support


Expand Down Expand Up @@ -50,6 +52,35 @@ class FunctionPropertiesTest(FuncAttrsTest):
def test_module(self):
self.assertEqual(self.b.__module__, __name__)

def test_qualname_replaced_during_argument_error(self):
class Qualname(str):
pass

def target(x, /):
pass

target.__qualname__ = Qualname('original_qualname')
ref = weakref.ref(target.__qualname__)

class Keyword(str):
__hash__ = str.__hash__

def __eq__(self, other):
# Replacing and freeing the qualname while comparisons
# execute Python must not leave a dangling pointer.
target.__qualname__ = 'new_qualname'
gc.collect()
return True

with self.assertRaisesRegex(
TypeError,
r"new_qualname\(\) got some positional-only arguments "
r"passed as keyword arguments: 'x'",
):
target(**{Keyword('x'): 1})
gc.collect()
self.assertIsNone(ref())

def test_dir_includes_correct_attrs(self):
self.b.known_attr = 7
self.assertIn('known_attr', dir(self.b),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
Setting ``__name__``, ``__qualname__``, ``__doc__`` or ``__module__`` on a
function no longer stops all threads in the free-threaded build. The setters
now use the per-object lock and defer freeing the old value with QSBR, the
same approach used for generator ``__name__``. This fixes a large slowdown
in multi-threaded code using :func:`functools.wraps` since 3.14.7. Readers
of these attributes now use atomic loads, and argument error reporting reads
``__qualname__`` after keyword comparisons, which may execute Python code and
replace it. Previously this could use a freed object.
8 changes: 8 additions & 0 deletions Modules/_testcapi/function.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,13 @@ function_get_module(PyObject *self, PyObject *func)
}


static PyObject *
function_get_module_macro(PyObject *self, PyObject *func)
{
return Py_XNewRef(PyFunction_GET_MODULE(func));
}


static PyObject *
function_get_defaults(PyObject *self, PyObject *func)
{
Expand Down Expand Up @@ -134,6 +141,7 @@ static PyMethodDef test_methods[] = {
{"function_get_code", function_get_code, METH_O, NULL},
{"function_get_globals", function_get_globals, METH_O, NULL},
{"function_get_module", function_get_module, METH_O, NULL},
{"function_get_module_macro", function_get_module_macro, METH_O, NULL},
{"function_get_defaults", function_get_defaults, METH_O, NULL},
{"function_set_defaults", function_set_defaults, METH_VARARGS, NULL},
{"function_get_kw_defaults", function_get_kw_defaults, METH_O, NULL},
Expand Down
6 changes: 4 additions & 2 deletions Modules/_testinternalcapi.c
Original file line number Diff line number Diff line change
Expand Up @@ -1182,7 +1182,8 @@ record_eval(PyThreadState *tstate, struct _PyInterpreterFrame *f, int exc)
assert(module != NULL);
module_state *state = get_module_state(module);
Py_DECREF(module);
int res = PyList_Append(state->record_list, func->func_name);
int res = PyList_Append(state->record_list,
FT_ATOMIC_LOAD_PTR_ACQUIRE(func->func_name));
if (res < 0) {
return NULL;
}
Expand Down Expand Up @@ -1243,7 +1244,8 @@ record_eval_interp(PyThreadState *tstate, struct _PyInterpreterFrame *f, int exc
assert(module != NULL);
module_state *state = get_module_state(module);
Py_DECREF(module);
int res = PyList_Append(state->record_list, func->func_name);
int res = PyList_Append(state->record_list,
FT_ATOMIC_LOAD_PTR_ACQUIRE(func->func_name));
if (res < 0) {
return NULL;
}
Expand Down
Loading
Loading