Skip to content

gh-128345: GIL is not properly disabled for _freeze_module.vcxproj #128344

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

Merged
merged 2 commits into from
Feb 15, 2025

Conversation

TheShermanTanker
Copy link
Contributor

@TheShermanTanker TheShermanTanker commented Dec 30, 2024

The _freeze_module.vcxproj file is looking for #undef Py_GIL_DISABLED, but that was changed to /* #define Py_GIL_DISABLED 1 */ some time ago, so the build system ends up looking to replace a line that doesn't exist, the result is that the freeze Python never has GIL disabled even if it was requested during the build

@TheShermanTanker TheShermanTanker requested a review from a team as a code owner December 30, 2024 10:31
@TheShermanTanker TheShermanTanker changed the title GIL is not properly disabled for _freeze_module.vcxproj gh-128345: GIL is not properly disabled for _freeze_module.vcxproj Dec 30, 2024
@rruuaanng
Copy link
Contributor

cc @ZeroIntensity

Copy link
Member

@ZeroIntensity ZeroIntensity left a comment

Choose a reason for hiding this comment

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

Are we able to add some sort of test?

@zooba zooba removed the needs backport to 3.13 bugs and security fixes label Jan 1, 2025
@zooba
Copy link
Member

zooba commented Jan 1, 2025

It hardly matters for this tool.

If it's somehow going to help the people working on free-threading to have it enabled for a build time tool, then sure. But there's no reason to backport it. Nobody is developing free-threading in 3.13 anymore - they're all working in main.

Copy link
Member

@ZeroIntensity ZeroIntensity left a comment

Choose a reason for hiding this comment

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

Thanks for the clarification, LGTM.

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 as well

@colesbury
Copy link
Contributor

!buildbot nogil windows

@bedevere-bot
Copy link

The regex 'nogil windows' did not match any buildbot builder. Is the requested builder in the list of stable builders?

@colesbury
Copy link
Contributor

!buildbot windows nogil

@bedevere-bot
Copy link

The regex 'windows nogil' did not match any buildbot builder. Is the requested builder in the list of stable builders?

@colesbury
Copy link
Contributor

!buildbot nogil

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @colesbury for commit cf12bef 🤖

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

The builders matched are:

  • AMD64 Windows Server 2022 NoGIL PR
  • AMD64 Fedora Rawhide NoGIL refleaks PR
  • AMD64 CentOS9 NoGIL Refleaks PR
  • x86-64 MacOS Intel ASAN NoGIL PR
  • AMD64 Windows PGO NoGIL PR
  • aarch64 Fedora Rawhide NoGIL PR
  • PPC64LE Fedora Rawhide NoGIL refleaks PR
  • aarch64 Fedora Rawhide NoGIL refleaks PR
  • ARM64 MacOS M1 NoGIL PR
  • AMD64 CentOS9 NoGIL PR
  • ARM64 MacOS M1 Refleaks NoGIL PR
  • AMD64 Fedora Rawhide NoGIL PR
  • PPC64LE Fedora Rawhide NoGIL PR
  • x86-64 MacOS Intel NoGIL PR

@kumaraditya303 kumaraditya303 enabled auto-merge (squash) February 15, 2025 13:51
@kumaraditya303 kumaraditya303 merged commit c26bed1 into python:main Feb 15, 2025
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants