Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-116322: Enable the GIL while loading C extension modules #118560

Merged
merged 9 commits into from
May 7, 2024

Conversation

swtaarrs
Copy link
Member

@swtaarrs swtaarrs commented May 3, 2024

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.

@swtaarrs
Copy link
Member Author

swtaarrs commented May 3, 2024

!buildbot nogil

@bedevere-bot
Copy link

🤖 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

@swtaarrs swtaarrs marked this pull request as ready for review May 3, 2024 19:21
@swtaarrs
Copy link
Member Author

swtaarrs commented May 3, 2024

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.

Copy link
Contributor

@colesbury colesbury left a 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.

Python/pystate.c Outdated Show resolved Hide resolved
Python/ceval_gil.c Outdated Show resolved Hide resolved
Python/importdl.c Outdated Show resolved Hide resolved
Copy link
Contributor

@colesbury colesbury left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@ericsnowcurrently ericsnowcurrently left a 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.

Include/internal/pycore_import.h Outdated Show resolved Hide resolved
Python/import.c Outdated Show resolved Hide resolved
Python/import.c Outdated Show resolved Hide resolved
Python/importdl.c Outdated Show resolved Hide resolved
Python/import.c Outdated Show resolved Hide resolved
Copy link
Member

@ericsnowcurrently ericsnowcurrently left a 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.)

Python/import.c Show resolved Hide resolved
@colesbury colesbury merged commit 853163d into python:main May 7, 2024
36 checks passed
SonicField pushed a commit to SonicField/cpython that referenced this pull request May 8, 2024
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-free-threading
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants