gh-111903: Add `@critical_section` directive to Argument Clinic. (#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.
This commit is contained in:
Sam Gross 2023-11-14 05:47:46 -05:00 committed by GitHub
parent 16055c1604
commit 324531df90
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 117 additions and 8 deletions

View File

@ -5467,3 +5467,78 @@ exit:
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]*/

View File

@ -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.

View File

@ -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;

View File

@ -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 @@ skip_optional_pos:
exit:
return return_value;
}
/*[clinic end generated code: output=090e70253e35fc22 input=a9049054013a1b77]*/
/*[clinic end generated code: output=b23847480eba3d9b input=a9049054013a1b77]*/

View File

@ -470,6 +470,10 @@ class CRenderData:
# 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 @@ class CLanguage(Language):
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 @@ class CLanguage(Language):
""") + "\n"
finale = normalize_snippet("""
{modifications}
{lock}
{return_value} = {c_basename}_impl({impl_arguments});
{unlock}
{return_conversion}
{post_parsing}
@ -1219,7 +1226,7 @@ class CLanguage(Language):
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 @@ class CLanguage(Language):
}}
""" % return_error, indent=4)]
if default_return_converter:
if simple_return:
parser_definition = '\n'.join([
parser_prototype,
'{{',
@ -1245,7 +1252,7 @@ class CLanguage(Language):
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 @@ class CLanguage(Language):
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 @@ class CLanguage(Language):
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 @@ class CLanguage(Language):
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 @@ class DSLParser:
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 @@ class DSLParser:
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 @@ class DSLParser:
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