GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-133296: Publicly expose critical section API that accepts PyMutex by ngoldbaum · Pull Request #135899 · python/cpython

Conversation

@ngoldbaum

@ngoldbaum ngoldbaum commented Jun 24, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bschoenmaeckers reacted with eyes emoji
All reactions
  • 👀 1 reaction
@ngoldbaum
pythongh-133296: Publicly expose critical section API that accepts Py…
176aa3d 
…Mutex
@ngoldbaum
ngoldbaum requested a review from markshannon as a code owner June 24, 2025 17:31
@bedevere-app bedevere-app Bot added the awaiting review label Jun 24, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Jun 24, 2025
Expose _PyCriticalSection_BeginMutex as a public API #133296
Closed
ngoldbaum added 2 commits June 24, 2025 11:37
@ngoldbaum
fix copy-paste error
e0691f5
@ngoldbaum
avoid compiler warning on GIL-enabled build
5640b48
colesbury
colesbury reviewed Jun 24, 2025
View reviewed changes

@colesbury colesbury left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Doc/c-api/init.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/c-api/init.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
colesbury
colesbury reviewed Jun 24, 2025
View reviewed changes
Comment thread Doc/c-api/init.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/c-api/init.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@ngoldbaum @colesbury
Apply suggestions from code review
6d987c7 
Co-authored-by: Sam Gross <[email protected]>
ngoldbaum
ngoldbaum commented Jun 24, 2025
View reviewed changes
Comment thread Misc/NEWS.d/next/C_API/2025-06-24-11-10-01.gh-issue-133296.lIEuVJ.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@ngoldbaum
Update changelog
59ac85a
kumaraditya303
kumaraditya303 reviewed Jul 3, 2025
View reviewed changes
Comment thread Doc/c-api/init.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@ngoldbaum
fix issue pointed out by Kumar
95a0be3
kumaraditya303
kumaraditya303 approved these changes Jul 10, 2025
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Jul 10, 2025
@kumaraditya303 kumaraditya303 added the topic-free-threading label Jul 10, 2025
@ngoldbaum

ngoldbaum commented Jul 21, 2025

Copy link
Copy Markdown
Contributor Author

The C API workgroup vote now shows unanimous approval for adding this: capi-workgroup/decisions#67 (comment)
colesbury reacted with hooray emoji
All reactions
  • 🎉 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
colesbury
colesbury approved these changes Jul 21, 2025
View reviewed changes
@colesbury
colesbury merged commit 89c220b into python:main Jul 21, 2025
@bedevere-app bedevere-app Bot removed the awaiting merge label Jul 21, 2025
@ngoldbaum

ngoldbaum commented Jul 21, 2025

Copy link
Copy Markdown
Contributor Author

@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.
hugovk reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@hugovk hugovk added the needs backport to 3.14 bugs and security fixes label Jul 22, 2025
@miss-islington-app

miss-islington-app Bot 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.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
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 <[email protected]>
@bedevere-app

bedevere-app Bot commented Jul 22, 2025

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot 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:

  • [3.14] gh-133296: Publicly expose critical section API that accepts PyMutex (gh-135899) #136969
  • [3.14] gh-134009: Expose PyMutex_IsLocked in the public C API (gh-134365) #136971

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
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 <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
@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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ngoldbaum ngoldbaum mentioned this pull request Jul 22, 2025
gh-133296: Fix versionadded for C API functions that were backported #137024
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-woods da-woods mentioned this pull request Jul 22, 2025
Let critical_section work with pymutex cython/cython#7031
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 <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
@mitsuhiko mitsuhiko mentioned this pull request Oct 22, 2025
Initial free threading port simplejson/simplejson#344
Closed
@ngoldbaum ngoldbaum mentioned this pull request Nov 21, 2025
Expose unsafe wrappers for Py_BEGIN_CRITICAL_SECTION_MUTEX API PyO3/pyo3#5642
Merged
@XuehaiPan XuehaiPan mentioned this pull request Feb 2, 2026
Fix race condition with py::make_key_iterator in free threading pybind/pybind11#5971
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ZeroIntensity ZeroIntensity ZeroIntensity left review comments
@colesbury colesbury colesbury approved these changes
@kumaraditya303 kumaraditya303 kumaraditya303 approved these changes
@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

topic-free-threading

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@ngoldbaum @hugovk @markshannon @colesbury @ZeroIntensity @kumaraditya303
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.