gh-128714: Fix function object races in __annotate__, __annotations__ and __type_params__ in free-threading build (#129016)

Lib/test/test_free_threading/test_func_annotations.py

@@ -0,0 +1,67 @@
+import concurrent.futures
+import unittest
+import inspect
+from threading import Thread, Barrier
+from unittest import TestCase
+
+from test.support import threading_helper, Py_GIL_DISABLED
+
+threading_helper.requires_working_threading(module=True)
+
+
+def get_func_annotation(f, b):
+    b.wait()
+    return inspect.get_annotations(f)
+
+
+def get_func_annotation_dunder(f, b):
+    b.wait()
+    return f.__annotations__
+
+
+def set_func_annotation(f, b):
+    b.wait()
+    f.__annotations__ = {'x': int, 'y': int, 'return': int}
+    return f.__annotations__
+
+
+@unittest.skipUnless(Py_GIL_DISABLED, "Enable only in FT build")
+class TestFTFuncAnnotations(TestCase):
+    NUM_THREADS = 8
+
+    def test_concurrent_read(self):
+        def f(x: int) -> int:
+            return x + 1
+
+        for _ in range(100):
+            with concurrent.futures.ThreadPoolExecutor(max_workers=self.NUM_THREADS) as executor:
+                b = Barrier(self.NUM_THREADS)
+                futures = {executor.submit(get_func_annotation, f, b): i for i in range(self.NUM_THREADS)}
+                for fut in concurrent.futures.as_completed(futures):
+                    annotate = fut.result()
+                    self.assertIsNotNone(annotate)
+                    self.assertEqual(annotate, {'x': int, 'return': int})
+
+            with concurrent.futures.ThreadPoolExecutor(max_workers=self.NUM_THREADS) as executor:
+                b = Barrier(self.NUM_THREADS)
+                futures = {executor.submit(get_func_annotation_dunder, f, b): i for i in range(self.NUM_THREADS)}
+                for fut in concurrent.futures.as_completed(futures):
+                    annotate = fut.result()
+                    self.assertIsNotNone(annotate)
+                    self.assertEqual(annotate, {'x': int, 'return': int})
+
+    def test_concurrent_write(self):
+        def bar(x: int, y: float) -> float:
+            return y ** x
+
+        for _ in range(100):
+            with concurrent.futures.ThreadPoolExecutor(max_workers=self.NUM_THREADS) as executor:
+                b = Barrier(self.NUM_THREADS)
+                futures = {executor.submit(set_func_annotation, bar, b): i for i in range(self.NUM_THREADS)}
+                for fut in concurrent.futures.as_completed(futures):
+                    annotate = fut.result()
+                    self.assertIsNotNone(annotate)
+                    self.assertEqual(annotate, {'x': int, 'y': int, 'return': int})
+
+            # func_get_annotations returns in-place dict, so bar.__annotations__ should be modified as well
+            self.assertEqual(bar.__annotations__, {'x': int, 'y': int, 'return': int})

Misc/NEWS.d/next/Core_and_Builtins/2025-01-19-09-07-44.gh-issue-128714.m1fyCB.rst

@@ -0,0 +1 @@
+Fix the potential races in get/set dunder methods ``__annotations__``, ``__annotate__`` and ``__type_params__`` for function object, and add related tests.

Objects/clinic/funcobject.c.h

@@ -6,8 +6,180 @@ preserve
 #  include "pycore_gc.h"          // PyGC_Head
 #  include "pycore_runtime.h"     // _Py_ID()
 #endif
+#include "pycore_critical_section.h"// Py_BEGIN_CRITICAL_SECTION()
 #include "pycore_modsupport.h"    // _PyArg_UnpackKeywords()
 
+PyDoc_STRVAR(function___annotate____doc__,
+"Get the code object for a function.");
+#if defined(function___annotate___DOCSTR)
+#   undef function___annotate___DOCSTR
+#endif
+#define function___annotate___DOCSTR function___annotate____doc__
+
+#if !defined(function___annotate___DOCSTR)
+#  define function___annotate___DOCSTR NULL
+#endif
+#if defined(FUNCTION___ANNOTATE___GETSETDEF)
+#  undef FUNCTION___ANNOTATE___GETSETDEF
+#  define FUNCTION___ANNOTATE___GETSETDEF {"__annotate__", (getter)function___annotate___get, (setter)function___annotate___set, function___annotate___DOCSTR},
+#else
+#  define FUNCTION___ANNOTATE___GETSETDEF {"__annotate__", (getter)function___annotate___get, NULL, function___annotate___DOCSTR},
+#endif
+
+static PyObject *
+function___annotate___get_impl(PyFunctionObject *self);
+
+static PyObject *
+function___annotate___get(PyObject *self, void *Py_UNUSED(context))
+{
+    PyObject *return_value = NULL;
+
+    Py_BEGIN_CRITICAL_SECTION(self);
+    return_value = function___annotate___get_impl((PyFunctionObject *)self);
+    Py_END_CRITICAL_SECTION();
+
+    return return_value;
+}
+
+#if !defined(function___annotate___DOCSTR)
+#  define function___annotate___DOCSTR NULL
+#endif
+#if defined(FUNCTION___ANNOTATE___GETSETDEF)
+#  undef FUNCTION___ANNOTATE___GETSETDEF
+#  define FUNCTION___ANNOTATE___GETSETDEF {"__annotate__", (getter)function___annotate___get, (setter)function___annotate___set, function___annotate___DOCSTR},
+#else
+#  define FUNCTION___ANNOTATE___GETSETDEF {"__annotate__", NULL, (setter)function___annotate___set, NULL},
+#endif
+
+static int
+function___annotate___set_impl(PyFunctionObject *self, PyObject *value);
+
+static int
+function___annotate___set(PyObject *self, PyObject *value, void *Py_UNUSED(context))
+{
+    int return_value;
+
+    Py_BEGIN_CRITICAL_SECTION(self);
+    return_value = function___annotate___set_impl((PyFunctionObject *)self, value);
+    Py_END_CRITICAL_SECTION();
+
+    return return_value;
+}
+
+PyDoc_STRVAR(function___annotations____doc__,
+"Dict of annotations in a function object.");
+#if defined(function___annotations___DOCSTR)
+#   undef function___annotations___DOCSTR
+#endif
+#define function___annotations___DOCSTR function___annotations____doc__
+
+#if !defined(function___annotations___DOCSTR)
+#  define function___annotations___DOCSTR NULL
+#endif
+#if defined(FUNCTION___ANNOTATIONS___GETSETDEF)
+#  undef FUNCTION___ANNOTATIONS___GETSETDEF
+#  define FUNCTION___ANNOTATIONS___GETSETDEF {"__annotations__", (getter)function___annotations___get, (setter)function___annotations___set, function___annotations___DOCSTR},
+#else
+#  define FUNCTION___ANNOTATIONS___GETSETDEF {"__annotations__", (getter)function___annotations___get, NULL, function___annotations___DOCSTR},
+#endif
+
+static PyObject *
+function___annotations___get_impl(PyFunctionObject *self);
+
+static PyObject *
+function___annotations___get(PyObject *self, void *Py_UNUSED(context))
+{
+    PyObject *return_value = NULL;
+
+    Py_BEGIN_CRITICAL_SECTION(self);
+    return_value = function___annotations___get_impl((PyFunctionObject *)self);
+    Py_END_CRITICAL_SECTION();
+
+    return return_value;
+}
+
+#if !defined(function___annotations___DOCSTR)
+#  define function___annotations___DOCSTR NULL
+#endif
+#if defined(FUNCTION___ANNOTATIONS___GETSETDEF)
+#  undef FUNCTION___ANNOTATIONS___GETSETDEF
+#  define FUNCTION___ANNOTATIONS___GETSETDEF {"__annotations__", (getter)function___annotations___get, (setter)function___annotations___set, function___annotations___DOCSTR},
+#else
+#  define FUNCTION___ANNOTATIONS___GETSETDEF {"__annotations__", NULL, (setter)function___annotations___set, NULL},
+#endif
+
+static int
+function___annotations___set_impl(PyFunctionObject *self, PyObject *value);
+
+static int
+function___annotations___set(PyObject *self, PyObject *value, void *Py_UNUSED(context))
+{
+    int return_value;
+
+    Py_BEGIN_CRITICAL_SECTION(self);
+    return_value = function___annotations___set_impl((PyFunctionObject *)self, value);
+    Py_END_CRITICAL_SECTION();
+
+    return return_value;
+}
+
+PyDoc_STRVAR(function___type_params____doc__,
+"Get the declared type parameters for a function.");
+#if defined(function___type_params___DOCSTR)
+#   undef function___type_params___DOCSTR
+#endif
+#define function___type_params___DOCSTR function___type_params____doc__
+
+#if !defined(function___type_params___DOCSTR)
+#  define function___type_params___DOCSTR NULL
+#endif
+#if defined(FUNCTION___TYPE_PARAMS___GETSETDEF)
+#  undef FUNCTION___TYPE_PARAMS___GETSETDEF
+#  define FUNCTION___TYPE_PARAMS___GETSETDEF {"__type_params__", (getter)function___type_params___get, (setter)function___type_params___set, function___type_params___DOCSTR},
+#else
+#  define FUNCTION___TYPE_PARAMS___GETSETDEF {"__type_params__", (getter)function___type_params___get, NULL, function___type_params___DOCSTR},
+#endif
+
+static PyObject *
+function___type_params___get_impl(PyFunctionObject *self);
+
+static PyObject *
+function___type_params___get(PyObject *self, void *Py_UNUSED(context))
+{
+    PyObject *return_value = NULL;
+
+    Py_BEGIN_CRITICAL_SECTION(self);
+    return_value = function___type_params___get_impl((PyFunctionObject *)self);
+    Py_END_CRITICAL_SECTION();
+
+    return return_value;
+}
+
+#if !defined(function___type_params___DOCSTR)
+#  define function___type_params___DOCSTR NULL
+#endif
+#if defined(FUNCTION___TYPE_PARAMS___GETSETDEF)
+#  undef FUNCTION___TYPE_PARAMS___GETSETDEF
+#  define FUNCTION___TYPE_PARAMS___GETSETDEF {"__type_params__", (getter)function___type_params___get, (setter)function___type_params___set, function___type_params___DOCSTR},
+#else
+#  define FUNCTION___TYPE_PARAMS___GETSETDEF {"__type_params__", NULL, (setter)function___type_params___set, NULL},
+#endif
+
+static int
+function___type_params___set_impl(PyFunctionObject *self, PyObject *value);
+
+static int
+function___type_params___set(PyObject *self, PyObject *value, void *Py_UNUSED(context))
+{
+    int return_value;
+
+    Py_BEGIN_CRITICAL_SECTION(self);
+    return_value = function___type_params___set_impl((PyFunctionObject *)self, value);
+    Py_END_CRITICAL_SECTION();
+
+    return return_value;
+}
+
 PyDoc_STRVAR(func_new__doc__,
 "function(code, globals, name=None, argdefs=None, closure=None,\n"
 "         kwdefaults=None)\n"
@@ -116,4 +288,4 @@ skip_optional_pos:
 exit:
     return return_value;
 }
-/*[clinic end generated code: output=bad4e19757dd26c3 input=a9049054013a1b77]*/
+/*[clinic end generated code: output=3cdce22867efe617 input=a9049054013a1b77]*/

Objects/funcobject.c

@@ -2,11 +2,11 @@
 /* Function object implementation */
 
 #include "Python.h"
-#include "pycore_dict.h"          // _Py_INCREF_DICT()
-#include "pycore_long.h"          // _PyLong_GetOne()
-#include "pycore_modsupport.h"    // _PyArg_NoKeywords()
-#include "pycore_object.h"        // _PyObject_GC_UNTRACK()
-#include "pycore_pyerrors.h"      // _PyErr_Occurred()
+#include "pycore_dict.h"                // _Py_INCREF_DICT()
+#include "pycore_long.h"                // _PyLong_GetOne()
+#include "pycore_modsupport.h"          // _PyArg_NoKeywords()
+#include "pycore_object.h"              // _PyObject_GC_UNTRACK()
+#include "pycore_pyerrors.h"            // _PyErr_Occurred()
 
 
 static const char *
@@ -635,6 +635,13 @@ static PyMemberDef func_memberlist[] = {
     {NULL}  /* Sentinel */
 };
 
+/*[clinic input]
+class function "PyFunctionObject *" "&PyFunction_Type"
+[clinic start generated code]*/
+/*[clinic end generated code: output=da39a3ee5e6b4b0d input=70af9c90aa2e71b0]*/
+
+#include "clinic/funcobject.c.h"
+
 static PyObject *
 func_get_code(PyObject *self, void *Py_UNUSED(ignored))
 {
@@ -824,32 +831,46 @@ func_set_kwdefaults(PyObject *self, PyObject *value, void *Py_UNUSED(ignored))
     return 0;
 }
 
+/*[clinic input]
+@critical_section
+@getter
+function.__annotate__
+
+Get the code object for a function.
+[clinic start generated code]*/
+
 static PyObject *
-func_get_annotate(PyObject *self, void *Py_UNUSED(ignored))
+function___annotate___get_impl(PyFunctionObject *self)
+/*[clinic end generated code: output=5ec7219ff2bda9e6 input=7f3db11e3c3329f3]*/
 {
-    PyFunctionObject *op = _PyFunction_CAST(self);
-    if (op->func_annotate == NULL) {
+    if (self->func_annotate == NULL) {
         Py_RETURN_NONE;
     }
-    return Py_NewRef(op->func_annotate);
+    return Py_NewRef(self->func_annotate);
 }
 
+/*[clinic input]
+@critical_section
+@setter
+function.__annotate__
+[clinic start generated code]*/
+
 static int
-func_set_annotate(PyObject *self, PyObject *value, void *Py_UNUSED(ignored))
+function___annotate___set_impl(PyFunctionObject *self, PyObject *value)
+/*[clinic end generated code: output=05b7dfc07ada66cd input=eb6225e358d97448]*/
 {
-    PyFunctionObject *op = _PyFunction_CAST(self);
     if (value == NULL) {
         PyErr_SetString(PyExc_TypeError,
             "__annotate__ cannot be deleted");
         return -1;
     }
     if (Py_IsNone(value)) {
-        Py_XSETREF(op->func_annotate, value);
+        Py_XSETREF(self->func_annotate, value);
         return 0;
     }
     else if (PyCallable_Check(value)) {
-        Py_XSETREF(op->func_annotate, Py_XNewRef(value));
-        Py_CLEAR(op->func_annotations);
+        Py_XSETREF(self->func_annotate, Py_XNewRef(value));
+        Py_CLEAR(self->func_annotations);
         return 0;
     }
     else {
@@ -859,24 +880,39 @@ func_set_annotate(PyObject *self, PyObject *value, void *Py_UNUSED(ignored))
     }
 }
 
+/*[clinic input]
+@critical_section
+@getter
+function.__annotations__
+
+Dict of annotations in a function object.
+[clinic start generated code]*/
+
 static PyObject *
-func_get_annotations(PyObject *self, void *Py_UNUSED(ignored))
+function___annotations___get_impl(PyFunctionObject *self)
+/*[clinic end generated code: output=a4cf4c884c934cbb input=92643d7186c1ad0c]*/
 {
-    PyFunctionObject *op = _PyFunction_CAST(self);
-    if (op->func_annotations == NULL &&
-        (op->func_annotate == NULL || !PyCallable_Check(op->func_annotate))) {
-        op->func_annotations = PyDict_New();
-        if (op->func_annotations == NULL)
+    PyObject *d = NULL;
+    if (self->func_annotations == NULL &&
+        (self->func_annotate == NULL || !PyCallable_Check(self->func_annotate))) {
+        self->func_annotations = PyDict_New();
+        if (self->func_annotations == NULL)
             return NULL;
     }
-    PyObject *d = func_get_annotation_dict(op);
+    d = func_get_annotation_dict(self);
     return Py_XNewRef(d);
 }
 
+/*[clinic input]
+@critical_section
+@setter
+function.__annotations__
+[clinic start generated code]*/
+
 static int
-func_set_annotations(PyObject *self, PyObject *value, void *Py_UNUSED(ignored))
+function___annotations___set_impl(PyFunctionObject *self, PyObject *value)
+/*[clinic end generated code: output=a61795d4a95eede4 input=5302641f686f0463]*/
 {
-    PyFunctionObject *op = _PyFunction_CAST(self);
     if (value == Py_None)
         value = NULL;
     /* Legal to del f.func_annotations.
@@ -887,35 +923,49 @@ func_set_annotations(PyObject *self, PyObject *value, void *Py_UNUSED(ignored))
             "__annotations__ must be set to a dict object");
         return -1;
     }
-    Py_XSETREF(op->func_annotations, Py_XNewRef(value));
-    Py_CLEAR(op->func_annotate);
+    Py_XSETREF(self->func_annotations, Py_XNewRef(value));
+    Py_CLEAR(self->func_annotate);
     return 0;
 }
 
+/*[clinic input]
+@critical_section
+@getter
+function.__type_params__
+
+Get the declared type parameters for a function.
+[clinic start generated code]*/
+
 static PyObject *
-func_get_type_params(PyObject *self, void *Py_UNUSED(ignored))
+function___type_params___get_impl(PyFunctionObject *self)
+/*[clinic end generated code: output=eb844d7ffca517a8 input=0864721484293724]*/
 {
-    PyFunctionObject *op = _PyFunction_CAST(self);
-    if (op->func_typeparams == NULL) {
+    if (self->func_typeparams == NULL) {
         return PyTuple_New(0);
     }
 
-    assert(PyTuple_Check(op->func_typeparams));
-    return Py_NewRef(op->func_typeparams);
+    assert(PyTuple_Check(self->func_typeparams));
+    return Py_NewRef(self->func_typeparams);
 }
 
+/*[clinic input]
+@critical_section
+@setter
+function.__type_params__
+[clinic start generated code]*/
+
 static int
-func_set_type_params(PyObject *self, PyObject *value, void *Py_UNUSED(ignored))
+function___type_params___set_impl(PyFunctionObject *self, PyObject *value)
+/*[clinic end generated code: output=038b4cda220e56fb input=3862fbd4db2b70e8]*/
 {
     /* Not legal to del f.__type_params__ or to set it to anything
      * other than a tuple object. */
-    PyFunctionObject *op = _PyFunction_CAST(self);
     if (value == NULL || !PyTuple_Check(value)) {
         PyErr_SetString(PyExc_TypeError,
                         "__type_params__ must be set to a tuple");
         return -1;
     }
-    Py_XSETREF(op->func_typeparams, Py_NewRef(value));
+    Py_XSETREF(self->func_typeparams, Py_NewRef(value));
     return 0;
 }
 
@@ -934,22 +984,15 @@ static PyGetSetDef func_getsetlist[] = {
     {"__code__", func_get_code, func_set_code},
     {"__defaults__", func_get_defaults, func_set_defaults},
     {"__kwdefaults__", func_get_kwdefaults, func_set_kwdefaults},
-    {"__annotations__", func_get_annotations, func_set_annotations},
-    {"__annotate__", func_get_annotate, func_set_annotate},
+    FUNCTION___ANNOTATIONS___GETSETDEF
+    FUNCTION___ANNOTATE___GETSETDEF
     {"__dict__", PyObject_GenericGetDict, PyObject_GenericSetDict},
     {"__name__", func_get_name, func_set_name},
     {"__qualname__", func_get_qualname, func_set_qualname},
-    {"__type_params__", func_get_type_params, func_set_type_params},
+    FUNCTION___TYPE_PARAMS___GETSETDEF
     {NULL} /* Sentinel */
 };
 
-/*[clinic input]
-class function "PyFunctionObject *" "&PyFunction_Type"
-[clinic start generated code]*/
-/*[clinic end generated code: output=da39a3ee5e6b4b0d input=70af9c90aa2e71b0]*/
-
-#include "clinic/funcobject.c.h"
-
 /* function.__new__() maintains the following invariants for closures.
    The closure must correspond to the free variables of the code object.