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

[3.12] gh-116510: Fix a Crash Due to Shared Immortal Interned Strings (gh-124865) #125205

Merged
merged 3 commits into from
Dec 3, 2024

Conversation

miss-islington
Copy link
Contributor

@miss-islington miss-islington commented Oct 9, 2024

Fix a crash caused by immortal interned strings being shared between
sub-interpreters that use basic single-phase init. In that case, the string
can be used by an interpreter that outlives the interpreter that created and
interned it. For interpreters that share obmalloc state, also share the
interned dict with the main interpreter.

This is an un-revert of gh-124646 that then addresses the Py_TRACE_REFS
failures identified by gh-124785.
(cherry picked from commit f2cb399)

Co-authored-by: Eric Snow [email protected]

…ythongh-124865)

Fix a crash caused by immortal interned strings being shared between
sub-interpreters that use basic single-phase init. In that case, the string
can be used by an interpreter that outlives the interpreter that created and
interned it. For interpreters that share obmalloc state, also share the
interned dict with the main interpreter.

This is an un-revert of pythongh-124646 that then addresses the Py_TRACE_REFS
failures identified by pythongh-124785.
(cherry picked from commit f2cb399)

Co-authored-by: Eric Snow <[email protected]>
@ericsnowcurrently
Copy link
Member

@nascheme I thought the backport wouldn't apply cleanly, but here we are. Do you recall what was different about the 3.12 fix?

@vstinner
Copy link
Member

Please don't merge until the TraceRefs regression is fixed: #124865 (comment)

@nascheme
Copy link
Member

I think this can be merged now with GH-125709 applied on top or squashed into it.

…ers (pythongh-125709)

They used to be shared, before 3.12.  Returning to sharing them resolves a failure on Py_TRACE_REFS builds.
@encukou
Copy link
Member

encukou commented Nov 13, 2024

@ericsnowcurrently, is this the way you'd do it in 3.12?

@encukou encukou added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 2, 2024
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @encukou for commit e9591b2 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 2, 2024
@encukou
Copy link
Member

encukou commented Dec 3, 2024

The failed buildbots aren't applicable to 3.12, or fail for unrelated & pre-existing reasons.

@ericsnowcurrently
Copy link
Member

@ericsnowcurrently, is this the way you'd do it in 3.12?

This should be good to go. It is backport of gh-124865 + gh-125709. There shouldn't be anything particular to 3.12 that would need to be addressed differently.

@ericsnowcurrently ericsnowcurrently merged commit 49da170 into python:3.12 Dec 3, 2024
90 of 105 checks passed
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.

6 participants