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-116322: Enable the GIL while loading C extension modules by swtaarrs · Pull Request #118560 · python/cpython

Conversation

@swtaarrs

@swtaarrs swtaarrs commented May 3, 2024 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Building on gh-116882, this PR adds the ability to enable/disable the GIL at runtime, and uses that in the C module loading code.

We can't know before running a module init function if it supports free-threading, so the GIL is temporarily enabled before doing so. If the module declares support for running without the GIL, the GIL is later disabled. Otherwise, the GIL is permanently enabled, and will never be disabled again for the life of the current interpreter.

  • Issue: Allow C extensions to declare compatibility with free-threading #116322

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@swtaarrs
Enable the GIL while loading C extension modules
e324003
@swtaarrs swtaarrs added interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-free-threading labels May 3, 2024
@swtaarrs
swtaarrs requested a review from colesbury May 3, 2024 17:48
@bedevere-app bedevere-app Bot mentioned this pull request May 3, 2024
Allow C extensions to declare compatibility with free-threading #116322
Closed
blurb-it Bot and others added 2 commits May 3, 2024 17:49
@blurb-it
📜🤖 Added by blurb_it.
3e8fee5
@swtaarrs
Mark _wmi module
637c3e5
@swtaarrs

swtaarrs commented May 3, 2024

Copy link
Copy Markdown
Member Author

!buildbot nogil
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot

bedevere-bot commented May 3, 2024

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @swtaarrs for commit 637c3e5 🤖

The command will test the builders whose names match following regular expression: nogil

The builders matched are:

  • x86-64 MacOS Intel ASAN NoGIL PR
  • AMD64 Ubuntu NoGIL Refleaks PR
  • ARM64 MacOS M1 NoGIL PR
  • AMD64 Windows Server 2022 NoGIL PR
  • AMD64 Ubuntu NoGIL PR
  • ARM64 MacOS M1 Refleaks NoGIL PR
  • x86-64 MacOS Intel NoGIL PR
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@swtaarrs
Use _PyEval_IsGILEnabled() in _is_gil_enabled()
fecf4bd
@swtaarrs
swtaarrs marked this pull request as ready for review May 3, 2024 19:21
@swtaarrs
swtaarrs requested review from a team, brettcannon, ericsnowcurrently, kumaraditya303, ncoghlan and warsaw as code owners May 3, 2024 19:21
@bedevere-app bedevere-app Bot added the awaiting review label May 3, 2024
@swtaarrs

swtaarrs commented May 3, 2024

Copy link
Copy Markdown
Member Author

I have a local commit to update sys._is_gil_enabled() to use the new function I added (_PyEval_IsGILEnabled()) but I'll wait until the buildbots finish to push it.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
colesbury
colesbury reviewed May 3, 2024
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.

This looks pretty good to me. Some comments below.

We should update sys._is_gil_enabled as part of this PR.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
swtaarrs reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread Python/pystate.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/importdl.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@swtaarrs
Review comments from Sam, plus some cleanup
0fd1967
colesbury
colesbury approved these changes May 4, 2024
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.

LGTM

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 May 4, 2024
ericsnowcurrently
ericsnowcurrently reviewed May 4, 2024
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.

I have some questions.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Include/internal/pycore_import.h Resolved

Uh oh!

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

Uh oh!

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

Uh oh!

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

Uh oh!

There was an error while loading. Please reload this page.
swtaarrs added 2 commits May 6, 2024 12:51
@swtaarrs
Merge remote-tracking branch 'upstream/main' into cpython-enable-gil
ab4fe48
@swtaarrs
Store GIL slot in cache instead of separate list, other fixes from Eric
0271a16
swtaarrs
swtaarrs commented May 6, 2024
View reviewed changes
Comment thread Python/import.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
ericsnowcurrently
ericsnowcurrently approved these changes May 6, 2024
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.

I have one small comment. Otherwise LGTM.

(There is stuff to follow up on though.)

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.
@swtaarrs
Always define singlephase_global_update.md_gil to clean up callsite
2d8c2b2
@swtaarrs
Merge remote-tracking branch 'upstream/main' into cpython-enable-gil
b430f7b
@colesbury
colesbury merged commit 853163d into python:main May 7, 2024
@bedevere-app bedevere-app Bot removed the awaiting merge label May 7, 2024
SonicField pushed a commit to SonicField/cpython that referenced this pull request May 8, 2024
@swtaarrs @SonicField
pythongh-116322: Enable the GIL while loading C extension modules (py…
0d76d9c 
…thon#118560)

Add the ability to enable/disable the GIL at runtime, and use that in
the C module loading code.

We can't know before running a module init function if it supports
free-threading, so the GIL is temporarily enabled before doing so. If
the module declares support for running without the GIL, the GIL is
later disabled. Otherwise, the GIL is permanently enabled, and will
never be disabled again for the life of the current interpreter.
x42005e1f added a commit to x42005e1f/aiologic that referenced this pull request Nov 10, 2026
@x42005e1f
feat!: rewrite is_nogil() and is_notso()
cd8ee12 
The GIL is temporarily enabled when an extension module is created (see python/cpython#118560), so we must not rely on `sys._is_gil_enabled()`, which does not distinguish between the temporarily enabled GIL and the permanently enabled GIL (and thus creates a dangerous race condition during parallel imports).
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
@colesbury colesbury colesbury approved these changes
@kumaraditya303 kumaraditya303 Awaiting requested review from kumaraditya303 kumaraditya303 is a code owner
@brettcannon brettcannon Awaiting requested review from brettcannon brettcannon 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

Uh oh!

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

Assignees

No one assigned

Labels

interpreter-core (Objects, Python, Grammar, and Parser dirs) 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.

4 participants

@swtaarrs @bedevere-bot @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 01402503 — 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.