Skip to content

gh-134745: Change PyThread_allocate_lock() implementation to PyMutex - #134747

Merged
vstinner merged 7 commits into
python:mainfrom
vstinner:pymutex
May 30, 2025
Merged

gh-134745: Change PyThread_allocate_lock() implementation to PyMutex#134747
vstinner merged 7 commits into
python:mainfrom
vstinner:pymutex

Conversation

@vstinner

@vstinnervstinner commented May 26, 2025

Copy link
Copy Markdown
Member

PyMutex is a fast and portable lock currently only used internally by Python. This change modifies PyThread_allocate_lock to reuse PyMutex.

@vstinner

Copy link
Copy Markdown
MemberAuthor

This draft PR currently only modify Python/thread_pthread.h. I left Python/thread_nt.h unchanged.

_PyThread_at_fork_reinit() is still implemented by allocating a new lock and leaking the old lock.

@vstinner

Copy link
Copy Markdown
MemberAuthor

@colesbury: What do you think of this idea? Does it make sense? Do you see any possible issue?

If the approach makes sense, I will update my PR to modify also the Windows implementation.

@colesbury

Copy link
Copy Markdown
Contributor

I think the idea is fine, but the PR title and description are confusing. threading.Lock / _thread.lock / _thread._thread.allocate_lock() already use PyMutex (all three are the same thing):

typedefstruct {
PyObject_HEAD
PyMutexlock;
} lockobject;

This PR changes the implementation of the C API PyThread_allocate_lock(). We are generally using PyMutex directly instead of PyThread_allocate_lock() in new code, and we've also changed a lot of existing uses too. Using PyMutex directly is simpler because it avoids an allocation that may fail.

I think it's okay to change the implementation of PyThread_allocate_lock(). It looks like it's going to simplify the code, but there's some risk of unknown behavioral changes.

@vstinnervstinner changed the title gh-134745: Change threading.Lock implementation to PyMutexgh-134745: Change PyThread_allocate_lock() implementation to PyMutexMay 27, 2025
Comment threadPython/thread.c Outdated
Comment threadPython/thread.c
Comment threadPython/thread.c Outdated
Don't call signal handlers if interrupted, only return PY_LOCK_INTR.

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

This LGTM, but I think you should include a NEWS entry. One behavioral change is that previously intr_flag was ignored on Windows, but now it actually makes the acquisitions interruptible.

Comment threadPython/thread.c Outdated
vstinnerand others added 2 commits May 30, 2025 11:29
Co-authored-by: Sam Gross <colesbury@gmail.com>
@vstinner
vstinner enabled auto-merge (squash) May 30, 2025 09:42
@vstinner
vstinner disabled auto-merge May 30, 2025 09:44
@vstinner
vstinner enabled auto-merge (squash) May 30, 2025 09:45
@vstinner
vstinner merged commit ebf6d13 into python:mainMay 30, 2025
@vstinner
vstinner deleted the pymutex branch May 30, 2025 10:15
@vstinner

Copy link
Copy Markdown
MemberAuthor

Merged, thanks for the review @colesbury.

Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull request Jul 12, 2025
…Mutex (python#134747)
Co-authored-by: Sam Gross <colesbury@gmail.com>
taegyunkim pushed a commit to taegyunkim/cpython that referenced this pull request Aug 4, 2025
…Mutex (python#134747)
Co-authored-by: Sam Gross <colesbury@gmail.com>
@vstinner

Copy link
Copy Markdown
MemberAuthor

Oh, I forgot to update sys.thread_info: I created #141140 for that.

gostak-dd pushed a commit to gostak-dd/cpython that referenced this pull request Jun 2, 2026
…Mutex (python#134747)
Co-authored-by: Sam Gross <colesbury@gmail.com>
vstinner pushed a commit that referenced this pull request Jul 24, 2026
…GH-154338) (#154655)
gh-134745: Remove dead code in `thread_nt.h` left by gh-134747 (GH-154338)
(cherry picked from commit 9560bd8)
Co-authored-by: Ivy Xu <fakeshadow1337@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@vstinner@colesbury