diff --git a/Lib/test/clinic.test.c b/Lib/test/clinic.test.c index 2ef7a6e8eb27952..da730c5b2495c8d 100644 --- a/Lib/test/clinic.test.c +++ b/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]*/ diff --git a/Misc/NEWS.d/next/Tools-Demos/2023-11-09-13-04-29.gh-issue-111903.7Prryr.rst b/Misc/NEWS.d/next/Tools-Demos/2023-11-09-13-04-29.gh-issue-111903.7Prryr.rst new file mode 100644 index 000000000000000..41601f7511797a2 --- /dev/null +++ b/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. diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c index e8caf9f0df6dbf2..169b2b3d105669e 100644 --- a/Modules/_io/bufferedio.c +++ b/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; diff --git a/Modules/_io/clinic/bufferedio.c.h b/Modules/_io/clinic/bufferedio.c.h index f6ac2699135917b..3fe03506b6cfa5c 100644 --- a/Modules/_io/clinic/bufferedio.c.h +++ b/Modules/_io/clinic/bufferedio.c.h @@ -324,7 +324,13 @@ _io__Buffered_close_impl(buffered *self); static PyObject * _io__Buffered_close(buffered *self, PyObject *Py_UNUSED(ignored)) { - return _io__Buffered_close_impl(self); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = _io__Buffered_close_impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; } PyDoc_STRVAR(_io__Buffered_detach__doc__, @@ -1075,4 +1081,4 @@ _io_BufferedRandom___init__(PyObject *self, PyObject *args, PyObject *kwargs) exit: return return_value; } -/*[clinic end generated code: output=090e70253e35fc22 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=b23847480eba3d9b input=a9049054013a1b77]*/ diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 2ea93e610b086b1..a205a5182ee99f5 100755 --- a/Tools/clinic/clinic.py +++ b/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