GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-47182: Fix crash by named unicode characters after interpreter reinitialization by tiran · Pull Request #32212 · python/cpython

Conversation

@tiran

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

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

https://bugs.python.org/issue47182

Automerge-Triggered-By: GH:tiran

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@tiran
bpo-47182: Fix crash by named unicode characters after interpreter re…
746ee08 
…initialization
@tiran tiran added needs backport to 3.7 labels Mar 31, 2022
@bedevere-bot bedevere-bot added the awaiting core review label Mar 31, 2022
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Mar 31, 2022
@tiran
tiran force-pushed the bpo-47182-ucn-crash branch from e44d28d to 746ee08 Compare March 31, 2022 13:40
@tiran tiran mentioned this pull request Mar 31, 2022
Using \N after exiting and restarting results in a function signature mismatch emmatyping/python-wasm#69
Closed
vstinner
vstinner approved these changes Mar 31, 2022
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Objects/unicodeobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Mar 31, 2022
@tiran
Add test case and comment
0b7a0f6
@tiran

tiran commented Mar 31, 2022

Copy link
Copy Markdown
Member Author

I have added a comment and a test case.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@tiran tiran added the 🤖 automerge label Mar 31, 2022
vstinner
vstinner approved these changes Mar 31, 2022
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/test_embed.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington

miss-islington commented Mar 31, 2022

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington
miss-islington merged commit 44e9150 into python:main Mar 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.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the awaiting merge label Mar 31, 2022
@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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington miss-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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
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 <[email protected]>
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 <[email protected]>
@bedevere-bot bedevere-bot removed the needs backport to 3.10 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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the needs backport to 3.9 label Mar 31, 2022
@tiran tiran removed needs backport to 3.7 labels Mar 31, 2022
@tiran

tiran commented Mar 31, 2022

Copy link
Copy Markdown
Member Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
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 <[email protected]>
@kumaraditya303 kumaraditya303 mentioned this pull request May 9, 2022
_PyUnicode_Fini should invalidate ucnhash_capi capsule pointer #91338
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@miss-islington miss-islington

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@tiran @miss-islington @vstinner @bedevere-bot @the-knights-who-say-ni
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.