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-119560: Drop an Invalid Assert in PyState_FindModule() #119561

Merged
merged 4 commits into from
May 25, 2024

Conversation

ericsnowcurrently
Copy link
Member

@ericsnowcurrently ericsnowcurrently commented May 25, 2024

The assertion was added in gh-118532 but was based on the invalid assumption that PyState_FindModule() would only be called with an already-initialized module def. I've added a test to make sure we don't make that assumption again.

@ericsnowcurrently ericsnowcurrently enabled auto-merge (squash) May 25, 2024 19:09
@ericsnowcurrently ericsnowcurrently merged commit 0c5ebe1 into python:main May 25, 2024
36 checks passed
@ericsnowcurrently ericsnowcurrently deleted the fix-ujson branch May 25, 2024 19:30
@ericsnowcurrently ericsnowcurrently added the needs backport to 3.13 bugs and security fixes label May 27, 2024
@miss-islington-app
Copy link

Thanks @ericsnowcurrently for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 27, 2024
…ongh-119561)

The assertion was added in pythongh-118532 but was based on the invalid assumption that PyState_FindModule() would only be called with an already-initialized module def.  I've added a test to make sure we don't make that assumption again.
(cherry picked from commit 0c5ebe1)

Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented May 27, 2024

GH-119632 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label May 27, 2024
ericsnowcurrently added a commit that referenced this pull request May 27, 2024
…119561) (gh-119632)

The assertion was added in gh-118532 but was based on the invalid assumption that PyState_FindModule() would only be called with an already-initialized module def.  I've added a test to make sure we don't make that assumption again.

(cherry picked from commit 0c5ebe1)

Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
ericsnowcurrently added a commit that referenced this pull request May 27, 2024
The fix in gh-119561 introduced an assertion that doesn't hold true if any of the three new test extension modules are loaded more than once.  This is fine normally but breaks if the new test_check_state_first() is run more than once, which happens for refleak checking and with the regrtest --forever flag.  We fix that here by clearing each of the three modules after loading them.  We also tweak a check in _modules_by_index_check().
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 27, 2024
The fix in pythongh-119561 introduced an assertion that doesn't hold true if any of the three new test extension modules are loaded more than once.  This is fine normally but breaks if the new test_check_state_first() is run more than once, which happens for refleak checking and with the regrtest --forever flag.  We fix that here by clearing each of the three modules after loading them.  We also tweak a check in _modules_by_index_check().
(cherry picked from commit ae7b176)

Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
ericsnowcurrently added a commit that referenced this pull request May 27, 2024
…9633)

The fix in gh-119561 introduced an assertion that doesn't hold true if any of the three new test extension modules are loaded more than once.  This is fine normally but breaks if the new test_check_state_first() is run more than once, which happens for refleak checking and with the regrtest --forever flag.  We fix that here by clearing each of the three modules after loading them.  We also tweak a check in _modules_by_index_check().

(cherry picked from commit ae7b176)

Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant