Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Argument Clinic: Add support for PEP 703's critical sections (Py_BEGIN_CRITICAL_SECTION()) #111903

Closed
Tracked by #108219
colesbury opened this issue Nov 9, 2023 · 2 comments
Assignees
Labels
3.13 bugs and security fixes topic-argument-clinic topic-free-threading type-feature A feature request or enhancement

Comments

@colesbury
Copy link
Contributor

colesbury commented Nov 9, 2023

Feature or enhancement

PEP 703 in large part relies on replacing the GIL with fine grained per-object locks. The primary way to acquire and release these locks is through the critical section API (#111569). It would be helpful if argument clinic could auto generate the API calls in the binding code when specified.

For example, the bufferedio.c will require per-object locking around most calls for thread-safety in --disable-gil builds.

As an example, the _io._Buffered.close function is written as:

/*[clinic input]
_io._Buffered.close
[clinic start generated code]*/
static PyObject *
_io__Buffered_close_impl(buffered *self)
/*[clinic end generated code: output=7280b7b42033be0c input=d20b83d1ddd7d805]*/

We might add a @critical_section directive to designate that argument clinic should generate Py_BEGIN_CRITICAL_SECTION() and Py_END_CRITICAL_SECTION() in the binding code before calling _io__Buffered_close_impl.

/*[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]*/

The generated binding code in bufferedio.c.h would then look like:

static PyObject *
_io__Buffered_close(buffered *self, PyObject *Py_UNUSED(ignored))
{
    PyObject *return_value = NULL;

    Py_BEGIN_CRITICAL_SECTION(self);
    return_value = _io__Buffered_close_impl(self);
    Py_END_CRITICAL_SECTION();

    return return_value;
}

Note that Py_BEGIN/END_CRITICAL_SECTION() are no-ops in the default build.

Linked PRs

@corona10
Copy link
Member

corona10 commented Nov 9, 2023

cc @erlend-aasland

@colesbury
Copy link
Contributor Author

I've updated the issue to use AC's existing "directive" feature and syntax.

colesbury added a commit to colesbury/cpython that referenced this issue Nov 9, 2023
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.
@colesbury colesbury self-assigned this Nov 9, 2023
colesbury added a commit to colesbury/devguide that referenced this issue Nov 10, 2023
AlexWaygood pushed a commit that referenced this issue Nov 14, 2023
…1904)

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.
AlexWaygood added a commit to python/devguide that referenced this issue Nov 14, 2023
See also python/cpython#111903.

---------

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
aisk pushed a commit to aisk/cpython that referenced this issue Feb 11, 2024
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.13 bugs and security fixes topic-argument-clinic topic-free-threading type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

2 participants