Skip to content

gh-133296: Publicly expose critical section API that accepts PyMutex - #135899

Merged
colesbury merged 6 commits into
python:mainfrom
ngoldbaum:expose-critical-section-begin-mutex-gh-133296
Jul 21, 2025
Merged

gh-133296: Publicly expose critical section API that accepts PyMutex#135899
colesbury merged 6 commits into
python:mainfrom
ngoldbaum:expose-critical-section-begin-mutex-gh-133296

Conversation

@ngoldbaum

@ngoldbaumngoldbaum commented Jun 24, 2025•
edited
Loading

Copy link
Copy Markdown
Contributor

xref gh-133296

See capi-workgroup/decisions#67 for a detailed proposal and vote from the C API workgroup.

I tagged this to be added for 3.15 but maybe @hugovk is willing to grant an exception so this lands in 3.14, since it's pretty straightforward and is new API so it shouldn't break anyone.

Docs preview: https://cpython-previews--135899.org.readthedocs.build/en/135899/c-api/init.html#python-critical-section-api

@bedevere-appbedevere-appBot mentioned this pull request Jun 24, 2025
Closed
colesbury
colesbury reviewed Jun 24, 2025
View reviewed changes

@colesburycolesbury left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Some small clarifications that the macros take PyMutex* as opposed to PyMutex.

Comment threadDoc/c-api/init.rst Outdated
Comment threadDoc/c-api/init.rst Outdated
colesbury
colesbury reviewed Jun 24, 2025
View reviewed changes
Comment threadDoc/c-api/init.rst Outdated
Comment threadDoc/c-api/init.rst Outdated
@ngoldbaum@colesbury
Apply suggestions from code review
6d987c7
Co-authored-by: Sam Gross <colesbury@gmail.com>
ngoldbaum
ngoldbaum commented Jun 24, 2025
View reviewed changes
Comment threadMisc/NEWS.d/next/C_API/2025-06-24-11-10-01.gh-issue-133296.lIEuVJ.rst Outdated
kumaraditya303
kumaraditya303 reviewed Jul 3, 2025
View reviewed changes
Comment threadDoc/c-api/init.rst Outdated
@ngoldbaum

ngoldbaum commented Jul 21, 2025

Copy link
Copy Markdown
ContributorAuthor

The C API workgroup vote now shows unanimous approval for adding this: capi-workgroup/decisions#67 (comment)

@colesbury
colesbury merged commit 89c220b into python:mainJul 21, 2025
46 checks passed
@ngoldbaum

ngoldbaum commented Jul 21, 2025

Copy link
Copy Markdown
ContributorAuthor

@hugovk I know it's pretty late, but I think it would help a few downstream use-cases if we could get this and #134365 backported to 3.14 in time for RC1.

@hugovkhugovk added the needs backport to 3.14 bugs and security fixes label Jul 22, 2025
@miss-islington-app

miss-islington-appBot commented Jul 22, 2025

Copy link
Copy Markdown

Thanks @ngoldbaum for the PR, and @colesbury for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 22, 2025
@ngoldbaum@miss-islington
pythongh-133296: Publicly expose critical section API that accepts Py…
518dc27
…Mutex (pythongh-135899)
This makes the following APIs public:
* `Py_BEGIN_CRITICAL_SECTION_MUTEX(mutex),`
* `Py_BEGIN_CRITICAL_SECTION2_MUTEX(mutex1, mutex2)`
* `void PyCriticalSection_BeginMutex(PyCriticalSection *c, PyMutex *mutex)`
* `void PyCriticalSection2_BeginMutex(PyCriticalSection2 *c, PyMutex *mutex1, PyMutex *mutex2)`
The macros are identical to the corresponding `Py_BEGIN_CRITICAL_SECTION` and
`Py_BEGIN_CRITICAL_SECTION2` macros (e.g., they include braces), but they
accept a `PyMutex` instead of an object.
The new macros are still paired with the existing END macros
(`Py_END_CRITICAL_SECTION`, `Py_END_CRITICAL_SECTION2`).
(cherry picked from commit 89c220b)
Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com>
@bedevere-app

bedevere-appBot commented Jul 22, 2025

Copy link
Copy Markdown

GH-136969 is a backport of this pull request to the 3.14 branch.

@bedevere-appbedevere-appBot removed the needs backport to 3.14 bugs and security fixes label Jul 22, 2025
@hugovk

hugovk commented Jul 22, 2025

Copy link
Copy Markdown
Member

@hugovk I know it's pretty late, but I think it would help a few downstream use-cases if we could get this and #134365 backported to 3.14 in time for RC1.

Sure, here's 3.14 backports:

In the backports, I adjusted the versionadded from "next" to "3.14", and moved the What's New entry from 3.15.rst to 3.14.rst.

