pythongh-111903: Add @critical_section directive to Argument Clinic. (

python#111904)

The `@critical_section` directive instructs Argument Clinic to generate calls
to `Py_BEGIN_CRITICAL_SECTION()` and `Py_END_CRITICAL_SECTION()` around the
bound function. In `--disable-gil` builds, these calls will lock and unlock
the `self` object. They are no-ops in the default build.

This is used in one place (`_io._Buffered.close`) as a demonstration.
Subsequent PRs will use it more widely in the `_io.Buffered` bindings.

Lib/test/clinic.test.c

@@ -5467,3 +5467,78 @@ docstr_fallback_to_converter_default(PyObject *module, PyObject *const *args, Py
 static PyObject *
 docstr_fallback_to_converter_default_impl(PyObject *module, str a)
 /*[clinic end generated code: output=ae24a9c6f60ee8a6 input=0cbe6a4d24bc2274]*/
+
+
+/*[clinic input]
+@critical_section
+test_critical_section
+[clinic start generated code]*/
+
+PyDoc_STRVAR(test_critical_section__doc__,
+"test_critical_section($module, /)\n"
+"--\n"
+"\n");
+
+#define TEST_CRITICAL_SECTION_METHODDEF    \
+    {"test_critical_section", (PyCFunction)test_critical_section, METH_NOARGS, test_critical_section__doc__},
+
+static PyObject *
+test_critical_section_impl(PyObject *module);
+
+static PyObject *
+test_critical_section(PyObject *module, PyObject *Py_UNUSED(ignored))
+{
+    PyObject *return_value = NULL;
+
+    Py_BEGIN_CRITICAL_SECTION(module);
+    return_value = test_critical_section_impl(module);
+    Py_END_CRITICAL_SECTION();
+
+    return return_value;
+}
+
+static PyObject *
+test_critical_section_impl(PyObject *module)
+/*[clinic end generated code: output=9d5a87bb28aa3f0c input=8c58956d6ff00f80]*/
+
+
+/*[clinic input]
+@critical_section
+test_critical_section_meth_o
+    a: object(subclass_of="&PyUnicode_Type")
+    /
+[clinic start generated code]*/
+
+PyDoc_STRVAR(test_critical_section_meth_o__doc__,
+"test_critical_section_meth_o($module, a, /)\n"
+"--\n"
+"\n");
+
+#define TEST_CRITICAL_SECTION_METH_O_METHODDEF    \
+    {"test_critical_section_meth_o", (PyCFunction)test_critical_section_meth_o, METH_O, test_critical_section_meth_o__doc__},
+
+static PyObject *
+test_critical_section_meth_o_impl(PyObject *module, PyObject *a);
+
+static PyObject *
+test_critical_section_meth_o(PyObject *module, PyObject *arg)
+{
+    PyObject *return_value = NULL;
+    PyObject *a;
+
+    if (!PyUnicode_Check(arg)) {
+        _PyArg_BadArgument("test_critical_section_meth_o", "argument", "str", arg);
+        goto exit;
+    }
+    a = arg;
+    Py_BEGIN_CRITICAL_SECTION(module);
+    return_value = test_critical_section_meth_o_impl(module, a);
+    Py_END_CRITICAL_SECTION();
+
+exit:
+    return return_value;
+}
+
+static PyObject *
+test_critical_section_meth_o_impl(PyObject *module, PyObject *a)
+/*[clinic end generated code: output=7a9d7420802d1202 input=376533f51eceb6c3]*/

Misc/NEWS.d/next/Tools-Demos/2023-11-09-13-04-29.gh-issue-111903.7Prryr.rst

@@ -0,0 +1,4 @@
+Argument Clinic now supports the ``@critical_section`` directive that
+instructs Argument Clinic to generate a critical section around the function
+call, which locks the ``self`` object in ``--disable-gil`` builds. Patch by
+Sam Gross.

Modules/_io/bufferedio.c

@@ -10,6 +10,7 @@
 #include "Python.h"
 #include "pycore_bytesobject.h"   // _PyBytes_Join()
 #include "pycore_call.h"          // _PyObject_CallNoArgs()
+#include "pycore_critical_section.h"  // Py_BEGIN_CRITICAL_SECTION()
 #include "pycore_object.h"        // _PyObject_GC_UNTRACK()
 #include "pycore_pyerrors.h"      // _Py_FatalErrorFormat()
 #include "pycore_pylifecycle.h"   // _Py_IsInterpreterFinalizing()
@@ -521,12 +522,13 @@ buffered_closed_get(buffered *self, void *context)
 }
 
 /*[clinic input]
+@critical_section
 _io._Buffered.close
 [clinic start generated code]*/
 
 static PyObject *
 _io__Buffered_close_impl(buffered *self)
-/*[clinic end generated code: output=7280b7b42033be0c input=d20b83d1ddd7d805]*/
+/*[clinic end generated code: output=7280b7b42033be0c input=56d95935b03fd326]*/
 {
     PyObject *res = NULL;
     int r;

Tools/clinic/clinic.py

@@ -470,6 +470,10 @@ def __init__(self) -> None:
         # The C statements required to clean up after the impl call.
         self.cleanup: list[str] = []
 
+        # The C statements to generate critical sections (per-object locking).
+        self.lock: list[str] = []
+        self.unlock: list[str] = []
+
 
 class FormatCounterFormatter(string.Formatter):
     """
@@ -1109,7 +1113,8 @@ def output_templates(
                                    condition=include.condition)
 
         has_option_groups = parameters and (parameters[0].group or parameters[-1].group)
-        default_return_converter = f.return_converter.type == 'PyObject *'
+        simple_return = (f.return_converter.type == 'PyObject *'
+                         and not f.critical_section)
         new_or_init = f.kind.new_or_init
 
         vararg: int | str = NO_VARARG
@@ -1183,7 +1188,9 @@ def parser_body(
             """) + "\n"
             finale = normalize_snippet("""
                     {modifications}
+                    {lock}
                     {return_value} = {c_basename}_impl({impl_arguments});
+                    {unlock}
                     {return_conversion}
                     {post_parsing}
 
@@ -1219,7 +1226,7 @@ def parser_body(
 
                 flags = "METH_METHOD|METH_FASTCALL|METH_KEYWORDS"
                 parser_prototype = self.PARSER_PROTOTYPE_DEF_CLASS
-                return_error = ('return NULL;' if default_return_converter
+                return_error = ('return NULL;' if simple_return
                                 else 'goto exit;')
                 parser_code = [normalize_snippet("""
                     if (nargs) {{
@@ -1228,7 +1235,7 @@ def parser_body(
                     }}
                     """ % return_error, indent=4)]
 
-            if default_return_converter:
+            if simple_return:
                 parser_definition = '\n'.join([
                     parser_prototype,
                     '{{',
@@ -1245,7 +1252,7 @@ def parser_body(
                 converters[0].format_unit == 'O'):
                 meth_o_prototype = self.METH_O_PROTOTYPE
 
-                if default_return_converter:
+                if simple_return:
                     # maps perfectly to METH_O, doesn't need a return converter.
                     # so we skip making a parse function
                     # and call directly into the impl function.
@@ -1858,6 +1865,10 @@ def render_function(
         selfless = parameters[1:]
         assert isinstance(f_self.converter, self_converter), "No self parameter in " + repr(f.full_name) + "!"
 
+        if f.critical_section:
+            data.lock.append('Py_BEGIN_CRITICAL_SECTION({self_name});')
+            data.unlock.append('Py_END_CRITICAL_SECTION();')
+
         last_group = 0
         first_optional = len(selfless)
         positional = selfless and selfless[-1].is_positional_only()
@@ -1937,6 +1948,8 @@ def render_function(
         template_dict['post_parsing'] = format_escape("".join(data.post_parsing).rstrip())
         template_dict['cleanup'] = format_escape("".join(data.cleanup))
         template_dict['return_value'] = data.return_value
+        template_dict['lock'] = "\n".join(data.lock)
+        template_dict['unlock'] = "\n".join(data.unlock)
 
         # used by unpack tuple code generator
         unpack_min = first_optional
@@ -1961,6 +1974,8 @@ def render_function(
                 modifications=template_dict['modifications'],
                 post_parsing=template_dict['post_parsing'],
                 cleanup=template_dict['cleanup'],
+                lock=template_dict['lock'],
+                unlock=template_dict['unlock'],
                 )
 
             # Only generate the "exit:" label
@@ -2954,6 +2969,7 @@ class Function:
     # functions with optional groups because we can't represent
     # those accurately with inspect.Signature in 3.4.
     docstring_only: bool = False
+    critical_section: bool = False
 
     def __post_init__(self) -> None:
         self.parent = self.cls or self.module
@@ -5108,6 +5124,7 @@ class DSLParser:
     coexist: bool
     parameter_continuation: str
     preserve_output: bool
+    critical_section: bool
     from_version_re = re.compile(r'([*/]) +\[from +(.+)\]')
 
     def __init__(self, clinic: Clinic) -> None:
@@ -5142,6 +5159,7 @@ def reset(self) -> None:
         self.forced_text_signature: str | None = None
         self.parameter_continuation = ''
         self.preserve_output = False
+        self.critical_section = False
 
     def directive_version(self, required: str) -> None:
         global version
@@ -5270,6 +5288,9 @@ def at_classmethod(self) -> None:
             fail("Can't set @classmethod, function is not a normal callable")
         self.kind = CLASS_METHOD
 
+    def at_critical_section(self) -> None:
+        self.critical_section = True
+
     def at_staticmethod(self) -> None:
         if self.kind is not CALLABLE:
             fail("Can't set @staticmethod, function is not a normal callable")
@@ -5492,7 +5513,8 @@ def state_modulename_name(self, line: str) -> None:
             return_converter = CReturnConverter()
 
         self.function = Function(name=function_name, full_name=full_name, module=module, cls=cls, c_basename=c_basename,
-                                 return_converter=return_converter, kind=self.kind, coexist=self.coexist)
+                                 return_converter=return_converter, kind=self.kind, coexist=self.coexist,
+                                 critical_section=self.critical_section)
         self.block.signatures.append(self.function)
 
         # insert a self converter automatically