gh-116510: Fix a Crash Due to Shared Immortal Interned Strings by ericsnowcurrently · Pull Request #124865 · python/cpython

ericsnowcurrently · 2024-10-01T18:36:08Z

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 addresses the Py_TRACE_REFS failures identified by gh-124785.

The 3.12 backport will be slightly different and will need to be done manually.

Issue: CPython 3.12 embedded in WeeChat causes segfault on subsequent calls to Py_EndInterpreter #116510

…ared immortal interned strings (pythongh-124646)" (pythongh-124807)" This reverts commit 7bdfabe.

ericsnowcurrently · 2024-10-01T18:56:09Z

!buildbot AMD64 Arch Linux TraceRefs

bedevere-bot · 2024-10-01T18:56:13Z

🤖 New build scheduled with the buildbot fleet by @ericsnowcurrently for commit 623fb87 🤖

The command will test the builders whose names match following regular expression: AMD64 Arch Linux TraceRefs

The builders matched are:

AMD64 Arch Linux TraceRefs PR

nascheme · 2024-10-01T20:04:26Z


 #ifdef Py_TRACE_REFS
+void
+_Py_NormalizeImmortalReference(PyObject *op)


Having some comment here would be helpful. Otherwise this function looks quite mysterious.

nascheme · 2024-10-01T20:05:15Z

LGTM other than adding a bit more comments.

ericsnowcurrently · 2024-10-01T23:28:29Z

The buildbot passed.

nascheme · 2024-10-08T21:05:20Z

Any reason not to merge this now?

ericsnowcurrently · 2024-10-08T23:33:01Z

I can merge it first thing tomorrow if no one objects.

miss-islington-app · 2024-10-09T17:32:19Z

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

…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 <ericsnowcurrently@gmail.com>

bedevere-app · 2024-10-09T17:32:36Z

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

bedevere-app · 2024-10-09T17:32:41Z

GH-125205 is a backport of this pull request to the 3.12 branch.

encukou · 2024-10-10T11:20:23Z

Due to either this change or #125199, the AMD64 Arch Linux TraceRefs 3.x buildbot is failing: https://buildbot.python.org/#/builders/484/builds/6022/steps/6/logs/stdio

vstinner · 2024-10-10T11:40:47Z

Due to either this change or #124865, the AMD64 Arch Linux TraceRefs 3.x buildbot is failing: https://buildbot.python.org/#/builders/484/builds/6022/steps/6/logs/stdio

I checked with git bisect and the regression was introduced by commit f2cb399:

commit f2cb39947093feda3ff85b8dc820922cc5e5f954
Author: Eric Snow <ericsnowcurrently@gmail.com>
Date: Wed Oct 9 11:32:16 2024 -0600
gh-116510: Fix a Crash Due to Shared Immortal Interned Strings (gh-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 gh-124646 that then addresses the Py_TRACE_REFS
failures identified by gh-124785.

ericsnowcurrently · 2024-10-10T15:13:29Z

I'll take a look.

…gh-124865) (gh-125709) (GH-125204) * gh-116510: Fix a Crash Due to Shared Immortal Interned Strings (gh-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 gh-124646 that then addresses the Py_TRACE_REFS failures identified by gh-124785. (cherry picked from commit f2cb399) Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com> * [3.13] gh-125286: Share the Main Refchain With Legacy Interpreters (gh-125709) They used to be shared, before 3.12. Returning to sharing them resolves a failure on Py_TRACE_REFS builds. --------- Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>

…gh-125205) 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 (i.e. backporting gh-125709 too). (cherry picked from commit f2cb399, AKA gh-124865) Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>

ericsnowcurrently added 2 commits October 1, 2024 12:24

Revert "pythongh-124785: Revert "pythongh-116510: Fix crash due to sh…
884f398
…ared immortal interned strings (pythongh-124646)" (pythongh-124807)" This reverts commit 7bdfabe.

Move non-isolated immortal objects to the right ref chain.
623fb87

ericsnowcurrently added needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Oct 1, 2024

ericsnowcurrently requested a review from nascheme October 1, 2024 18:36

bedevere-appBot mentioned this pull request Oct 1, 2024
CPython 3.12 embedded in WeeChat causes segfault on subsequent calls to Py_EndInterpreter #116510
Closed

bedevere-appBot added the awaiting core review label Oct 1, 2024

ericsnowcurrently mentioned this pull request Oct 1, 2024
[TraceRefs build] Tests fail: _PyRefchain_Remove: Assertion `value == REFCHAIN_VALUE' failed #124785
Closed

nascheme mentioned this pull request Oct 1, 2024
gh-124785: re-work fix so tracerefs test passes #124808
Closed

nascheme reviewed Oct 1, 2024
View reviewed changes

Add some comments.
fcc6a06

ericsnowcurrently merged commit f2cb399 into python:mainOct 9, 2024

bedevere-appBot removed the awaiting core review label Oct 9, 2024

ericsnowcurrently deleted the fix-tracerefs branch October 9, 2024 17:32

bedevere-appBot removed the needs backport to 3.13 bugs and security fixes label Oct 9, 2024

bedevere-appBot removed the needs backport to 3.12 only security fixes label Oct 9, 2024

encukou mentioned this pull request Oct 10, 2024
gh-125196: Use PyUnicodeWriter in symtable.c #125199
Merged

This was referenced Oct 10, 2024
[3.13] gh-116510: Fix a Crash Due to Shared Immortal Interned Strings (gh-124865) #125204
Merged
[3.12] gh-116510: Fix a Crash Due to Shared Immortal Interned Strings (gh-124865) #125205
Merged

This was referenced Oct 10, 2024
test_audit_subinterpreter crashes with tracerefs enabled #125286
Closed
Memory leak on executables embedded with 3.13 #113055
Closed

Uh oh!

Uh oh!

Conversation

ericsnowcurrently commented Oct 1, 2024• edited by bedevere-app BotLoading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ericsnowcurrently commented Oct 1, 2024

Uh oh!

bedevere-bot commented Oct 1, 2024

Uh oh!

naschemeOct 1, 2024

Choose a reason for hiding this comment

Uh oh!

ericsnowcurrentlyOct 1, 2024

Choose a reason for hiding this comment

Uh oh!

nascheme commented Oct 1, 2024

Uh oh!

ericsnowcurrently commented Oct 1, 2024

Uh oh!

nascheme commented Oct 8, 2024

Uh oh!

ericsnowcurrently commented Oct 8, 2024

Uh oh!

miss-islington-appBot commented Oct 9, 2024

Uh oh!

bedevere-appBot commented Oct 9, 2024

Uh oh!

bedevere-appBot commented Oct 9, 2024

Uh oh!

encukou commented Oct 10, 2024

Uh oh!

vstinner commented Oct 10, 2024

Uh oh!

ericsnowcurrently commented Oct 10, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ericsnowcurrently commented Oct 1, 2024•
edited by bedevere-app Bot
Loading