And we'll need a follow-up to update these in main.

hugovk added a commit that referenced this pull request Jul 22, 2025
@miss-islington@ngoldbaum@hugovk
[3.14]gh-133296: Publicly expose critical section API that accepts P…
11f5101
…yMutex (gh-135899) (#136969)
Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com>
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
@markshannon

markshannon commented Jul 22, 2025

Copy link
Copy Markdown
Member

The C API workgroup vote now shows unanimous approval for adding this: capi-workgroup/decisions#67 (comment)

This doesn't sound unanimous to me: I'll be honest I don't quite get the point of a critical section that doesn't actually exclude other threads from running the same code, but I don't have energy to argue about it right now.

I agree with Steve. Calling something a "critical section", when it isn't, seems badly misleading.

@ngoldbaumngoldbaum mentioned this pull request Jul 22, 2025
Merged
da-woods added a commit to da-woods/cython that referenced this pull request Jul 22, 2025
@da-woods
Let critical_section work with pymutex
7382496
Based on python/cpython#135899 this'll
be supported in 3.14 (and can be backported to 3.13 fairly easily).
@da-woodsda-woods mentioned this pull request Jul 22, 2025
Merged
da-woods added a commit to cython/cython that referenced this pull request Jul 26, 2025
@da-woods
Let critical_section work with pymutex (#7031)
e7139a8
Based on python/cpython#135899 this'll be
supported in 3.14 (and can be backported to 3.13 fairly easily).
taegyunkim pushed a commit to taegyunkim/cpython that referenced this pull request Aug 4, 2025
@ngoldbaum@taegyunkim
pythongh-133296: Publicly expose critical section API that accepts Py…
5899156
…Mutex (pythongh-135899)
This makes the following APIs public:
* `Py_BEGIN_CRITICAL_SECTION_MUTEX(mutex),`
* `Py_BEGIN_CRITICAL_SECTION2_MUTEX(mutex1, mutex2)`
* `void PyCriticalSection_BeginMutex(PyCriticalSection *c, PyMutex *mutex)`
* `void PyCriticalSection2_BeginMutex(PyCriticalSection2 *c, PyMutex *mutex1, PyMutex *mutex2)`
The macros are identical to the corresponding `Py_BEGIN_CRITICAL_SECTION` and
`Py_BEGIN_CRITICAL_SECTION2` macros (e.g., they include braces), but they
accept a `PyMutex` instead of an object.
The new macros are still paired with the existing END macros
(`Py_END_CRITICAL_SECTION`, `Py_END_CRITICAL_SECTION2`).
Agent-Hellboy pushed a commit to Agent-Hellboy/cpython that referenced this pull request Aug 19, 2025
@ngoldbaum@Agent-Hellboy
pythongh-133296: Publicly expose critical section API that accepts Py…
a10aa97
…Mutex (pythongh-135899)
This makes the following APIs public:
* `Py_BEGIN_CRITICAL_SECTION_MUTEX(mutex),`
* `Py_BEGIN_CRITICAL_SECTION2_MUTEX(mutex1, mutex2)`
* `void PyCriticalSection_BeginMutex(PyCriticalSection *c, PyMutex *mutex)`
* `void PyCriticalSection2_BeginMutex(PyCriticalSection2 *c, PyMutex *mutex1, PyMutex *mutex2)`
The macros are identical to the corresponding `Py_BEGIN_CRITICAL_SECTION` and
`Py_BEGIN_CRITICAL_SECTION2` macros (e.g., they include braces), but they
accept a `PyMutex` instead of an object.
The new macros are still paired with the existing END macros
(`Py_END_CRITICAL_SECTION`, `Py_END_CRITICAL_SECTION2`).
kumaraditya303 pushed a commit to miss-islington/cpython that referenced this pull request Sep 9, 2025
@miss-islington@ngoldbaum
@hugovk@kumaraditya303
[3.14]pythongh-133296: Publicly expose critical section API that acc…
b24c78c
…epts PyMutex (pythongh-135899) (python#136969)
Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com>
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
@mitsuhikomitsuhiko mentioned this pull request Oct 22, 2025
Closed
@ngoldbaumngoldbaum mentioned this pull request Nov 21, 2025
Merged
@XuehaiPanXuehaiPan mentioned this pull request Feb 2, 2026
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ZeroIntensityZeroIntensityZeroIntensity left review comments
@colesburycolesburycolesbury approved these changes
@kumaraditya303kumaraditya303kumaraditya303 approved these changes
@markshannonmarkshannonAwaiting requested review from markshannonmarkshannon is a code owner

Assignees

No one assigned

Labels

topic-free-threading

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@ngoldbaum@hugovk@markshannon@colesbury@ZeroIntensity@kumaraditya303