Skip to content

Commit

Permalink
pythongh-111903: Add @critical_section directive to Argument Clinic.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
colesbury committed Nov 9, 2023
1 parent 6f09f69 commit 83da40a
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -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.
4 changes: 3 additions & 1 deletion Modules/_io/bufferedio.c
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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;
Expand Down
10 changes: 8 additions & 2 deletions Modules/_io/clinic/bufferedio.c.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

28 changes: 25 additions & 3 deletions Tools/clinic/clinic.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
"""
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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,
'{{',
Expand All @@ -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.
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 83da40a

Please sign in to comment.