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 00000000000000..41601f7511797a --- /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 e8caf9f0df6dbf..169b2b3d105669 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 f6ac2699135917..3fe03506b6cfa5 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 5f94b90ae09bd0..321882b6362432 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_statement = [] + self.unlock_statement = [] + class FormatCounterFormatter(string.Formatter): """ @@ -1183,7 +1187,9 @@ def parser_body( """) + "\n" finale = normalize_snippet(""" {modifications} + {lock_statement} {return_value} = {c_basename}_impl({impl_arguments}); + {unlock_statement} {return_conversion} {post_parsing} @@ -1228,7 +1234,7 @@ def parser_body( }} """ % return_error, indent=4)] - if default_return_converter: + if default_return_converter and not f.critical_section: parser_definition = '\n'.join([ parser_prototype, '{{', @@ -1245,7 +1251,7 @@ def parser_body( converters[0].format_unit == 'O'): meth_o_prototype = self.METH_O_PROTOTYPE - if default_return_converter: + if default_return_converter and not f.critical_section: # 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 +1864,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_statement.append('Py_BEGIN_CRITICAL_SECTION({self_name});') + data.unlock_statement.append('Py_END_CRITICAL_SECTION();') + last_group = 0 first_optional = len(selfless) positional = selfless and selfless[-1].is_positional_only() @@ -1937,6 +1947,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_statement'] = "\n".join(data.lock_statement) + template_dict['unlock_statement'] = "\n".join(data.unlock_statement) # used by unpack tuple code generator unpack_min = first_optional @@ -1961,6 +1973,8 @@ def render_function( modifications=template_dict['modifications'], post_parsing=template_dict['post_parsing'], cleanup=template_dict['cleanup'], + lock_statement=template_dict['lock_statement'], + unlock_statement=template_dict['unlock_statement'], ) # Only generate the "exit:" label @@ -2954,6 +2968,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 @@ -5110,6 +5125,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: @@ -5144,6 +5160,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 @@ -5272,6 +5289,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") @@ -5460,6 +5480,7 @@ def state_modulename_name(self, line: str) -> None: return line, _, returns = line.partition('->') + returns = returns.strip() full_name, c_basename = self.parse_function_names(line) @@ -5494,7 +5515,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