Skip to content

gh-111924: Use PyMutex for runtime global locks. - #112207

Merged
ericsnowcurrently merged 4 commits into
python:mainfrom
colesbury:global_locks
Dec 7, 2023
Merged

gh-111924: Use PyMutex for runtime global locks.#112207
ericsnowcurrently merged 4 commits into
python:mainfrom
colesbury:global_locks

Conversation

@colesbury

@colesburycolesbury commented Nov 17, 2023•
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Contributor

This replaces some usages of PyThread_type_lock with PyMutex, which does not require memory allocation to initialize.

This simplifies some of the runtime initialization and is also one step towards avoiding changing the default raw memory allocator during initialize/finalization, which can be non-thread-safe in some circumstances.

@colesbury
pythongh-111924: Use PyMutex for runtime global locks.
628f6eb
This replaces some usages of PyThread_type_lock with PyMutex, which
does not require memory allocation to initialize.
@colesburycolesbury added interpreter-core (Objects, Python, Grammar, and Parser dirs) 3.13 bugs and security fixes topic-free-threading labels Nov 17, 2023
@bedevere-appbedevere-appBot mentioned this pull request Nov 17, 2023
Closed
@colesbury
colesbury marked this pull request as ready for review November 17, 2023 17:36
@colesbury

colesbury commented Nov 17, 2023

Copy link
Copy Markdown
ContributorAuthor

@ericsnowcurrently, when you get a chance, would you please review this?

PyMutex_Lock() behaves similarly to PyThread_acquire_lock() except that PyMutex_Lock() will temporarily release the GIL if it needs to block. That's mostly fine, but I think not what we want for the HEAD_LOCK() macro, so that explicitly passes a flag to not release the GIL (or detach in --disable-gil builds).

@brettcannon
brettcannon removed their request for review November 17, 2023 20:25
ericsnowcurrently
ericsnowcurrently reviewed Dec 7, 2023
View reviewed changes

@ericsnowcurrentlyericsnowcurrently left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

mostly LGTM

I've left a few very minor comments.

Comment threadPython/import.c
Comment threadPython/pylifecycle.c
Comment threadPython/ceval_gil.c Outdated
Comment threadPython/crossinterp.c
Comment threadPython/pystate.c Outdated
Comment threadPython/pystate.c
ericsnowcurrently
ericsnowcurrently reviewed Dec 7, 2023
View reviewed changes
Comment threadInclude/internal/pycore_lock.h
Comment threadPython/pystate.c Outdated
ericsnowcurrently
ericsnowcurrently approved these changes Dec 7, 2023
View reviewed changes

@ericsnowcurrentlyericsnowcurrently left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

I've left one comment that I'll leave to your discretion. I'll wait for your response before merging.

@ericsnowcurrently
ericsnowcurrently merged commit cf6110b into python:mainDec 7, 2023
@ericsnowcurrently

ericsnowcurrently commented Dec 7, 2023

Copy link
Copy Markdown
Member

Thanks for tackling this, @colesbury!

@colesbury
colesbury deleted the global_locks branch December 12, 2023 19:37
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
@colesbury@aisk
pythongh-111924: Use PyMutex for Runtime-global Locks. (pythongh-112207)
0640620
This replaces some usages of PyThread_type_lock with PyMutex, which does not require memory allocation to initialize.
This simplifies some of the runtime initialization and is also one step towards avoiding changing the default raw memory allocator during initialize/finalization, which can be non-thread-safe in some circumstances.
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
@colesbury@Glyphack
pythongh-111924: Use PyMutex for Runtime-global Locks. (pythongh-112207)
18d70b6
This replaces some usages of PyThread_type_lock with PyMutex, which does not require memory allocation to initialize.
This simplifies some of the runtime initialization and is also one step towards avoiding changing the default raw memory allocator during initialize/finalization, which can be non-thread-safe in some circumstances.
This was referenced Dec 12, 2024
Closed
Merged
colesbury pushed a commit that referenced this pull request Dec 12, 2024
@velemas
gh-127865: Fix build failure for systems without thread local support (…
f823910
…GH-127866)
This PR fixes the build issue introduced by the commit 628f6eb from
GH-112207 on systems without thread local support.
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 12, 2024
@velemas@miss-islington
pythongh-127865: Fix build failure for systems without thread local s…
52a58bd
…upport (pythonGH-127866)
This PR fixes the build issue introduced by the commit 628f6eb from
pythonGH-112207 on systems without thread local support.
(cherry picked from commit f823910)
Co-authored-by: velemas <10437413+velemas@users.noreply.github.com>
@miss-islingtonmiss-islington mentioned this pull request Dec 12, 2024
Merged
colesbury pushed a commit that referenced this pull request Dec 12, 2024
@miss-islington@velemas
[3.13]gh-127865: Fix build failure for systems without thread local …
f21b38c
…support (GH-127866) (GH-127882)
This PR fixes the build issue introduced by the commit 628f6eb from
GH-112207 on systems without thread local support.
(cherry picked from commit f823910)
Co-authored-by: velemas <10437413+velemas@users.noreply.github.com>
srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request Jan 8, 2025
@velemas@srinivasreddy
pythongh-127865: Fix build failure for systems without thread local s…
8dd7f1a
…upport (pythonGH-127866)
This PR fixes the build issue introduced by the commit 628f6eb from
pythonGH-112207 on systems without thread local support.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ericsnowcurrentlyericsnowcurrentlyericsnowcurrently approved these changes
@kumaraditya303kumaraditya303Awaiting requested review from kumaraditya303kumaraditya303 is a code owner
@ncoghlanncoghlanAwaiting requested review from ncoghlanncoghlan is a code owner
@warsawwarsawAwaiting requested review from warsawwarsaw is a code owner
@markshannonmarkshannonAwaiting requested review from markshannon
@gvanrossumgvanrossumAwaiting requested review from gvanrossum

Assignees

No one assigned

Labels

3.13bugs and security fixesinterpreter-core(Objects, Python, Grammar, and Parser dirs)skip newstopic-free-threading

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@colesbury@ericsnowcurrently