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-111924: Use PyMutex for runtime global locks. by colesbury · Pull Request #112207 · python/cpython

Conversation

@colesbury

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

Uh oh!

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

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.

  • Issue: Avoid changing the PYMEM_DOMAIN_RAW allocator during initialization and finalization #111924

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@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.
@colesbury colesbury added interpreter-core (Objects, Python, Grammar, and Parser dirs) 3.13 only security fixes topic-free-threading labels Nov 17, 2023
@colesbury
colesbury requested a review from ericsnowcurrently November 17, 2023 16:55
@bedevere-app bedevere-app Bot mentioned this pull request Nov 17, 2023
Avoid changing the PYMEM_DOMAIN_RAW allocator during initialization and finalization #111924
Closed
@colesbury colesbury added the skip news label Nov 17, 2023
@colesbury
colesbury marked this pull request as ready for review November 17, 2023 17:36
@colesbury
colesbury requested review from brettcannon, gvanrossum, kumaraditya303, markshannon, ncoghlan and warsaw as code owners November 17, 2023 17:36
@bedevere-app bedevere-app Bot added the awaiting review label Nov 17, 2023
@colesbury

colesbury commented Nov 17, 2023

Copy link
Copy Markdown
Contributor Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@brettcannon
brettcannon removed their request for review November 17, 2023 20:25
ericsnowcurrently
ericsnowcurrently reviewed Dec 7, 2023
View reviewed changes

@ericsnowcurrently ericsnowcurrently left a comment

Copy link
Copy Markdown
Member

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.

mostly LGTM

I've left a few very minor comments.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Python/import.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Python/pylifecycle.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Python/ceval_gil.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Python/crossinterp.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Python/pystate.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Python/pystate.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
colesbury added 3 commits December 7, 2023 13:09
@colesbury
Changes from review
e2227da
@colesbury
Use _PyMutex_at_fork_reinit in ceval_gil.c
3ab46f5
@colesbury
Use memset instead C99 initializer to fix Windows build
8ef8314
ericsnowcurrently
ericsnowcurrently reviewed Dec 7, 2023
View reviewed changes
Comment thread Include/internal/pycore_lock.h Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Python/pystate.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
ericsnowcurrently
ericsnowcurrently approved these changes Dec 7, 2023
View reviewed changes

@ericsnowcurrently ericsnowcurrently left a comment

Copy link
Copy Markdown
Member

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.

LGTM

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Dec 7, 2023
@ericsnowcurrently
ericsnowcurrently merged commit cf6110b into python:main Dec 7, 2023
@bedevere-app bedevere-app Bot removed the awaiting merge label Dec 7, 2023
@ericsnowcurrently

ericsnowcurrently commented Dec 7, 2023

Copy link
Copy Markdown
Member

Thanks for tackling this, @colesbury!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@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
Build failure without thread local support #127865
Closed
gh-127865: build failure fix for systems without thread-locals support #127866
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 <[email protected]>
@miss-islington miss-islington mentioned this pull request Dec 12, 2024
[3.13] gh-127865: Fix build failure for systems without thread local support (GH-127866) #127882
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 <[email protected]>
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 free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ericsnowcurrently ericsnowcurrently ericsnowcurrently approved these changes
@kumaraditya303 kumaraditya303 Awaiting requested review from kumaraditya303 kumaraditya303 is a code owner
@ncoghlan ncoghlan Awaiting requested review from ncoghlan ncoghlan is a code owner
@warsaw warsaw Awaiting requested review from warsaw warsaw is a code owner
@markshannon markshannon Awaiting requested review from markshannon
@gvanrossum gvanrossum Awaiting requested review from gvanrossum

Uh oh!

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

Assignees

No one assigned

Labels

3.13 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) skip news 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.

2 participants

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