Skip to content

bpo-47182: Fix crash by named unicode characters after interpreter reinitialization - #32212

Merged
miss-islington merged 2 commits into
python:mainfrom
tiran:bpo-47182-ucn-crash
Mar 31, 2022
Merged

bpo-47182: Fix crash by named unicode characters after interpreter reinitialization#32212
miss-islington merged 2 commits into
python:mainfrom
tiran:bpo-47182-ucn-crash

Conversation

@tiran

@tirantiran commented Mar 31, 2022•
edited by miss-islington
Loading

Copy link
Copy Markdown
Member

https://bugs.python.org/issue47182

Automerge-Triggered-By: GH:tiran

@tiran
tiranforce-pushed the bpo-47182-ucn-crash branch from e44d28d to 746ee08CompareMarch 31, 2022 13:40
@tirantiran mentioned this pull request Mar 31, 2022
Closed
vstinner
vstinner approved these changes Mar 31, 2022
View reviewed changes

@vstinnervstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, but consider adding a comment ;-)

If you are motivated, you can add a test to test_embed using test_repeated_init_exec: see test_finalize_structseq() of test_embed for example. You can run arbitrary Python code, it's executed multiple times, each time the interpreter is created/destroyed in the same process.

Comment threadObjects/unicodeobject.c
@tiran

tiran commented Mar 31, 2022

Copy link
Copy Markdown
MemberAuthor

I have added a comment and a test case.

vstinner
vstinner approved these changes Mar 31, 2022
View reviewed changes

@vstinnervstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks for adding a regression test!

Comment threadLib/test/test_embed.py
@miss-islington

miss-islington commented Mar 31, 2022

Copy link
Copy Markdown
Contributor

@tiran: Status check is done, and it's a success ✅ .

@miss-islington
miss-islington merged commit 44e9150 into python:mainMar 31, 2022
@miss-islington

miss-islington commented Mar 31, 2022

Copy link
Copy Markdown
Contributor

Thanks @tiran for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8, 3.9, 3.10.
🐍🍒⛏🤖

@miss-islington

miss-islington commented Mar 31, 2022

Copy link
Copy Markdown
Contributor

Sorry, @tiran, I could not cleanly backport this to 3.10 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 44e915028d75f7cef141aa1aada962465a5907d6 3.10

@miss-islingtonmiss-islington self-assigned this Mar 31, 2022
@miss-islington

miss-islington commented Mar 31, 2022

Copy link
Copy Markdown
Contributor

Sorry @tiran, I had trouble checking out the 3.9 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 44e915028d75f7cef141aa1aada962465a5907d6 3.9

@miss-islington

miss-islington commented Mar 31, 2022

Copy link
Copy Markdown
Contributor

Sorry, @tiran, I could not cleanly backport this to 3.8 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 44e915028d75f7cef141aa1aada962465a5907d6 3.8

@miss-islington

miss-islington commented Mar 31, 2022

Copy link
Copy Markdown
Contributor

Sorry @tiran, I had trouble checking out the 3.7 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 44e915028d75f7cef141aa1aada962465a5907d6 3.7

@vstinner

vstinner commented Mar 31, 2022

Copy link
Copy Markdown
Member

Are you sure that Python 3.10 and older are affected?

Clearing interned strings is new in Python 3.10. Python 3.11 changed that with immortal code objects and immortal strings.

In Python 3.9, strings were not really cleared by Py_Finalize(). They just leaked.

tiran added a commit to tiran/cpython that referenced this pull request Mar 31, 2022
@tiran
[3.10] bpo-47182: Fix crash by named unicode characters after interpr…
e0e582e
…eter reinitialization (pythonGH-32212)
Automerge-Triggered-By: GH:tiran.
(cherry picked from commit 44e9150)
Co-authored-by: Christian Heimes <christian@python.org>
tiran added a commit to tiran/cpython that referenced this pull request Mar 31, 2022
@tiran
[3.9] bpo-47182: Fix crash by named unicode characters after interpre…
2d7b8ce
…ter reinitialization (pythonGH-32212)
Automerge-Triggered-By: GH:tiran.
(cherry picked from commit 44e9150)
Co-authored-by: Christian Heimes <christian@python.org>
@bedevere-botbedevere-bot removed the needs backport to 3.10 only security fixes label Mar 31, 2022
@bedevere-bot

bedevere-bot commented Mar 31, 2022

Copy link
Copy Markdown

GH-32216 is a backport of this pull request to the 3.10 branch.

@bedevere-bot

bedevere-bot commented Mar 31, 2022

Copy link
Copy Markdown

GH-32217 is a backport of this pull request to the 3.9 branch.

@tiran

tiran commented Mar 31, 2022

Copy link
Copy Markdown
MemberAuthor

3.9 and earlier are definitely not affected. It doesn't hurt to backport the change to 3.10.

@vstinner

vstinner commented Mar 31, 2022

Copy link
Copy Markdown
Member

3.9 and earlier are definitely not affected. It doesn't hurt to backport the change to 3.10.

Yeah, I agree to backport to 3.10. But you have to backport manually, _testembed doesn't have test_repeated_init_exec.

tiran added a commit that referenced this pull request Apr 1, 2022
@tiran
[3.10]bpo-47182: Fix crash by named unicode characters after interpr…
55d5c96
…eter reinitialization (GH-32212) (GH-32216)
Co-authored-by: Christian Heimes <christian@python.org>
@kumaraditya303kumaraditya303 mentioned this pull request May 9, 2022
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinnervstinnervstinner approved these changes

Assignees

@miss-islingtonmiss-islington

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@tiran@miss-islington@vstinner@bedevere-bot@the-knights-who-say-ni